restore profile switching
This commit is contained in:
@@ -667,27 +667,13 @@ export async function openGatewayForAgent(connectionId: null | string, profile:
|
||||
}
|
||||
}
|
||||
|
||||
// The agent-scoped analogue of prepareGatewayForProfile, and the same
|
||||
// publication seam: dial the agent's socket without publishing anything, and
|
||||
// hand back the synchronous activation thunk. A null connection id falls
|
||||
// through to the profile seam, so both doors into an activation share one
|
||||
// atomicity contract instead of drifting apart; an explicit `local` id is a
|
||||
// registry identity (`registryBackendScopeKey` keeps its own scope for it) and
|
||||
// stays on the registry route.
|
||||
//
|
||||
// The thunk reports whether it actually published, preserving the `activated`
|
||||
// contract callers rely on: a source edit/remove can dispose this entry while
|
||||
// its dial is in flight, and a caller must be able to tell "switched" from
|
||||
// "the target stopped existing" rather than assume the former.
|
||||
export async function prepareGatewayForAgent(connectionId: null | string, profile: string): Promise<() => boolean> {
|
||||
export async function ensureGatewayForAgent(connectionId: null | string, profile: string): Promise<boolean> {
|
||||
const scope = registryBackendScopeKey(connectionId, profile)
|
||||
|
||||
// Genuinely-local scope: the profile door owns this route, so hand back ITS
|
||||
// thunk unchanged. Wrapping it to return an unconditional `true` would have
|
||||
// reported a rejected activation as a successful one and let the agent
|
||||
// caller publish companion state for a switch that never happened.
|
||||
if (scope === normKey(profile)) {
|
||||
return prepareGatewayForProfile(profile)
|
||||
await ensureGatewayForProfile(profile)
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
if (!window.hermesDesktop?.getConnectionFor) {
|
||||
@@ -716,54 +702,42 @@ export async function prepareGatewayForAgent(connectionId: null | string, profil
|
||||
}
|
||||
}
|
||||
|
||||
// Bind the entry this dial settled on; see prepareGatewayForProfile.
|
||||
const prepared = entry
|
||||
// A source edit/remove may dispose this entry while its dial is still in
|
||||
// flight. Only the still-registered, still-owned activation may publish.
|
||||
const activated =
|
||||
entry.wantOpen &&
|
||||
g.secondaries.get(scope) === entry &&
|
||||
Boolean(entry.connection) &&
|
||||
applyActive(scope, activationEpoch)
|
||||
|
||||
return () => {
|
||||
// A source edit/remove may dispose this entry while its dial is still in
|
||||
// flight. Only the still-registered, still-owned activation may publish.
|
||||
const activated =
|
||||
prepared.wantOpen &&
|
||||
g.secondaries.get(scope) === prepared &&
|
||||
Boolean(prepared.connection) &&
|
||||
applyActive(scope, activationEpoch)
|
||||
|
||||
if (activated && prepared.connection) {
|
||||
publishActiveConnection(prepared.connection)
|
||||
}
|
||||
|
||||
return activated
|
||||
if (activated && entry.connection) {
|
||||
publishActiveConnection(entry.connection)
|
||||
}
|
||||
|
||||
return activated
|
||||
}
|
||||
|
||||
export async function ensureGatewayForAgent(connectionId: null | string, profile: string): Promise<boolean> {
|
||||
return (await prepareGatewayForAgent(connectionId, profile))()
|
||||
}
|
||||
|
||||
// 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<() => boolean> {
|
||||
// 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> {
|
||||
const key = normKey(profile)
|
||||
const activationEpoch = beginGatewayActivation()
|
||||
|
||||
if (key === g.primaryProfile) {
|
||||
return () => applyActive(key, activationEpoch)
|
||||
applyActive(key, activationEpoch)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
// 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. 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.
|
||||
// descriptor — $activeGatewayProfile still moves to `key`, so request
|
||||
// scoping and profile-aware surfaces behave identically.
|
||||
if (await sharedPrimaryRoute(key)) {
|
||||
return () => applyActive(g.primaryProfile, activationEpoch)
|
||||
applyActive(g.primaryProfile, activationEpoch)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
let entry = g.secondaries.get(key)
|
||||
@@ -786,36 +760,11 @@ export async function prepareGatewayForProfile(profile: string): Promise<() => b
|
||||
}
|
||||
}
|
||||
|
||||
// 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
|
||||
|
||||
// Reports whether the ACTIVATION was accepted, which is a different question
|
||||
// from whether a descriptor was published: an accepted activation with no
|
||||
// cached connection still moved the gateway, so the caller must still move
|
||||
// its companion state. Only a rejected activation (disposed entry, or an
|
||||
// epoch superseded by a newer switch while this one was dialing) must leave
|
||||
// every companion store alone.
|
||||
return () => {
|
||||
const activated = prepared.wantOpen && g.secondaries.get(key) === prepared && applyActive(key, activationEpoch)
|
||||
|
||||
if (activated && prepared.connection) {
|
||||
publishActiveConnection(prepared.connection)
|
||||
}
|
||||
|
||||
return activated
|
||||
if (entry.wantOpen && g.secondaries.get(key) === entry && applyActive(key, activationEpoch) && entry.connection) {
|
||||
publishActiveConnection(entry.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> {
|
||||
|
||||
@@ -13,56 +13,18 @@ import type { HermesConnection } from '@/global'
|
||||
// 2. Agent activations share the gatewaySwitch mutex with profile switches —
|
||||
// without it, two rapid activations could complete out of order and the
|
||||
// EARLIER setActive() landed last.
|
||||
// 3. A SUCCEEDING activation publishes the gateway, the profile pointer and
|
||||
// the connection descriptor with no asynchronous gap between them.
|
||||
// Activating first and awaiting the descriptor after left $gateway on the
|
||||
// new backend while $connection still described the old one.
|
||||
// 4. A FAILING descriptor lookup publishes none of the three. Swallowing the
|
||||
// rejection and publishing anyway produced the same mixed state as (3),
|
||||
// except permanent: (3) closes when the descriptor arrives, whereas a
|
||||
// failed lookup never arrives and the split survived until an unrelated
|
||||
// reconnect or switch repaired it.
|
||||
//
|
||||
// Both doors go through the prepare/publish seam (prepareGatewayFor*, which
|
||||
// dial without publishing and return the activation thunk), so these mocks
|
||||
// hand back a spy thunk instead of activating on call.
|
||||
|
||||
// Distinct gateway identities so a listener can tell WHICH backend it was
|
||||
// handed. A bare vi.fn() thunk never touches $gateway, which would let an
|
||||
// out-of-order publication pass unnoticed.
|
||||
const INITIAL_GATEWAY = { id: 'live-socket' }
|
||||
const AGENT_GATEWAY = { id: 'agent-socket' }
|
||||
const PROFILE_GATEWAY = { id: 'profile-socket' }
|
||||
|
||||
const activateAgent = vi.fn(() => {
|
||||
$gateway.set(AGENT_GATEWAY)
|
||||
|
||||
return true
|
||||
})
|
||||
|
||||
const activateProfile = vi.fn(() => {
|
||||
$gateway.set(PROFILE_GATEWAY)
|
||||
|
||||
return true
|
||||
})
|
||||
|
||||
// Annotated with the SEAM's thunk types, not the spies' own. Inferred, the
|
||||
// resolved type is the MockInstance itself, and a test can no longer hand back
|
||||
// a plain `() => false` to stand in for a disposed entry.
|
||||
const prepareGatewayForAgent = vi.fn(
|
||||
async (_connectionId: null | string, _profile: string): Promise<() => boolean> => activateAgent
|
||||
)
|
||||
|
||||
const prepareGatewayForProfile = vi.fn(async (_profile: string): Promise<() => boolean> => activateProfile)
|
||||
const ensureGatewayForAgent = vi.fn(async (_connectionId: null | string, _profile: string) => true)
|
||||
const ensureGatewayForProfile = vi.fn(async (_profile: string) => undefined)
|
||||
const openGatewayForProfile = vi.fn(async (_profile: string) => undefined)
|
||||
const $gateway = atom<unknown>(INITIAL_GATEWAY)
|
||||
const $gateway = atom<unknown>({ id: 'live-socket' })
|
||||
const resetStarmapGraph = vi.fn()
|
||||
|
||||
vi.mock('@/store/gateway', () => ({
|
||||
$gateway,
|
||||
openGatewayForProfile,
|
||||
prepareGatewayForAgent,
|
||||
prepareGatewayForProfile
|
||||
ensureGatewayForAgent,
|
||||
ensureGatewayForProfile,
|
||||
openGatewayForProfile
|
||||
}))
|
||||
vi.mock('@/hermes', () => ({
|
||||
getProfiles: vi.fn(async () => ({ profiles: [] })),
|
||||
@@ -98,13 +60,9 @@ function deferred(): { promise: Promise<void>; resolve: () => void } {
|
||||
beforeEach(() => {
|
||||
getConnection.mockReset()
|
||||
getConnectionFor.mockReset()
|
||||
prepareGatewayForAgent.mockReset()
|
||||
prepareGatewayForAgent.mockResolvedValue(activateAgent)
|
||||
prepareGatewayForProfile.mockReset()
|
||||
prepareGatewayForProfile.mockResolvedValue(activateProfile)
|
||||
activateAgent.mockClear()
|
||||
activateProfile.mockClear()
|
||||
$gateway.set(INITIAL_GATEWAY)
|
||||
ensureGatewayForAgent.mockClear()
|
||||
ensureGatewayForProfile.mockClear()
|
||||
$gateway.set({ id: 'live-socket' })
|
||||
$activeGatewayProfile.set('default')
|
||||
$connection.set(localConn())
|
||||
vi.stubGlobal('window', { hermesDesktop: { getConnection, getConnectionFor } })
|
||||
@@ -123,83 +81,31 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () =
|
||||
|
||||
await ensureGatewayAgent('homelab', 'research')
|
||||
|
||||
expect(prepareGatewayForAgent).toHaveBeenCalledWith('homelab', 'research')
|
||||
expect(activateAgent).toHaveBeenCalledTimes(1)
|
||||
expect(ensureGatewayForAgent).toHaveBeenCalledWith('homelab', 'research')
|
||||
expect(getConnectionFor).toHaveBeenCalledWith({ connectionId: 'homelab', profile: 'research' })
|
||||
expect($activeGatewayProfile.get()).toBe('research')
|
||||
expect($connection.get()?.mode).toBe('remote')
|
||||
expect($connection.get()?.profile).toBe('research')
|
||||
})
|
||||
|
||||
it('fails the switch closed when the descriptor lookup rejects', async () => {
|
||||
// Previously this path swallowed the rejection and published anyway, which
|
||||
// left $gateway and $activeGatewayProfile on the NEW backend while
|
||||
// $connection still described the old one. Unlike the pending-descriptor
|
||||
// race below, that state did not close on its own: it survived until some
|
||||
// later reconnect or switch happened to repair it.
|
||||
it('leaves the prior connection intact when the descriptor fetch fails', async () => {
|
||||
getConnectionFor.mockRejectedValue(new Error('source unreachable'))
|
||||
|
||||
await expect(ensureGatewayAgent('homelab', 'research')).rejects.toThrow('source unreachable')
|
||||
await ensureGatewayAgent('homelab', 'research')
|
||||
|
||||
// Nothing published: all three still describe the previous backend, and the
|
||||
// caller can retry the switch.
|
||||
expect(activateAgent).not.toHaveBeenCalled()
|
||||
expect($gateway.get()).toBe(INITIAL_GATEWAY)
|
||||
expect($activeGatewayProfile.get()).toBe('default')
|
||||
expect($activeGatewayProfile.get()).toBe('research')
|
||||
// Best-effort: boot/reconnect resyncs later; we must not null it out here.
|
||||
expect($connection.get()?.mode).toBe('local')
|
||||
expect($connection.get()?.profile).toBe('default')
|
||||
})
|
||||
|
||||
it('does not republish a registry identity invalidated during activation', async () => {
|
||||
// The thunk reports false: the entry was disposed (source edited/removed)
|
||||
// between dial and publish. Nothing may publish, $gateway included.
|
||||
prepareGatewayForAgent.mockResolvedValueOnce(() => false)
|
||||
ensureGatewayForAgent.mockResolvedValueOnce(false)
|
||||
|
||||
await ensureGatewayAgent('removed-source', 'research')
|
||||
|
||||
expect($activeGatewayProfile.get()).toBe('default')
|
||||
expect($connection.get()?.mode).toBe('local')
|
||||
expect($gateway.get()).toBe(INITIAL_GATEWAY)
|
||||
// The descriptor lookup DOES run: it is issued concurrently with the dial
|
||||
// so both can be resolved before anything is published, which is the whole
|
||||
// point of the seam. Resolving it lazily (only after the thunk reports a
|
||||
// live entry) would put an await between the identity check and the
|
||||
// publication and reopen the gap. The cost is one redundant read-only
|
||||
// lookup in the rare disposed-entry case; the invariant that matters -
|
||||
// nothing is PUBLISHED - is asserted above.
|
||||
expect(getConnectionFor).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('never shows a $gateway listener the new backend beside stale companions', async () => {
|
||||
// The assertion the earlier tests could not make. A spy thunk that never
|
||||
// touches $gateway proves only that it was CALLED at the right moment;
|
||||
// it cannot prove that the three public stores become visible together.
|
||||
// Nanostores drains listeners synchronously on every .set(), so without
|
||||
// batch() a $gateway listener runs between the writes and reads the new
|
||||
// gateway next to the previous profile and descriptor.
|
||||
getConnectionFor.mockResolvedValue(agentConn())
|
||||
const seen: { connection?: string; gateway: unknown; profile: string }[] = []
|
||||
|
||||
const stop = $gateway.listen(gateway => {
|
||||
seen.push({
|
||||
connection: $connection.get()?.profile,
|
||||
gateway,
|
||||
profile: $activeGatewayProfile.get()
|
||||
})
|
||||
})
|
||||
|
||||
try {
|
||||
await ensureGatewayAgent('homelab', 'research')
|
||||
} finally {
|
||||
stop()
|
||||
}
|
||||
|
||||
expect(seen).toHaveLength(1)
|
||||
// When the listener sees the agent's gateway, the profile pointer and the
|
||||
// descriptor must ALREADY identify that same backend.
|
||||
expect(seen[0].gateway).toBe(AGENT_GATEWAY)
|
||||
expect(seen[0].profile).toBe('research')
|
||||
expect(seen[0].connection).toBe('research')
|
||||
expect(getConnectionFor).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('falls through to the profile path for a null connectionId', async () => {
|
||||
@@ -207,8 +113,8 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () =
|
||||
|
||||
await ensureGatewayAgent(null, 'research')
|
||||
|
||||
expect(prepareGatewayForProfile).toHaveBeenCalledWith('research')
|
||||
expect(prepareGatewayForAgent).not.toHaveBeenCalled()
|
||||
expect(ensureGatewayForProfile).toHaveBeenCalledWith('research')
|
||||
expect(ensureGatewayForAgent).not.toHaveBeenCalled()
|
||||
expect(getConnectionFor).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
@@ -217,90 +123,10 @@ describe('ensureGatewayAgent → $connection / $activeGatewayProfile sync', () =
|
||||
|
||||
await ensureGatewayAgent('local', 'research')
|
||||
|
||||
expect(prepareGatewayForAgent).toHaveBeenCalledWith('local', 'research')
|
||||
expect(prepareGatewayForProfile).not.toHaveBeenCalled()
|
||||
expect(ensureGatewayForAgent).toHaveBeenCalledWith('local', 'research')
|
||||
expect(ensureGatewayForProfile).not.toHaveBeenCalled()
|
||||
expect(getConnectionFor).toHaveBeenCalledWith({ connectionId: 'local', profile: 'research' })
|
||||
})
|
||||
|
||||
it('never publishes the agent gateway before its connection descriptor', async () => {
|
||||
// The same mixed-state window the profile path closes, through the door
|
||||
// added for the SDK's ensureAgent. A slow getConnectionFor must not leave
|
||||
// $gateway/$activeGatewayProfile on the agent's backend while $connection
|
||||
// still describes the previous one — anything requesting in that window
|
||||
// announces the WRONG mode to the new backend.
|
||||
let resolveDescriptor: (conn: HermesConnection) => void = () => undefined
|
||||
getConnectionFor.mockReturnValue(
|
||||
new Promise<HermesConnection>(resolve => {
|
||||
resolveDescriptor = resolve
|
||||
})
|
||||
)
|
||||
|
||||
const switching = ensureGatewayAgent('homelab', 'research')
|
||||
// Let the socket-dial half settle; the descriptor is still pending.
|
||||
await Promise.resolve()
|
||||
await Promise.resolve()
|
||||
|
||||
expect(activateAgent).not.toHaveBeenCalled()
|
||||
expect($gateway.get()).toBe(INITIAL_GATEWAY)
|
||||
expect($activeGatewayProfile.get()).toBe('default')
|
||||
expect($connection.get()?.mode).toBe('local')
|
||||
|
||||
resolveDescriptor(agentConn())
|
||||
await switching
|
||||
|
||||
expect(activateAgent).toHaveBeenCalledTimes(1)
|
||||
expect($activeGatewayProfile.get()).toBe('research')
|
||||
expect($connection.get()?.mode).toBe('remote')
|
||||
})
|
||||
})
|
||||
|
||||
describe('ensureGatewayProfile publishes under the same activation guard', () => {
|
||||
it('publishes nothing when the profile activation is superseded', async () => {
|
||||
// The profile-door mirror of "does not republish a registry identity
|
||||
// invalidated during activation". applyActive() returns false when its
|
||||
// captured epoch has been superseded — a newer switch or a teardown
|
||||
// landed while this preparation was awaiting its route or socket.
|
||||
//
|
||||
// Discarding that boolean does not produce a torn publication; batch()
|
||||
// makes the writes observer-atomic either way. It produces something
|
||||
// subtler and worse: ONE complete, internally inconsistent tuple, the
|
||||
// CURRENT gateway paired with the stale target's profile pointer and
|
||||
// descriptor. Atomicity cannot make a rejected activation correct, so the
|
||||
// caller has to decline to publish at all.
|
||||
getConnection.mockResolvedValue(localConn({ profile: 'worker' }))
|
||||
prepareGatewayForProfile.mockResolvedValueOnce(() => false)
|
||||
|
||||
const seen: unknown[] = []
|
||||
const stop = $gateway.listen(gateway => seen.push(gateway))
|
||||
|
||||
try {
|
||||
await ensureGatewayProfile('worker')
|
||||
} finally {
|
||||
stop()
|
||||
}
|
||||
|
||||
// All three still describe the complete route that was already active.
|
||||
expect($gateway.get()).toBe(INITIAL_GATEWAY)
|
||||
expect($activeGatewayProfile.get()).toBe('default')
|
||||
expect($connection.get()?.profile).toBe('default')
|
||||
expect($connection.get()?.mode).toBe('local')
|
||||
// And no subscriber was handed a tuple to disagree about.
|
||||
expect(seen).toEqual([])
|
||||
})
|
||||
|
||||
it('publishes the companions when the profile activation is accepted', async () => {
|
||||
// The other half: the guard must not swallow a legitimate switch. Without
|
||||
// this, returning a constant false from every thunk would pass the test
|
||||
// above and break the feature.
|
||||
getConnection.mockResolvedValue(localConn({ profile: 'worker' }))
|
||||
|
||||
await ensureGatewayProfile('worker')
|
||||
|
||||
expect(activateProfile).toHaveBeenCalledTimes(1)
|
||||
expect($gateway.get()).toBe(PROFILE_GATEWAY)
|
||||
expect($activeGatewayProfile.get()).toBe('worker')
|
||||
expect($connection.get()?.profile).toBe('worker')
|
||||
})
|
||||
})
|
||||
|
||||
describe('ensureGatewayAgent shares the gatewaySwitch mutex with profile switches', () => {
|
||||
@@ -308,16 +134,14 @@ describe('ensureGatewayAgent shares the gatewaySwitch mutex with profile switche
|
||||
const profileGate = deferred()
|
||||
const order: string[] = []
|
||||
|
||||
prepareGatewayForProfile.mockImplementation(async (profile: string) => {
|
||||
ensureGatewayForProfile.mockImplementation(async (profile: string) => {
|
||||
order.push(`profile:${profile}`)
|
||||
await profileGate.promise
|
||||
|
||||
return activateProfile
|
||||
})
|
||||
prepareGatewayForAgent.mockImplementation(async (_connectionId, profile) => {
|
||||
ensureGatewayForAgent.mockImplementation(async (_connectionId, profile) => {
|
||||
order.push(`agent:${profile}`)
|
||||
|
||||
return activateAgent
|
||||
return true
|
||||
})
|
||||
getConnection.mockResolvedValue(localConn({ profile: 'worker' }))
|
||||
getConnectionFor.mockResolvedValue(agentConn())
|
||||
@@ -346,16 +170,14 @@ describe('ensureGatewayAgent shares the gatewaySwitch mutex with profile switche
|
||||
const agentGate = deferred()
|
||||
const order: string[] = []
|
||||
|
||||
prepareGatewayForAgent.mockImplementation(async (_connectionId, profile) => {
|
||||
ensureGatewayForAgent.mockImplementation(async (_connectionId, profile) => {
|
||||
order.push(`agent:${profile}`)
|
||||
await agentGate.promise
|
||||
|
||||
return activateAgent
|
||||
return true
|
||||
})
|
||||
prepareGatewayForProfile.mockImplementation(async (profile: string) => {
|
||||
ensureGatewayForProfile.mockImplementation(async (profile: string) => {
|
||||
order.push(`profile:${profile}`)
|
||||
|
||||
return activateProfile
|
||||
})
|
||||
getConnection.mockResolvedValue(localConn({ profile: 'worker' }))
|
||||
getConnectionFor.mockResolvedValue(agentConn())
|
||||
|
||||
@@ -6,25 +6,13 @@ 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.
|
||||
// Returns true: both prepare seams hand back a thunk reporting whether the
|
||||
// activation was ACCEPTED, and a caller publishes its companion state only on
|
||||
// true. A bare vi.fn() returns undefined, which now reads as "superseded" and
|
||||
// would silently suppress every publication these tests assert on.
|
||||
const activateGateway = vi.fn(() => true)
|
||||
const ensureGatewayForProfile = vi.fn(async () => undefined)
|
||||
const prepareGatewayForAgent = vi.fn(async (_connectionId: null | string, _profile: string) => activateGateway)
|
||||
const prepareGatewayForProfile = vi.fn(async (_profile: string) => activateGateway)
|
||||
const ensureGatewayForAgent = vi.fn(async () => undefined)
|
||||
const openGatewayForProfile = vi.fn(async (_profile: string) => undefined)
|
||||
const $gateway = atom<unknown>({ id: 'live-socket' })
|
||||
const resetStarmapGraph = vi.fn()
|
||||
|
||||
vi.mock('@/store/gateway', () => ({
|
||||
$gateway,
|
||||
ensureGatewayForProfile,
|
||||
openGatewayForProfile,
|
||||
prepareGatewayForAgent,
|
||||
prepareGatewayForProfile
|
||||
}))
|
||||
vi.mock('@/store/gateway', () => ({ $gateway, ensureGatewayForAgent, ensureGatewayForProfile, openGatewayForProfile }))
|
||||
vi.mock('@/hermes', () => ({
|
||||
getProfiles: vi.fn(async () => ({ profiles: [] })),
|
||||
setApiRequestProfile: vi.fn()
|
||||
@@ -65,9 +53,7 @@ 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')
|
||||
@@ -93,8 +79,7 @@ describe('ensureGatewayProfile → $connection sync (#46651)', () => {
|
||||
|
||||
await ensureGatewayProfile('vps-remote')
|
||||
|
||||
expect(prepareGatewayForProfile).toHaveBeenCalledWith('vps-remote')
|
||||
expect(activateGateway).toHaveBeenCalledTimes(1)
|
||||
expect(ensureGatewayForProfile).toHaveBeenCalledWith('vps-remote')
|
||||
expect(getConnection).toHaveBeenCalledWith('vps-remote')
|
||||
expect($connection.get()?.mode).toBe('remote')
|
||||
expect($connection.get()?.profile).toBe('vps-remote')
|
||||
@@ -111,51 +96,15 @@ describe('ensureGatewayProfile → $connection sync (#46651)', () => {
|
||||
expect($connection.get()?.mode).toBe('local')
|
||||
})
|
||||
|
||||
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.
|
||||
it('leaves the prior connection intact when the descriptor fetch fails', async () => {
|
||||
getConnection.mockRejectedValue(new Error('backend unreachable'))
|
||||
|
||||
await ensureGatewayProfile('vps-remote')
|
||||
|
||||
expect(activateGateway).not.toHaveBeenCalled()
|
||||
expect($activeGatewayProfile.get()).toBe('default')
|
||||
// Best-effort: boot/reconnect resyncs later; we must not null it out here.
|
||||
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())
|
||||
@@ -163,7 +112,7 @@ describe('ensureGatewayProfile → $connection sync (#46651)', () => {
|
||||
await ensureGatewayProfile('vps-remote')
|
||||
|
||||
expect(getConnection).not.toHaveBeenCalled()
|
||||
expect(prepareGatewayForProfile).not.toHaveBeenCalled()
|
||||
expect(ensureGatewayForProfile).not.toHaveBeenCalled()
|
||||
expect($connection.get()?.mode).toBe('remote')
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import { atom, batch, computed } from 'nanostores'
|
||||
import { atom, computed } from 'nanostores'
|
||||
|
||||
import type { HermesConnection } from '@/global'
|
||||
import { getProfiles, hermesApi, setApiRequestProfile, STARTUP_REQUEST_TIMEOUT_MS } from '@/hermes'
|
||||
import { invalidateProfileScopedQueries } from '@/lib/query-client'
|
||||
import {
|
||||
@@ -13,7 +12,7 @@ import {
|
||||
storedStringRecord
|
||||
} from '@/lib/storage'
|
||||
import { invalidateCronModelImpactScopeState } from '@/store/cron-model-impact-scope'
|
||||
import { $gateway, openGatewayForProfile, prepareGatewayForAgent, prepareGatewayForProfile } from '@/store/gateway'
|
||||
import { $gateway, ensureGatewayForAgent, ensureGatewayForProfile, openGatewayForProfile } from '@/store/gateway'
|
||||
import { setConnection } from '@/store/session'
|
||||
import { resetStarmapGraph } from '@/store/starmap'
|
||||
import type { ProfileInfo } from '@/types/hermes'
|
||||
@@ -270,24 +269,30 @@ export function prewarmProfileBackend(name: string): void {
|
||||
|
||||
let gatewaySwitch: Promise<void> | null = null
|
||||
|
||||
// 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
|
||||
// Keep the renderer's $connection (mode / baseUrl / profile) in lockstep with
|
||||
// the profile the live gateway is now on. $connection seeds from the PRIMARY
|
||||
// (window) backend at boot and otherwise only refreshes on a sleep/wake
|
||||
// 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) {
|
||||
// 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> {
|
||||
const getConnection = window.hermesDesktop?.getConnection
|
||||
|
||||
return getConnection ? getConnection(profile) : null
|
||||
if (!getConnection) {
|
||||
return
|
||||
}
|
||||
|
||||
try {
|
||||
setConnection(await getConnection(profile))
|
||||
} catch {
|
||||
// Leave the prior connection in place; boot/reconnect resyncs it later.
|
||||
}
|
||||
}
|
||||
|
||||
// Make `profile`'s backend the active gateway, lazily opening its socket if it
|
||||
@@ -326,46 +331,14 @@ export async function ensureGatewayProfile(profile: string | null | undefined):
|
||||
|
||||
$gatewaySwapTarget.set(target)
|
||||
gatewaySwitch = (async () => {
|
||||
// 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 publication. batch() defers Nanostores' notifications to the end of
|
||||
// the callback, so the active gateway, $activeGatewayProfile and
|
||||
// $connection become visible together. Without it these are sequential
|
||||
// .set() calls that each drain their listeners synchronously, and a
|
||||
// $gateway listener runs while the other two still name the old backend.
|
||||
batch(() => {
|
||||
// A rejected activation publishes NOTHING, exactly like the agent path.
|
||||
// applyActive() returns false when its captured epoch was superseded --
|
||||
// a newer switch (or a teardown) landed while this one was awaiting its
|
||||
// route or socket. Publishing the companions anyway would leave the
|
||||
// CURRENT gateway paired with the stale profile pointer and descriptor,
|
||||
// and batch() cannot rescue that: it would make the mismatched tuple
|
||||
// atomically observable rather than prevent it.
|
||||
if (!activate()) {
|
||||
return
|
||||
}
|
||||
|
||||
$activeGatewayProfile.set(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.
|
||||
})
|
||||
// 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)
|
||||
$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)
|
||||
})()
|
||||
|
||||
try {
|
||||
await gatewaySwitch
|
||||
@@ -377,30 +350,25 @@ export async function ensureGatewayProfile(profile: string | null | undefined):
|
||||
|
||||
// Registry-aware sibling of syncConnectionToActiveProfile: a connection-scoped
|
||||
// agent's descriptor comes from getConnectionFor (its SOURCE connection), not
|
||||
// getConnection (the local pool).
|
||||
// Resolve only — publication is the caller's, so the descriptor can be in hand
|
||||
// BEFORE the activation frame rather than an await after it.
|
||||
//
|
||||
// Null means "no desktop bridge" (plain browser) and nothing else, matching
|
||||
// resolveConnectionForProfile. A bridge REJECTION propagates so the caller
|
||||
// aborts the whole switch. Collapsing the two into null instead let a failed
|
||||
// lookup publish the new gateway and profile while $connection kept describing
|
||||
// the OLD backend, and unlike the pending-descriptor race that state did not
|
||||
// close on its own: it survived until some later reconnect or switch happened
|
||||
// to repair it, which is the same invariant this path exists to establish.
|
||||
async function resolveConnectionForActiveAgent(
|
||||
connectionId: string,
|
||||
profile: string
|
||||
): Promise<null | HermesConnection> {
|
||||
// getConnection (the local pool). Same best-effort contract.
|
||||
async function syncConnectionToActiveAgent(connectionId: string, profile: string): Promise<void> {
|
||||
const getConnectionFor = window.hermesDesktop?.getConnectionFor
|
||||
|
||||
return getConnectionFor ? getConnectionFor({ connectionId, profile }) : null
|
||||
if (!getConnectionFor) {
|
||||
return
|
||||
}
|
||||
|
||||
try {
|
||||
setConnection(await getConnectionFor({ connectionId, profile }))
|
||||
} catch {
|
||||
// Leave the prior connection in place; boot/reconnect resyncs it later.
|
||||
}
|
||||
}
|
||||
|
||||
// Activate a connection-scoped agent's gateway — the (connectionId, profile)
|
||||
// analogue of ensureGatewayProfile, and the door the SDK's ensureAgent goes
|
||||
// through. Three invariants the raw store call (ensureGatewayForAgent) does
|
||||
// not provide on its own:
|
||||
// through. Two invariants the raw store call (ensureGatewayForAgent) does not
|
||||
// provide on its own:
|
||||
// - Every activation moves $activeGatewayProfile and resyncs $connection,
|
||||
// exactly like the profile path — otherwise activating an ALREADY-OPEN
|
||||
// registry agent left both describing the previous backend, routing
|
||||
@@ -409,10 +377,6 @@ async function resolveConnectionForActiveAgent(
|
||||
// - Activations share the gatewaySwitch mutex with profile switches, so a
|
||||
// rapid agent↔profile (or agent↔agent) interleave can't finish out of
|
||||
// order and leave the EARLIER setActive() as the last write.
|
||||
// - The gateway, the profile pointer and the connection descriptor publish in
|
||||
// ONE synchronous frame, via the same prepare/publish seam the profile path
|
||||
// uses, so no subscriber sees the new backend paired with the old
|
||||
// descriptor.
|
||||
// Only a null connectionId falls through to the legacy profile path. Explicit
|
||||
// `local` is a registry identity and must use the genuinely-local route.
|
||||
export async function ensureGatewayAgent(connectionId: null | string, profile: string): Promise<void> {
|
||||
@@ -430,40 +394,16 @@ export async function ensureGatewayAgent(connectionId: null | string, profile: s
|
||||
|
||||
$gatewaySwapTarget.set(target)
|
||||
gatewaySwitch = (async () => {
|
||||
// Dial the agent's socket and resolve its descriptor without publishing
|
||||
// either, exactly like the profile path above. Activating first and then
|
||||
// awaiting the descriptor left $gateway on the new backend while
|
||||
// $connection still described the old one, so anything requesting during
|
||||
// that window announced the WRONG mode to the new backend.
|
||||
const [descriptor, activate] = await Promise.all([
|
||||
resolveConnectionForActiveAgent(connection, target),
|
||||
prepareGatewayForAgent(connection, target)
|
||||
])
|
||||
const activated = await ensureGatewayForAgent(connection, target)
|
||||
|
||||
// ONE publication. batch() defers Nanostores' notifications to the end of
|
||||
// the callback, so a $gateway listener cannot run while the profile
|
||||
// pointer and the connection descriptor still name the previous backend.
|
||||
// Without it these are three sequential .set() calls, each draining its
|
||||
// listeners synchronously, and the first listener observes exactly the
|
||||
// mismatch this seam exists to prevent.
|
||||
batch(() => {
|
||||
// A disposed target (source edited/removed mid-dial) publishes nothing
|
||||
// at all, rather than moving the profile pointer to a backend that no
|
||||
// longer has a socket.
|
||||
if (!activate()) {
|
||||
return
|
||||
}
|
||||
if (!activated) {
|
||||
return
|
||||
}
|
||||
|
||||
$activeGatewayProfile.set(target)
|
||||
|
||||
// Remote-aware paths (image.attach_bytes vs image.attach, /api/fs/*,
|
||||
// /api/media) follow $connection. Null here is only the no-bridge case,
|
||||
// so keeping the previous descriptor is correct; a failed lookup
|
||||
// rejected above and never reached this frame.
|
||||
if (descriptor) {
|
||||
setConnection(descriptor)
|
||||
}
|
||||
})
|
||||
$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 syncConnectionToActiveAgent(connection, target)
|
||||
})()
|
||||
|
||||
try {
|
||||
|
||||
Reference in New Issue
Block a user