mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-11-06 05:17:15 +00:00
fix(desktop): never idle-reap a local profile backend (#134942)
* fix(desktop): never idle-reap a local profile backend Support f502bc6f: with a remote primary, a "This device" profile's local `hermes serve` child was SIGTERM'd by the pool idle reaper after 60 min with nothing streamed (the #105239 pinned-tier TTL), leaving "This device · Backend offline" while the user still had the chat open. A local backend runs its profile's cron jobs and bot chats with nobody watching, so idleness is not a reason to kill it. The idle reaper now only reclaims connection descriptors (remote routes: no child, no slot). Local children are still bounded by the maxBackends slot cap and foreground reclaim (activeTurn vetoed), unchanged. Removes POOL_PINNED_IDLE_MS. The Backend Idle Timeout setting description now says it applies to remote connections only. E2E no-idle-reap.spec.ts: a pooled local profile backend survives two reaper ticks past both idle windows. Fails on main (backend reaped), passes here. * chore(desktop): drop the dead pinned-tier idle policy (retireIdle, lastStreamedAt) Greptile P2 on #134942: with local children no longer idle-reaped, retireIdle has no production caller and lastStreamedAt no reader. Removes both, the stamp write in touchPoolBackend, and the pinned-tier test. The shared retirement safeguards stay covered through evictTo (identity, admission permit, activeTurn veto).
This commit is contained in:
Teknium authored and GitHub committed
1 parent
b1a39a1b1b
commit
167e9fdb84
5 files changed
+124
-168
No files matched your search
@@ -0,0 +1,66 @@
|
||||
/**
|
||||
* Support f502bc6f: a "This device" profile's local `hermes serve` child was SIGTERM'd by the pool
|
||||
* idle reaper (60 min with nothing streamed), leaving "This device · Backend offline". A local
|
||||
* child hosts its profile's cron jobs and bot chats with nobody watching, so it is never retired
|
||||
* for being idle. The idle windows are pinned to their 60s floor so two reaper ticks fit the test.
|
||||
*/
|
||||
|
||||
import * as path from 'node:path'
|
||||
|
||||
import { expect, test } from '@playwright/test'
|
||||
|
||||
import {
|
||||
backendProcesses,
|
||||
coreAppEnv,
|
||||
createCoreSandbox,
|
||||
launchCoreApp,
|
||||
waitForInteractive,
|
||||
writeProviderHome
|
||||
} from './harness'
|
||||
import { startScriptedProvider } from './provider'
|
||||
|
||||
const IDLE_MS = 60_000
|
||||
|
||||
test('a local profile backend is never idle-reaped', async () => {
|
||||
test.setTimeout(300_000)
|
||||
const provider = await startScriptedProvider()
|
||||
const box = createCoreSandbox('no-idle-reap')
|
||||
writeProviderHome(box.hermesHome, provider.url)
|
||||
writeProviderHome(path.join(box.hermesHome, 'profiles', 'reviewer'), provider.url)
|
||||
|
||||
const { app, page } = await launchCoreApp(
|
||||
coreAppEnv(box, {
|
||||
HERMES_DESKTOP_POOL_IDLE_MS: String(IDLE_MS),
|
||||
HERMES_DESKTOP_POOL_PINNED_IDLE_MS: String(IDLE_MS),
|
||||
// One process per profile: the pooled child the reaper used to kill.
|
||||
HERMES_DESKTOP_ISOLATED_BACKEND: '1'
|
||||
})
|
||||
)
|
||||
|
||||
try {
|
||||
await waitForInteractive(app, page)
|
||||
const before = new Set(backendProcesses(box).map(p => p.pid))
|
||||
|
||||
await page.evaluate(() =>
|
||||
(window as any).hermesDesktop.getConnectionFor({
|
||||
connectionId: 'local',
|
||||
profile: 'reviewer',
|
||||
priority: 'foreground'
|
||||
})
|
||||
)
|
||||
|
||||
const child = backendProcesses(box).find(p => !before.has(p.pid))
|
||||
expect(child, 'a pooled backend for reviewer').toBeDefined()
|
||||
|
||||
// Two reaper ticks past both idle windows, with nothing touching or streaming.
|
||||
await page.waitForTimeout(IDLE_MS + 2 * 60_000 + 5_000)
|
||||
|
||||
expect(
|
||||
backendProcesses(box).map(p => p.pid),
|
||||
'reviewer backend still running'
|
||||
).toContain(child!.pid)
|
||||
} finally {
|
||||
await app.close().catch(() => undefined)
|
||||
await provider.close()
|
||||
}
|
||||
})
|
||||
@@ -432,7 +432,7 @@ import {
|
||||
localRouteFallbackProfiles,
|
||||
undialedSshRouteSeeds
|
||||
} from './plugin-profile-routes'
|
||||
import { clampPoolLimits, parsePoolLimits, POOL_LIMITS_DEFAULTS, POOL_LIMITS_MIN } from './pool-limits'
|
||||
import { clampPoolLimits, parsePoolLimits, POOL_LIMITS_DEFAULTS } from './pool-limits'
|
||||
import { createPoolRetirer } from './pool-retire'
|
||||
import { createPoolRetirementClient } from './pool-retire-http'
|
||||
import {
|
||||
@@ -2188,20 +2188,6 @@ const POOL_KEEPALIVE_FRESH_MS = Math.max(
|
||||
Number(process.env.HERMES_DESKTOP_POOL_KEEPALIVE_FRESH_MS) || 4 * 60_000
|
||||
)
|
||||
|
||||
// Pinned-tier TTL (#105239): the renderer's 60s keepalive (touchPoolBackend)
|
||||
// refreshes lastActiveAt for every OPEN chat, so the idle reaper's only clock
|
||||
// never fires for the pinned tier — every profile whose chat was ever opened
|
||||
// held its ~120 MB serve child until app quit (126 processes / 7.5 GB on the
|
||||
// reporter's machine, all parented to Hermes.exe). A keepalive proves the
|
||||
// chat is open, not that anything streamed: retire a local child whose last
|
||||
// streamed turn is older than this window. Re-focusing the chat re-ensures it
|
||||
// idempotently (ensureBackend/ensureRegistryBackend reuse), and mid-stream
|
||||
// safety is unchanged — activeTurn entries are excluded by the retirer.
|
||||
const POOL_PINNED_IDLE_MS = Math.max(
|
||||
POOL_LIMITS_MIN.idleMs,
|
||||
Number(process.env.HERMES_DESKTOP_POOL_PINNED_IDLE_MS) || 60 * 60_000
|
||||
)
|
||||
|
||||
let poolIdleReaper = null
|
||||
let backendOrphanReapPromise = null
|
||||
// Auto-reload budget for renderer crashes, shared by EVERY window (primary,
|
||||
@@ -11967,13 +11953,6 @@ function touchPoolBackend(profile, options: { activeTurn?: boolean } = {}) {
|
||||
|
||||
if (typeof options.activeTurn === 'boolean') {
|
||||
entry.activeTurn = options.activeTurn
|
||||
|
||||
// A prompt turn leasing this backend IS streamed activity (#105239):
|
||||
// the keepalive touch alone only proves the chat is open, so the
|
||||
// pinned-tier TTL reads this stamp, not lastActiveAt.
|
||||
if (options.activeTurn) {
|
||||
entry.lastStreamedAt = Date.now()
|
||||
}
|
||||
}
|
||||
|
||||
return
|
||||
@@ -12004,29 +11983,15 @@ function startPoolIdleReaper() {
|
||||
const now = Date.now()
|
||||
|
||||
for (const [profile, entry] of [...backendPool.entries()]) {
|
||||
// Remote descriptors hold no child/slot. Local children require the
|
||||
// same admission authority as foreground and LRU reclamation.
|
||||
// Pinned-tier TTL (#105239): the keepalive refreshes lastActiveAt for
|
||||
// every open chat, so that clock alone never fires for the pinned tier.
|
||||
// A local child whose last STREAMED turn (activeTurn touch) is older
|
||||
// than POOL_PINNED_IDLE_MS is idle even while keepalive-fresh; entries
|
||||
// without the stamp keep the legacy lastActiveAt clock.
|
||||
const idleFor = now - (entry.lastActiveAt || 0)
|
||||
const streamedIdleFor = entry.lastStreamedAt ? now - entry.lastStreamedAt : null
|
||||
const reapable = idleFor > poolIdleMs() || (streamedIdleFor !== null && streamedIdleFor > POOL_PINNED_IDLE_MS)
|
||||
|
||||
if (reapable) {
|
||||
const retiring = entry.process
|
||||
? poolRetirer.retireIdle(profile, poolIdleMs(), candidate =>
|
||||
Boolean(
|
||||
Date.now() - (candidate.lastActiveAt || 0) > poolIdleMs() ||
|
||||
(candidate.lastStreamedAt ? Date.now() - candidate.lastStreamedAt > POOL_PINNED_IDLE_MS : false)
|
||||
)
|
||||
)
|
||||
: stopPoolBackend(profile)
|
||||
|
||||
void retiring.catch(error => rememberLog(`Pool idle retirement failed: ${String(error)}`))
|
||||
// Only connection descriptors (no child, no slot) are idle-reclaimed. A local `hermes serve` child is
|
||||
// never retired for being idle: it runs its profile's cron jobs and bot chats with nobody watching,
|
||||
// and a reaped one left "This device · Backend offline" (support f502bc6f). The slot cap and
|
||||
// foreground reclaim still bound how many run; activeTurn is vetoed there as before.
|
||||
if (entry.process || now - (entry.lastActiveAt || 0) <= poolIdleMs()) {
|
||||
continue
|
||||
}
|
||||
|
||||
void stopPoolBackend(profile).catch(error => rememberLog(`Pool idle retirement failed: ${String(error)}`))
|
||||
}
|
||||
|
||||
if (backendPool.size === 0 && poolIdleReaper) {
|
||||
|
||||
@@ -5,66 +5,60 @@ import { test } from 'vitest'
|
||||
import { createPoolRetirer, type PoolRetireEntry, selectRetirementCandidates } from './pool-retire'
|
||||
import { LocalBackendSpawnCoordinator } from './pool-spawn-coordinator'
|
||||
|
||||
test('idle and LRU retirement require backend authority, unchanged identity and current eligibility', async () => {
|
||||
for (const path of ['idle', 'lru'] as const) {
|
||||
for (const outcome of ['busy', 'unknown', 'expired', 'replaced', 'fresh', 'idle'] as const) {
|
||||
const entry: PoolRetireEntry = { process: {}, lastActiveAt: 1 }
|
||||
const pool = new Map([['a', entry]])
|
||||
const cancelled: string[] = []
|
||||
const stopped: string[] = []
|
||||
const events: string[] = []
|
||||
test('LRU retirement requires backend authority, unchanged identity and current eligibility', async () => {
|
||||
for (const outcome of ['busy', 'unknown', 'expired', 'replaced', 'fresh', 'idle'] as const) {
|
||||
const entry: PoolRetireEntry = { process: {}, lastActiveAt: 1 }
|
||||
const pool = new Map([['a', entry]])
|
||||
const cancelled: string[] = []
|
||||
const stopped: string[] = []
|
||||
const events: string[] = []
|
||||
|
||||
const retirer = createPoolRetirer({
|
||||
pool,
|
||||
coordinator: new LocalBackendSpawnCoordinator(3),
|
||||
prepare: async () => {
|
||||
if (outcome === 'replaced') {
|
||||
pool.set('a', { process: {}, lastActiveAt: 1 })
|
||||
}
|
||||
|
||||
if (outcome === 'fresh') {
|
||||
entry.lastActiveAt = Date.now()
|
||||
}
|
||||
|
||||
return outcome === 'busy' || outcome === 'unknown' ? null : 'permit'
|
||||
},
|
||||
commit: async () => {
|
||||
events.push('commit')
|
||||
|
||||
return outcome !== 'expired'
|
||||
},
|
||||
cancel: async key => {
|
||||
cancelled.push(key)
|
||||
},
|
||||
onRetiring: () => {
|
||||
events.push('park')
|
||||
},
|
||||
stopBackend: async key => {
|
||||
events.push('stop')
|
||||
stopped.push(key)
|
||||
pool.delete(key)
|
||||
}
|
||||
})
|
||||
|
||||
try {
|
||||
if (path === 'idle') {
|
||||
await retirer.retireIdle('a', 1000)
|
||||
} else {
|
||||
await retirer.evictTo(0, 1000)
|
||||
const retirer = createPoolRetirer({
|
||||
pool,
|
||||
coordinator: new LocalBackendSpawnCoordinator(3),
|
||||
prepare: async () => {
|
||||
if (outcome === 'replaced') {
|
||||
pool.set('a', { process: {}, lastActiveAt: 1 })
|
||||
}
|
||||
|
||||
assert.deepEqual(stopped, outcome === 'idle' ? ['a'] : [], `${path}: ${outcome}`)
|
||||
|
||||
if (outcome === 'idle') {
|
||||
assert.deepEqual(events, ['commit', 'park', 'stop'])
|
||||
if (outcome === 'fresh') {
|
||||
entry.lastActiveAt = Date.now()
|
||||
}
|
||||
|
||||
if (['expired', 'replaced', 'fresh'].includes(outcome)) {
|
||||
assert.deepEqual(cancelled, ['a'])
|
||||
}
|
||||
} finally {
|
||||
retirer.dispose()
|
||||
return outcome === 'busy' || outcome === 'unknown' ? null : 'permit'
|
||||
},
|
||||
commit: async () => {
|
||||
events.push('commit')
|
||||
|
||||
return outcome !== 'expired'
|
||||
},
|
||||
cancel: async key => {
|
||||
cancelled.push(key)
|
||||
},
|
||||
onRetiring: () => {
|
||||
events.push('park')
|
||||
},
|
||||
stopBackend: async key => {
|
||||
events.push('stop')
|
||||
stopped.push(key)
|
||||
pool.delete(key)
|
||||
}
|
||||
})
|
||||
|
||||
try {
|
||||
await retirer.evictTo(0, 1000)
|
||||
|
||||
assert.deepEqual(stopped, outcome === 'idle' ? ['a'] : [], outcome)
|
||||
|
||||
if (outcome === 'idle') {
|
||||
assert.deepEqual(events, ['commit', 'park', 'stop'])
|
||||
}
|
||||
|
||||
if (['expired', 'replaced', 'fresh'].includes(outcome)) {
|
||||
assert.deepEqual(cancelled, ['a'])
|
||||
}
|
||||
} finally {
|
||||
retirer.dispose()
|
||||
}
|
||||
}
|
||||
})
|
||||
@@ -83,48 +77,3 @@ test('candidate selection excludes processless descriptors, renderer-leased work
|
||||
['old', 'fresh']
|
||||
)
|
||||
})
|
||||
|
||||
test('retireIdle honours the pinned-tier eligibility override while every other safeguard stays armed (#105239)', async () => {
|
||||
// The keepalive refreshes lastActiveAt for every open chat, so a pinned
|
||||
// backend's legacy clock never expires. The reaper passes an override that
|
||||
// also reads lastStreamedAt; the retirer must still veto an active turn,
|
||||
// require the admission permit, and keep identity checks — only the idle
|
||||
// PREDICATE changes.
|
||||
for (const outcome of ['stale-streamed', 'fresh-streamed', 'mid-turn'] as const) {
|
||||
const entry: PoolRetireEntry = {
|
||||
process: {},
|
||||
lastActiveAt: Date.now(),
|
||||
// keepalive-fresh in every case; only streamed-activity age varies.
|
||||
lastStreamedAt: outcome === 'fresh-streamed' ? Date.now() - 5 * 60_000 : Date.now() - 2 * 60 * 60_000,
|
||||
...(outcome === 'mid-turn' ? { activeTurn: true } : {})
|
||||
}
|
||||
|
||||
const pool = new Map([['pinned', entry]])
|
||||
const stopped: string[] = []
|
||||
|
||||
const retirer = createPoolRetirer({
|
||||
pool,
|
||||
coordinator: new LocalBackendSpawnCoordinator(3),
|
||||
prepare: async () => 'permit',
|
||||
commit: async () => true,
|
||||
cancel: async () => {},
|
||||
stopBackend: async key => {
|
||||
stopped.push(key)
|
||||
pool.delete(key)
|
||||
}
|
||||
})
|
||||
|
||||
try {
|
||||
await retirer.retireIdle('pinned', 10 * 60_000, candidate =>
|
||||
Boolean(candidate.lastStreamedAt && Date.now() - (candidate.lastStreamedAt || 0) > 60 * 60_000)
|
||||
)
|
||||
} finally {
|
||||
retirer.dispose()
|
||||
}
|
||||
|
||||
// stale-streamed (keepalive-fresh, no streamed turn for 2h) retires;
|
||||
// fresh-streamed fails the override and stays; mid-turn is vetoed by the
|
||||
// retirer even though the override said yes.
|
||||
assert.deepEqual(stopped, outcome === 'stale-streamed' ? ['pinned'] : [], outcome)
|
||||
}
|
||||
})
|
||||
@@ -4,13 +4,6 @@ export interface PoolRetireEntry {
|
||||
/** An early veto only; backend admission owns the proof. */
|
||||
activeTurn?: boolean
|
||||
lastActiveAt?: null | number
|
||||
/**
|
||||
* Last prompt turn that leased this backend (#105239): a keepalive touch
|
||||
* only proves the chat is open, while this stamp proves streamed activity.
|
||||
* The idle reaper reads it for the pinned-tier TTL; absent means the entry
|
||||
* predates the stamp and keeps the legacy lastActiveAt clock.
|
||||
*/
|
||||
lastStreamedAt?: null | number
|
||||
process?: unknown
|
||||
}
|
||||
|
||||
@@ -154,23 +147,6 @@ export function createPoolRetirer<E extends PoolRetireEntry>(deps: PoolRetirerDe
|
||||
throw new Error(`Backend for "${key}" was retired; open it explicitly to reconnect.`)
|
||||
}
|
||||
},
|
||||
retireIdle: (key: string, idleMs: number, idleEligibility?: (entry: E) => boolean) =>
|
||||
enqueue(async () => {
|
||||
const entry = deps.pool.get(key)
|
||||
|
||||
if (!entry) {
|
||||
return false
|
||||
}
|
||||
|
||||
// The default clock is the keepalive-touched lastActiveAt; the reaper
|
||||
// passes an override that also honours the pinned-tier TTL
|
||||
// (lastStreamedAt) so a keepalive-fresh but stream-idle entry can
|
||||
// still be retired (#105239). The retirer keeps every other
|
||||
// safeguard: identity, admission permit, activeTurn veto, proof probe.
|
||||
return retire(key, entry, () =>
|
||||
idleEligibility ? idleEligibility(entry) : Date.now() - (entry.lastActiveAt || 0) > idleMs
|
||||
)
|
||||
}),
|
||||
evictTo: (keep: number, freshMs: number) =>
|
||||
enqueue(async () => {
|
||||
const retired: string[] = []
|
||||
|
||||
@@ -109,7 +109,7 @@ export function PoolLimitsSetting() {
|
||||
<span className="text-[length:var(--conversation-caption-font-size)] text-(--ui-text-tertiary)">ms</span>
|
||||
</div>
|
||||
}
|
||||
description="How long an unused bot backend stays warm before it is shut down. Raise this so bots you revisit every few minutes never pay a cold start."
|
||||
description="How long an unused remote bot connection stays cached before it is dropped. Local backends keep running while idle, since they run cron jobs and bot chats."
|
||||
id={settingElementId(SETTING_IDS.advanced.backendIdleTimeout)}
|
||||
title={t.settings.poolLimits.backendIdleTimeoutTitle}
|
||||
/>
|
||||
|
||||
Reference in new issue
Block a user