fix(desktop): avoid local profile REST backend spawns
This commit is contained in:
@@ -41,6 +41,7 @@ import {
|
||||
profileSshOverride,
|
||||
remoteRequestMatchesBaseUrl,
|
||||
resolveAuthMode,
|
||||
resolveProfileApiRequest,
|
||||
resolveProfileBackendRoute,
|
||||
resolveRemoteSshDashboardProfile,
|
||||
resolveTestWsUrl,
|
||||
@@ -354,9 +355,15 @@ const ROUTES = [
|
||||
expected: { backend: 'pool', descriptorProfile: null, scopePath: false }
|
||||
},
|
||||
{
|
||||
name: 'a local non-primary profile gets its own pooled backend',
|
||||
name: 'an unscoped local profile request keeps its pooled backend',
|
||||
profile: 'coder',
|
||||
opts: { primaryProfile: 'default', globalRemote: false, profileRemoteOverride: false },
|
||||
opts: {
|
||||
primaryProfile: 'default',
|
||||
globalRemote: false,
|
||||
profileRemoteOverride: false,
|
||||
requestMethod: 'POST',
|
||||
requestPath: '/api/memory/reset'
|
||||
},
|
||||
expected: { backend: 'pool', descriptorProfile: null, scopePath: false }
|
||||
},
|
||||
{
|
||||
@@ -382,6 +389,44 @@ const ROUTES = [
|
||||
ownEntry: true
|
||||
},
|
||||
expected: { backend: 'pool', descriptorProfile: null, scopePath: false }
|
||||
},
|
||||
{
|
||||
name: 'a profile-aware local REST request reuses the primary backend',
|
||||
profile: 'coder',
|
||||
opts: {
|
||||
primaryProfile: 'default',
|
||||
globalRemote: false,
|
||||
profileRemoteOverride: false,
|
||||
requestMethod: 'GET',
|
||||
requestPath: '/api/config'
|
||||
},
|
||||
expected: { backend: 'primary', descriptorProfile: 'coder', scopePath: true }
|
||||
},
|
||||
{
|
||||
name: 'a profile-management request uses the primary without a query scope',
|
||||
profile: 'coder',
|
||||
opts: {
|
||||
primaryProfile: 'default',
|
||||
globalRemote: false,
|
||||
profileRemoteOverride: false,
|
||||
requestMethod: 'DELETE',
|
||||
requestPath: '/api/profiles/worker'
|
||||
},
|
||||
expected: { backend: 'primary', descriptorProfile: null, scopePath: false }
|
||||
},
|
||||
{
|
||||
name: 'a stored local profile never reuses a remote primary for an eligible REST route',
|
||||
profile: 'coder',
|
||||
opts: {
|
||||
primaryProfile: 'default',
|
||||
globalRemote: false,
|
||||
profileRemoteOverride: false,
|
||||
primaryRemoteActive: true,
|
||||
ownEntry: true,
|
||||
requestMethod: 'GET',
|
||||
requestPath: '/api/config'
|
||||
},
|
||||
expected: { backend: 'pool', descriptorProfile: null, scopePath: false }
|
||||
}
|
||||
]
|
||||
|
||||
@@ -547,6 +592,27 @@ test('translateSelfProfileQuery no-ops when alias and backend profile agree or a
|
||||
assert.equal(translateSelfProfileQuery('/api/cron/jobs?profile=mara', '', 'default'), '/api/cron/jobs?profile=mara')
|
||||
})
|
||||
|
||||
test('pathWithGlobalRemoteProfile appends local-primary profile scope only for eligible routes', () => {
|
||||
assert.equal(
|
||||
pathWithGlobalRemoteProfile('/api/config', 'iris', {
|
||||
globalRemote: false,
|
||||
profileRemoteOverride: false,
|
||||
requestMethod: 'GET',
|
||||
requestPath: '/api/config'
|
||||
}),
|
||||
'/api/config?profile=iris'
|
||||
)
|
||||
assert.equal(
|
||||
pathWithGlobalRemoteProfile('/api/memory/reset', 'iris', {
|
||||
globalRemote: false,
|
||||
profileRemoteOverride: false,
|
||||
requestMethod: 'POST',
|
||||
requestPath: '/api/memory/reset'
|
||||
}),
|
||||
'/api/memory/reset'
|
||||
)
|
||||
})
|
||||
|
||||
test('pathWithGlobalRemoteProfile skips empty profile/path safely', () => {
|
||||
assert.equal(
|
||||
pathWithGlobalRemoteProfile('/api/model/info', '', {
|
||||
@@ -564,6 +630,130 @@ test('pathWithGlobalRemoteProfile skips empty profile/path safely', () => {
|
||||
)
|
||||
})
|
||||
|
||||
// --- resolveProfileApiRequest ---
|
||||
|
||||
test('resolveProfileApiRequest keeps eligible local REST on the primary backend', () => {
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/config?view=desktop', {
|
||||
globalRemote: false,
|
||||
profileRemoteOverride: false,
|
||||
requestMethod: 'GET'
|
||||
}),
|
||||
{
|
||||
backendProfile: null,
|
||||
requestPath: '/api/config?view=desktop&profile=iris'
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
test('resolveProfileApiRequest keeps unscoped destructive routes on the profile backend', () => {
|
||||
for (const [method, path] of [
|
||||
['POST', '/api/memory/reset'],
|
||||
['POST', '/api/curator/run'],
|
||||
['PUT', '/api/curator/paused'],
|
||||
['POST', '/api/webhooks']
|
||||
]) {
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', path, {
|
||||
globalRemote: false,
|
||||
profileRemoteOverride: false,
|
||||
requestMethod: method
|
||||
}),
|
||||
{ backendProfile: 'iris', requestPath: path }
|
||||
)
|
||||
}
|
||||
})
|
||||
|
||||
test('resolveProfileApiRequest uses exact method and path eligibility for mixed families', () => {
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/skills', {
|
||||
requestMethod: 'GET'
|
||||
}),
|
||||
{ backendProfile: null, requestPath: '/api/skills?profile=iris' }
|
||||
)
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/skills', {
|
||||
requestMethod: 'POST'
|
||||
}),
|
||||
{ backendProfile: 'iris', requestPath: '/api/skills' }
|
||||
)
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/config/defaults', {
|
||||
requestMethod: 'GET'
|
||||
}),
|
||||
{ backendProfile: 'iris', requestPath: '/api/config/defaults' }
|
||||
)
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/model/recommended-default?provider=nous', {
|
||||
requestMethod: 'GET'
|
||||
}),
|
||||
{
|
||||
backendProfile: 'iris',
|
||||
requestPath: '/api/model/recommended-default?provider=nous'
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
test('resolveProfileApiRequest scopes complete safe families according to their contracts', () => {
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/tools/toolsets/image_gen/config', {
|
||||
requestMethod: 'GET'
|
||||
}),
|
||||
{
|
||||
backendProfile: null,
|
||||
requestPath: '/api/tools/toolsets/image_gen/config?profile=iris'
|
||||
}
|
||||
)
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/profiles/worker', {
|
||||
requestMethod: 'DELETE'
|
||||
}),
|
||||
{
|
||||
backendProfile: null,
|
||||
requestPath: '/api/profiles/worker'
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
test('resolveProfileApiRequest preserves remote routing precedence', () => {
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/memory/reset', {
|
||||
globalRemote: true,
|
||||
profileRemoteOverride: false,
|
||||
requestMethod: 'POST'
|
||||
}),
|
||||
{
|
||||
backendProfile: null,
|
||||
requestPath: '/api/memory/reset?profile=iris'
|
||||
}
|
||||
)
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/config', {
|
||||
globalRemote: true,
|
||||
profileRemoteOverride: true,
|
||||
requestMethod: 'GET'
|
||||
}),
|
||||
{
|
||||
backendProfile: 'iris',
|
||||
requestPath: '/api/config'
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
test('resolveProfileApiRequest keeps a stored local profile off a remote primary', () => {
|
||||
assert.deepEqual(
|
||||
resolveProfileApiRequest('iris', '/api/config', {
|
||||
primaryRemoteActive: true,
|
||||
ownEntry: true,
|
||||
requestMethod: 'GET'
|
||||
}),
|
||||
{
|
||||
backendProfile: 'iris',
|
||||
requestPath: '/api/config'
|
||||
}
|
||||
)
|
||||
})
|
||||
|
||||
// --- normalizeRemoteBaseUrl ---
|
||||
|
||||
test('normalizeRemoteBaseUrl strips trailing slashes, hash, and query', () => {
|
||||
|
||||
@@ -506,6 +506,8 @@ export interface ProfileRouteOptions {
|
||||
primaryRemoteActive?: boolean
|
||||
/** A stored per-profile entry exists for this profile (local or remote). */
|
||||
ownEntry?: boolean
|
||||
requestMethod?: null | string
|
||||
requestPath?: null | string
|
||||
}
|
||||
|
||||
export interface ProfileBackendRoute {
|
||||
@@ -521,16 +523,83 @@ export interface ProfileBackendRoute {
|
||||
scopePath: boolean
|
||||
}
|
||||
|
||||
const LOCAL_PRIMARY_SCOPED_ROUTES = new Set([
|
||||
'GET /api/config',
|
||||
'PUT /api/config',
|
||||
'GET /api/config/raw',
|
||||
'PUT /api/config/raw',
|
||||
'GET /api/config/schema',
|
||||
'DELETE /api/env',
|
||||
'GET /api/env',
|
||||
'PUT /api/env',
|
||||
'POST /api/env/reveal',
|
||||
'GET /api/model/auxiliary',
|
||||
'GET /api/model/info',
|
||||
'GET /api/model/moa',
|
||||
'PUT /api/model/moa',
|
||||
'GET /api/model/options',
|
||||
'POST /api/model/set',
|
||||
'GET /api/skills',
|
||||
'GET /api/skills/content',
|
||||
'PUT /api/skills/toggle',
|
||||
'POST /api/skills/hub/install',
|
||||
'GET /api/skills/hub/preview',
|
||||
'GET /api/skills/hub/scan',
|
||||
'GET /api/skills/hub/search',
|
||||
'GET /api/skills/hub/sources',
|
||||
'POST /api/skills/hub/uninstall',
|
||||
'POST /api/skills/hub/update'
|
||||
])
|
||||
|
||||
function localPrimaryRequestScope(opts: ProfileRouteOptions): boolean | null {
|
||||
const rawPath = String(opts.requestPath || '')
|
||||
|
||||
if (!rawPath) {
|
||||
return null
|
||||
}
|
||||
|
||||
let pathname
|
||||
|
||||
try {
|
||||
pathname = new URL(rawPath, 'https://example.invalid').pathname
|
||||
} catch {
|
||||
return null
|
||||
}
|
||||
|
||||
const method = String(opts.requestMethod || 'GET').toUpperCase()
|
||||
|
||||
if (LOCAL_PRIMARY_SCOPED_ROUTES.has(`${method} ${pathname}`)) {
|
||||
return true
|
||||
}
|
||||
|
||||
// Every current /api/tools handler accepts `profile`; every /api/profiles
|
||||
// handler either aggregates profiles or names its target in the path/body.
|
||||
// These are the only whole families safe to route through the primary.
|
||||
if (pathname === '/api/tools' || pathname.startsWith('/api/tools/')) {
|
||||
return true
|
||||
}
|
||||
|
||||
if (pathname === '/api/profiles' || pathname.startsWith('/api/profiles/')) {
|
||||
return false
|
||||
}
|
||||
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* The one place that answers "which backend serves profile P, and does its
|
||||
* REST path need a profile scope?". Four routes, in precedence order:
|
||||
* REST path need a profile scope?". Six routes, in precedence order:
|
||||
*
|
||||
* 1. The primary profile owns the window backend outright.
|
||||
* 2. A profile with its own remote override gets a pooled descriptor for that
|
||||
* host, which is already scoped to it.
|
||||
* 3. A profile inheriting the app-global remote shares the primary backend —
|
||||
* one host serves every profile — so it is scoped per request instead.
|
||||
* 4. Any other local profile gets its own pooled backend, spawned with
|
||||
* 4. An unknown profile under a remote primary shares that remote backend.
|
||||
* A stored local profile remains isolated in its own backend.
|
||||
* 5. A local profile REST request that the primary backend can safely scope
|
||||
* reuses that backend, with `?profile=` when the handler accepts it.
|
||||
* 6. Any other local profile gets its own pooled backend, spawned with
|
||||
* `--profile`, so its `HERMES_HOME` scopes it.
|
||||
*
|
||||
* Routing used to be spread across three overlapping predicates that each
|
||||
@@ -553,12 +622,28 @@ function resolveProfileBackendRoute(profile, opts: ProfileRouteOptions = {}): Pr
|
||||
return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true }
|
||||
}
|
||||
|
||||
if (opts.primaryRemoteActive && !opts.ownEntry) {
|
||||
// The primary profile's own backend is a remote gateway (per-profile
|
||||
// override or env) and this sub-profile has no stored entry of its own.
|
||||
// Route through that gateway with profile scoping instead of spawning a
|
||||
// fresh local backend that shares nothing but the name (#88296).
|
||||
return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true }
|
||||
if (opts.primaryRemoteActive) {
|
||||
if (!opts.ownEntry) {
|
||||
// The primary profile's own backend is a remote gateway (per-profile
|
||||
// override or env) and this sub-profile has no stored entry of its own.
|
||||
// Route through that gateway with profile scoping instead of spawning a
|
||||
// fresh local backend that shares nothing but the name (#88296).
|
||||
return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true }
|
||||
}
|
||||
|
||||
// A stored local profile must not be redirected into the remote primary,
|
||||
// even when its REST endpoint supports profile scoping.
|
||||
return { backend: 'pool', descriptorProfile: null, scopePath: false }
|
||||
}
|
||||
|
||||
const localScope = localPrimaryRequestScope(opts)
|
||||
|
||||
if (localScope !== null) {
|
||||
return {
|
||||
backend: 'primary',
|
||||
descriptorProfile: localScope ? scopedProfile : null,
|
||||
scopePath: localScope
|
||||
}
|
||||
}
|
||||
|
||||
return { backend: 'pool', descriptorProfile: null, scopePath: false }
|
||||
@@ -680,6 +765,29 @@ function apiRequestRegistryConnectionId(request): null | string {
|
||||
return id
|
||||
}
|
||||
|
||||
export interface ProfileApiRequestRoute {
|
||||
/** Profile passed to ensureBackend; null selects the primary backend. */
|
||||
backendProfile: null | string
|
||||
requestPath: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the two decisions made by the `hermes:api` IPC handler from the same
|
||||
* routing table: which backend serves the request, and whether its URL needs a
|
||||
* profile query scope.
|
||||
*/
|
||||
function resolveProfileApiRequest(profile, path, opts: ProfileRouteOptions = {}): ProfileApiRequestRoute {
|
||||
const scopedProfile = connectionScopeKey(profile)
|
||||
const requestPath = String(path || '')
|
||||
const routeOpts = { ...opts, requestPath }
|
||||
const route = resolveProfileBackendRoute(scopedProfile, routeOpts)
|
||||
|
||||
return {
|
||||
backendProfile: route.backend === 'pool' ? scopedProfile : null,
|
||||
requestPath: pathWithGlobalRemoteProfile(requestPath, scopedProfile, routeOpts)
|
||||
}
|
||||
}
|
||||
|
||||
function tokenPreview(value) {
|
||||
const raw = String(value || '')
|
||||
|
||||
@@ -822,6 +930,7 @@ export {
|
||||
profileSshOverride,
|
||||
remoteRequestMatchesBaseUrl,
|
||||
resolveAuthMode,
|
||||
resolveProfileApiRequest,
|
||||
resolveProfileBackendRoute,
|
||||
resolveRemoteSshDashboardProfile,
|
||||
resolveTestWsUrl,
|
||||
|
||||
@@ -86,6 +86,7 @@ import {
|
||||
profileSshOverride,
|
||||
remoteRequestMatchesBaseUrl,
|
||||
resolveAuthMode,
|
||||
resolveProfileApiRequest,
|
||||
resolveProfileBackendRoute,
|
||||
resolveRemoteSshDashboardProfile,
|
||||
resolveTestWsUrl,
|
||||
@@ -9484,7 +9485,7 @@ function primaryProfileKey() {
|
||||
}
|
||||
|
||||
// Options describing the current connection setup for `resolveProfileBackendRoute`.
|
||||
function profileRouteOptions(profile) {
|
||||
function profileRouteOptions(profile, request?) {
|
||||
const config = readDesktopConnectionConfig()
|
||||
const sshOverride = profileSshOverride(config, profile)
|
||||
const key = connectionScopeKey(profile) || primaryProfileKey()
|
||||
@@ -9502,7 +9503,9 @@ function profileRouteOptions(profile) {
|
||||
primaryRemoteActive: primaryBackendIsRemote(),
|
||||
// A stored per-profile entry (local or remote) — pins this profile to
|
||||
// its own backend; absent entries inherit the primary's remote.
|
||||
ownEntry: Boolean((readDesktopConnectionConfig().profiles || {})[key])
|
||||
ownEntry: Boolean((config.profiles || {})[key]),
|
||||
requestMethod: request?.method,
|
||||
requestPath: request?.path
|
||||
}
|
||||
}
|
||||
|
||||
@@ -13228,13 +13231,17 @@ async function handleHermesApiRequest(request) {
|
||||
// backend instead of spawning a fresh pool backend. A freshly spawned
|
||||
// backend calls ensure_hermes_home() which recreates the profile directory,
|
||||
// defeating the deletion and leaving a zombie process.
|
||||
const routeProfile = resolveRouteProfile(tornDownProfile, profile)
|
||||
//
|
||||
// Safe local-profile REST calls also stay on the primary dashboard and carry
|
||||
// ?profile=. Endpoints that cannot honor that scope retain their pooled
|
||||
// backend so a destructive call can never fall through to the primary home.
|
||||
const apiRoute = resolveProfileApiRequest(profile, request.path, profileRouteOptions(profile, request))
|
||||
const routeProfile = resolveRouteProfile(tornDownProfile, apiRoute.backendProfile)
|
||||
|
||||
const connection = await ensureBackend(routeProfile)
|
||||
const timeoutMs = resolveTimeoutMs(request?.timeoutMs, DEFAULT_FETCH_TIMEOUT_MS)
|
||||
|
||||
const requestPath = pathWithGlobalRemoteProfile(request.path, profile, profileRouteOptions(profile))
|
||||
|
||||
const url = `${connection.baseUrl}${requestPath}`
|
||||
const url = `${connection.baseUrl}${apiRoute.requestPath}`
|
||||
|
||||
// OAuth gateways authenticate REST via EITHER a native bearer token
|
||||
// (cookieless RFC 8252 flow) OR the HttpOnly session cookie held in the OAuth
|
||||
|
||||
@@ -147,3 +147,7 @@ test('localProfilePoolKeys returns every local process scope for one profile', (
|
||||
assert.deepEqual(localProfilePoolKeys('Selena'), ['selena', 'conn:local::selena'])
|
||||
assert.deepEqual(localProfilePoolKeys(''), [])
|
||||
})
|
||||
|
||||
test('resolveRouteProfile preserves a primary-backend route from another routing policy', () => {
|
||||
assert.equal(resolveRouteProfile(null, null), null)
|
||||
})
|
||||
|
||||
@@ -180,7 +180,7 @@ export function decideProfileDeleteAction(
|
||||
*/
|
||||
export function resolveRouteProfile(
|
||||
tornDownProfile: string | null,
|
||||
profile: string | undefined
|
||||
profile: string | null | undefined
|
||||
): string | null | undefined {
|
||||
return tornDownProfile ? null : profile
|
||||
}
|
||||
|
||||
@@ -249,9 +249,10 @@ export class HermesGateway extends JsonRpcGatewayClient {
|
||||
// Profile that profile-scoped REST settings (config/env/skills/tools/model/…)
|
||||
// should target. Mirrors $activeGatewayProfile, pushed in from the store via
|
||||
// setApiRequestProfile so this module needs no store import (avoids a cycle).
|
||||
// Electron main consumes request.profile to pick which backend *process* serves
|
||||
// the call; each pooled backend already has its own HERMES_HOME, so no backend
|
||||
// change is needed. Null → primary, so single-profile users are unaffected.
|
||||
// Electron main consumes request.profile as request scope. Local calls whose
|
||||
// REST handlers accept profile reuse the primary dashboard via ?profile=;
|
||||
// unscoped handlers retain a profile backend. Remote overrides still route to
|
||||
// their owning backend. Null → primary, so single-profile users are unaffected.
|
||||
let _apiProfile: null | string = null
|
||||
|
||||
export function setApiRequestProfile(profile: null | string): void {
|
||||
@@ -678,10 +679,8 @@ export async function listSidebarSessions(req: SidebarSessionsRequest): Promise<
|
||||
}
|
||||
}
|
||||
|
||||
// Mutations take the owning `profile` so Electron routes them to that profile's
|
||||
// backend (remote pool or local primary) via request.profile — matching the
|
||||
// read path. A remote session's row lives only on its remote host, so a mutation
|
||||
// that hit the local primary would no-op or 404. Omit for the current/default.
|
||||
// Mutations take the owning `profile` so Electron can route them to the correct
|
||||
// remote backend or local profile scope. Omit for the current/default profile.
|
||||
export function setSessionArchived(id: string, archived: boolean, profile?: string | null): Promise<{ ok: boolean }> {
|
||||
return window.hermesDesktop.api<{ ok: boolean }>({
|
||||
...(profile ? { profile } : {}),
|
||||
|
||||
Reference in New Issue
Block a user