diff --git a/apps/desktop/electron/connection-config.test.ts b/apps/desktop/electron/connection-config.test.ts index e8a4ca8e3803..5bae2e8e708a 100644 --- a/apps/desktop/electron/connection-config.test.ts +++ b/apps/desktop/electron/connection-config.test.ts @@ -35,8 +35,8 @@ import { profileHasRemoteConnection, profileRemoteOverride, profileSshOverride, - profileUsesPrimaryBackend, resolveAuthMode, + resolveProfileBackendRoute, resolveTestWsUrl, RT_COOKIE_VARIANTS, savedProfileSsh, @@ -188,42 +188,63 @@ test('saved SSH drafts are inactive and explicit overrides take precedence', () assert.equal(profileHasRemoteConnection(config, 'coder'), true) }) -// --- profileUsesPrimaryBackend --- +// --- resolveProfileBackendRoute --- -test('profileUsesPrimaryBackend keeps primary and empty scopes on the window backend', () => { - assert.equal(profileUsesPrimaryBackend('default', { primaryProfile: 'default' }), true) - assert.equal(profileUsesPrimaryBackend(' coder ', { primaryProfile: 'coder' }), true) - assert.equal(profileUsesPrimaryBackend('', { primaryProfile: 'default' }), true) -}) +const ROUTES = [ + { + name: 'the primary profile owns the window backend', + profile: 'default', + opts: { primaryProfile: 'default' }, + expected: { backend: 'primary', descriptorProfile: null, scopePath: false } + }, + { + name: 'a renamed primary profile still owns the window backend', + profile: ' coder ', + opts: { primaryProfile: 'coder', globalRemote: true }, + expected: { backend: 'primary', descriptorProfile: null, scopePath: false } + }, + { + name: 'an unset profile resolves to the primary', + profile: '', + opts: { primaryProfile: 'default', globalRemote: true }, + expected: { backend: 'primary', descriptorProfile: null, scopePath: false } + }, + { + name: 'a profile inheriting the app-global remote shares the primary backend, scoped per request', + profile: 'coder', + opts: { primaryProfile: 'default', globalRemote: true, profileRemoteOverride: false }, + expected: { backend: 'primary', descriptorProfile: 'coder', scopePath: true } + }, + { + name: 'a profile with its own remote override gets a pooled descriptor for that host', + profile: 'coder', + opts: { primaryProfile: 'default', globalRemote: true, profileRemoteOverride: true }, + expected: { backend: 'pool', descriptorProfile: null, scopePath: false } + }, + { + name: 'a local non-primary profile gets its own pooled backend', + profile: 'coder', + opts: { primaryProfile: 'default', globalRemote: false, profileRemoteOverride: false }, + expected: { backend: 'pool', descriptorProfile: null, scopePath: false } + } +] -test('profileUsesPrimaryBackend shares one app-global remote across profiles', () => { - assert.equal( - profileUsesPrimaryBackend('coder', { - primaryProfile: 'default', - globalRemote: true, - profileRemoteOverride: false - }), - true - ) -}) +for (const route of ROUTES) { + test(`resolveProfileBackendRoute: ${route.name}`, () => { + assert.deepEqual(resolveProfileBackendRoute(route.profile, route.opts), route.expected) + }) +} -test('profileUsesPrimaryBackend preserves profile overrides and local profile backends', () => { - assert.equal( - profileUsesPrimaryBackend('coder', { - primaryProfile: 'default', - globalRemote: true, - profileRemoteOverride: true - }), - false - ) - assert.equal( - profileUsesPrimaryBackend('coder', { - primaryProfile: 'default', - globalRemote: false, - profileRemoteOverride: false - }), - false - ) +test('resolveProfileBackendRoute only tags a descriptor when the backend is shared', () => { + // A pooled backend is already scoped to its profile, so tagging it would + // imply a second scope the caller must reconcile. Only the shared + // global-remote route carries one. + for (const route of ROUTES) { + const resolved = resolveProfileBackendRoute(route.profile, route.opts) + + assert.equal(Boolean(resolved.descriptorProfile), resolved.scopePath) + assert.ok(!resolved.descriptorProfile || resolved.backend === 'primary') + } }) // --- pathWithGlobalRemoteProfile --- @@ -238,6 +259,17 @@ test('pathWithGlobalRemoteProfile appends profile in global remote mode', () => ) }) +test('pathWithGlobalRemoteProfile skips the primary profile, which the remote already serves', () => { + assert.equal( + pathWithGlobalRemoteProfile('/api/model/info', 'coder', { + globalRemote: true, + primaryProfile: 'coder', + profileRemoteOverride: false + }), + '/api/model/info' + ) +}) + test('pathWithGlobalRemoteProfile preserves existing query params', () => { assert.equal( pathWithGlobalRemoteProfile('/api/model/options?force=1', 'iris', { diff --git a/apps/desktop/electron/connection-config.ts b/apps/desktop/electron/connection-config.ts index f5cd7763d7d5..c15644b112a0 100644 --- a/apps/desktop/electron/connection-config.ts +++ b/apps/desktop/electron/connection-config.ts @@ -350,34 +350,68 @@ function profileRemoteOverride(config, profile) { return { url, authMode: normAuthMode(entry.authMode), token: entry.token } } +export interface ProfileRouteOptions { + globalRemote?: boolean + primaryProfile?: null | string + profileRemoteOverride?: boolean +} + +export interface ProfileBackendRoute { + /** Which backend serves this profile: the window backend, or a pooled one. */ + backend: 'pool' | 'primary' + /** + * Profile to tag on the returned descriptor when the backend is shared and + * therefore not itself scoped to that profile. Null when the backend already + * belongs to the profile. + */ + descriptorProfile: null | string + /** Whether REST paths on this route must carry `?profile=` to be scoped. */ + scopePath: boolean +} + /** - * Decide whether a profile should reuse the primary backend connection. + * The one place that answers "which backend serves profile P, and does its + * REST path need a profile scope?". Four routes, in precedence order: * - * The primary profile always owns the window backend. In app-global remote - * mode, that same backend serves every profile through request-level profile - * scoping, unless a profile has its own remote connection override. + * 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 + * `--profile`, so its `HERMES_HOME` scopes it. + * + * Routing used to be spread across three overlapping predicates that each + * re-derived part of this table, which is how case 3 ended up registering + * reapable pool entries for backends it never owned. */ -function profileUsesPrimaryBackend(profile, opts: any = {}) { +function resolveProfileBackendRoute(profile, opts: ProfileRouteOptions = {}): ProfileBackendRoute { const scopedProfile = connectionScopeKey(profile) const primaryProfile = connectionScopeKey(opts.primaryProfile) || 'default' if (!scopedProfile || scopedProfile === primaryProfile) { - return true + return { backend: 'primary', descriptorProfile: null, scopePath: false } } - return Boolean(opts.globalRemote) && !opts.profileRemoteOverride + if (opts.profileRemoteOverride) { + return { backend: 'pool', descriptorProfile: null, scopePath: false } + } + + if (opts.globalRemote) { + return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true } + } + + return { backend: 'pool', descriptorProfile: null, scopePath: false } } /** - * In global-remote mode one backend serves every Desktop profile, so REST calls - * that are scoped by renderer-side `request.profile` must carry that scope as a - * query parameter. Local pooled backends and per-profile remote overrides do not - * need this: they already run against a backend scoped to the target profile. + * Add renderer-side `request.profile` to a REST path when the route says the + * serving backend is not already scoped to that profile. */ -function pathWithGlobalRemoteProfile(path, profile, opts: any = {}) { +function pathWithGlobalRemoteProfile(path, profile, opts: ProfileRouteOptions = {}) { const scopedProfile = connectionScopeKey(profile) - if (!scopedProfile || !opts.globalRemote || opts.profileRemoteOverride) { + if (!resolveProfileBackendRoute(profile, opts).scopePath) { return path } @@ -523,8 +557,8 @@ export { profileHasRemoteConnection, profileRemoteOverride, profileSshOverride, - profileUsesPrimaryBackend, resolveAuthMode, + resolveProfileBackendRoute, resolveTestWsUrl, RT_COOKIE_VARIANTS, savedProfileSsh, diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 25154f9161c3..d88e1f787601 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -63,8 +63,8 @@ import { profileHasRemoteConnection, profileRemoteOverride, profileSshOverride, - profileUsesPrimaryBackend, resolveAuthMode, + resolveProfileBackendRoute, resolveTestWsUrl, savedProfileSsh, tokenPreview @@ -7742,33 +7742,28 @@ function primaryProfileKey() { return readActiveDesktopProfile() || 'default' } -// Resolve a backend connection for the given profile. The primary profile uses -// startHermes() (the window backend: boot UI, bootstrap, remote mode). Profiles -// inheriting an app-global remote share that same connection because the remote -// backend scopes their requests by profile. Other profiles use lazily-spawned -// pool backends. An empty / unknown profile resolves to the primary. -async function ensureBackend(profile) { - const primaryProfile = primaryProfileKey() - const key = profile && String(profile).trim() ? String(profile).trim() : primaryProfile - - if (key === primaryProfile) { - return startHermes() +// Options describing the current connection setup for `resolveProfileBackendRoute`. +function profileRouteOptions(profile) { + return { + globalRemote: globalRemoteActive(), + primaryProfile: primaryProfileKey(), + profileRemoteOverride: Boolean(profileHasRemoteOverride(profile)) } +} - if ( - globalRemoteActive() && - profileUsesPrimaryBackend(key, { - primaryProfile, - globalRemote: true, - profileRemoteOverride: profileHasRemoteOverride(key) - }) - ) { +// 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) { + const key = profile && String(profile).trim() ? String(profile).trim() : primaryProfileKey() + const route = resolveProfileBackendRoute(key, profileRouteOptions(key)) + + if (route.backend === 'primary') { const connection = await startHermes() - // Non-primary global-remote profiles still need their scope on the - // descriptor so renderer-side WebSocket, filesystem, and cache routing use - // the selected profile while sharing the one underlying backend. - return { ...connection, profile: key } + // A shared backend still owes the caller its profile scope, so renderer-side + // WebSocket, filesystem, and cache routing target the selected profile. + return route.descriptorProfile ? { ...connection, profile: route.descriptorProfile } : connection } const existing = backendPool.get(key) @@ -9908,10 +9903,7 @@ ipcMain.handle('hermes:api', async (_event, request) => { const connection = await ensureBackend(routeProfile) const timeoutMs = resolveTimeoutMs(request?.timeoutMs, DEFAULT_FETCH_TIMEOUT_MS) - const requestPath = pathWithGlobalRemoteProfile(request.path, profile, { - globalRemote: globalRemoteActive(), - profileRemoteOverride: profileHasRemoteOverride(profile) - }) + const requestPath = pathWithGlobalRemoteProfile(request.path, profile, profileRouteOptions(profile)) const url = `${connection.baseUrl}${requestPath}`