diff --git a/apps/desktop/src/app/messaging/index.tsx b/apps/desktop/src/app/messaging/index.tsx index 0a1cc8e7ff..1611dcf3c2 100644 --- a/apps/desktop/src/app/messaging/index.tsx +++ b/apps/desktop/src/app/messaging/index.tsx @@ -28,7 +28,7 @@ import { normalize } from '@/lib/text' import { cn } from '@/lib/utils' import { $changeEventsAvailable, $pairingChangeTick, $platformsChangeTick } from '@/store/live-sync' import { notify, notifyError } from '@/store/notifications' -import { $settingsScopeOverride } from '@/store/settings-scope' +import { $settingsRequestProfile } from '@/store/settings-scope' import { runGatewayRestart } from '@/store/system-actions' import { useRefreshHotkey } from '../hooks/use-refresh-hotkey' @@ -128,10 +128,9 @@ function fieldCopy(field: MessagingEnvVarInfo, m: Translations['messaging']) { export function MessagingView({ setStatusbarItemGroup: _setStatusbarItemGroup, ...props }: MessagingViewProps) { const { t } = useI18n() const m = t.messaging - // Shared settings "Applies to" scope. At this layer `null` means "follow - // the active profile", while the API helpers use `undefined` for that - // contract; passing null would deliberately target the primary profile. - const scopeProfile = useStore($settingsScopeOverride) ?? undefined + // Shared settings "Applies to" scope, request-shaped (undefined → follow + // the active profile; the API helpers treat null as "target primary"). + const scopeProfile = useStore($settingsRequestProfile) // Both save/toggle toasts offer the same one-click restart. const restartGatewayAction = { label: t.commandCenter.restartGateway, onClick: () => void runGatewayRestart() } const [platforms, setPlatforms] = useState(null) diff --git a/apps/desktop/src/app/settings/config-settings.tsx b/apps/desktop/src/app/settings/config-settings.tsx index c44dd8b6a7..ee69a841fc 100644 --- a/apps/desktop/src/app/settings/config-settings.tsx +++ b/apps/desktop/src/app/settings/config-settings.tsx @@ -24,7 +24,7 @@ import { $keepAwake, setKeepAwake } from '@/store/keep-awake' import { notify, notifyError } from '@/store/notifications' import { normalizeProfileKey } from '@/store/profile' import { repoDiscoveryPolicyFromConfig, repoDiscoveryPolicySignature, scanAndRecordRepos } from '@/store/projects' -import { $settingsScopeOverride } from '@/store/settings-scope' +import { $settingsRequestProfile } from '@/store/settings-scope' import type { ConfigFieldSchema, HermesConfigRecord } from '@/types/hermes' import { hermesConfigCacheWriter, useHermesConfigRecord } from '../hooks/use-config-record' @@ -58,7 +58,7 @@ export function ConfigSettings({ // inner page per scope so every draft/seed/autosave ref resets wholesale // when the target profile changes — the same guarantee useOnProfileSwitch // provides for app-wide switches, without hand-clearing each piece. - const scopeProfile = useStore($settingsScopeOverride) + const scopeProfile = useStore($settingsRequestProfile) return ( getHermesConfigSchema(scopeProfile ?? undefined), + queryFn: () => getHermesConfigSchema(scopeProfile), staleTime: 5 * 60 * 1000 }) @@ -147,7 +147,7 @@ function ConfigSettingsInner({ useEffect(() => { let cancelled = false - getElevenLabsVoices(scopeProfile ?? undefined) + getElevenLabsVoices(scopeProfile) .then(result => { if (cancelled || !result.available) { return @@ -178,7 +178,7 @@ function ConfigSettingsInner({ const t = window.setTimeout(() => { void (async () => { try { - const result = await saveHermesConfig(config, scopeProfile ?? undefined) + const result = await saveHermesConfig(config, scopeProfile) if (!result.ok) { throw new Error(c.autosaveFailed) diff --git a/apps/desktop/src/app/settings/env-credentials.tsx b/apps/desktop/src/app/settings/env-credentials.tsx index 0b4daf8e2c..490d08c066 100644 --- a/apps/desktop/src/app/settings/env-credentials.tsx +++ b/apps/desktop/src/app/settings/env-credentials.tsx @@ -43,8 +43,10 @@ export function SettingsCategoryHeading({ count, icon: Icon, title }: CategoryHe // credential pages (Providers, Keys) share one source of truth and one set of // mutation handlers instead of duplicating the plumbing. An optional `profile` // targets another profile's env store (the shared settings "Applies to" -// scope); undefined/null keeps the app-wide active profile. -export function useEnvCredentials(profile: null | string = null): UseEnvCredentials { +// scope); undefined keeps the app-wide active profile. Request-shaped on +// purpose: the API helpers treat an explicit `null` as "target the +// primary/default backend", which is never what a settings page means. +export function useEnvCredentials(profile?: string): UseEnvCredentials { const { t } = useI18n() const credentials = t.settings.credentials const toolsets = t.settings.toolsets diff --git a/apps/desktop/src/app/settings/keys-settings.test.tsx b/apps/desktop/src/app/settings/keys-settings.test.tsx index 87217b2e15..699fd0e4a7 100644 --- a/apps/desktop/src/app/settings/keys-settings.test.tsx +++ b/apps/desktop/src/app/settings/keys-settings.test.tsx @@ -12,7 +12,7 @@ stubResizeObserver() vi.mock('@/hermes', () => ({ deleteEnvVar: vi.fn(), - getEnvVars: () => getEnvVars(), + getEnvVars: (profile?: null | string) => getEnvVars(profile), revealEnvVar: vi.fn(), setApiRequestProfile: () => undefined, setEnvVar: vi.fn() @@ -54,6 +54,15 @@ function DeepLinkButton({ target }: { target: string }) { } describe('KeysSettings', () => { + it('fetches env vars for the active profile (undefined, never null) when unscoped', async () => { + // #90549 class: getEnvVars(null) targets the primary profile's env store, + // so a non-default profile's Keys page would read (and edit) the wrong + // profile. Unscoped must send undefined so the active profile applies. + await renderKeysSettings('tools') + + await waitFor(() => expect(getEnvVars).toHaveBeenCalledWith(undefined)) + }) + it('lists tools and excludes settings / channel-managed credentials', async () => { getEnvVars.mockResolvedValue({ BRAVE_SEARCH_API_KEY: envVar('tool', { description: 'Search the web with Brave.' }), diff --git a/apps/desktop/src/app/settings/keys-settings.tsx b/apps/desktop/src/app/settings/keys-settings.tsx index aeb24b49be..c5f239cc6f 100644 --- a/apps/desktop/src/app/settings/keys-settings.tsx +++ b/apps/desktop/src/app/settings/keys-settings.tsx @@ -2,7 +2,7 @@ import { useStore } from '@nanostores/react' import { useCallback, useEffect, useMemo, useState } from 'react' import { useI18n } from '@/i18n' -import { $settingsScopeOverride } from '@/store/settings-scope' +import { $settingsRequestProfile } from '@/store/settings-scope' import { CredentialKeyCard, credentialPlaceholder, credentialRowLabel } from './credential-key-ui' import { useEnvCredentials } from './env-credentials' @@ -35,8 +35,10 @@ const credentialElementId = (key: string) => `credential-key-${key}` export function KeysSettings({ view }: KeysSettingsProps) { const { t } = useI18n() // Shared settings "Applies to" scope: fetch + edit the selected profile's - // env store instead of the active one (null → active, the default path). - const scopeProfile = useStore($settingsScopeOverride) + // env store instead of the active one (undefined → active, the default + // path — request-shaped so the API helpers never see a primary-targeting + // null). + const scopeProfile = useStore($settingsRequestProfile) const { rowProps, vars } = useEnvCredentials(scopeProfile) const [openKey, setOpenKey] = useState(null) diff --git a/apps/desktop/src/app/settings/memory/connect.tsx b/apps/desktop/src/app/settings/memory/connect.tsx index 64d9cfa6fa..1c093179fe 100644 --- a/apps/desktop/src/app/settings/memory/connect.tsx +++ b/apps/desktop/src/app/settings/memory/connect.tsx @@ -12,7 +12,7 @@ const POLL_TIMEOUT_MS = 120_000 // Small connect affordance rendered under the provider dropdown. Capability is // backend-driven: the status route 404s for providers without an oauth_flow // module, so non-OAuth providers render nothing. -export function MemoryConnect({ profile = null, provider }: { profile?: null | string; provider: string }) { +export function MemoryConnect({ profile, provider }: { profile?: string; provider: string }) { const [capable, setCapable] = useState<'no' | 'unknown' | 'yes'>('unknown') const [connected, setConnected] = useState(false) const [auth, setAuth] = useState(null) diff --git a/apps/desktop/src/app/settings/memory/provider-config-panel.tsx b/apps/desktop/src/app/settings/memory/provider-config-panel.tsx index 2d7b6ae0c4..b4b5fcc1b2 100644 --- a/apps/desktop/src/app/settings/memory/provider-config-panel.tsx +++ b/apps/desktop/src/app/settings/memory/provider-config-panel.tsx @@ -20,7 +20,7 @@ function seedValues(config: MemoryProviderConfig): Record { ) } -export function ProviderConfigPanel({ profile = null, provider }: { profile?: null | string; provider: string }) { +export function ProviderConfigPanel({ profile, provider }: { profile?: string; provider: string }) { const [config, setConfig] = useState(null) const [loadError, setLoadError] = useState(null) const [values, setValues] = useState>({}) diff --git a/apps/desktop/src/app/settings/model-settings.test.tsx b/apps/desktop/src/app/settings/model-settings.test.tsx index 5343e2ef00..97d9eb84b4 100644 --- a/apps/desktop/src/app/settings/model-settings.test.tsx +++ b/apps/desktop/src/app/settings/model-settings.test.tsx @@ -28,11 +28,12 @@ const startManualProviderOAuth = vi.fn() let profileSwitchHandler: (() => void) | null = null vi.mock('@/hermes', () => ({ - getGlobalModelInfo: () => getGlobalModelInfo(), - getGlobalModelOptions: () => getGlobalModelOptions(), - getAuxiliaryModels: () => getAuxiliaryModels(), + getGlobalModelInfo: (profile?: null | string) => getGlobalModelInfo(profile), + getGlobalModelOptions: (opts?: unknown, profile?: null | string) => getGlobalModelOptions(opts, profile), + getAuxiliaryModels: (profile?: null | string) => getAuxiliaryModels(profile), getApiRequestProfile: () => 'default', - getMoaModels: () => getMoaModels(), + getMoaModels: (profile?: null | string) => getMoaModels(profile), + profileScopeKey: (scope?: null | string) => (scope ?? '').trim() || 'default', setModelAssignment: (body: unknown) => setModelAssignment(body), getRecommendedDefaultModel: (slug: string) => getRecommendedDefaultModel(slug), saveMoaModels: (body: unknown) => saveMoaModels(body), @@ -85,7 +86,7 @@ afterEach(() => { profileSwitchHandler = null }) -async function renderModelSettings() { +async function renderModelSettings(scopeProfile?: string) { const { ModelSettings } = await import('./model-settings') const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }) @@ -94,12 +95,36 @@ async function renderModelSettings() { // needs a router context in tests (the app provides HashRouter at root). - + ) } +describe('ModelSettings profile scope', () => { + // #90549: the API helpers treat `null` as "deliberately target the + // primary/default profile". A page following the active profile must pass + // `undefined`, or every read repaints the primary's model and the user's + // change looks reverted. + it('follows the active profile (undefined, never null) when unscoped', async () => { + await renderModelSettings() + + await waitFor(() => expect(getGlobalModelInfo).toHaveBeenCalledWith(undefined)) + expect(getGlobalModelOptions).toHaveBeenCalledWith(undefined, undefined) + expect(getAuxiliaryModels).toHaveBeenCalledWith(undefined) + expect(getMoaModels).toHaveBeenCalledWith(undefined) + }) + + it('reads through the explicit scope override when one is set', async () => { + await renderModelSettings('research') + + await waitFor(() => expect(getGlobalModelInfo).toHaveBeenCalledWith('research')) + expect(getGlobalModelOptions).toHaveBeenCalledWith(undefined, 'research') + expect(getAuxiliaryModels).toHaveBeenCalledWith('research') + expect(getMoaModels).toHaveBeenCalledWith('research') + }) +}) + describe('ModelSettings', () => { it('loads the current main model and lists configured providers only', async () => { await renderModelSettings() diff --git a/apps/desktop/src/app/settings/model-settings.tsx b/apps/desktop/src/app/settings/model-settings.tsx index 3ad3af8ac2..6ae1590c8e 100644 --- a/apps/desktop/src/app/settings/model-settings.tsx +++ b/apps/desktop/src/app/settings/model-settings.tsx @@ -182,17 +182,15 @@ interface ModelSettingsProps { /** Notified after the main model is applied, so live UI stores can sync. */ onMainModelChanged?: (provider: string, model: string) => void /** Shared settings "Applies to" scope: a concrete profile to edit instead of - * the app's active one, or null to follow the active profile (default). */ - scopeProfile?: null | string + * the app's active one, or undefined to follow the active profile (default). + * Request-shaped on purpose — the API helpers treat `null` as "deliberately + * target the primary/default backend", so this prop never carries null. */ + scopeProfile?: string } -export function ModelSettings({ onMainModelChanged, scopeProfile = null }: ModelSettingsProps) { +export function ModelSettings({ onMainModelChanged, scopeProfile }: ModelSettingsProps) { const { t } = useI18n() const m = t.settings.model - // `null` means "follow the active profile" at the settings layer. The - // Hermes API helpers use `undefined` for that contract; passing null would - // deliberately suppress the active profile and read the primary/default. - const requestProfile = scopeProfile ?? undefined const [loading, setLoading] = useState(true) const [error, setError] = useState('') const [mainModel, setMainModel] = useState<{ model: string; provider: string } | null>(null) @@ -239,10 +237,10 @@ export function ModelSettings({ onMainModelChanged, scopeProfile = null }: Model try { const [modelInfo, modelOptions, auxiliaryModels, moaModels] = await Promise.all([ - getGlobalModelInfo(requestProfile), - getGlobalModelOptions(undefined, requestProfile), - getAuxiliaryModels(requestProfile), - getMoaModels(requestProfile).catch(() => null) + getGlobalModelInfo(scopeProfile), + getGlobalModelOptions(undefined, scopeProfile), + getAuxiliaryModels(scopeProfile), + getMoaModels(scopeProfile).catch(() => null) ]) if (profileEpoch.current !== epoch) { @@ -280,7 +278,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile = null }: Model } } }, - [requestProfile, scopeProfile] + [scopeProfile] ) useEffect(() => { @@ -543,7 +541,7 @@ export function ModelSettings({ onMainModelChanged, scopeProfile = null }: Model setConfig(next) try { - await saveHermesConfig(next, scopeProfile ?? undefined) + await saveHermesConfig(next, scopeProfile) } catch (err) { setConfig(prev) notifyError(err, m.defaultsFailed) diff --git a/apps/desktop/src/store/settings-scope.test.ts b/apps/desktop/src/store/settings-scope.test.ts index 85824a91c2..321ae5d31c 100644 --- a/apps/desktop/src/store/settings-scope.test.ts +++ b/apps/desktop/src/store/settings-scope.test.ts @@ -17,7 +17,7 @@ vi.mock('@/lib/query-client', () => ({ invalidateProfileScopedQueries: vi.fn() } vi.mock('@/store/starmap', () => ({ resetStarmapGraph: vi.fn() })) const { $activeGatewayProfile } = await import('./profile') -const { $settingsScopeOverride, $settingsScopeProfile, setSettingsScope } = await import('./settings-scope') +const { $settingsRequestProfile, $settingsScopeOverride, $settingsScopeProfile, setSettingsScope } = await import('./settings-scope') beforeEach(() => { $activeGatewayProfile.set('default') @@ -58,6 +58,18 @@ describe('settings scope store', () => { expect($settingsScopeProfile.get()).toBe('default') }) + it('exposes a request-shaped scope: undefined (never null) without an override', () => { + // api/client.ts profileScoped() treats null as "target primary/default" — + // the #90549 bug class. The request form must therefore never be null. + expect($settingsRequestProfile.get()).toBeUndefined() + + setSettingsScope('research') + expect($settingsRequestProfile.get()).toBe('research') + + setSettingsScope('default') + expect($settingsRequestProfile.get()).toBeUndefined() + }) + it('drops the override on an app-wide profile switch', () => { setSettingsScope('research') expect($settingsScopeOverride.get()).toBe('research') diff --git a/apps/desktop/src/store/settings-scope.ts b/apps/desktop/src/store/settings-scope.ts index c8178f22df..115707b8d5 100644 --- a/apps/desktop/src/store/settings-scope.ts +++ b/apps/desktop/src/store/settings-scope.ts @@ -16,6 +16,23 @@ export const $settingsScopeProfile = computed([$settingsScopeOverride, $activeGa normalizeProfileKey(override ?? active) ) +// ── Request-scope form (THE value to hand to API helpers) ────────────────── +// The store contract and the API contract disagree about `null`: +// - here, `null` means "follow the app's active profile" (no override); +// - in api/client.ts `profileScoped()`/`capabilityScoped()`, `null` means +// "deliberately suppress the active profile and target primary/default" — +// only `undefined` falls back to the active profile. +// Passing the raw override into an API helper therefore silently retargets +// every read/write to the primary profile whenever no override is set — the +// "model change reverts when I re-enter the tab" class of bug (#90549: the +// page WROTE the right profile but READ primary back). Always send this +// computed (or `override ?? undefined`) on requests; keep the raw override +// only for UI concerns (selector highlight, cache keys, remount keys). +export const $settingsRequestProfile = computed( + $settingsScopeOverride, + (override): string | undefined => override ?? undefined +) + // Select the profile the settings pages should edit. Picking the app's active // profile stores `null` (no override) so the scope keeps following the app on // profile switches — and requests keep their unscoped default shape.