diff --git a/apps/desktop/electron/backend-dial-claim.test.ts b/apps/desktop/electron/backend-dial-claim.test.ts index 6aa7ce1353893..30c7e8195c34d 100644 --- a/apps/desktop/electron/backend-dial-claim.test.ts +++ b/apps/desktop/electron/backend-dial-claim.test.ts @@ -126,10 +126,10 @@ describe('main.ts wiring for #90812', () => { it('routes the profile-scoped dial IPC through the single-owner claim', () => { const handlerStart = mainSource.indexOf("ipcMain.handle('hermes:connection', ") expect(handlerStart).toBeGreaterThan(-1) - const body = mainSource.slice(handlerStart, handlerStart + 900) + const body = mainSource.slice(handlerStart, handlerStart + 1200) expect(body).toContain('backendDialClaims.run(') - expect(body).toContain('ensureBackend(profile)') + expect(body).toContain('ensureBackend(profile, { spawnPriority })') }) it('routes the registry-scoped dial IPC through the claim keyed by backendScopeKey(connectionId, profile)', () => { @@ -137,8 +137,9 @@ describe('main.ts wiring for #90812', () => { expect(handlerStart).toBeGreaterThan(-1) const body = mainSource.slice(handlerStart, handlerStart + 1_200) - expect(body).toContain('backendDialClaims.run(backendScopeKey(id, profile)') - expect(body).toContain('ensureRegistryBackend(id, profile)') + expect(body).toContain('const scopeKey = backendScopeKey(id, profile)') + expect(body).toContain('backendDialClaims.run(scopeKey, ') + expect(body).toContain("ensureRegistryBackend(id, profile, '', { spawnPriority })") }) // The four IPC/probe surfaces below call ensureRegistryBackend()/ensureBackend() diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 5116cdb7bc656..ecbc3ebb71662 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -287,7 +287,9 @@ import { import { selectPoolEvictions } from './pool-eviction' import { clampPoolLimits, parsePoolLimits, POOL_LIMITS_DEFAULTS } from './pool-limits' import { + isBackgroundSlotWaitTimeout, LocalBackendSpawnCoordinator, + type LocalBackendSpawnPriority, type LocalBackendSpawnRequest, releaseLocalBackendSlotAfterExit } from './pool-spawn-coordinator' @@ -1492,6 +1494,70 @@ const localBackendSpawnCoordinator = new LocalBackendSpawnCoordinator(poolLimits // the queued ticket fails before the renderer does and the user sees why. const POOL_SLOT_WAIT_MS = 30_000 +function spawnPriorityFrom(value: unknown): LocalBackendSpawnPriority { + return value === 'foreground' ? 'foreground' : 'background' +} + +// Foreground intent for a dial whose pool entry does not exist yet: a user +// click that joins an in-flight backendDialClaims claim never re-enters +// ensureBackend(), and the claim owner may still be awaiting poolStopper / +// registry resolution before backendPool.set(). The local spawn takes the mark +// right before its slot request; the IPC handler that set it clears it once +// the claim settles, so a dial that never reaches a slot request (primary +// route, remote scope, a guard rejection) cannot leave it for a later +// hydration spawn of the same key to pick up. +const pendingForegroundSpawns = new Set() + +function takeForegroundSpawn(...poolKeys: string[]): boolean { + let marked = false + + for (const poolKey of poolKeys) { + marked = pendingForegroundSpawns.delete(poolKey) || marked + } + + return marked +} + +// Upgrade a pooled entry (running, spawning, or queued for a slot) to +// foreground so a queued slot wait can take the reserved foreground slot. +function promotePoolEntry(entry: any): void { + entry.spawnPriority = 'foreground' + entry.localBackendSpawnRequest?.promote?.('foreground') +} + +// Land a spawn failure in desktop.log. A background slot-wait timeout is +// routine under a saturated pool (the next hydration pass retries), so it is +// logged as such instead of as a backend-start failure. +function logPoolSpawnFailure(label: string, error: unknown): void { + if (isBackgroundSlotWaitTimeout(error)) { + rememberLog(`Profile backend ${label} slot wait timed out (background); will retry on the next hydration`) + } else { + rememberLog( + `Hermes backend for profile ${label} failed to start: ${error instanceof Error ? error.message : String(error)}` + ) + } +} + +// Apply foreground intent to the dial claim for `scopeKey`: an entry already +// in the pool is promoted directly, otherwise the intent is marked for the +// spawn the claim owner is about to start. Returns the cleanup that clears a +// mark the dial never consumed. +function applySpawnPriority(scopeKey: string, spawnPriority: LocalBackendSpawnPriority): () => void { + if (spawnPriority !== 'foreground') { + return () => undefined + } + + const existing = backendPool.get(scopeKey) + + if (existing) { + promotePoolEntry(existing) + } else { + pendingForegroundSpawns.add(scopeKey) + } + + return () => void pendingForegroundSpawns.delete(scopeKey) +} + function poolMaxBackends() { return poolLimits.maxBackends } @@ -11382,8 +11448,9 @@ function profileRouteOptions(profile, request?) { // Resolve a backend connection for the given profile, per the routing table in // resolveProfileBackendRoute(). An empty / unknown profile resolves to the // primary, so legacy callers are unchanged. -async function ensureBackend(profile) { +async function ensureBackend(profile, opts: { spawnPriority?: LocalBackendSpawnPriority } = {}) { const key = profile && String(profile).trim() ? String(profile).trim() : primaryProfileKey() + const spawnPriority = spawnPriorityFrom(opts.spawnPriority) profileDeletionGate.assertCanStart(key) @@ -11415,6 +11482,11 @@ async function ensureBackend(profile) { if (existing) { existing.lastActiveAt = Date.now() + + if (spawnPriority === 'foreground') { + promotePoolEntry(existing) + } + const connection = await existing.connectionPromise setWslBridgeProfileState(key, connection.mode !== 'remote') @@ -11432,16 +11504,15 @@ async function ensureBackend(profile) { remoteBaseUrl: null, releaseLocalBackendSlot: null, localBackendSlotKey: null, - localBackendSpawnRequest: null + localBackendSpawnRequest: null, + spawnPriority } entry.connectionPromise = spawnPoolBackend(key, entry).catch(async error => { // Land the failure in desktop.log: without this a spawn that dies before // its child exists (guard rejection, runtime resolution) leaves no trace // beyond renderer-side rejections users never see in a bundle. - rememberLog( - `Hermes backend for profile "${key}" failed to start: ${error instanceof Error ? error.message : String(error)}` - ) + logPoolSpawnFailure(`"${key}"`, error) await teardownFailedLocalBackend(key, entry) throw error @@ -11462,7 +11533,13 @@ async function ensureBackend(profile) { // a genuinely-local child when the v1 mode says remote; non-local connections // pool under the composite key from backendScopeKey() and reuse the same pool // entry lifecycle (LRU, idle reaper, touch) as per-profile local backends. -async function ensureRegistryBackend(connectionId, profile, managedUpdateCorrelation = '') { +async function ensureRegistryBackend( + connectionId, + profile, + managedUpdateCorrelation = '', + opts: { spawnPriority?: LocalBackendSpawnPriority } = {} +) { + const spawnPriority = spawnPriorityFrom(opts.spawnPriority) const registry = readDesktopConnectionsRegistry() const id = String(connectionId || '').trim() || registry.primary const source = registry.connections.find(c => c.id === id) @@ -11519,7 +11596,7 @@ async function ensureRegistryBackend(connectionId, profile, managedUpdateCorrela const primary = await reuseMatchingPrimarySshBackend({ connectionId: id, effectiveFingerprint: resolveRegistryEffectiveFingerprint, - ensurePrimary: () => ensureBackend(profile), + ensurePrimary: () => ensureBackend(profile, { spawnPriority }), profile, registry, source @@ -11570,7 +11647,7 @@ async function ensureRegistryBackend(connectionId, profile, managedUpdateCorrela }) if (localRoute.delegate) { - return ensureBackend(profile) + return ensureBackend(profile, { spawnPriority }) } const stoppingLocal = poolStopper.inFlight(localRoute.poolKey) @@ -11584,6 +11661,10 @@ async function ensureRegistryBackend(connectionId, profile, managedUpdateCorrela if (existingLocal) { existingLocal.lastActiveAt = Date.now() + if (spawnPriority === 'foreground') { + promotePoolEntry(existingLocal) + } + return existingLocal.connectionPromise } @@ -11598,7 +11679,8 @@ async function ensureRegistryBackend(connectionId, profile, managedUpdateCorrela remoteBaseUrl: null, releaseLocalBackendSlot: null, localBackendSlotKey: null, - localBackendSpawnRequest: null + localBackendSpawnRequest: null, + spawnPriority } localEntry.connectionPromise = spawnPoolBackend(profileKey, localEntry, { @@ -11607,9 +11689,7 @@ async function ensureRegistryBackend(connectionId, profile, managedUpdateCorrela }).catch(async error => { // Same trace rule as the v1 pool path: a forced-local child whose spawn // rejects before the child exists must still land in desktop.log. - rememberLog( - `Hermes backend for profile "${profileKey}" (forced-local) failed to start: ${error instanceof Error ? error.message : String(error)}` - ) + logPoolSpawnFailure(`"${profileKey}" (forced-local)`, error) await teardownFailedLocalBackend(localRoute.poolKey, localEntry) throw error @@ -12447,11 +12527,23 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po // pool-idle window (10 min) would hold the pool key hostage and every // later click on the profile would join that stale wait. Failing here // surfaces the "all N slots busy" reason instead of a generic boot timeout. - const spawnRequest = localBackendSpawnCoordinator.request(poolKey, { timeoutMs: POOL_SLOT_WAIT_MS }) + // The caller stamped entry.spawnPriority from its own request; a foreground + // dial that joined the claim before this entry existed left a mark instead. + if (takeForegroundSpawn(poolKey, profile)) { + entry.spawnPriority = 'foreground' + } + + const spawnPriority: LocalBackendSpawnPriority = spawnPriorityFrom(entry.spawnPriority) + + const spawnRequest = localBackendSpawnCoordinator.request(poolKey, { + timeoutMs: POOL_SLOT_WAIT_MS, + priority: spawnPriority + }) + entry.localBackendSlotKey = poolKey entry.localBackendSpawnRequest = spawnRequest - if (localBackendSpawnCoordinator.activeCount >= poolMaxBackends()) { + if (spawnRequest.queued) { rememberLog( `Profile backend "${profile}" waiting for a free local slot (${localBackendSpawnCoordinator.activeCount}/${poolMaxBackends()} busy, ${localBackendSpawnCoordinator.queuedCount} queued)` ) @@ -14711,13 +14803,27 @@ function createWindow() { }) } -ipcMain.handle('hermes:connection', async (_event, profile) => { +ipcMain.handle('hermes:connection', async (_event, profile, extra) => { // Coalesce concurrent renderer dials for one profile scope (#90812): the // renderer-side reconnect lock is per-window, so two windows waking at once // both land here. The claim key mirrors ensureBackend()'s own profile // normalization so every spelling of the primary coalesces onto one dial. const profileKey = profile && String(profile).trim() ? String(profile).trim() : primaryProfileKey() - const connection = await backendDialClaims.run(backendScopeKey(null, profileKey), () => ensureBackend(profile)) + // A user click may join an in-flight hydration claim; the foreground intent + // is applied to that claim so its slot wait can take the reserved slot. + const spawnPriority = spawnPriorityFrom(extra?.priority) + + const scopeKey = backendScopeKey(null, profileKey) + const clearSpawnPriority = applySpawnPriority(scopeKey, spawnPriority) + + let connection + + try { + connection = await backendDialClaims.run(scopeKey, () => ensureBackend(profile, { spawnPriority })) + } finally { + clearSpawnPriority() + } + const connectionId = resolvedConnectionId(readDesktopConnectionsRegistry(), connection) return connectionId ? { ...connection, connectionId } : connection @@ -14728,13 +14834,24 @@ ipcMain.handle('hermes:connection', async (_event, profile) => { // forces a genuinely-local child when the v1 global mode is remote (the // registry 'local' entry always means this machine). ipcMain.handle('hermes:connection:for', async (_event, payload) => { - const { connectionId, profile } = payload && typeof payload === 'object' ? (payload as any) : ({} as any) + const { connectionId, profile, priority } = payload && typeof payload === 'object' ? (payload as any) : ({} as any) const registry = readDesktopConnectionsRegistry() const id = String(connectionId || '').trim() || registry.primary + const spawnPriority = spawnPriorityFrom(priority) + // Same single-owner claim as 'hermes:connection', keyed by the composite // (connectionId, profile) scope (#90812): concurrent registry dials for one // scope share the first spawn instead of bootstrapping duplicate remotes. - const connection = await backendDialClaims.run(backendScopeKey(id, profile), () => ensureRegistryBackend(id, profile)) + const scopeKey = backendScopeKey(id, profile) + const clearSpawnPriority = applySpawnPriority(scopeKey, spawnPriority) + + let connection + + try { + connection = await backendDialClaims.run(scopeKey, () => ensureRegistryBackend(id, profile, '', { spawnPriority })) + } finally { + clearSpawnPriority() + } return { ...connection, connectionId: id, registryScoped: true } }) diff --git a/apps/desktop/electron/pool-spawn-coordinator.test.ts b/apps/desktop/electron/pool-spawn-coordinator.test.ts index 73b2291fe1edd..b7cf4ee30ec91 100644 --- a/apps/desktop/electron/pool-spawn-coordinator.test.ts +++ b/apps/desktop/electron/pool-spawn-coordinator.test.ts @@ -6,7 +6,11 @@ import { fileURLToPath } from 'node:url' import { test } from 'vitest' -import { LocalBackendSpawnCoordinator, releaseLocalBackendSlotAfterExit } from './pool-spawn-coordinator' +import { + LocalBackendSlotWaitTimeoutError, + LocalBackendSpawnCoordinator, + releaseLocalBackendSlotAfterExit +} from './pool-spawn-coordinator' const deferred = () => { let resolve!: () => void @@ -306,6 +310,197 @@ test('setLimit rejects a non-positive or fractional cap', () => { assert.equal(coordinator.limit, 2) }) +test('cap 3: two background leases leave a reserved slot for foreground', async () => { + const coordinator = new LocalBackendSpawnCoordinator(3) + const bg1 = await coordinator.request('bg-1', { priority: 'background' }).acquired + const bg2 = await coordinator.request('bg-2', { priority: 'background' }).acquired + assert.equal(coordinator.activeCount, 2) + assert.equal(coordinator.queuedCount, 0) + + let fgGranted = false + + const fgPromise = coordinator.request('fg', { priority: 'foreground' }).acquired.then(release => { + fgGranted = true + + return release + }) + + await flush() + assert.equal(fgGranted, true) + assert.equal(coordinator.activeCount, 3) + + const releaseFg = await fgPromise + bg1() + bg2() + releaseFg() + assert.equal(coordinator.activeCount, 0) +}) + +test('untagged acquire still fills the cap (foreground default)', async () => { + const coordinator = new LocalBackendSpawnCoordinator(3) + const releases = await Promise.all(['a', 'b', 'c'].map(key => coordinator.acquire(key))) + assert.equal(coordinator.activeCount, 3) + assert.equal(coordinator.queuedCount, 0) + + for (const release of releases) { + release() + } + + assert.equal(coordinator.activeCount, 0) +}) + +test('foreground is granted the reserved slot ahead of a background hydration queue', async () => { + const coordinator = new LocalBackendSpawnCoordinator(3) + + const bgRunning = await Promise.all( + ['bg-run-1', 'bg-run-2'].map(key => coordinator.request(key, { priority: 'background' }).acquired) + ) + + const queued = Array.from({ length: 20 }, (_, index) => + coordinator.request(`bg-wait-${index}`, { priority: 'background', timeoutMs: 5_000 }) + ) + + await flush() + assert.equal(coordinator.activeCount, 2) + assert.equal(coordinator.queuedCount, 20) + + const started = Date.now() + const releaseFg = await coordinator.request('user-click', { priority: 'foreground', timeoutMs: 100 }).acquired + assert.ok(Date.now() - started < 80, 'foreground must not wait behind the background queue') + assert.equal(coordinator.activeCount, 3) + + for (const request of queued) { + request.cancel() + } + + releaseFg() + + for (const release of bgRunning) { + release() + } + + await Promise.all( + queued.map(request => + request.acquired.then( + () => undefined, + () => undefined + ) + ) + ) + assert.equal(coordinator.activeCount, 0) + assert.equal(coordinator.queuedCount, 0) +}) + +test('drain prefers a foreground waiter over an earlier background waiter', async () => { + const coordinator = new LocalBackendSpawnCoordinator(1) + const releaseHolder = await coordinator.acquire('holder') + const background = coordinator.request('background', { priority: 'background' }) + const foreground = coordinator.request('foreground', { priority: 'foreground' }) + await flush() + assert.equal(coordinator.queuedCount, 2) + + let backgroundEntered = false + let foregroundEntered = false + + const backgroundGrant = background.acquired.then(release => { + backgroundEntered = true + + return release + }) + + const foregroundGrant = foreground.acquired.then(release => { + foregroundEntered = true + + return release + }) + + releaseHolder() + await flush() + assert.equal(foregroundEntered, true) + assert.equal(backgroundEntered, false) + assert.equal(coordinator.activeCount, 1) + + const releaseForeground = await foregroundGrant + releaseForeground() + const releaseBackground = await backgroundGrant + assert.equal(backgroundEntered, true) + releaseBackground() + assert.equal(coordinator.activeCount, 0) +}) + +test('background slot-wait timeout is distinguishable; foreground keeps a user-facing message', async () => { + const coordinator = new LocalBackendSpawnCoordinator(1) + const releaseFirst = await coordinator.acquire('first') + + const background = coordinator.request('bg', { priority: 'background', timeoutMs: 10 }) + await assert.rejects(background.acquired, error => { + assert.ok(error instanceof LocalBackendSlotWaitTimeoutError) + assert.equal(error.name, 'LocalBackendSlotWaitTimeoutError') + assert.equal(error.priority, 'background') + assert.equal(error.silent, true) + assert.match(error.message, /timed out while waiting for a free slot/) + assert.match(error.message, /\(background\)/) + + return true + }) + + const foreground = coordinator.request('fg', { priority: 'foreground', timeoutMs: 10 }) + await assert.rejects(foreground.acquired, error => { + assert.ok(error instanceof Error) + assert.match(error.message, /timed out while waiting for a free slot/) + assert.doesNotMatch(error.message, /\(background\)/) + assert.notEqual(error.name, 'LocalBackendSlotWaitTimeoutError') + + return true + }) + + releaseFirst() + assert.equal(coordinator.activeCount, 0) +}) + +test('request() reports whether the caller actually waited behind the queue', async () => { + const coordinator = new LocalBackendSpawnCoordinator(3) + const bg1 = coordinator.request('bg-1', { priority: 'background' }) + const bg2 = coordinator.request('bg-2', { priority: 'background' }) + const bgWait = coordinator.request('bg-3', { priority: 'background' }) + assert.equal(bg1.queued, false) + assert.equal(bg2.queued, false) + assert.equal(bgWait.queued, true) + + // The reserved slot is free: a foreground request is granted immediately + // even though a background waiter is queued. + const fg = coordinator.request('fg', { priority: 'foreground' }) + assert.equal(fg.queued, false) + assert.equal(coordinator.activeCount, 3) + + bgWait.cancel() + await bgWait.acquired.catch(() => undefined) + ;(await fg.acquired)() + ;(await bg1.acquired)() + ;(await bg2.acquired)() + assert.equal(coordinator.activeCount, 0) +}) + +test('promoting a queued background waiter lets it take the reserved foreground slot', async () => { + const coordinator = new LocalBackendSpawnCoordinator(3) + const bg1 = await coordinator.request('bg-1', { priority: 'background' }).acquired + const bg2 = await coordinator.request('bg-2', { priority: 'background' }).acquired + const queued = coordinator.request('same-bot', { priority: 'background' }) + await flush() + assert.equal(coordinator.activeCount, 2) + assert.equal(coordinator.queuedCount, 1) + + assert.equal(queued.promote('foreground'), true) + const releasePromoted = await queued.acquired + assert.equal(coordinator.activeCount, 3) + assert.equal(coordinator.queuedCount, 0) + + releasePromoted() + bg1() + bg2() + assert.equal(coordinator.activeCount, 0) +}) + // ── main.ts wiring ────────────────────────────────────────────────────────── // The coordinator is only as good as the timeout main.ts hands it. A queued // ticket that outlives the renderer's backend-boot budget holds the pool key @@ -330,7 +525,10 @@ test('setLimit rejects a non-positive or fractional cap', () => { assert.ok(Number.isFinite(slotWait) && slotWait > 0, 'POOL_SLOT_WAIT_MS must be a literal in main.ts') assert.ok(Number.isFinite(bootBudget), 'BACKEND_BOOT_WAIT_TIMEOUT_MS must be a literal') assert.ok(slotWait < bootBudget, `slot wait ${slotWait}ms must be below the boot budget ${bootBudget}ms`) - assert.match(mainSource, /localBackendSpawnCoordinator\.request\(poolKey, \{ timeoutMs: POOL_SLOT_WAIT_MS \}\)/) + assert.match( + mainSource, + /localBackendSpawnCoordinator\.request\(poolKey, \{\s*timeoutMs: POOL_SLOT_WAIT_MS,\s*priority: spawnPriority\s*\}\)/ + ) assert.doesNotMatch(mainSource, /request\(poolKey, \{ timeoutMs: POOL_IDLE_MS \}\)/) }) diff --git a/apps/desktop/electron/pool-spawn-coordinator.ts b/apps/desktop/electron/pool-spawn-coordinator.ts index 8e565ed133343..5f80af332d304 100644 --- a/apps/desktop/electron/pool-spawn-coordinator.ts +++ b/apps/desktop/electron/pool-spawn-coordinator.ts @@ -1,17 +1,47 @@ export type ReleaseLocalBackendSlot = () => void +export type LocalBackendSpawnPriority = 'foreground' | 'background' + export type LocalBackendSpawnRequest = { acquired: Promise cancel: () => boolean + promote: (priority: LocalBackendSpawnPriority) => boolean + /** False when the slot was granted without waiting behind the queue. */ + queued: boolean } type Waiter = { key: string + priority: LocalBackendSpawnPriority resolve: (release: ReleaseLocalBackendSlot) => void reject: (error: Error) => void timer: ReturnType | null } +const SLOT_WAIT_TIMEOUT_MESSAGE = (key: string) => + `Local backend start for "${key}" timed out while waiting for a free slot.` + +/** + * Slot-wait timeout. Background hydrations set `silent` so call sites can fail + * quiet instead of toasting a user-visible backend-start failure. + */ +export class LocalBackendSlotWaitTimeoutError extends Error { + readonly priority: LocalBackendSpawnPriority + readonly silent: boolean + + constructor(key: string, priority: LocalBackendSpawnPriority) { + const suffix = priority === 'background' ? ' (background)' : '' + super(`${SLOT_WAIT_TIMEOUT_MESSAGE(key)}${suffix}`) + this.name = 'LocalBackendSlotWaitTimeoutError' + this.priority = priority + this.silent = priority === 'background' + } +} + +export function isBackgroundSlotWaitTimeout(error: unknown): boolean { + return error instanceof LocalBackendSlotWaitTimeoutError && error.silent +} + export async function releaseLocalBackendSlotAfterExit( release: ReleaseLocalBackendSlot, waitForExit: () => Promise @@ -25,10 +55,15 @@ export async function releaseLocalBackendSlotAfterExit( * * A lease is acquired immediately before local start work and is held until * the child exits or the start fails. Remote descriptors never call request(). + * + * When the cap is at least 2, one slot is reserved for foreground (user-open) + * requests so background roster hydration cannot occupy the whole pool. + * Untagged acquire() is foreground, so existing cap tests still fill `limit`. */ export class LocalBackendSpawnCoordinator { #limit: number - #active = 0 + #activeForeground = 0 + #activeBackground = 0 #queue: Waiter[] = [] constructor(limit: number) { @@ -40,7 +75,7 @@ export class LocalBackendSpawnCoordinator { } get activeCount(): number { - return this.#active + return this.#activeForeground + this.#activeBackground } get limit(): number { @@ -66,39 +101,47 @@ export class LocalBackendSpawnCoordinator { return this.#queue.length } - request(key: string, options: { timeoutMs?: number } = {}): LocalBackendSpawnRequest { + request( + key: string, + options: { timeoutMs?: number; priority?: LocalBackendSpawnPriority } = {} + ): LocalBackendSpawnRequest { if (options.timeoutMs !== undefined && (!Number.isFinite(options.timeoutMs) || options.timeoutMs < 1)) { throw new RangeError('Local backend spawn timeout must be a positive number.') } - if (this.#active < this.#limit) { + const priority: LocalBackendSpawnPriority = options.priority === 'background' ? 'background' : 'foreground' + + if (this.#queue.length === 0 && this.#canGrant(priority)) { return { - acquired: Promise.resolve(this.#grant()), - cancel: () => false + acquired: Promise.resolve(this.#grant(priority)), + cancel: () => false, + promote: () => false, + queued: false } } let waiter!: Waiter const acquired = new Promise((resolve, reject) => { - waiter = { key, resolve, reject, timer: null } + waiter = { key, priority, resolve, reject, timer: null } this.#queue.push(waiter) if (options.timeoutMs !== undefined) { waiter.timer = setTimeout(() => { - this.#rejectWaiter( - waiter, - new Error(`Local backend start for "${key}" timed out while waiting for a free slot.`) - ) + this.#rejectWaiter(waiter, this.#timeoutError(waiter)) }, options.timeoutMs) waiter.timer.unref?.() } }) + this.#drain() + return { acquired, cancel: () => - this.#rejectWaiter(waiter, new Error(`Local backend start for "${key}" was cancelled while queued.`)) + this.#rejectWaiter(waiter, new Error(`Local backend start for "${key}" was cancelled while queued.`)), + promote: (nextPriority: LocalBackendSpawnPriority) => this.#promoteWaiter(waiter, nextPriority), + queued: this.#queue.includes(waiter) } } @@ -106,6 +149,45 @@ export class LocalBackendSpawnCoordinator { return this.request(key).acquired } + #timeoutError(waiter: Waiter): Error { + if (waiter.priority === 'background') { + return new LocalBackendSlotWaitTimeoutError(waiter.key, 'background') + } + + return new Error(SLOT_WAIT_TIMEOUT_MESSAGE(waiter.key)) + } + + #backgroundLimit(): number { + return this.#limit >= 2 ? this.#limit - 1 : this.#limit + } + + #canGrant(priority: LocalBackendSpawnPriority): boolean { + if (this.activeCount >= this.#limit) { + return false + } + + if (priority === 'background' && this.#activeBackground >= this.#backgroundLimit()) { + return false + } + + return true + } + + #promoteWaiter(waiter: Waiter, priority: LocalBackendSpawnPriority): boolean { + if (!this.#queue.includes(waiter)) { + return false + } + + if (waiter.priority === priority) { + return false + } + + waiter.priority = priority + this.#drain() + + return true + } + #rejectWaiter(waiter: Waiter, error: Error): boolean { const index = this.#queue.indexOf(waiter) @@ -127,8 +209,13 @@ export class LocalBackendSpawnCoordinator { } } - #grant(): ReleaseLocalBackendSlot { - this.#active += 1 + #grant(priority: LocalBackendSpawnPriority): ReleaseLocalBackendSlot { + if (priority === 'background') { + this.#activeBackground += 1 + } else { + this.#activeForeground += 1 + } + let released = false return () => { @@ -137,17 +224,49 @@ export class LocalBackendSpawnCoordinator { } released = true - this.#active -= 1 + + if (priority === 'background') { + this.#activeBackground -= 1 + } else { + this.#activeForeground -= 1 + } + this.#drain() } } - /** Hand free slots to queued waiters while under the (possibly lowered) cap. */ + #takeWaiter(priority: LocalBackendSpawnPriority): Waiter | undefined { + const index = this.#queue.findIndex(waiter => waiter.priority === priority) + + if (index === -1) { + return undefined + } + + return this.#queue.splice(index, 1)[0] + } + + /** Hand free slots to queued waiters. Foreground waiters always go first. */ #drain(): void { - while (this.#active < this.#limit && this.#queue.length > 0) { - const next = this.#queue.shift()! + while (this.#canGrant('foreground')) { + const next = this.#takeWaiter('foreground') + + if (!next) { + break + } + + this.#clearTimer(next) + next.resolve(this.#grant('foreground')) + } + + while (this.#canGrant('background')) { + const next = this.#takeWaiter('background') + + if (!next) { + break + } + this.#clearTimer(next) - next.resolve(this.#grant()) + next.resolve(this.#grant('background')) } } } diff --git a/apps/desktop/electron/preload.ts b/apps/desktop/electron/preload.ts index fd9668752b193..ca8bb45a28200 100644 --- a/apps/desktop/electron/preload.ts +++ b/apps/desktop/electron/preload.ts @@ -18,7 +18,7 @@ contextBridge.exposeInMainWorld('hermesDesktop', { // Launch-flag fact: the app was started with --local, so the renderer may // show the local-models surfaces. Static for the window's lifetime. localModelsEnabled: launchFlags?.localModels === true, - getConnection: profile => ipcRenderer.invoke('hermes:connection', profile), + getConnection: (profile, opts) => ipcRenderer.invoke('hermes:connection', profile, opts), // Registry-scoped backend resolution: { connectionId, profile } → descriptor. getConnectionFor: payload => ipcRenderer.invoke('hermes:connection:for', payload), getProfileRoutes: profiles => ipcRenderer.invoke('hermes:plugin-profile-routes', profiles), diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 9c0b509641348..6a24a791c988e 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -1001,9 +1001,11 @@ export function useSessionActions({ // dial the owning backend without moving $activeGatewayProfile. if ($showAllProfiles.get()) { if (resolvedConnectionId) { - await openGatewayForAgent(resolvedConnectionId, ownerRoute?.profile || sessionProfile || 'default') + await openGatewayForAgent(resolvedConnectionId, ownerRoute?.profile || sessionProfile || 'default', { + spawnPriority: 'foreground' + }) } else if (sessionProfile) { - await openGatewayForProfile(normalizeProfileKey(sessionProfile)) + await openGatewayForProfile(normalizeProfileKey(sessionProfile), { spawnPriority: 'foreground' }) } } else if (resolvedConnectionId) { await ensureGatewayAgent(resolvedConnectionId, ownerRoute?.profile || sessionProfile || 'default') diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index 0362b8ff127df..43ff09810e729 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -20,12 +20,16 @@ declare global { // Resolve a backend connection. Omit `profile` (or pass the primary) for // the window's backend; pass a named profile to lazily spawn/reuse that // profile's backend from the pool. - getConnection: (profile?: string | null) => Promise + getConnection: ( + profile?: string | null, + opts?: { priority?: 'foreground' | 'background' } + ) => Promise // Registry-scoped backend resolution: dial (connectionId, profile). An // empty/local connectionId delegates to the legacy getConnection path. getConnectionFor?: (payload: { connectionId?: null | string profile?: null | string + priority?: 'foreground' | 'background' }) => Promise // Registry-scoped fresh WS URL (same result contract as getGatewayWsUrl). getGatewayWsUrlFor?: (payload: { diff --git a/apps/desktop/src/sdk/index.ts b/apps/desktop/src/sdk/index.ts index bf28c8fb16816..e69550dac7792 100644 --- a/apps/desktop/src/sdk/index.ts +++ b/apps/desktop/src/sdk/index.ts @@ -911,11 +911,14 @@ export const host = { // not the registry-secondary path openGatewayForAgent takes for a 'local' // connection id. Behavior for a plain local open is unchanged. const dial = explicitRoute - ? () => openGatewayForAgent(explicitRoute.connectionId, explicitRoute.profile) + ? () => + openGatewayForAgent(explicitRoute.connectionId, explicitRoute.profile, { + spawnPriority: 'foreground' + }) : plan.switchWorkspace ? () => ensureGatewayProfile(plan.switchWorkspace as string) : plan.dialWithoutSwitching - ? () => openGatewayForProfile(plan.dialWithoutSwitching as string) + ? () => openGatewayForProfile(plan.dialWithoutSwitching as string, { spawnPriority: 'foreground' }) : null if (dial) { diff --git a/apps/desktop/src/sdk/profile-routing.test.ts b/apps/desktop/src/sdk/profile-routing.test.ts index 83328ad1204b9..202b9fbaef4e0 100644 --- a/apps/desktop/src/sdk/profile-routing.test.ts +++ b/apps/desktop/src/sdk/profile-routing.test.ts @@ -607,7 +607,7 @@ describe('profile-aware plugin session opens', () => { await host.openSession('remote-chat', { route }) - expect(openGatewayForAgent).toHaveBeenCalledWith('source-a', 'default') + expect(openGatewayForAgent).toHaveBeenCalledWith('source-a', 'default', expect.objectContaining({ spawnPriority: 'foreground' })) expect(ensureGatewayProfile).not.toHaveBeenCalled() expect(setShowAllProfiles).toHaveBeenCalledWith(true) expect($activeGatewayProfile.get()).toBe('remote-worker') @@ -1158,7 +1158,7 @@ describe('profile-aware plugin session opens', () => { }) expect(ensureGatewayProfile).not.toHaveBeenCalled() - expect(openGatewayForProfile).toHaveBeenCalledWith('worker') + expect(openGatewayForProfile).toHaveBeenCalledWith('worker', expect.objectContaining({ spawnPriority: 'foreground' })) expect(setShowAllProfiles).toHaveBeenCalledWith(true) expect($activeGatewayProfile.get()).toBe('default') }) @@ -1169,7 +1169,7 @@ describe('profile-aware plugin session opens', () => { await host.openSession('bot-chat', { profile: 'worker' }) expect(ensureGatewayProfile).not.toHaveBeenCalled() - expect(openGatewayForProfile).toHaveBeenCalledWith('worker') + expect(openGatewayForProfile).toHaveBeenCalledWith('worker', expect.objectContaining({ spawnPriority: 'foreground' })) expect(setShowAllProfiles).toHaveBeenCalledWith(true) expect($activeGatewayProfile.get()).toBe('default') }) diff --git a/apps/desktop/src/store/gateway-spawn-priority.test.ts b/apps/desktop/src/store/gateway-spawn-priority.test.ts new file mode 100644 index 0000000000000..58ea0a5eb2886 --- /dev/null +++ b/apps/desktop/src/store/gateway-spawn-priority.test.ts @@ -0,0 +1,112 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +// #102281: a user-initiated open must reach Electron main as a FOREGROUND dial +// on its FIRST IPC, not only on the secondary's connect. Every open first +// probes the route (sharedPrimaryRoute / isAttachedSharedRemote) with +// getConnection / getConnectionFor; if that probe is untagged, main starts the +// spawn as a background slot wait and the click waits out the probe's 20s +// timeout before anything promotes it. + +vi.mock('@/hermes', () => ({ + setApiRequestConnection: vi.fn(), + HermesGateway: class { + connectionState = 'closed' + connect = async (): Promise => { + this.connectionState = 'open' + } + close = (): void => { + this.connectionState = 'closed' + } + onEvent = vi.fn(() => () => {}) + onState = vi.fn(() => () => {}) + } +})) +vi.mock('@/store/session', () => ({ setConnection: vi.fn(), setGatewayState: vi.fn() })) +vi.mock('@/store/notify-baseline', () => ({ markNativeNotifyBaseline: vi.fn() })) + +const { + closeSecondaryGateways, + configureGatewayRegistry, + ensureGatewayForAgent, + ensureGatewayForProfile, + openGatewayForAgent, + openGatewayForProfile, + setPrimaryGateway +} = await import('./gateway') + +const conn = { + 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 installDesktop(): { getConnection: ReturnType; getConnectionFor: ReturnType } { + const stub = { + getConnection: vi.fn(async () => conn), + getConnectionFor: vi.fn(async () => conn) + } + + ;(window as unknown as { hermesDesktop: unknown }).hermesDesktop = stub + + return stub +} + +function priorities(mock: ReturnType, pick: (args: unknown[]) => unknown): unknown[] { + return mock.mock.calls.map(args => pick(args)) +} + +beforeEach(() => { + configureGatewayRegistry({ onEvent: vi.fn() }) + setPrimaryGateway({ connectionState: 'open' } as never, 'default') +}) + +afterEach(() => { + closeSecondaryGateways() + vi.clearAllMocks() + delete (window as unknown as { hermesDesktop?: unknown }).hermesDesktop +}) + +describe('user opens dial main as foreground from the first IPC (#102281)', () => { + it('ensureGatewayForProfile tags the route probe AND the connect dial', async () => { + const desktop = installDesktop() + + await ensureGatewayForProfile('research') + + const seen = priorities(desktop.getConnection, args => (args[1] as { priority?: string } | undefined)?.priority) + expect(seen.length).toBeGreaterThanOrEqual(2) + expect(seen.every(priority => priority === 'foreground')).toBe(true) + }) + + it('openGatewayForProfile without a priority never tags a dial as foreground', async () => { + const desktop = installDesktop() + + await openGatewayForProfile('research') + + const seen = priorities(desktop.getConnection, args => (args[1] as { priority?: string } | undefined)?.priority) + expect(seen.length).toBeGreaterThanOrEqual(1) + expect(seen.every(priority => priority === undefined)).toBe(true) + }) + + it('openGatewayForAgent forwards spawnPriority to every registry dial', async () => { + const desktop = installDesktop() + + await openGatewayForAgent('homelab', 'research', { spawnPriority: 'foreground' }) + + const seen = priorities(desktop.getConnectionFor, args => (args[0] as { priority?: string }).priority) + expect(seen.length).toBeGreaterThanOrEqual(1) + expect(seen.every(priority => priority === 'foreground')).toBe(true) + }) + + it('ensureGatewayForAgent is always a foreground open', async () => { + const desktop = installDesktop() + + await ensureGatewayForAgent('homelab', 'research') + + const seen = priorities(desktop.getConnectionFor, args => (args[0] as { priority?: string }).priority) + expect(seen.length).toBeGreaterThanOrEqual(1) + expect(seen.every(priority => priority === 'foreground')).toBe(true) + }) +}) diff --git a/apps/desktop/src/store/gateway.ts b/apps/desktop/src/store/gateway.ts index 5bf36abaea48f..0812aec00f735 100644 --- a/apps/desktop/src/store/gateway.ts +++ b/apps/desktop/src/store/gateway.ts @@ -20,6 +20,26 @@ import { setConnection, setGatewayState } from '@/store/session' const normKey = (profile: string | null | undefined): string => (profile ?? '').trim() || 'default' +// Spawn-slot priority handed to Electron main with every backend dial. A +// user-initiated open is 'foreground' and may take the pool's reserved slot; +// roster hydration, hover prewarm and untagged dials are 'background' — main's +// default, so background dials keep the pre-priority IPC payload shape. +type SpawnPriority = 'foreground' | 'background' + +function dialPriority(spawnPriority: SpawnPriority): { priority: 'foreground' } | Record { + return spawnPriority === 'foreground' ? { priority: 'foreground' } : {} +} + +function dialProfile( + desktop: NonNullable, + profile: string, + spawnPriority: SpawnPriority +): Promise { + return spawnPriority === 'foreground' + ? desktop.getConnection(profile, { priority: 'foreground' }) + : desktop.getConnection(profile) +} + // Read connection state through a call so TS control-flow analysis doesn't // narrow the getter to a constant across guards (it genuinely changes). const isOpen = (gateway: HermesGateway | null): boolean => gateway?.connectionState === 'open' @@ -302,7 +322,11 @@ function isPrimaryRegistryRoute(connectionId: null | string, profile: string): b * dials a second WebSocket at the same Tailscale URL, which accept/closes in * ~30ms (`messages=1`) and never runs `session.create` (#96493). Isolated * SSH/pooled backends (`sharedRemote: false`) still get their own secondary. */ -async function isAttachedSharedRemote(connectionId: null | string, profile: string): Promise { +async function isAttachedSharedRemote( + connectionId: null | string, + profile: string, + spawnPriority: SpawnPriority = 'background' +): Promise { const id = String(connectionId ?? '').trim() const key = normKey(profile) @@ -322,7 +346,7 @@ async function isAttachedSharedRemote(connectionId: null | string, profile: stri try { const conn = await withTimeout( - desktop.getConnectionFor({ connectionId: id, profile: key }), + desktop.getConnectionFor({ connectionId: id, profile: key, ...dialPriority(spawnPriority) }), RECONNECT_ATTEMPT_TIMEOUT_MS, `Timed out resolving shared-remote route for "${key}"` ) @@ -489,7 +513,7 @@ function clearTimer(entry: Secondary): void { } } -async function openSecondary(entry: Secondary): Promise { +async function openSecondary(entry: Secondary, spawnPriority: SpawnPriority = 'background'): Promise { const desktop = window.hermesDesktop if (!desktop) { @@ -497,6 +521,20 @@ async function openSecondary(entry: Secondary): Promise { } if (entry.connectPromise) { + if (spawnPriority === 'foreground') { + // Hydration may already own this dial as a background slot wait. Kick a + // foreground IPC so main can promote it onto the reserved slot. + void ( + entry.connectionId && desktop.getConnectionFor + ? desktop.getConnectionFor({ + connectionId: entry.connectionId, + profile: entry.profile, + priority: 'foreground' + }) + : desktop.getConnection(entry.profile, { priority: 'foreground' }) + ).catch(() => undefined) + } + await entry.connectPromise return @@ -543,12 +581,16 @@ async function openSecondary(entry: Secondary): Promise { const conn = entry.connectionId && desktop.getConnectionFor ? await withTimeout( - desktop.getConnectionFor({ connectionId: entry.connectionId, profile: entry.profile }), + desktop.getConnectionFor({ + connectionId: entry.connectionId, + profile: entry.profile, + ...dialPriority(spawnPriority) + }), RECONNECT_ATTEMPT_TIMEOUT_MS, `Timed out connecting to profile "${entry.profile}"` ) : await withTimeout( - desktop.getConnection(entry.profile), + dialProfile(desktop, entry.profile, spawnPriority), RECONNECT_ATTEMPT_TIMEOUT_MS, `Timed out connecting to profile "${entry.profile}"` ) @@ -752,7 +794,7 @@ function createSecondary(profile: string, connectionId: null | string = null): S // the second dial fails (tunnel/token are per-backend) and the closed socket // poisons the active gateway with "not connected" even though the primary is // open right next to it. -async function sharedPrimaryRoute(profile: string): Promise { +async function sharedPrimaryRoute(profile: string, spawnPriority: SpawnPriority = 'background'): Promise { const desktop = window.hermesDesktop if (!desktop) { @@ -764,8 +806,11 @@ async function sharedPrimaryRoute(profile: string): Promise { // like any other failure, not hang the route decision forever, since // every caller (gatewayForProfile → requestGatewayForProfile/Agent) awaits // this before it can fall back to dialing a secondary. + // This is the FIRST dial main sees for a user open, so it must already + // carry the foreground priority — otherwise the spawn it starts queues as + // background and the click waits out this probe before being promoted. const conn = await withTimeout( - desktop.getConnection(profile), + dialProfile(desktop, profile, spawnPriority), RECONNECT_ATTEMPT_TIMEOUT_MS, `Timed out resolving the shared-primary route for profile "${profile}"` ) @@ -781,7 +826,8 @@ async function sharedPrimaryRoute(profile: string): Promise { // request-scope flag; dedicated local/remote profiles use their pooled socket. async function gatewayForProfile( profile: string, - leaseRequest = false + leaseRequest = false, + spawnPriority: SpawnPriority = 'background' ): Promise<{ gateway: HermesGateway | null; key: string; release: () => void; scopeProfile: boolean }> { const key = normKey(profile) const noRelease = () => undefined @@ -790,7 +836,7 @@ async function gatewayForProfile( return { gateway: g.primaryGateway, key, release: noRelease, scopeProfile: false } } - if (await sharedPrimaryRoute(key)) { + if (await sharedPrimaryRoute(key, spawnPriority)) { return { gateway: g.primaryGateway, key, release: noRelease, scopeProfile: true } } @@ -840,7 +886,7 @@ async function gatewayForProfile( try { if (!isOpen(entry.gateway)) { - await openSecondary(entry) + await openSecondary(entry, spawnPriority) } } catch (error) { release() @@ -1300,8 +1346,11 @@ function releaseTerminalTurnLease(scope: string, event: GatewayEvent): void { // it. No scheduleReconnect on failure: a hover is speculative, so a dead // backend must not start a background retry loop — the real switch owns retry // and error UX. An already-open (or primary) profile is a no-op. -export async function openGatewayForProfile(profile: string): Promise { - await gatewayForProfile(profile) +export async function openGatewayForProfile( + profile: string, + { spawnPriority = 'background' }: { spawnPriority?: SpawnPriority } = {} +): Promise { + await gatewayForProfile(profile, false, spawnPriority) } // ── Connection-scoped agents (multi-source roster) ───────────────────────── @@ -1321,15 +1370,18 @@ export async function openGatewayForProfile(profile: string): Promise { export async function openGatewayForAgent( connectionId: null | string, profile: string, - { activationLease = false }: { activationLease?: boolean } = {} + { + activationLease = false, + spawnPriority = 'background' + }: { activationLease?: boolean; spawnPriority?: SpawnPriority } = {} ): Promise { const scope = registryBackendScopeKey(connectionId, profile) if (scope === normKey(profile) || isPrimaryRegistryRoute(connectionId, profile)) { - return openGatewayForProfile(profile) + return openGatewayForProfile(profile, { spawnPriority }) } - if (await isAttachedSharedRemote(connectionId, profile)) { + if (await isAttachedSharedRemote(connectionId, profile, spawnPriority)) { if (!isOpen(g.primaryGateway)) { throw new Error('Hermes gateway unavailable') } @@ -1356,7 +1408,7 @@ export async function openGatewayForAgent( } try { - await openSecondary(entry) + await openSecondary(entry, spawnPriority) } catch (error) { if (activationLease) { entry.activationLeaseUntil = 0 @@ -1383,7 +1435,7 @@ export async function ensureGatewayForAgent( return !signal?.aborted } - if (await isAttachedSharedRemote(connectionId, profile)) { + if (await isAttachedSharedRemote(connectionId, profile, 'foreground')) { return Boolean(isOpen(g.primaryGateway) && !signal?.aborted) } @@ -1412,7 +1464,7 @@ export async function ensureGatewayForAgent( entry.reconnectAttempt = 0 try { - await openSecondary(entry) + await openSecondary(entry, 'foreground') } catch { scheduleReconnect(entry) } @@ -1465,7 +1517,7 @@ export async function ensureGatewayForProfile(profile: string): Promise { // 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. - if (await sharedPrimaryRoute(key)) { + if (await sharedPrimaryRoute(key, 'foreground')) { applyActive(g.primaryProfile, activationEpoch) return @@ -1489,7 +1541,7 @@ export async function ensureGatewayForProfile(profile: string): Promise { entry.reconnectAttempt = 0 try { - await openSecondary(entry) + await openSecondary(entry, 'foreground') } 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 diff --git a/apps/desktop/src/store/profile.ts b/apps/desktop/src/store/profile.ts index b608f165f0f19..44d4d6b1cad59 100644 --- a/apps/desktop/src/store/profile.ts +++ b/apps/desktop/src/store/profile.ts @@ -603,7 +603,10 @@ export async function openGatewayAgent(connectionId: string, profile: string): P return } - await openGatewayForAgent(connection, normalizeProfileKey(profile), { activationLease: true }) + await openGatewayForAgent(connection, normalizeProfileKey(profile), { + activationLease: true, + spawnPriority: 'foreground' + }) } // Activate a connection-scoped agent's gateway — the (connectionId, profile) diff --git a/contributors/emails/128559392+bounce12340@users.noreply.github.com b/contributors/emails/128559392+bounce12340@users.noreply.github.com new file mode 100644 index 0000000000000..cebf5875b559d --- /dev/null +++ b/contributors/emails/128559392+bounce12340@users.noreply.github.com @@ -0,0 +1,2 @@ +bounce12340 +# PR #102496 salvage