From 136767294212b3c0aa527ef9714b4c23032effbc Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 6 Aug 2026 12:39:32 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=81=20fix:=20Re-Arm=20Soft=20Default?= =?UTF-8?q?=20When=20a=20Stored=20Agent=20No=20Longer=20Resolves=20(#14664?= =?UTF-8?q?)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔁 fix: Re-Arm Soft Default When a Stored Agent No Longer Resolves * 🔁 fix: Scope Agent-List Gate to Storage-Derived Selections, Reuse Shared Agents Map * 🔁 fix: Trust Stored Agent Pick When the Catalog Request Fails * 🔁 fix: Mount Keyboard Shortcuts Inside the Agents-Map Provider * 🔁 fix: Always Gate 404 Fallback on Agent List, Skip Wait When Selector Disabled --- client/src/Providers/AgentsMapContext.tsx | 4 +- client/src/hooks/useNewConvo.ts | 5 +- client/src/routes/ChatRoute.tsx | 63 ++++++-- client/src/routes/Root.tsx | 2 +- .../__tests__/getDefaultModelSpec.test.ts | 147 +++++++++++++++++- client/src/utils/endpoints.ts | 63 +++++++- e2e/specs/mock/soft-default.spec.ts | 38 +++++ 7 files changed, 298 insertions(+), 24 deletions(-) diff --git a/client/src/Providers/AgentsMapContext.tsx b/client/src/Providers/AgentsMapContext.tsx index 904d74754d..e86c188aaf 100644 --- a/client/src/Providers/AgentsMapContext.tsx +++ b/client/src/Providers/AgentsMapContext.tsx @@ -2,5 +2,7 @@ import { createContext, useContext } from 'react'; import useAgentsMap from '~/hooks/Agents/useAgentsMap'; type AgentsMapContextType = ReturnType; -export const AgentsMapContext = createContext({} as AgentsMapContextType); +/** Defaults to undefined (map unknown): an `{}` default outside the provider would + * read as a loaded-but-empty catalog and misclassify stored agent picks as deleted. */ +export const AgentsMapContext = createContext(undefined); export const useAgentsMapContext = () => useContext(AgentsMapContext); diff --git a/client/src/hooks/useNewConvo.ts b/client/src/hooks/useNewConvo.ts index df6df51edd..1ad671c021 100644 --- a/client/src/hooks/useNewConvo.ts +++ b/client/src/hooks/useNewConvo.ts @@ -40,6 +40,7 @@ import useGetConversation from './Conversations/useGetConversation'; import useAssistantListMap from './Assistants/useAssistantListMap'; import { useResetChatBadges } from './useChatBadges'; import { useApplyModelSpecEffects } from './Agents'; +import { useAgentsMapContext } from '~/Providers'; import { usePauseGlobalAudio } from './Audio'; import { useHasAccess } from '~/hooks'; import store from '~/store'; @@ -57,6 +58,7 @@ const useNewConvo = (index = 0) => { const saveBadgesState = useRecoilValue(store.saveBadgesState); const setSubmission = useSetRecoilState(store.submissionByIndex(index)); const { data: endpointsConfig = {} as TEndpointsConfig } = useGetEndpointsQuery(); + const agentsMap = useAgentsMapContext(); const hasAgentAccess = useHasAccess({ permissionType: PermissionTypes.AGENTS, @@ -330,7 +332,7 @@ const useNewConvo = (index = 0) => { }; let preset = _preset; - const result = getDefaultModelSpec(startupConfig, endpointsConfig); + const result = getDefaultModelSpec(startupConfig, endpointsConfig, agentsMap); const defaultModelSpec = result?.default ?? result?.last ?? result?.softDefault; const shouldApplyModelSpec = result?.softDefault != null @@ -390,6 +392,7 @@ const useNewConvo = (index = 0) => { [ files, setFiles, + agentsMap, saveDrafts, mutateAsync, resetBadges, diff --git a/client/src/routes/ChatRoute.tsx b/client/src/routes/ChatRoute.tsx index a514a60bfe..94797e6f5b 100644 --- a/client/src/routes/ChatRoute.tsx +++ b/client/src/routes/ChatRoute.tsx @@ -3,14 +3,16 @@ import { useQueryClient } from '@tanstack/react-query'; import { useRecoilCallback, useRecoilValue } from 'recoil'; import { Spinner, useToastContext } from '@librechat/client'; import { useParams, useSearchParams } from 'react-router-dom'; -import { Constants, EModelEndpoint } from 'librechat-data-provider'; import { useGetModelsQuery } from 'librechat-data-provider/react-query'; -import type { TPreset } from 'librechat-data-provider'; +import { Constants, EModelEndpoint, PermissionBits } from 'librechat-data-provider'; +import type { TPreset, TAgentsMap } from 'librechat-data-provider'; import { + defaultSpecAwaitsAgents, mergeQuerySettingsWithSpec, processValidSettings, getDefaultModelSpec, getModelSpecPreset, + hasModelSelection, isNotFoundError, isTemporaryConversation, logger, @@ -20,6 +22,7 @@ import { useGetConvoIdQuery, useGetStartupConfig, useGetEndpointsQuery, + useListAgentsQuery, useProjectQuery, } from '~/data-provider'; import { @@ -29,7 +32,7 @@ import { useNewConvo, useLocalize, } from '~/hooks'; -import { ToolCallsMapProvider } from '~/Providers'; +import { ToolCallsMapProvider, useAgentsMapContext } from '~/Providers'; import ChatView from '~/components/Chat/ChatView'; import { NotificationSeverity } from '~/common'; import useAuthRedirect from './useAuthRedirect'; @@ -124,6 +127,16 @@ export default function ChatRoute() { }); const endpointsQuery = useGetEndpointsQuery({ enabled: isAuthenticated }); const assistantListMap = useAssistantListMap(); + /** The map comes from Root's shared context (one mapping pass app-wide); the + * select-less observer only tracks settle state. Only a loaded list may + * invalidate a stored agent pick: on a transient catalog failure (retries are + * disabled) the map stays unknown, the pick stays trusted, and the gate below + * releases so the landing never hangs on the error. */ + const agentsMap: TAgentsMap | undefined = useAgentsMapContext(); + const agentsQuery = useListAgentsQuery( + { requiredPermission: PermissionBits.VIEW }, + { enabled: isAuthenticated }, + ); const isTemporaryChat = isTemporaryConversation(conversation); @@ -165,15 +178,35 @@ export default function ChatRoute() { return; } - const getNewConvoPreset = () => { - const queryParams: Record = {}; - searchParams.forEach((value, key) => { - if (key !== 'prompt' && key !== 'q' && key !== 'submit' && key !== 'projectId') { - queryParams[key] = value; - } - }); - const querySettings = processValidSettings(queryParams); + const queryParams: Record = {}; + searchParams.forEach((value, key) => { + if (key !== 'prompt' && key !== 'q' && key !== 'submit' && key !== 'projectId') { + queryParams[key] = value; + } + }); + const querySettings = processValidSettings(queryParams); + const notFoundConvo = + Boolean(conversationId) && + !isNewConvo && + initialConvoQuery.isError && + isNotFoundError(initialConvoQuery.error); + + /** A stored agent pick can only be validated against the loaded agent list + * (it may name an agent since deleted, or one from another org sharing this + * browser storage). Defer the first conversation until the list settles. + * A URL naming its own selection skips the wait only on the new-chat branch, + * where it takes precedence over the stored pick; the 404 fallback never + * applies query settings, so it always waits. */ + const awaitsAgentList = + agentsMap == null && + !agentsQuery.isError && + defaultSpecAwaitsAgents(startupConfig, endpointsQuery.data); + if (awaitsAgentList && (notFoundConvo || (isNewConvo && !hasModelSelection(querySettings)))) { + return; + } + + const getNewConvoPreset = () => { /** A spec named in the URL is an explicit selection: it must resolve to its own * full preset, or stale last-selection state (endpoint/agent) fills the gaps. * Names absent from the client config (e.g. `showInMenu: false`) stay in the @@ -182,7 +215,9 @@ export default function ChatRoute() { ? startupConfig?.modelSpecs?.list?.find((spec) => spec.name === querySettings.spec) : undefined; - const result = urlSpec ? undefined : getDefaultModelSpec(startupConfig, endpointsQuery.data); + const result = urlSpec + ? undefined + : getDefaultModelSpec(startupConfig, endpointsQuery.data, agentsMap); const spec = urlSpec ?? result?.default ?? result?.last ?? result?.softDefault; const specPreset = spec ? getModelSpecPreset(spec) : undefined; @@ -220,7 +255,7 @@ export default function ChatRoute() { initialConvoQuery.isError && isNotFoundError(initialConvoQuery.error) ) { - const result = getDefaultModelSpec(startupConfig, endpointsQuery.data); + const result = getDefaultModelSpec(startupConfig, endpointsQuery.data, agentsMap); const spec = result?.default ?? result?.last ?? result?.softDefault; showToast({ message: localize('com_ui_conversation_not_found'), @@ -267,6 +302,8 @@ export default function ChatRoute() { // eslint-disable-next-line react-hooks/exhaustive-deps }, [ roles, + agentsMap, + agentsQuery.isError, startupConfig, initialConvoQuery.data, initialConvoQuery.isError, diff --git a/client/src/routes/Root.tsx b/client/src/routes/Root.tsx index d792cae0bd..3adb6ec1d9 100644 --- a/client/src/routes/Root.tsx +++ b/client/src/routes/Root.tsx @@ -101,6 +101,7 @@ export default function Root() { + {config?.interface?.termsOfService?.modalAcceptance === true && ( )} - diff --git a/client/src/utils/__tests__/getDefaultModelSpec.test.ts b/client/src/utils/__tests__/getDefaultModelSpec.test.ts index 268f222b61..0246127dc8 100644 --- a/client/src/utils/__tests__/getDefaultModelSpec.test.ts +++ b/client/src/utils/__tests__/getDefaultModelSpec.test.ts @@ -1,6 +1,11 @@ import { Constants, EModelEndpoint, LocalStorageKeys } from 'librechat-data-provider'; -import type { TModelSpec, TStartupConfig, TEndpointsConfig } from 'librechat-data-provider'; -import { getDefaultModelSpec } from '../endpoints'; +import type { + TModelSpec, + TStartupConfig, + TEndpointsConfig, + TAgentsMap, +} from 'librechat-data-provider'; +import { getDefaultModelSpec, defaultSpecAwaitsAgents } from '../endpoints'; const createModelSpec = (name: string, overrides: Partial = {}): TModelSpec => ({ @@ -551,4 +556,142 @@ describe('getDefaultModelSpec', () => { expect(result).toBeUndefined(); }); }); + + describe('stored agent picks validated against the agent list', () => { + const softSpec = createModelSpec('soft-spec', { softDefault: true }); + const liveAgentsMap = { agent_123: { id: 'agent_123' } } as unknown as TAgentsMap; + + it('yields to a stored agent present in the loaded agent list', () => { + persistAgentSelection('agent_123'); + + const result = getDefaultModelSpec( + createStartupConfig([softSpec]), + fullEndpointsConfig, + liveAgentsMap, + ); + + expect(result).toBeUndefined(); + }); + + it('re-arms the soft default when the stored agent is missing from the agent list', () => { + persistAgentSelection('agent_other_org'); + + const result = getDefaultModelSpec( + createStartupConfig([softSpec]), + fullEndpointsConfig, + liveAgentsMap, + ); + + expect(result).toEqual({ softDefault: softSpec }); + }); + + it('re-arms the soft default over a stored agent when the agent list is empty', () => { + persistAgentSelection('agent_123'); + + const result = getDefaultModelSpec(createStartupConfig([softSpec]), fullEndpointsConfig, {}); + + expect(result).toEqual({ softDefault: softSpec }); + }); + + it('re-arms in an agents-only allow-list when the stored agent is missing', () => { + persistAgentSelection('agent_other_org'); + + const result = getDefaultModelSpec( + createStartupConfig([softSpec], { addedEndpoints: [EModelEndpoint.agents] }), + agentsOnlyEndpointsConfig, + liveAgentsMap, + ); + + expect(result).toEqual({ softDefault: softSpec }); + }); + + it('trusts the stored agent while the agent list is unknown', () => { + persistAgentSelection('agent_other_org'); + + const result = getDefaultModelSpec( + createStartupConfig([softSpec]), + fullEndpointsConfig, + undefined, + ); + + expect(result).toBeUndefined(); + }); + }); +}); + +describe('defaultSpecAwaitsAgents', () => { + beforeEach(() => { + localStorage.clear(); + }); + + const softSpec = createModelSpec('soft-spec', { softDefault: true }); + + it('awaits the agent list for a stored agent pick with a soft default configured', () => { + persistAgentSelection('agent_123'); + + expect(defaultSpecAwaitsAgents(createStartupConfig([softSpec]), fullEndpointsConfig)).toBe( + true, + ); + }); + + it('does not await without a stored agent pick', () => { + expect(defaultSpecAwaitsAgents(createStartupConfig([softSpec]), fullEndpointsConfig)).toBe( + false, + ); + }); + + it('does not await when a stored spec decides first', () => { + persistAppliedSpec(softSpec); + + expect(defaultSpecAwaitsAgents(createStartupConfig([softSpec]), fullEndpointsConfig)).toBe( + false, + ); + }); + + it('does not await when a hard default decides first', () => { + const hardSpec = createModelSpec('hard-spec', { default: true }); + persistAgentSelection('agent_123'); + + expect( + defaultSpecAwaitsAgents(createStartupConfig([hardSpec, softSpec]), fullEndpointsConfig), + ).toBe(false); + }); + + it('does not await without a soft default configured', () => { + const regularSpec = createModelSpec('regular-spec'); + persistAgentSelection('agent_123'); + + expect(defaultSpecAwaitsAgents(createStartupConfig([regularSpec]), fullEndpointsConfig)).toBe( + false, + ); + }); + + it('does not await when the endpoints config lacks agents', () => { + persistAgentSelection('agent_123'); + + expect( + defaultSpecAwaitsAgents(createStartupConfig([softSpec]), { + [EModelEndpoint.openAI]: { order: 0 }, + } as TEndpointsConfig), + ).toBe(false); + }); + + it('does not await for an ephemeral stored agent id', () => { + persistAgentSelection('ephemeral_agent_123'); + + expect(defaultSpecAwaitsAgents(createStartupConfig([softSpec]), fullEndpointsConfig)).toBe( + false, + ); + }); + + it('does not await when model selection is disabled', () => { + persistAgentSelection('agent_123'); + + expect( + defaultSpecAwaitsAgents( + createStartupConfig([softSpec], { modelSelect: false }), + fullEndpointsConfig, + ), + ).toBe(false); + }); }); diff --git a/client/src/utils/endpoints.ts b/client/src/utils/endpoints.ts index 5d79c78726..3fea0c5cc1 100644 --- a/client/src/utils/endpoints.ts +++ b/client/src/utils/endpoints.ts @@ -139,8 +139,8 @@ interface InitiatedTemplateResult { type StoredModelSelection = Pick< t.TConversation, - 'endpoint' | 'model' | 'spec' | 'agent_id' | 'assistant_id' ->; + 'model' | 'spec' | 'agent_id' | 'assistant_id' +> & { endpoint?: EModelEndpoint | string | null }; function hasSelectionValue(value?: string | null): boolean { return typeof value === 'string' && value.trim() !== ''; @@ -160,6 +160,16 @@ function parseStoredModelSelection( } } +function isStoredAgentPick( + selection?: Partial | null, +): selection is Partial & { agent_id: string } { + return ( + isAgentsEndpoint(selection?.endpoint ?? '') && + hasSelectionValue(selection?.agent_id) && + !isEphemeralAgentId(selection?.agent_id ?? '') + ); +} + export function hasModelSelection(selection?: Partial | null): boolean { if (!selection) { return false; @@ -234,10 +244,7 @@ function hasSelectableEntitySelection({ if (!modelSelect || !endpoint) { return false; } - const isAgentPick = - isAgentsEndpoint(endpoint) && - hasSelectionValue(selection.agent_id) && - !isEphemeralAgentId(selection.agent_id); + const isAgentPick = isStoredAgentPick(selection); const isAssistantPick = isAssistantsEndpoint(endpoint) && hasSelectionValue(selection.assistant_id); if (!isAgentPick && !isAssistantPick) { @@ -399,10 +406,17 @@ export function applyModelSpecEphemeralAgent({ * strands a new chat on an unselectable endpoint. The legacy first-spec fallback applies * only when specs are prioritized (or the model menu is hidden) and no soft default is * configured. + * + * A stored agent pick is trusted only until the agent list can weigh in: pass `agentsMap` + * once loaded, and a pick naming an agent missing from it (deleted, or selected in another + * org sharing this browser storage) is residue the soft default overrides. An undefined + * map means the list is unknown and leaves the pick trusted; gate on + * `defaultSpecAwaitsAgents` to defer the decision until the map settles. */ export function getDefaultModelSpec( startupConfig?: t.TStartupConfig, endpointsConfig?: t.TEndpointsConfig, + agentsMap?: t.TAgentsMap, ): | { default?: t.TModelSpec; @@ -437,6 +451,11 @@ export function getDefaultModelSpec( if (lastSpec?.name === softDefaultSpec.name) { return { softDefault: softDefaultSpec }; } + const staleAgentPick = + agentsMap != null && isStoredAgentPick(lastSetup) && agentsMap[lastSetup.agent_id] == null; + if (staleAgentPick) { + return { softDefault: softDefaultSpec }; + } const modelSelect = interfaceConfig?.modelSelect; const yieldsToSelection = (hasModelSelection(lastSetup) && @@ -456,6 +475,38 @@ export function getDefaultModelSpec( return; } +/** + * Whether resolving the default spec for a new chat hinges on the agent list: + * a soft default is configured, no hard default or stored spec decides first, + * and the stored last setup names a concrete agent whose existence only the + * loaded agent map can confirm. Callers should defer `getDefaultModelSpec` + * until the agent list query settles (data or error) while this returns true. + */ +export function defaultSpecAwaitsAgents( + startupConfig?: t.TStartupConfig, + endpointsConfig?: t.TEndpointsConfig, +): boolean { + const list = startupConfig?.modelSpecs?.list; + if (!list || list.some((spec) => spec.default) || !list.some((spec) => spec.softDefault)) { + return false; + } + + /** With the selector disabled the soft default can never yield to a stored + * pick, so the decision is deterministic without the agent list. */ + if (!startupConfig?.interface?.modelSelect) { + return false; + } + + const lastSetup = parseStoredModelSelection( + localStorage.getItem(LocalStorageKeys.LAST_CONVO_SETUP + '_0'), + ); + if (hasSelectionValue(lastSetup?.spec) && list.some((spec) => spec.name === lastSetup?.spec)) { + return false; + } + + return isStoredAgentPick(lastSetup) && endpointsConfig?.[EModelEndpoint.agents] != null; +} + export function getModelSpecPreset(modelSpec?: t.TModelSpec) { if (!modelSpec) { return; diff --git a/e2e/specs/mock/soft-default.spec.ts b/e2e/specs/mock/soft-default.spec.ts index 640eed2bd1..3bac47e1c6 100644 --- a/e2e/specs/mock/soft-default.spec.ts +++ b/e2e/specs/mock/soft-default.spec.ts @@ -2,6 +2,7 @@ import { expect, test } from '@playwright/test'; import type { TStartupConfig } from 'librechat-data-provider'; import type { Page } from '@playwright/test'; import { getPrimaryE2EUser } from '../../setup/users.mock'; +import { cleanupAgent } from './agents.helpers'; import { NEW_CHAT_PATH, getAccessToken, @@ -272,4 +273,41 @@ test.describe('soft default model spec', () => { await expect(modelTrigger(page)).toContainText(SOFT_DEFAULT_LABEL, { timeout: 15000 }); await expect(main).not.toContainText(agentName); }); + + // Regression: a stored agent selection can outlive the agent itself (deletion here; + // switching orgs that share browser storage behaves the same, since the other org's + // agent id never resolves). The dead pick used to keep suppressing the soft default + // and strand a cold New Chat on the agents endpoint with nothing selected. Once the + // agent list loads without the stored id, the pick is residue and the soft default + // must re-arm. + test('a stored agent that no longer exists yields to the soft default on a cold load', async ({ + page, + }) => { + test.setTimeout(120000); + await startFresh(page); + await expect(modelTrigger(page)).toContainText(SOFT_DEFAULT_LABEL, { timeout: 15000 }); + + const agentName = uniqueName('E2E Stale Agent'); + const agent = await createAgent(page, agentName); + await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); + await selectAgent(page, agentName); + + await page.reload({ timeout: 10000 }); + await expect(modelTrigger(page)).toContainText(agentName, { timeout: 15000 }); + + await cleanupAgent(page, agent.id); + + await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); + await expect(modelTrigger(page)).toContainText(SOFT_DEFAULT_LABEL, { timeout: 15000 }); + await expect(modelTrigger(page)).not.toHaveText('Select a model'); + + // A live selection must still outrank the soft default after the fix: recreate, + // select, and confirm the carry-forward behavior is intact on a cold load. + const survivorName = uniqueName('E2E Live Agent'); + await createAgent(page, survivorName); + await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); + await selectAgent(page, survivorName); + await page.goto(NEW_CHAT_PATH, { timeout: 10000 }); + await expect(modelTrigger(page)).toContainText(survivorName, { timeout: 15000 }); + }); });