fix(desktop): probe a cached pooled remote backend before dispatching to it
A pooled remote backend (Bot Mode, group chat) keeps its descriptor and SSH forward cached in the backend pool. When the remote Desktop relaunches, the remote process dies but the local forward stays LISTENing, so ensureRegistryBackend() keeps returning the dead descriptor and every dispatch to that machine fails until the app is restarted. The background sweep cannot cover this: revalidatePooledRemoteBackends() only runs from the renderer reconnect IPC, which never fires while the primary connection stays healthy. Validate the exact cached descriptor at dispatch time with a short /api/status probe (2.5 s). On failure, retire the pool entry and its SSH forward, then reconnect on demand. Concurrent dispatches share one retire/reconnect sequence through a RemoteRevalidationCoordinator keyed on the cached promise, and identity checks make a late failure from an old descriptor unable to tear down a replacement another caller already installed. Verified on a two-Mac setup (MacBook + Mac mini over SSH): after relaunching the Mac mini's Desktop, a group-chat turn from the MacBook now reaches the mini's backend and its reply lands, where it previously failed forever.
This commit is contained in:
@@ -284,6 +284,7 @@ import { createQuickEntryShortcut, quickEntryWindowBounds, sanitizeQuickEntrySet
|
||||
import { type ActiveWork, mergeActiveWork, normalizeActiveWork, quitPromptFor } from './quit-guard'
|
||||
import * as remoteLifecycle from './remote-lifecycle'
|
||||
import {
|
||||
ensureHealthyPooledRemoteBackendForDispatch,
|
||||
RemoteLivenessTracker,
|
||||
RemoteRevalidationCoordinator,
|
||||
revalidatePooledRemoteBackends,
|
||||
@@ -1331,6 +1332,7 @@ let mainWindow = null
|
||||
const backendConnectionState = createBackendConnectionState<ReturnType<typeof spawn>, any>()
|
||||
const remoteLiveness = new RemoteLivenessTracker()
|
||||
const remoteRevalidation = new RemoteRevalidationCoordinator()
|
||||
const registryDispatchRevalidation = new RemoteRevalidationCoordinator()
|
||||
// True while connection-config:apply soft-rehomes the primary — suppresses the
|
||||
// backend-exit toast so an intentional kill doesn't look like a crash.
|
||||
let softRehomeInProgress = false
|
||||
@@ -10581,8 +10583,37 @@ async function ensureRegistryBackend(connectionId, profile) {
|
||||
|
||||
if (existing) {
|
||||
existing.lastActiveAt = Date.now()
|
||||
const connectionPromise = existing.connectionPromise
|
||||
|
||||
return existing.connectionPromise
|
||||
// A remote process can die while its local SSH forward stays LISTENing.
|
||||
// Validate the exact cached descriptor at dispatch time; background
|
||||
// revalidation is renderer-driven and may never run while the Bots pane is
|
||||
// closed. Concurrent clicks share one retire/reconnect sequence.
|
||||
return registryDispatchRevalidation.run(connectionPromise, () =>
|
||||
ensureHealthyPooledRemoteBackendForDispatch({
|
||||
connectionPromise,
|
||||
currentConnectionPromise: () => backendPool.get(key)?.connectionPromise || null,
|
||||
probe: (connection, requestPath, options) => fetchJsonForBackend(connection, requestPath, options),
|
||||
reconnect: () => ensureRegistryBackend(id, profile),
|
||||
retire: async (error: any) => {
|
||||
// A late failure from an old descriptor must never tear down a newer
|
||||
// entry that another caller has already installed.
|
||||
if (backendPool.get(key) !== existing) {
|
||||
return
|
||||
}
|
||||
|
||||
rememberLog(
|
||||
`Pooled remote backend "${key}" failed its dispatch probe (${error?.message || error}); reconnecting on demand.`
|
||||
)
|
||||
await stopPoolBackend(key)
|
||||
|
||||
if (source.kind === 'ssh') {
|
||||
await sshBootstrapCoordinator.cancelAndWait(key)
|
||||
await teardownSshConnection(key)
|
||||
}
|
||||
}
|
||||
})
|
||||
)
|
||||
}
|
||||
|
||||
evictLruPoolBackends(POOL_MAX_BACKENDS - 1)
|
||||
|
||||
@@ -1,6 +1,8 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import {
|
||||
ensureHealthyPooledRemoteBackendForDispatch,
|
||||
POOLED_REMOTE_DISPATCH_PROBE_TIMEOUT_MS,
|
||||
REMOTE_LIVENESS_FAILURE_LIMIT,
|
||||
REMOTE_LIVENESS_FAILURE_WINDOW_MS,
|
||||
REMOTE_LIVENESS_TIMEOUT_MS,
|
||||
@@ -257,6 +259,47 @@ describe('revalidateRemoteConnection', () => {
|
||||
})
|
||||
})
|
||||
|
||||
describe('ensureHealthyPooledRemoteBackendForDispatch', () => {
|
||||
it('retires a dead cached descriptor and gives dispatch the replacement', async () => {
|
||||
const stale = { baseUrl: 'http://127.0.0.1:49525', mode: 'remote' }
|
||||
const replacement = { baseUrl: 'http://127.0.0.1:53968', mode: 'remote' }
|
||||
const stalePromise = Promise.resolve(stale)
|
||||
let currentPromise: Promise<typeof stale> | null = stalePromise
|
||||
|
||||
const retire = vi.fn(async () => {
|
||||
currentPromise = null
|
||||
})
|
||||
|
||||
const reconnect = vi.fn(async () => {
|
||||
currentPromise = Promise.resolve(replacement)
|
||||
|
||||
return replacement
|
||||
})
|
||||
|
||||
const probe = vi.fn(async connection => {
|
||||
if (connection === stale) {
|
||||
throw new Error('connect ECONNREFUSED 127.0.0.1:49525')
|
||||
}
|
||||
})
|
||||
|
||||
await expect(
|
||||
ensureHealthyPooledRemoteBackendForDispatch({
|
||||
connectionPromise: stalePromise,
|
||||
currentConnectionPromise: () => currentPromise,
|
||||
probe,
|
||||
reconnect,
|
||||
retire
|
||||
})
|
||||
).resolves.toBe(replacement)
|
||||
|
||||
expect(probe).toHaveBeenCalledWith(stale, '/api/status', {
|
||||
timeoutMs: POOLED_REMOTE_DISPATCH_PROBE_TIMEOUT_MS
|
||||
})
|
||||
expect(retire).toHaveBeenCalledOnce()
|
||||
expect(reconnect).toHaveBeenCalledOnce()
|
||||
})
|
||||
})
|
||||
|
||||
describe('revalidatePooledRemoteBackends', () => {
|
||||
interface TestRemoteConnection {
|
||||
authMode?: string
|
||||
|
||||
@@ -1,4 +1,9 @@
|
||||
export const REMOTE_LIVENESS_TIMEOUT_MS = 10_000
|
||||
// Dispatch is synchronous user intent: a cached descriptor must prove its
|
||||
// forwarded endpoint is alive before it can be returned. Keep this probe much
|
||||
// shorter than the background liveness budget so a dead tunnel reconnects
|
||||
// promptly instead of making the click feel hung.
|
||||
export const POOLED_REMOTE_DISPATCH_PROBE_TIMEOUT_MS = 2_500
|
||||
export const REMOTE_LIVENESS_FAILURE_LIMIT = 3
|
||||
// Even at the capped retry path, consecutive liveness observations are at most
|
||||
// about 48s apart (ticket mint + socket open + backoff + the next status probe).
|
||||
@@ -63,6 +68,60 @@ export class RemoteRevalidationCoordinator {
|
||||
}
|
||||
}
|
||||
|
||||
interface EnsureHealthyPooledRemoteBackendForDispatchOptions<
|
||||
TConnection extends RemoteConnectionDescriptor
|
||||
> {
|
||||
connectionPromise: Promise<TConnection>
|
||||
currentConnectionPromise: () => null | Promise<TConnection>
|
||||
probe: (connection: TConnection, path: string, options: { timeoutMs: number }) => Promise<unknown>
|
||||
reconnect: () => Promise<TConnection>
|
||||
retire: (error: unknown) => Promise<void> | void
|
||||
}
|
||||
|
||||
/**
|
||||
* Gate dispatch through a cheap health probe of the exact cached descriptor.
|
||||
*
|
||||
* A failed descriptor is retired before reconnecting, while identity checks
|
||||
* prevent a late probe from tearing down a replacement installed by another
|
||||
* caller. The caller should single-flight this function per cached promise so
|
||||
* concurrent dispatches share one retire/reconnect sequence.
|
||||
*/
|
||||
export async function ensureHealthyPooledRemoteBackendForDispatch<
|
||||
TConnection extends RemoteConnectionDescriptor
|
||||
>({
|
||||
connectionPromise,
|
||||
currentConnectionPromise,
|
||||
probe,
|
||||
reconnect,
|
||||
retire
|
||||
}: EnsureHealthyPooledRemoteBackendForDispatchOptions<TConnection>): Promise<TConnection> {
|
||||
let connection: TConnection
|
||||
|
||||
try {
|
||||
connection = await connectionPromise
|
||||
|
||||
if (currentConnectionPromise() !== connectionPromise) {
|
||||
return reconnect()
|
||||
}
|
||||
|
||||
await probe(connection, '/api/status', {
|
||||
timeoutMs: POOLED_REMOTE_DISPATCH_PROBE_TIMEOUT_MS
|
||||
})
|
||||
} catch (error) {
|
||||
if (currentConnectionPromise() === connectionPromise) {
|
||||
await retire(error)
|
||||
}
|
||||
|
||||
return reconnect()
|
||||
}
|
||||
|
||||
if (currentConnectionPromise() !== connectionPromise) {
|
||||
return reconnect()
|
||||
}
|
||||
|
||||
return connection
|
||||
}
|
||||
|
||||
/**
|
||||
* Tracks consecutive remote liveness failures independently per gateway.
|
||||
* A successful probe clears the streak, and reaching the limit consumes it so
|
||||
|
||||
Reference in New Issue
Block a user