Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 43 additions & 11 deletions apps/desktop/electron/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,14 @@ import { poolTouchKeys } from './pool-touch-scope'
import { createKeepAwake } from './power-save'
import { FirstRunSetupResetError, runPrimaryBackendStartup } from './primary-backend-startup'
import { rehomePrimaryConnection } from './primary-connection-rehome'
import { decideProfileDeleteAction, profileNameFromDeleteRequest, resolveRouteProfile } from './profile-delete-routing'
import {
assertLocalProfileCanStart,
decideProfileDeleteAction,
localProfilePoolKeys,
ProfileDeletionGate,
profileNameFromDeleteRequest,
resolveRouteProfile
} from './profile-delete-routing'
import {
buildSidebarSessionSliceParams,
fetchPrimaryProfileSessions,
Expand Down Expand Up @@ -1176,6 +1183,7 @@ let softRehomeInProgress = false
// with no named profiles never populates this map, so their experience is
// byte-for-byte the single-backend behavior.
const backendPool = new Map() // profile -> { process, port, token, connectionPromise, lastActiveAt }
const profileDeletionGate = new ProfileDeletionGate()
// Keep the pool light: cap concurrent profile backends (LRU eviction) and reap
// idle ones. A user idles at exactly the primary backend; pool backends only
// exist while a non-primary profile is actively being chatted through.
Expand Down Expand Up @@ -9485,6 +9493,9 @@ function profileRouteOptions(profile) {
// primary, so legacy callers are unchanged.
async function ensureBackend(profile) {
const key = profile && String(profile).trim() ? String(profile).trim() : primaryProfileKey()

profileDeletionGate.assertCanStart(key)

const route = resolveProfileBackendRoute(key, profileRouteOptions(key))

if (route.backend === 'primary') {
Expand Down Expand Up @@ -9561,6 +9572,8 @@ async function ensureRegistryBackend(connectionId, profile) {
// can't collide with the v1 remote descriptor cached at the bare key.
const profileKey = String(profile ?? '').trim() || 'default'

profileDeletionGate.assertCanStart(profileKey)

const localRoute = resolveRegistryLocalRoute(profileKey, {
globalRemote: globalRemoteActive(),
profileRemoteOverride: Boolean(profileHasRemoteOverride(profileKey))
Expand Down Expand Up @@ -9813,13 +9826,16 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po
const poolKey = opts.poolKey || profile

await reapOrphanedBackendsOnce()
profileDeletionGate.assertCanStart(profile)

// A profile may point at its OWN remote backend (connection.json
// `profiles[name]`), or inherit the app-wide remote (env / global settings).
// In either case there is no local child to spawn β€” we just verify the
// remote is reachable and hand back its connection descriptor. The pool
// entry keeps `entry.process === null`, which stopPoolBackend/evict already
// tolerate.
const remote = opts.forceLocal ? null : await resolveRemoteBackend(profile)
profileDeletionGate.assertCanStart(profile)

if (remote) {
await waitForHermes(remote.baseUrl, remote.token, undefined, remote.authMode, remote.headers)
Expand Down Expand Up @@ -9858,6 +9874,8 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po
})
}

profileDeletionGate.assertCanStart(profile)

// --profile wins over the inherited HERMES_HOME env (see _apply_profile_override
// step 3 in hermes_cli/main.py), so the child re-homes to this profile.
// --port 0: the OS assigns an ephemeral port; the child announces it on stdout.
Expand All @@ -9872,6 +9890,9 @@ async function spawnPoolBackend(profile, entry, opts: { forceLocal?: boolean; po
rememberLog(`Starting Hermes backend for profile "${profile}" via ${backend.label}`)

const parentStartMarker = await desktopParentStartMarker()
assertLocalProfileCanStart(profile, profileDeletionGate, key =>
directoryExists(path.join(HERMES_HOME, 'profiles', key))
)
const backendNonce = crypto.randomBytes(16).toString('hex')
const parentIdentityEnv = parentWatchdogEnv(process.pid, parentStartMarker, backendNonce)

Expand Down Expand Up @@ -9993,17 +10014,16 @@ function stopPoolBackend(profile) {
}

async function teardownPoolBackendAndWait(profile) {
const entry = backendPool.get(profile)
const entries = localProfilePoolKeys(profile)
.map(key => ({ entry: backendPool.get(key), key }))
.filter(item => item.entry)

if (!entry) {
return
for (const { entry, key } of entries) {
backendPool.delete(key)
stopBackendChild(entry.process)
}

backendPool.delete(profile)

stopBackendChild(entry.process)

await waitForBackendExit(entry.process)
await Promise.all(entries.map(({ entry }) => waitForBackendExit(entry.process)))
}

function stopAllPoolBackends() {
Expand Down Expand Up @@ -10056,7 +10076,7 @@ async function prepareProfileDeleteRequest(request) {

if (decision.action === 'teardown-primary') {
writeActiveDesktopProfile('default')
await teardownPrimaryBackendAndWait()
await Promise.all([teardownPrimaryBackendAndWait(), teardownPoolBackendAndWait(decision.profile)])

return decision.profile
}
Expand Down Expand Up @@ -13026,7 +13046,7 @@ async function mergeRemoteProfileSessions(searchParams, remoteProfiles) {
}
}

ipcMain.handle('hermes:api', async (_event, request) => {
async function handleHermesApiRequest(request) {
// Registry-pinned request (request.connectionId): the renderer is working
// against a REGISTERED gateway connection, so the data β€” cron jobs and their
// run sessions included β€” lives in THAT host's state.db, not any local
Expand Down Expand Up @@ -13123,6 +13143,18 @@ ipcMain.handle('hermes:api', async (_event, request) => {
upload: request?.upload,
timeoutMs
})
}

ipcMain.handle('hermes:api', async (_event, request) => {
const deletingProfile = profileNameFromDeleteRequest(request)

if (!deletingProfile) {
return handleHermesApiRequest(request)
}

const releaseProfileDeletion = profileDeletionGate.acquire(deletingProfile)

return handleHermesApiRequest(request).finally(releaseProfileDeletion)
})

// One deduper per cross-window cue β€” the choke point every window shares. Main
Expand Down
69 changes: 68 additions & 1 deletion apps/desktop/electron/profile-delete-routing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@ import assert from 'node:assert/strict'

import { test } from 'vitest'

import { decideProfileDeleteAction, profileNameFromDeleteRequest, resolveRouteProfile } from './profile-delete-routing'
import {
assertLocalProfileCanStart,
decideProfileDeleteAction,
localProfilePoolKeys,
ProfileDeletionGate,
profileNameFromDeleteRequest,
resolveRouteProfile
} from './profile-delete-routing'

// ---------------------------------------------------------------------------
// profileNameFromDeleteRequest
Expand Down Expand Up @@ -80,3 +87,63 @@ test('resolveRouteProfile passes the requested profile through when nothing was
test('resolveRouteProfile passes through undefined when nothing was torn down and no profile was requested', () => {
assert.equal(resolveRouteProfile(null, undefined), undefined)
})

// ---------------------------------------------------------------------------
// ProfileDeletionGate / localProfilePoolKeys
// ---------------------------------------------------------------------------

test('ProfileDeletionGate blocks concurrent starts until deletion releases', () => {
const gate = new ProfileDeletionGate()
const release = gate.acquire('Selena')

assert.equal(gate.blocks('selena'), true)
assert.equal(gate.blocks('trina'), false)

release()
assert.equal(gate.blocks('selena'), false)
})

test('ProfileDeletionGate keeps overlapping deletion leases blocked', () => {
const gate = new ProfileDeletionGate()
const releaseFirst = gate.acquire('selena')
const releaseSecond = gate.acquire('selena')

releaseFirst()
assert.equal(gate.blocks('selena'), true)

releaseSecond()
assert.equal(gate.blocks('selena'), false)
})

test('ProfileDeletionGate rejects a deferred start when deletion begins while it waits', async () => {
const gate = new ProfileDeletionGate()
let continueStart = () => undefined

const waiting = new Promise<void>(resolve => {
continueStart = resolve
})

const start = (async () => {
await waiting
gate.assertCanStart('selena')
})()

const release = gate.acquire('selena')

continueStart()
await assert.rejects(start, /Profile "selena" is being deleted/)
release()
})

test('assertLocalProfileCanStart rejects a delayed retry after the profile directory is removed', () => {
const gate = new ProfileDeletionGate()

assert.throws(() => assertLocalProfileCanStart('selena', gate, () => false), /Profile "selena" no longer exists/)
assert.doesNotThrow(() => assertLocalProfileCanStart('default', gate, () => false))
assert.doesNotThrow(() => assertLocalProfileCanStart('selena', gate, profile => profile === 'selena'))
})

test('localProfilePoolKeys returns every local process scope for one profile', () => {
assert.deepEqual(localProfilePoolKeys('Selena'), ['selena', 'conn:local::selena'])
assert.deepEqual(localProfilePoolKeys(''), [])
})
97 changes: 94 additions & 3 deletions apps/desktop/electron/profile-delete-routing.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
// Profile-delete routing logic for the `hermes:api` IPC handler.
//
// When the renderer issues DELETE /api/profiles/<name>, the handler must
// tear down that profile's backend (primary window backend or pool backend)
// and then route the *next* request away from the just-deleted profile's
// pool backend -- spawning a fresh one would call ensure_hermes_home() and
// tear down every local backend for that profile and route the DELETE itself
// away from the just-deleted profile. Concurrent and delayed starts must also
// be rejected: spawning a fresh backend would call ensure_hermes_home() and
// recreate the profile directory the delete just removed, leaving a zombie
// process behind (issue #52279).
//
Expand Down Expand Up @@ -59,6 +59,97 @@ export interface ProfileDeleteDecisionDeps {
primaryProfileKey: () => string
}

/**
* Process-local barrier for profile deletion. Electron IPC handlers run
* concurrently, so tearing down a pooled backend is not enough by itself: a
* renderer reconnect can enter ensureBackend() while the DELETE request is
* still removing the profile and recreate its HERMES_HOME.
*
* Counts instead of a Set keep overlapping requests for the same profile
* blocked until the last request releases its lease.
*/
export class ProfileDeletionGate {
readonly #active = new Map<string, number>()

acquire(profile: unknown): () => void {
const key = String(profile ?? '')
.trim()
.toLowerCase()

if (!key) {
return () => undefined
}

this.#active.set(key, (this.#active.get(key) ?? 0) + 1)
let released = false

return () => {
if (released) {
return
}

released = true
const remaining = (this.#active.get(key) ?? 1) - 1

if (remaining > 0) {
this.#active.set(key, remaining)
} else {
this.#active.delete(key)
}
}
}

blocks(profile: unknown): boolean {
const key = String(profile ?? '')
.trim()
.toLowerCase()

return Boolean(key && this.#active.has(key))
}

assertCanStart(profile: unknown): void {
const key = String(profile ?? '').trim()

if (this.blocks(key)) {
throw new Error(`Profile "${key}" is being deleted.`)
}
}
}

/**
* Validate the final boundary before spawning a local profile backend. The
* deletion gate closes the in-flight race; the directory check rejects a
* delayed renderer retry after the DELETE request has already completed.
*/
export function assertLocalProfileCanStart(
profile: unknown,
gate: ProfileDeletionGate,
profileDirectoryExists: (profile: string) => boolean
): void {
const key = String(profile ?? '')
.trim()
.toLowerCase()

gate.assertCanStart(key)

if (key && key !== 'default' && !profileDirectoryExists(key)) {
throw new Error(`Profile "${key}" no longer exists.`)
}
}

/**
* A local profile can occupy the legacy bare pool slot or the explicit-local
* registry slot when the v1 route points elsewhere. Both processes own the
* same on-disk profile and must be stopped before deleting it.
*/
export function localProfilePoolKeys(profile: unknown): string[] {
const key = String(profile ?? '')
.trim()
.toLowerCase()

return key ? [key, `conn:local::${key}`] : []
}

/**
* Pure decision logic for prepareProfileDeleteRequest: given the parsed
* profile name (or null), decide which side-effecting branch the caller
Expand Down
Loading