From 79d418f32a05446cee919046cf99ed97c0757323 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 9 Sep 2026 17:59:08 +0100 Subject: [PATCH 1/7] feat(agents): make sources optional and drop the synthetic "Default" source The sources list used to start with a fake "Default" entry that had no id and, at run time, meant "no source, skip retrieval". The agent form pre-selected it, snapped back to it when the last source was deselected, and refused to publish without it, so a new agent always looked like it had a knowledge base when it had none. Backend - /api/sources returns only ingested sources; no placeholder row. - Publishing an agent no longer requires a source on create or update. The legacy "default" value is still accepted and maps to NULL. Frontend - The agent source picker starts empty, can be cleared, and shows a hint that a source-less agent answers from the model and its tools only. - The picker groups sources into "Your sources" and "Shared with team" when any team-shared source exists, shows "N sources selected" for a multi-selection, and gets the same "Go to Sources" / "Upload new" footer as the chat picker. A source uploaded from the form is selected when it lands. - Source selection serialisation and the picker id live in one helper shared with the chat picker; the four copies in the form are gone. - The client no longer seeds a placeholder source in the store, and the dead auto-select of a "default" document is removed. --- .../api/answer/services/stream_processor.py | 6 +- docsgpt/api/user/agents/routes.py | 43 +-- docsgpt/api/user/sources/routes.py | 14 +- frontend/src/agents/NewAgent.tsx | 308 +++++++----------- .../src/agents/workflow/WorkflowBuilder.tsx | 21 +- frontend/src/components/MessageInput.tsx | 4 +- .../src/components/SourcesPopoverFooter.tsx | 39 +++ .../message-input/SourcesTrigger.tsx | 33 +- frontend/src/hooks/useDataInitializer.ts | 20 +- frontend/src/locale/de.json | 8 +- frontend/src/locale/en.json | 8 +- frontend/src/locale/es.json | 8 +- frontend/src/locale/jp.json | 8 +- frontend/src/locale/ru.json | 8 +- frontend/src/locale/zh-TW.json | 8 +- frontend/src/locale/zh.json | 8 +- .../src/modals/ShareConversationModal.tsx | 10 +- frontend/src/preferences/preferenceSlice.ts | 11 +- frontend/src/store.ts | 11 +- frontend/src/utils/sourceUtils.test.ts | 144 ++++++++ frontend/src/utils/sourceUtils.ts | 73 +++++ tests/api/user/agents/test_routes_happy.py | 84 ++++- tests/api/user/sources/test_routes.py | 20 +- 23 files changed, 532 insertions(+), 365 deletions(-) create mode 100644 frontend/src/components/SourcesPopoverFooter.tsx create mode 100644 frontend/src/utils/sourceUtils.test.ts create mode 100644 frontend/src/utils/sourceUtils.ts diff --git a/docsgpt/api/answer/services/stream_processor.py b/docsgpt/api/answer/services/stream_processor.py index ab3e6f1f..004b616c 100644 --- a/docsgpt/api/answer/services/stream_processor.py +++ b/docsgpt/api/answer/services/stream_processor.py @@ -647,9 +647,9 @@ class StreamProcessor: def _configure_source(self): """Configure the source based on agent data. - The literal string ``"default"`` is a placeholder meaning "no - ingested source" and is normalized to an empty source so that no - retrieval is attempted. + The literal string ``"default"`` is a legacy placeholder (older + clients sent it for "no ingested source") and is normalized to an + empty source so that no retrieval is attempted. """ if self._agent_data: agent_data = self._agent_data diff --git a/docsgpt/api/user/agents/routes.py b/docsgpt/api/user/agents/routes.py index 3e873416..16a5875c 100644 --- a/docsgpt/api/user/agents/routes.py +++ b/docsgpt/api/user/agents/routes.py @@ -76,7 +76,8 @@ AGENT_TYPE_SCHEMAS = { # is acceptable and maps to NULL downstream. "validate_published": ["name", "description"], "validate_draft": [], - "require_source": True, + # No source is required: a source-less agent answers from the model + # and its tools, and ``source``/``sources`` map to NULL downstream. "fields": [ "name", "description", @@ -661,20 +662,6 @@ class CreateAgent(Resource): if data.get("status") == "published": required_fields = schema["required_published"] validate_fields = schema["validate_published"] - if ( - schema.get("require_source") - and not data.get("source") - and not data.get("sources") - ): - return make_response( - jsonify( - { - "success": False, - "message": "Either 'source' or 'sources' field is required for published agents", - } - ), - 400, - ) else: required_fields = schema["required_draft"] validate_fields = schema["validate_draft"] @@ -1190,29 +1177,9 @@ class UpdateAgent(Resource): ) if not final_value: missing_published_fields.append(field_label) - source_final = update_fields.get( - "source_id", existing_agent.get("source_id"), - ) - extra_final = update_fields.get( - "extra_source_ids", existing_agent.get("extra_source_ids") or [], - ) - # ``retriever`` carries the runtime identity for - # agents that publish against the synthetic - # "Default" source (frontend's auto-selected - # ``{name: "Default", retriever: "classic"}`` - # entry has no ``id``, so ``source_id`` ends up - # NULL even though the user picked something). - # Without this fallback the most common new-agent - # publish flow gets a 400. - retriever_final = update_fields.get( - "retriever", existing_agent.get("retriever"), - ) - if ( - not source_final - and not extra_final - and not retriever_final - ): - missing_published_fields.append("Source or retriever") + # Sources are optional: a published agent with no + # ``source_id`` and no ``extra_source_ids`` skips + # retrieval and answers from the model and its tools. if missing_published_fields: return _reject( "Cannot publish agent. Missing or invalid required " diff --git a/docsgpt/api/user/sources/routes.py b/docsgpt/api/user/sources/routes.py index 25ed0b6d..dc675063 100644 --- a/docsgpt/api/user/sources/routes.py +++ b/docsgpt/api/user/sources/routes.py @@ -73,16 +73,10 @@ class CombinedJson(Resource): if not decoded_token: return make_response(jsonify({"success": False}), 401) user = decoded_token.get("sub") - data = [ - { - "name": "Default", - "date": "default", - "model": settings.EMBEDDINGS_NAME, - "location": "remote", - "tokens": "", - "retriever": "classic", - } - ] + # Only ingested sources are listed. There is no synthetic "Default" + # entry: "no source" is expressed by an empty selection on the client, + # and the answer path treats a missing source as "skip retrieval". + data = [] try: with db_readonly() as conn: diff --git a/frontend/src/agents/NewAgent.tsx b/frontend/src/agents/NewAgent.tsx index 24ee5373..ba49aac4 100644 --- a/frontend/src/agents/NewAgent.tsx +++ b/frontend/src/agents/NewAgent.tsx @@ -1,6 +1,12 @@ import isEqual from 'lodash/isEqual'; import { MoreHorizontal } from 'lucide-react'; -import React, { useCallback, useEffect, useRef, useState } from 'react'; +import React, { + useCallback, + useEffect, + useMemo, + useRef, + useState, +} from 'react'; import { useTranslation } from 'react-i18next'; import { useDispatch, useSelector } from 'react-redux'; import { useNavigate, useParams, useSearchParams } from 'react-router-dom'; @@ -32,12 +38,13 @@ import { MultiSelectPopover, type MultiSelectPopoverItem, } from '../components/MultiSelectPopover'; +import SourcesPopoverFooter from '../components/SourcesPopoverFooter'; import Spinner from '../components/Spinner'; import ToolIcon from '../components/ToolIcon'; import AgentDetailsModal from '../modals/AgentDetailsModal'; import ShareToTeamModal from '../teams/ShareToTeamModal'; import ConfirmationModal from '../modals/ConfirmationModal'; -import { ActiveState, Doc, Prompt } from '../models/misc'; +import { ActiveState, Prompt } from '../models/misc'; import { selectAgentFolders, selectSelectedAgent, @@ -51,6 +58,13 @@ import { import PromptsModal from '../preferences/PromptsModal'; import Prompts from '../settings/Prompts'; import { UserToolType } from '../settings/types'; +import Upload from '../upload/Upload'; +import { + selectedSourceIdsFromAgent, + serializeAgentSources, + sourceItemId, + toSourcePickerItems, +} from '../utils/sourceUtils'; import { getToolDisplayName, isClassicAgentToolVisible, @@ -137,6 +151,11 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { const [isSourcePopupOpen, setIsSourcePopupOpen] = useState(false); const [isToolsPopupOpen, setIsToolsPopupOpen] = useState(false); const [isModelsPopupOpen, setIsModelsPopupOpen] = useState(false); + const [uploadModalState, setUploadModalState] = + useState('INACTIVE'); + // Set once an upload started from this form finishes, so the source it + // created can be selected as soon as the refreshed list arrives. + const [pendingUploadSelect, setPendingUploadSelect] = useState(false); const [selectedSourceIds, setSelectedSourceIds] = useState>( new Set(), ); @@ -162,6 +181,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { const sourceAnchorButtonRef = useRef(null); const toolAnchorButtonRef = useRef(null); const modelAnchorButtonRef = useRef(null); + const sourceIdsBeforeUploadRef = useRef>(new Set()); const modeConfig = { new: { @@ -199,11 +219,10 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { agent.name && agent.description && agent.prompt_id && agent.agent_type; const isJsonSchemaValidOrEmpty = jsonSchemaText.trim() === '' || jsonSchemaValid; - const hasSource = selectedSourceIds.size > 0; const guardrailsOk = !guardrailsIncomplete(agent.config?.guardrails); - return ( - hasRequiredFields && isJsonSchemaValidOrEmpty && hasSource && guardrailsOk - ); + // Sources are optional: an agent without one answers from the model and + // its tools only. + return hasRequiredFields && isJsonSchemaValidOrEmpty && guardrailsOk; }; const isJsonSchemaInvalid = () => { @@ -222,8 +241,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { // source name instead of "External KB"; only then show the generic label. const resolveSourceLabel = (id: string): string => { const matchedDoc = sourceDocs?.find( - (source) => - source.id === id || source.name === id || source.retriever === id, + (source) => sourceItemId(source) === id, ); if (matchedDoc?.name) return matchedDoc.name; const detail = agent.source_details?.find((d) => d.id === id); @@ -231,6 +249,50 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { return t('agents.form.externalKb'); }; + const sourceItems = useMemo( + () => + toSourcePickerItems( + sourceDocs, + { + own: t('agents.form.sourcePopup.groupOwn'), + team: t('agents.form.sourcePopup.groupTeam'), + }, + SourceIcon, + ), + [sourceDocs, t], + ); + + const selectedSourceNames = Array.from(selectedSourceIds) + .map((id) => resolveSourceLabel(id)) + .filter(Boolean); + const sourceTriggerLabel = + selectedSourceIds.size === 0 + ? t('agents.form.placeholders.selectSources') + : selectedSourceIds.size === 1 + ? selectedSourceNames[0] + : t('conversation.sources.selectedCount', { + count: selectedSourceIds.size, + }); + + const handleUploadClick = () => { + sourceIdsBeforeUploadRef.current = new Set( + (sourceDocs ?? []).map(sourceItemId), + ); + setIsSourcePopupOpen(false); + setUploadModalState('ACTIVE'); + }; + + // Sources go out as the legacy single ``source`` for one selection and as + // the ``sources`` list for several; an empty selection sends both empty. + const appendSourceFields = (formData: FormData) => { + const { source, sources } = serializeAgentSources( + selectedSourceIds, + sourceDocs, + ); + formData.append('source', source); + formData.append('sources', JSON.stringify(sources)); + }; + const handleUpload = useCallback((files: File[]) => { if (files && files.length > 0) { const file = files[0]; @@ -261,39 +323,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { formData.append('name', agent.name); formData.append('description', agent.description); - if (selectedSourceIds.size > 1) { - const sourcesArray = Array.from(selectedSourceIds) - .map((id) => { - const sourceDoc = sourceDocs?.find( - (source) => - source.id === id || source.retriever === id || source.name === id, - ); - if (sourceDoc?.name === 'Default' && !sourceDoc?.id) { - return 'default'; - } - return sourceDoc?.id || id; - }) - .filter(Boolean); - formData.append('sources', JSON.stringify(sourcesArray)); - formData.append('source', ''); - } else if (selectedSourceIds.size === 1) { - const singleSourceId = Array.from(selectedSourceIds)[0]; - const sourceDoc = sourceDocs?.find( - (source) => - source.id === singleSourceId || - source.retriever === singleSourceId || - source.name === singleSourceId, - ); - let finalSourceId; - if (sourceDoc?.name === 'Default' && !sourceDoc?.id) - finalSourceId = 'default'; - else finalSourceId = sourceDoc?.id || singleSourceId; - formData.append('source', String(finalSourceId)); - formData.append('sources', JSON.stringify([])); - } else { - formData.append('source', ''); - formData.append('sources', JSON.stringify([])); - } + appendSourceFields(formData); formData.append('chunks', agent.chunks); formData.append('retriever', agent.retriever); @@ -386,39 +416,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { formData.append('name', agent.name); formData.append('description', agent.description); - if (selectedSourceIds.size > 1) { - const sourcesArray = Array.from(selectedSourceIds) - .map((id) => { - const sourceDoc = sourceDocs?.find( - (source) => - source.id === id || source.retriever === id || source.name === id, - ); - if (sourceDoc?.name === 'Default' && !sourceDoc?.id) { - return 'default'; - } - return sourceDoc?.id || id; - }) - .filter(Boolean); - formData.append('sources', JSON.stringify(sourcesArray)); - formData.append('source', ''); - } else if (selectedSourceIds.size === 1) { - const singleSourceId = Array.from(selectedSourceIds)[0]; - const sourceDoc = sourceDocs?.find( - (source) => - source.id === singleSourceId || - source.retriever === singleSourceId || - source.name === singleSourceId, - ); - let finalSourceId; - if (sourceDoc?.name === 'Default' && !sourceDoc?.id) - finalSourceId = 'default'; - else finalSourceId = sourceDoc?.id || singleSourceId; - formData.append('source', String(finalSourceId)); - formData.append('sources', JSON.stringify([])); - } else { - formData.append('source', ''); - formData.append('sources', JSON.stringify([])); - } + appendSourceFields(formData); formData.append('chunks', agent.chunks); formData.append('retriever', agent.retriever); @@ -658,16 +656,15 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { validateAndSetFolder(); }, [folderIdFromUrl, agentFolders, token, dispatch]); - // Auto-select default source if none selected + // After an upload started from this form, select the source it created. useEffect(() => { - if (sourceDocs && sourceDocs.length > 0 && selectedSourceIds.size === 0) { - const defaultSource = sourceDocs.find((s) => s.name === 'Default'); - const fallback = defaultSource || sourceDocs[0]; - setSelectedSourceIds( - new Set([String(fallback.id || fallback.retriever || fallback.name)]), - ); - } - }, [sourceDocs, selectedSourceIds.size]); + if (!pendingUploadSelect || !sourceDocs) return; + const before = sourceIdsBeforeUploadRef.current; + const added = sourceDocs.map(sourceItemId).filter((id) => !before.has(id)); + if (added.length === 0) return; + setPendingUploadSelect(false); + setSelectedSourceIds((prev) => new Set([...prev, ...added])); + }, [pendingUploadSelect, sourceDocs]); useEffect(() => { if ((mode === 'edit' || mode === 'draft') && agentId) { @@ -679,31 +676,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { } const data = await response.json(); - if (data.sources && data.sources.length > 0) { - const mappedSources = data.sources.map((sourceId: string) => { - if (sourceId === 'default') { - const defaultSource = sourceDocs?.find( - (source) => source.name === 'Default', - ); - return defaultSource?.retriever || 'classic'; - } - return sourceId; - }); - setSelectedSourceIds(new Set(mappedSources)); - } else if (data.source) { - if (data.source === 'default') { - const defaultSource = sourceDocs?.find( - (source) => source.name === 'Default', - ); - setSelectedSourceIds( - new Set([defaultSource?.retriever || 'classic']), - ); - } else { - setSelectedSourceIds(new Set([data.source])); - } - } else if (data.retriever) { - setSelectedSourceIds(new Set([data.retriever])); - } + setSelectedSourceIds(new Set(selectedSourceIdsFromAgent(data))); if (data.tool_details) setSelectedTools(data.tool_details); if (data.status === 'draft') setEffectiveMode('draft'); @@ -763,56 +736,18 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { }, [selectedModelIds]); useEffect(() => { - const selectedSources = Array.from(selectedSourceIds) - .map((id) => - sourceDocs?.find( - (source) => - source.id === id || source.retriever === id || source.name === id, - ), - ) - .filter(Boolean); - - if (selectedSources.length > 0) { - // Handle multiple sources - if (selectedSources.length > 1) { - // Multiple sources selected - store in sources array - const sourceIds = selectedSources - .map((source) => source?.id) - .filter((id): id is string => Boolean(id)); - setAgent((prev) => ({ - ...prev, - sources: sourceIds, - source: '', // Clear single source for multiple sources - retriever: '', - })); - } else { - // Single source selected - maintain backward compatibility - const selectedSource = selectedSources[0]; - if (selectedSource && 'id' in selectedSource) { - setAgent((prev) => ({ - ...prev, - source: selectedSource?.id || 'default', - sources: [], // Clear sources array for single source - retriever: '', - })); - } else { - setAgent((prev) => ({ - ...prev, - source: '', - sources: [], // Clear sources array - retriever: selectedSource?.retriever || 'classic', - })); - } - } - } else { - // No sources selected - setAgent((prev) => ({ - ...prev, - source: '', - sources: [], - retriever: '', - })); - } + const { source, sources } = serializeAgentSources( + selectedSourceIds, + sourceDocs, + ); + setAgent((prev) => ({ + ...prev, + source, + sources, + // A source-less agent keeps the default retriever name so its runtime + // config stays valid; real sources carry their own retriever. + retriever: selectedSourceIds.size === 0 ? 'classic' : '', + })); }, [selectedSourceIds]); useEffect(() => { @@ -999,63 +934,46 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { open={isSourcePopupOpen} onOpenChange={setIsSourcePopupOpen} title={t('agents.form.sourcePopup.title')} - items={ - sourceDocs?.map((doc: Doc) => ({ - id: String(doc.id || doc.retriever || doc.name), - label: doc.name, - icon: , - })) || [] - } + items={sourceItems} selectedIds={Array.from(selectedSourceIds)} onToggle={(id) => { const next = new Set(selectedSourceIds); if (next.has(id)) next.delete(id); else next.add(id); - if ( - next.size === 0 && - sourceDocs && - sourceDocs.length > 0 - ) { - const defaultSource = sourceDocs.find( - (s) => s.name === 'Default', - ); - const fallback = defaultSource || sourceDocs[0]; - setSelectedSourceIds( - new Set([ - String( - fallback.id || fallback.retriever || fallback.name, - ), - ]), - ); - } else { - setSelectedSourceIds(next); - } + setSelectedSourceIds(next); }} searchPlaceholder={t( 'agents.form.sourcePopup.searchPlaceholder', )} emptyMessage={t('agents.form.sourcePopup.noOptionsMessage')} + footer={ + setIsSourcePopupOpen(false)} + onUploadClick={handleUploadClick} + /> + } trigger={ } /> + {selectedSourceIds.size === 0 && ( +

+ {t('agents.form.sourcePopup.noSourceHint')} +

+ )}
@@ -1551,6 +1469,16 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { onClose={() => setShareModalOpen(false)} /> )} + {uploadModalState === 'ACTIVE' && ( + setUploadModalState('INACTIVE')} + onSuccessfulUpload={() => setPendingUploadSelect(true)} + /> + )} ({ id: node.id, type: node.type as - | 'start' - | 'end' - | 'agent' - | 'note' - | 'state' - | 'condition' - | 'code', + 'start' | 'end' | 'agent' | 'note' | 'state' | 'condition' | 'code', title: node.data.title || node.data.label || node.type, position: node.position, data: @@ -327,10 +321,9 @@ function WorkflowBuilderInner() { const [availableTools, setAvailableTools] = useState([]); const sourceOptions = useMemo( () => - (sourceDocs ?? []).map((doc) => ({ - value: doc.id ?? 'default', - label: doc.name, - })), + (sourceDocs ?? []) + .filter((doc) => Boolean(doc.id)) + .map((doc) => ({ value: doc.id as string, label: doc.name })), [sourceDocs], ); const [agentJsonSchemaDrafts, setAgentJsonSchemaDrafts] = useState< @@ -3042,11 +3035,7 @@ function WorkflowBuilderInner() { .map((n) => ({ id: n.id, type: n.type as - | 'start' - | 'end' - | 'agent' - | 'state' - | 'code', + 'start' | 'end' | 'agent' | 'state' | 'code', title: n.data.title || n.data.label || n.type, position: n.position, data: diff --git a/frontend/src/components/MessageInput.tsx b/frontend/src/components/MessageInput.tsx index 1fc75d1d..3a3c9a58 100644 --- a/frontend/src/components/MessageInput.tsx +++ b/frontend/src/components/MessageInput.tsx @@ -55,6 +55,7 @@ import { partitionAttachmentFiles, } from '../constants/fileUpload'; import { UserToolType } from '../settings/types'; +import { sourceItemId } from '../utils/sourceUtils'; import { isChatToolVisible } from '../utils/toolUtils'; const generateId = (): string => @@ -1459,9 +1460,6 @@ export default function MessageInput({ void _docs; }; - // Stable id for matching selected sources: prefer ``id``, fall back to ``date``. - const sourceItemId = (doc: Doc): string => doc.id || doc.date; - const sourceItems: MultiSelectPopoverItem[] = (sourceDocs || []).map( (doc) => ({ id: sourceItemId(doc), diff --git a/frontend/src/components/SourcesPopoverFooter.tsx b/frontend/src/components/SourcesPopoverFooter.tsx new file mode 100644 index 00000000..e918e718 --- /dev/null +++ b/frontend/src/components/SourcesPopoverFooter.tsx @@ -0,0 +1,39 @@ +import { useTranslation } from 'react-i18next'; + +import RedirectIcon from '../assets/redirect.svg'; +import { Button } from './ui/button'; + +type SourcesPopoverFooterProps = { + /** Fires when the sources link is followed so the caller can close its popover. */ + onNavigate: () => void; + onUploadClick: () => void; +}; + +/** Shared footer for source pickers: a link to the sources page and an upload shortcut. */ +export default function SourcesPopoverFooter({ + onNavigate, + onUploadClick, +}: SourcesPopoverFooterProps) { + const { t } = useTranslation(); + + return ( +
+ + {t('settings.sources.goToSources')} + + + +
+ ); +} diff --git a/frontend/src/components/message-input/SourcesTrigger.tsx b/frontend/src/components/message-input/SourcesTrigger.tsx index 2e527f59..7feee685 100644 --- a/frontend/src/components/message-input/SourcesTrigger.tsx +++ b/frontend/src/components/message-input/SourcesTrigger.tsx @@ -1,12 +1,12 @@ import { useTranslation } from 'react-i18next'; -import RedirectIcon from '../../assets/redirect.svg'; import SourceIcon from '../../assets/source.svg'; import type { Doc } from '../../models/misc'; import { MultiSelectPopover, type MultiSelectPopoverItem, } from '../MultiSelectPopover'; +import SourcesPopoverFooter from '../SourcesPopoverFooter'; import { Button } from '../ui/button'; type SourcesTriggerProps = { @@ -41,29 +41,10 @@ export default function SourcesTrigger({ searchPlaceholder={t('settings.sources.searchPlaceholder')} emptyMessage={t('conversation.sources.noSourcesAvailable')} footer={ - + onOpenChange(false)} + onUploadClick={onUploadClick} + /> } trigger={ diff --git a/frontend/src/hooks/useDataInitializer.ts b/frontend/src/hooks/useDataInitializer.ts index ab5e3e3c..b91f3b5f 100644 --- a/frontend/src/hooks/useDataInitializer.ts +++ b/frontend/src/hooks/useDataInitializer.ts @@ -1,7 +1,6 @@ import { useEffect } from 'react'; import { useDispatch, useSelector } from 'react-redux'; -import { Doc } from '../models/misc'; import { getDocs, getConversations, @@ -9,11 +8,9 @@ import { } from '../preferences/preferenceApi'; import { selectConversations, - selectSelectedDocs, selectToken, setConversations, setPrompts, - setSelectedDocs, setSourceDocs, } from '../preferences/preferenceSlice'; @@ -22,7 +19,7 @@ import { * * Custom hook responsible for initializing all application data on mount. * This hook handles: - * - Fetching and setting up documents (source docs and selected docs) + * - Fetching and setting up source documents * - Fetching and setting up prompts * - Fetching and setting up conversations * @@ -31,7 +28,6 @@ import { export default function useDataInitializer(isAuthLoading: boolean) { const dispatch = useDispatch(); const token = useSelector(selectToken); - const selectedDoc = useSelector(selectSelectedDocs); const conversations = useSelector(selectConversations); // Initialize documents @@ -45,20 +41,6 @@ export default function useDataInitializer(isAuthLoading: boolean) { try { const data = await getDocs(token); dispatch(setSourceDocs(data)); - - // Auto-select default document if none selected - if ( - !selectedDoc || - (Array.isArray(selectedDoc) && selectedDoc.length === 0) - ) { - if (Array.isArray(data)) { - data.forEach((doc: Doc) => { - if (doc.model && doc.name === 'default') { - dispatch(setSelectedDocs([doc])); - } - }); - } - } } catch (error) { console.error('Failed to fetch documents:', error); } diff --git a/frontend/src/locale/de.json b/frontend/src/locale/de.json index acad9fdc..55c8763d 100644 --- a/frontend/src/locale/de.json +++ b/frontend/src/locale/de.json @@ -1088,7 +1088,8 @@ "text": "Wähle deine Quellen", "link": "Quellen-Link", "view_more": "{{count}} weitere Quellen", - "noSourcesAvailable": "Keine Quellen verfügbar" + "noSourcesAvailable": "Keine Quellen verfügbar", + "selectedCount": "{{count}} Quellen ausgewählt" }, "attachments": { "attach": "Anhängen", @@ -1248,7 +1249,10 @@ "sourcePopup": { "title": "Quellen auswählen", "searchPlaceholder": "Quellen suchen...", - "noOptionsMessage": "Keine Quellen verfügbar" + "noOptionsMessage": "Keine Quellen verfügbar", + "groupOwn": "Deine Quellen", + "groupTeam": "Mit dem Team geteilt", + "noSourceHint": "Optional. Ohne Quelle antwortet der Agent nur anhand des Modells und seiner Werkzeuge." }, "toolsPopup": { "title": "Werkzeuge auswählen", diff --git a/frontend/src/locale/en.json b/frontend/src/locale/en.json index d97d0b94..a1c86fdc 100644 --- a/frontend/src/locale/en.json +++ b/frontend/src/locale/en.json @@ -1093,7 +1093,8 @@ "text": "Choose Your Sources", "link": "Source link", "view_more": "{{count}} more sources", - "noSourcesAvailable": "No sources available" + "noSourcesAvailable": "No sources available", + "selectedCount": "{{count}} sources selected" }, "attachments": { "attach": "Attach", @@ -1258,7 +1259,10 @@ "sourcePopup": { "title": "Select Sources", "searchPlaceholder": "Search sources...", - "noOptionsMessage": "No sources available" + "noOptionsMessage": "No sources available", + "groupOwn": "Your sources", + "groupTeam": "Shared with team", + "noSourceHint": "Optional. Without a source the agent answers from the model and its tools only." }, "toolsPopup": { "title": "Select Tools", diff --git a/frontend/src/locale/es.json b/frontend/src/locale/es.json index 3fcb19ef..4b5aadac 100644 --- a/frontend/src/locale/es.json +++ b/frontend/src/locale/es.json @@ -1088,7 +1088,8 @@ "text": "Elegir tus fuentes", "link": "Enlace fuente", "view_more": "Ver {{count}} más fuentes", - "noSourcesAvailable": "No hay fuentes disponibles" + "noSourcesAvailable": "No hay fuentes disponibles", + "selectedCount": "{{count}} fuentes seleccionadas" }, "attachments": { "attach": "Adjuntar", @@ -1248,7 +1249,10 @@ "sourcePopup": { "title": "Seleccionar Fuentes", "searchPlaceholder": "Buscar fuentes...", - "noOptionsMessage": "No hay fuentes disponibles" + "noOptionsMessage": "No hay fuentes disponibles", + "groupOwn": "Tus fuentes", + "groupTeam": "Compartidas con el equipo", + "noSourceHint": "Opcional. Sin una fuente, el agente responde solo con el modelo y sus herramientas." }, "toolsPopup": { "title": "Seleccionar Herramientas", diff --git a/frontend/src/locale/jp.json b/frontend/src/locale/jp.json index faa486f4..bb407b25 100644 --- a/frontend/src/locale/jp.json +++ b/frontend/src/locale/jp.json @@ -1088,7 +1088,8 @@ "text": "ソーステキスト", "link": "ソースリンク", "view_more": "さらに{{count}}個のソース", - "noSourcesAvailable": "利用可能なソースがありません" + "noSourcesAvailable": "利用可能なソースがありません", + "selectedCount": "{{count}} 件のソースを選択中" }, "attachments": { "attach": "添付", @@ -1248,7 +1249,10 @@ "sourcePopup": { "title": "ソースを選択", "searchPlaceholder": "ソースを検索...", - "noOptionsMessage": "利用可能なソースがありません" + "noOptionsMessage": "利用可能なソースがありません", + "groupOwn": "自分のソース", + "groupTeam": "チームと共有", + "noSourceHint": "任意。ソースがない場合、エージェントはモデルとツールのみで回答します。" }, "toolsPopup": { "title": "ツールを選択", diff --git a/frontend/src/locale/ru.json b/frontend/src/locale/ru.json index 52ffa34d..1ee14216 100644 --- a/frontend/src/locale/ru.json +++ b/frontend/src/locale/ru.json @@ -1088,7 +1088,8 @@ "text": "Выберите ваши источники", "link": "Ссылка на источник", "view_more": "ещё {{count}} источников", - "noSourcesAvailable": "Нет доступных источников" + "noSourcesAvailable": "Нет доступных источников", + "selectedCount": "Выбрано источников: {{count}}" }, "attachments": { "attach": "Прикрепить", @@ -1248,7 +1249,10 @@ "sourcePopup": { "title": "Выберите Источники", "searchPlaceholder": "Поиск источников...", - "noOptionsMessage": "Нет доступных источников" + "noOptionsMessage": "Нет доступных источников", + "groupOwn": "Ваши источники", + "groupTeam": "Общие с командой", + "noSourceHint": "Необязательно. Без источника агент отвечает только на основе модели и своих инструментов." }, "toolsPopup": { "title": "Выберите Инструменты", diff --git a/frontend/src/locale/zh-TW.json b/frontend/src/locale/zh-TW.json index 0b77cb1e..39981712 100644 --- a/frontend/src/locale/zh-TW.json +++ b/frontend/src/locale/zh-TW.json @@ -1088,7 +1088,8 @@ "text": "來源文字", "link": "來源連結", "view_more": "查看更多 {{count}} 個來源", - "noSourcesAvailable": "沒有可用的來源" + "noSourcesAvailable": "沒有可用的來源", + "selectedCount": "已選擇 {{count}} 個來源" }, "attachments": { "attach": "附件", @@ -1248,7 +1249,10 @@ "sourcePopup": { "title": "選擇來源", "searchPlaceholder": "搜尋來源...", - "noOptionsMessage": "沒有可用的來源" + "noOptionsMessage": "沒有可用的來源", + "groupOwn": "我的來源", + "groupTeam": "團隊共享", + "noSourceHint": "可選。未選擇來源時,代理僅根據模型和工具作答。" }, "toolsPopup": { "title": "選擇工具", diff --git a/frontend/src/locale/zh.json b/frontend/src/locale/zh.json index 7e664a4f..ecfe0205 100644 --- a/frontend/src/locale/zh.json +++ b/frontend/src/locale/zh.json @@ -1088,7 +1088,8 @@ "text": "来源文本", "link": "来源链接", "view_more": "还有{{count}}个来源", - "noSourcesAvailable": "没有可用的来源" + "noSourcesAvailable": "没有可用的来源", + "selectedCount": "已选择 {{count}} 个来源" }, "attachments": { "attach": "附件", @@ -1248,7 +1249,10 @@ "sourcePopup": { "title": "选择来源", "searchPlaceholder": "搜索来源...", - "noOptionsMessage": "没有可用的来源" + "noOptionsMessage": "没有可用的来源", + "groupOwn": "我的来源", + "groupTeam": "团队共享", + "noSourceHint": "可选。未选择来源时,智能体仅根据模型和工具作答。" }, "toolsPopup": { "title": "选择工具", diff --git a/frontend/src/modals/ShareConversationModal.tsx b/frontend/src/modals/ShareConversationModal.tsx index b6242a4d..d0ac889c 100644 --- a/frontend/src/modals/ShareConversationModal.tsx +++ b/frontend/src/modals/ShareConversationModal.tsx @@ -47,14 +47,12 @@ export const ShareConversationModal = ({ const selectedPrompt = useSelector(selectPrompt); const selectedChunk = useSelector(selectChunks); + // Only ingested sources (rows with an id) can be shared. const extractDocPaths = (docs: Doc[]) => docs - ? docs.map((doc: Doc) => { - return { - label: doc.name, - value: doc.id ?? 'default', - }; - }) + ? docs + .filter((doc: Doc) => Boolean(doc.id)) + .map((doc: Doc) => ({ label: doc.name, value: doc.id as string })) : []; const [sourcePath, setSourcePath] = useState<{ diff --git a/frontend/src/preferences/preferenceSlice.ts b/frontend/src/preferences/preferenceSlice.ts index 37cb7d4b..6f8bfc31 100644 --- a/frontend/src/preferences/preferenceSlice.ts +++ b/frontend/src/preferences/preferenceSlice.ts @@ -55,16 +55,7 @@ const initialState: Preference = { { name: 'strict', id: 'strict', type: 'public' }, ], chunks: '2', - selectedDocs: [ - { - id: 'default', - name: 'default', - type: 'remote', - date: 'default', - model: 'openai_text-embedding-ada-002', - retriever: 'classic', - }, - ] as Doc[], + selectedDocs: [], sourceDocs: null, conversations: { data: null, diff --git a/frontend/src/store.ts b/frontend/src/store.ts index a2b9ee97..ff3330af 100644 --- a/frontend/src/store.ts +++ b/frontend/src/store.ts @@ -43,16 +43,7 @@ const preloadedState: { preference: Preference } = { data: null, loading: false, }, - sourceDocs: [ - { - name: 'default', - date: '', - model: '1.0', - type: 'remote', - id: 'default', - retriever: 'clasic', - }, - ], + sourceDocs: null, modalState: 'INACTIVE', paginatedDocuments: null, templateAgents: null, diff --git a/frontend/src/utils/sourceUtils.test.ts b/frontend/src/utils/sourceUtils.test.ts new file mode 100644 index 00000000..913c43f7 --- /dev/null +++ b/frontend/src/utils/sourceUtils.test.ts @@ -0,0 +1,144 @@ +import { describe, expect, it } from 'vitest'; + +import type { Doc } from '../models/misc'; +import { + selectedSourceIdsFromAgent, + serializeAgentSources, + sourceItemId, + toSourcePickerItems, +} from './sourceUtils'; + +const doc = (overrides: Partial): Doc => ({ + name: 'doc', + date: '2026-01-01', + model: 'm', + ...overrides, +}); + +const labels = { own: 'Your sources', team: 'Shared with team' }; + +describe('sourceItemId', () => { + it('uses the source id when present', () => { + expect(sourceItemId(doc({ id: 'abc', date: 'd' }))).toBe('abc'); + }); + + it('falls back to the date for legacy rows without an id', () => { + expect(sourceItemId(doc({ date: '2026-02-02' }))).toBe('2026-02-02'); + }); +}); + +describe('toSourcePickerItems', () => { + it('returns no items for a missing list', () => { + expect(toSourcePickerItems(null, labels)).toEqual([]); + expect(toSourcePickerItems(undefined, labels)).toEqual([]); + }); + + it('stays flat when every source is the caller’s own', () => { + const items = toSourcePickerItems( + [ + doc({ id: 'a', name: 'A' }), + doc({ id: 'b', name: 'B', ownership: 'user' }), + ], + labels, + 'icon.svg', + ); + expect(items).toEqual([ + { id: 'a', label: 'A', icon: 'icon.svg' }, + { id: 'b', label: 'B', icon: 'icon.svg' }, + ]); + expect(items.every((item) => item.group === undefined)).toBe(true); + }); + + it('groups own sources first, then team-shared ones', () => { + const items = toSourcePickerItems( + [ + doc({ id: 't1', name: 'Team one', ownership: 'team' }), + doc({ id: 'o1', name: 'Own one', ownership: 'user' }), + doc({ id: 't2', name: 'Team two', ownership: 'team' }), + doc({ id: 'o2', name: 'Own two' }), + ], + labels, + ); + expect(items.map((item) => [item.id, item.group])).toEqual([ + ['o1', 'Your sources'], + ['o2', 'Your sources'], + ['t1', 'Shared with team'], + ['t2', 'Shared with team'], + ]); + }); +}); + +describe('serializeAgentSources', () => { + const docs = [doc({ id: 'a' }), doc({ id: 'b' })]; + + it('sends both fields empty when nothing is selected', () => { + expect(serializeAgentSources([], docs)).toEqual({ + source: '', + sources: [], + }); + }); + + it('uses the legacy single source field for one selection', () => { + expect(serializeAgentSources(['a'], docs)).toEqual({ + source: 'a', + sources: [], + }); + }); + + it('uses the sources list for several selections', () => { + expect(serializeAgentSources(new Set(['a', 'b']), docs)).toEqual({ + source: '', + sources: ['a', 'b'], + }); + }); + + it('resolves picker ids to source ids and passes unknown ids through', () => { + const legacy = doc({ id: undefined, date: 'legacy-date' }); + expect( + serializeAgentSources(['legacy-date', 'shared-by-owner'], [legacy]), + ).toEqual({ + source: '', + sources: ['legacy-date', 'shared-by-owner'], + }); + expect(serializeAgentSources(['shared-by-owner'], null)).toEqual({ + source: 'shared-by-owner', + sources: [], + }); + }); + + it('drops duplicates and empty ids', () => { + expect(serializeAgentSources(['a', 'a', ''], docs)).toEqual({ + source: 'a', + sources: [], + }); + }); +}); + +describe('selectedSourceIdsFromAgent', () => { + it('prefers the sources list over the single source', () => { + expect( + selectedSourceIdsFromAgent({ source: 'x', sources: ['a', 'b'] }), + ).toEqual(['a', 'b']); + }); + + it('falls back to the single source', () => { + expect(selectedSourceIdsFromAgent({ source: 'a', sources: [] })).toEqual([ + 'a', + ]); + }); + + it('treats the legacy default placeholder and empty values as no source', () => { + expect(selectedSourceIdsFromAgent({ source: 'default' })).toEqual([]); + expect(selectedSourceIdsFromAgent({ sources: ['default', ''] })).toEqual( + [], + ); + expect(selectedSourceIdsFromAgent({ source: '' })).toEqual([]); + expect(selectedSourceIdsFromAgent({})).toEqual([]); + }); + + it('keeps real ids next to a legacy placeholder', () => { + expect(selectedSourceIdsFromAgent({ sources: ['default', 'a'] })).toEqual([ + 'a', + ]); + }); +}); diff --git a/frontend/src/utils/sourceUtils.ts b/frontend/src/utils/sourceUtils.ts new file mode 100644 index 00000000..3683ee79 --- /dev/null +++ b/frontend/src/utils/sourceUtils.ts @@ -0,0 +1,73 @@ +import type { MultiSelectPopoverItem } from '../components/MultiSelectPopover'; +import type { Doc } from '../models/misc'; + +/** Stable picker id for a source: its id, or its date for legacy rows without one. */ +export function sourceItemId(doc: Pick): string { + return doc.id || doc.date; +} + +export type SourceGroupLabels = { own: string; team: string }; + +/** + * Build picker items for a source list. When any source is shared through a + * team, items are grouped into the caller's own sources followed by the + * team-shared ones; otherwise the list stays flat. + */ +export function toSourcePickerItems( + docs: Doc[] | null | undefined, + labels: SourceGroupLabels, + icon?: MultiSelectPopoverItem['icon'], +): MultiSelectPopoverItem[] { + const list = docs ?? []; + const isTeam = (doc: Doc) => doc.ownership === 'team'; + const hasTeam = list.some(isTeam); + const toItem = (doc: Doc): MultiSelectPopoverItem => ({ + id: sourceItemId(doc), + label: doc.name, + icon, + ...(hasTeam ? { group: isTeam(doc) ? labels.team : labels.own } : {}), + }); + if (!hasTeam) return list.map(toItem); + return [ + ...list.filter((doc) => !isTeam(doc)).map(toItem), + ...list.filter(isTeam).map(toItem), + ]; +} + +export type AgentSourcePayload = { source: string; sources: string[] }; + +/** + * Serialise a picker selection into the agent payload: one selection goes in + * the legacy single ``source`` field, several go in ``sources``, and none + * leaves both empty (the agent then skips retrieval). Picker ids resolve to + * source ids; ids not in ``docs`` (e.g. an owner's source a team editor can't + * list) pass through unchanged. + */ +export function serializeAgentSources( + selectedIds: Iterable, + docs: Doc[] | null | undefined, +): AgentSourcePayload { + const ids = Array.from(new Set(selectedIds)) + .map((id) => docs?.find((doc) => sourceItemId(doc) === id)?.id ?? id) + .filter(Boolean); + if (ids.length > 1) return { source: '', sources: ids }; + if (ids.length === 1) return { source: ids[0], sources: [] }; + return { source: '', sources: [] }; +} + +/** + * Selected source ids for a stored agent. ``sources`` wins over the legacy + * single ``source``; the legacy ``"default"`` placeholder means no source. + */ +export function selectedSourceIdsFromAgent(agent: { + source?: string; + sources?: string[]; +}): string[] { + const ids = + agent.sources && agent.sources.length > 0 + ? agent.sources + : agent.source + ? [agent.source] + : []; + return ids.filter((id) => Boolean(id) && id !== 'default'); +} diff --git a/tests/api/user/agents/test_routes_happy.py b/tests/api/user/agents/test_routes_happy.py index 8f8938cc..0936ab2e 100644 --- a/tests/api/user/agents/test_routes_happy.py +++ b/tests/api/user/agents/test_routes_happy.py @@ -782,12 +782,10 @@ class TestUpdateAgent: response = UpdateAgent().put(str(agent["id"])) assert response.status_code == 400 - def test_publish_with_default_source_succeeds(self, app, pg_conn): - """The frontend's auto-selected "Default" source has no UUID — it - sends ``source: ''`` and ``sources: []`` with ``retriever: 'classic'``. - Pre-fix, the publish gate rejected this with - ``Missing or invalid required fields: Source``. The retriever - carries the runtime identity, so the gate now accepts it. + def test_publish_with_no_source_succeeds(self, app, pg_conn): + """An agent with no source is a valid published state: it answers + from the model and its tools. The form sends ``source: ''`` and + ``sources: []`` with the default ``retriever``. """ from docsgpt.api.user.agents.routes import UpdateAgent from docsgpt.storage.db.repositories.agents import AgentsRepository @@ -827,11 +825,11 @@ class TestUpdateAgent: f"unexpected error: {response.json}" ) - def test_publish_without_source_or_retriever_returns_400( + def test_publish_without_source_or_retriever_succeeds( self, app, pg_conn, ): - """If neither a source nor a retriever is configured, the gate - still trips — the agent has no way to retrieve anything.""" + """Neither a source nor a retriever is required to publish; a + source-less agent simply skips retrieval at run time.""" from docsgpt.api.user.agents.routes import UpdateAgent from docsgpt.storage.db.repositories.agents import AgentsRepository from docsgpt.storage.db.repositories.prompts import PromptsRepository @@ -864,8 +862,12 @@ class TestUpdateAgent: from flask import request request.decoded_token = {"sub": user} response = UpdateAgent().put(str(agent["id"])) - assert response.status_code == 400 - assert "Source or retriever" in response.json.get("message", "") + assert response.status_code == 200, ( + f"unexpected error: {response.json}" + ) + stored = AgentsRepository(pg_conn).get_by_id(str(agent["id"])) + assert stored["status"] == "published" + assert stored["source_id"] is None def test_publishing_generates_api_key(self, app, pg_conn): from docsgpt.api.user.agents.routes import UpdateAgent @@ -1075,25 +1077,71 @@ class TestCreateAgentMore: ) assert status == 400 - def test_publish_classic_without_source_returns_400(self, app): + def test_publish_classic_without_source_succeeds(self, app, pg_conn): + """Publishing with ``source: ''`` and ``sources: []`` creates a + source-less agent (``source_id`` NULL) instead of a 400.""" from docsgpt.api.user.agents.routes import CreateAgent + from docsgpt.storage.db.repositories.agents import AgentsRepository - with app.test_request_context( + user = "u-create-no-source" + with _patch_db(pg_conn), app.test_request_context( "/api/create_agent", method="POST", json={ - "name": "n", + "name": "no-source", "description": "d", "status": "published", "agent_type": "classic", + "chunks": "2", + "retriever": "classic", + "prompt_id": "default", + "source": "", + "sources": [], }, ): from flask import request - request.decoded_token = {"sub": "u"} + request.decoded_token = {"sub": user} response = CreateAgent().post() - status = ( - response[1] if isinstance(response, tuple) else response.status_code + assert response.status_code == 201, ( + f"unexpected error: {response.json}" ) - assert status == 400 + agents = AgentsRepository(pg_conn).list_for_user(user) + created = next(a for a in agents if a["name"] == "no-source") + assert created["status"] == "published" + assert created["source_id"] is None + assert not created.get("extra_source_ids") + assert created["key"] + + def test_publish_classic_with_legacy_default_sentinel_succeeds( + self, app, pg_conn, + ): + """Older clients still send ``source: "default"``; it keeps mapping + to a NULL source rather than being rejected.""" + from docsgpt.api.user.agents.routes import CreateAgent + from docsgpt.storage.db.repositories.agents import AgentsRepository + + user = "u-create-legacy-default" + with _patch_db(pg_conn), app.test_request_context( + "/api/create_agent", method="POST", + json={ + "name": "legacy-default", + "description": "d", + "status": "published", + "agent_type": "classic", + "chunks": "2", + "retriever": "classic", + "prompt_id": "default", + "source": "default", + }, + ): + from flask import request + request.decoded_token = {"sub": user} + response = CreateAgent().post() + assert response.status_code == 201, ( + f"unexpected error: {response.json}" + ) + agents = AgentsRepository(pg_conn).list_for_user(user) + created = next(a for a in agents if a["name"] == "legacy-default") + assert created["source_id"] is None def test_unknown_agent_type_falls_back_classic(self, app, pg_conn): from docsgpt.api.user.agents.routes import CreateAgent diff --git a/tests/api/user/sources/test_routes.py b/tests/api/user/sources/test_routes.py index 32b39bdf..35a57a68 100644 --- a/tests/api/user/sources/test_routes.py +++ b/tests/api/user/sources/test_routes.py @@ -80,7 +80,9 @@ class TestCombinedJson: response = CombinedJson().get() assert response.status_code == 401 - def test_returns_default_plus_user_sources(self, app, pg_conn): + def test_returns_only_real_user_sources(self, app, pg_conn): + """No synthetic "Default" row: every entry is an ingested source + with an id, so clients never have to special-case a placeholder.""" from docsgpt.api.user.sources.routes import CombinedJson user = "u-list-sources" @@ -93,8 +95,20 @@ class TestCombinedJson: assert response.status_code == 200 names = [d["name"] for d in response.json] - assert "Default" in names - assert "doc1" in names + assert "Default" not in names + assert names == ["doc1"] + assert all(d.get("id") for d in response.json) + + def test_returns_empty_list_when_user_has_no_sources(self, app, pg_conn): + from docsgpt.api.user.sources.routes import CombinedJson + + with _patch_db(pg_conn), app.test_request_context("/api/sources"): + from flask import request + request.decoded_token = {"sub": "u-no-sources"} + response = CombinedJson().get() + + assert response.status_code == 200 + assert response.json == [] def test_db_error_returns_400(self, app): from docsgpt.api.user.sources.routes import CombinedJson From 0fe059db12790a0f63dbcf8909274bed83ad1eb4 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 9 Sep 2026 18:39:09 +0100 Subject: [PATCH 2/7] fix(agents): list source-less agents and select only the uploaded source Review follow-ups: - The agents list and the pinned list dropped agents that had neither a source nor a retriever. Publishing such an agent is now allowed, so it vanished from the list right after publish. Both filters are gone; a source-less agent skips retrieval and is still runnable. Tests cover both listings. - The agent form selected an uploaded source by diffing the source catalog before and after the upload, which would also pick up any source created or shared in the meantime. Upload now passes the id of the source it created to onSuccessfulUpload, and the form selects only that id. --- docsgpt/api/user/agents/routes.py | 14 +------ frontend/src/agents/NewAgent.tsx | 27 +++++-------- frontend/src/upload/Upload.tsx | 20 ++++++---- tests/api/user/agents/test_routes_happy.py | 46 ++++++++++++++++++++++ 4 files changed, 69 insertions(+), 38 deletions(-) diff --git a/docsgpt/api/user/agents/routes.py b/docsgpt/api/user/agents/routes.py index 16a5875c..c65b0df6 100644 --- a/docsgpt/api/user/agents/routes.py +++ b/docsgpt/api/user/agents/routes.py @@ -481,18 +481,11 @@ class GetAgents(Resource): shared_ids = [aid for aid in team_shared if aid not in owned_ids] shared_agents = agents_repo.list_by_ids(shared_ids) - def _is_runnable(agent: dict) -> bool: - return bool( - agent.get("source_id") - or (agent.get("extra_source_ids") or []) - or agent.get("retriever") - or agent.get("agent_type") == "workflow" - ) - + # Every agent is listed: one with no source skips retrieval and + # answers from the model and its tools, so it is still runnable. list_agents = [ _format_agent_output(agent, pinned=str(agent["id"]) in pinned_ids) for agent in agents - if _is_runnable(agent) ] list_agents += [ _format_agent_output( @@ -501,7 +494,6 @@ class GetAgents(Resource): team_access=team_shared.get(str(agent["id"])), ) for agent in shared_agents - if _is_runnable(agent) ] except Exception as err: current_app.logger.error(f"Error retrieving agents: {err}", exc_info=True) @@ -1511,8 +1503,6 @@ class PinnedAgents(Resource): list_pinned_agents = [] for agent in pinned_agents: source_id = agent.get("source_id") - if not source_id and not agent.get("retriever"): - continue list_pinned_agents.append( { "id": str(agent["id"]), diff --git a/frontend/src/agents/NewAgent.tsx b/frontend/src/agents/NewAgent.tsx index ba49aac4..3f588a9e 100644 --- a/frontend/src/agents/NewAgent.tsx +++ b/frontend/src/agents/NewAgent.tsx @@ -153,9 +153,6 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { const [isModelsPopupOpen, setIsModelsPopupOpen] = useState(false); const [uploadModalState, setUploadModalState] = useState('INACTIVE'); - // Set once an upload started from this form finishes, so the source it - // created can be selected as soon as the refreshed list arrives. - const [pendingUploadSelect, setPendingUploadSelect] = useState(false); const [selectedSourceIds, setSelectedSourceIds] = useState>( new Set(), ); @@ -181,7 +178,6 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { const sourceAnchorButtonRef = useRef(null); const toolAnchorButtonRef = useRef(null); const modelAnchorButtonRef = useRef(null); - const sourceIdsBeforeUploadRef = useRef>(new Set()); const modeConfig = { new: { @@ -275,13 +271,18 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { }); const handleUploadClick = () => { - sourceIdsBeforeUploadRef.current = new Set( - (sourceDocs ?? []).map(sourceItemId), - ); setIsSourcePopupOpen(false); setUploadModalState('ACTIVE'); }; + // Select the source an upload started from this form created. The id comes + // from the upload itself, so a source that appeared meanwhile for another + // reason is never picked up by accident. + const handleUploadedSource = (sourceId?: string) => { + if (!sourceId) return; + setSelectedSourceIds((prev) => new Set([...prev, sourceId])); + }; + // Sources go out as the legacy single ``source`` for one selection and as // the ``sources`` list for several; an empty selection sends both empty. const appendSourceFields = (formData: FormData) => { @@ -656,16 +657,6 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { validateAndSetFolder(); }, [folderIdFromUrl, agentFolders, token, dispatch]); - // After an upload started from this form, select the source it created. - useEffect(() => { - if (!pendingUploadSelect || !sourceDocs) return; - const before = sourceIdsBeforeUploadRef.current; - const added = sourceDocs.map(sourceItemId).filter((id) => !before.has(id)); - if (added.length === 0) return; - setPendingUploadSelect(false); - setSelectedSourceIds((prev) => new Set([...prev, ...added])); - }, [pendingUploadSelect, sourceDocs]); - useEffect(() => { if ((mode === 'edit' || mode === 'draft') && agentId) { const getAgent = async () => { @@ -1476,7 +1467,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { isOnboarding={false} renderTab={null} close={() => setUploadModalState('INACTIVE')} - onSuccessfulUpload={() => setPendingUploadSelect(true)} + onSuccessfulUpload={handleUploadedSource} /> )} void; - onSuccessfulUpload?: () => void; + /** Fires once the upload is ingested, with the id of the source it created. */ + onSuccessfulUpload?: (sourceId?: string) => void; }) { const token = useSelector(selectToken); const selectedDocs = useSelector(selectSelectedDocs); @@ -442,7 +443,10 @@ function Upload({ (clientTaskId: string) => { let handled = false; - const handleTerminal = (status: 'completed' | 'failed') => { + const handleTerminal = ( + status: 'completed' | 'failed', + sourceId?: string, + ) => { if (handled) return; handled = true; if (status !== 'completed') return; @@ -468,7 +472,7 @@ function Upload({ } } } - onSuccessfulUpload?.(); + onSuccessfulUpload?.(sourceId); }) .catch((err) => { console.error( @@ -483,7 +487,7 @@ function Upload({ const task = state.upload.tasks.find((t) => t.id === clientTaskId); if (!task) return false; if (task.status === 'completed' || task.status === 'failed') { - handleTerminal(task.status); + handleTerminal(task.status, task.sourceId); return true; } // Recover from the race where the terminal SSE landed before @@ -494,7 +498,7 @@ function Upload({ for (const event of state.notifications.recentEvents) { if (event.scope?.id !== task.sourceId) continue; if (event.type === 'source.ingest.completed') { - handleTerminal('completed'); + handleTerminal('completed', task.sourceId); return true; } if (event.type === 'source.ingest.failed') { @@ -626,7 +630,7 @@ function Upload({ updates: { status: 'completed', progress: 100 }, }), ); - onSuccessfulUpload?.(); + onSuccessfulUpload?.(parsed.source_id); } } catch (error) { handleTaskFailure(clientTaskId); @@ -762,7 +766,7 @@ function Upload({ updates: { status: 'completed', progress: 100 }, }), ); - onSuccessfulUpload?.(); + onSuccessfulUpload?.(response.source_id); } } catch (error) { handleTaskFailure(clientTaskId); @@ -813,7 +817,7 @@ function Upload({ }, }), ); - onSuccessfulUpload?.(); + onSuccessfulUpload?.(data.source_id); }) .catch(() => handleTaskFailure(clientTaskId)); }; diff --git a/tests/api/user/agents/test_routes_happy.py b/tests/api/user/agents/test_routes_happy.py index 0936ab2e..66e6c95f 100644 --- a/tests/api/user/agents/test_routes_happy.py +++ b/tests/api/user/agents/test_routes_happy.py @@ -128,6 +128,23 @@ class TestGetAgents: names = [a["name"] for a in response.json] assert "B1" in names and "B2" in names + def test_lists_source_less_agent_without_retriever(self, app, pg_conn): + """A published agent with no source and no retriever skips retrieval + at run time, so the list must not hide it.""" + from docsgpt.api.user.agents.routes import GetAgents + + user = "u-list-no-source" + _seed_agent( + pg_conn, user=user, name="bare", with_source=False, retriever="", + ) + + with _patch_db(pg_conn), app.test_request_context("/api/get_agents"): + from flask import request + request.decoded_token = {"sub": user} + response = GetAgents().get() + assert response.status_code == 200 + assert "bare" in [a["name"] for a in response.json] + def test_db_error_returns_400(self, app): from docsgpt.api.user.agents.routes import GetAgents @@ -1693,6 +1710,35 @@ class TestPinnedAgentsListing: assert len(response.json) == 1 assert response.json[0]["pinned"] is True + def test_lists_pinned_source_less_agent_without_retriever( + self, app, pg_conn, + ): + """Pinning follows the main list: no source and no retriever is + still a runnable agent and must not be dropped.""" + from docsgpt.api.user.agents.routes import PinAgent, PinnedAgents + + user = "u-pinned-no-source" + agent = _seed_agent( + pg_conn, user=user, with_source=False, retriever="", + ) + + with _patch_db(pg_conn), app.test_request_context( + f"/api/pin_agent?id={agent['id']}", method="POST" + ): + from flask import request + request.decoded_token = {"sub": user} + PinAgent().post() + + with _patch_db(pg_conn), app.test_request_context( + "/api/pinned_agents" + ): + from flask import request + request.decoded_token = {"sub": user} + response = PinnedAgents().get() + + assert response.status_code == 200 + assert [a["id"] for a in response.json] == [str(agent["id"])] + def test_pinned_db_error_returns_400(self, app): from docsgpt.api.user.agents.routes import PinnedAgents From 9778910abe93c45c79a632868bb676fc64143dd5 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 10 Sep 2026 08:36:10 +0100 Subject: [PATCH 3/7] test: list route keeps agents with neither source nor retriever The coverage suite still asserted the old filter that hid such agents from /api/get_agents. They are listed now, so the test checks that. --- tests/test_extra_coverage.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/test_extra_coverage.py b/tests/test_extra_coverage.py index cda347d0..58a6822e 100644 --- a/tests/test_extra_coverage.py +++ b/tests/test_extra_coverage.py @@ -341,8 +341,9 @@ class TestSeederExtra: class TestAgentsRoutesRemainingGaps: - def test_get_agents_filters_out_incomplete(self, app, pg_conn): - """Agents missing both source and retriever are filtered from the list.""" + def test_get_agents_lists_source_less_agents(self, app, pg_conn): + """An agent with neither source nor retriever is still listed: it + skips retrieval at run time and answers from the model and tools.""" from docsgpt.api.user.agents.routes import GetAgents from docsgpt.storage.db.repositories.agents import AgentsRepository @@ -351,12 +352,11 @@ class TestAgentsRoutesRemainingGaps: yield pg_conn user = "u-filter" - # Complete agent: has retriever AgentsRepository(pg_conn).create( user, "ok", "published", retriever="classic", ) - # Incomplete agent: no source, no retriever, not workflow type - AgentsRepository(pg_conn).create(user, "bad", "published") + # No source, no retriever, not a workflow agent. + AgentsRepository(pg_conn).create(user, "bare", "published") with patch( "docsgpt.api.user.agents.routes.db_session", _yield @@ -369,4 +369,4 @@ class TestAgentsRoutesRemainingGaps: assert response.status_code == 200 names = [a["name"] for a in response.json] assert "ok" in names - assert "bad" not in names + assert "bare" in names From c26b8a3403ac216c060704563a6451b84febb5a6 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 11 Sep 2026 11:28:14 +0100 Subject: [PATCH 4/7] fix(agents): gate agent pinning on visibility PinAgent looked agents up with no ownership, share or team predicate, and PinnedAgents returned those rows through a hand-rolled dict that bypassed the blanking _format_agent_output applies. Anyone holding an agent id -- a share-link recipient learns the real UUID -- could pin it and keep reading its name, description, prompt, tools, type, status and masked key after the share was revoked. Add _user_may_pin: the owner, a team grantee, a recipient of a still-live share link, or a system template. PinAgent checks it before pinning and PinnedAgents re-checks every row on read, so a revoked grant drops out of the list instead of persisting. Unpinning stays open regardless of current access, or a revoked share would strand a pin the user cannot clear. The masked key is now owner-only, matching _format_agent_output. Adds the first cross-user pin tests: every existing one pinned the caller's own agent. --- docsgpt/api/user/agents/routes.py | 80 +++++++++++-- tests/api/user/agents/test_routes_happy.py | 132 +++++++++++++++++++++ 2 files changed, 200 insertions(+), 12 deletions(-) diff --git a/docsgpt/api/user/agents/routes.py b/docsgpt/api/user/agents/routes.py index c65b0df6..2ddcda5d 100644 --- a/docsgpt/api/user/agents/routes.py +++ b/docsgpt/api/user/agents/routes.py @@ -1456,6 +1456,33 @@ class DeleteAgent(Resource): return make_response(jsonify({"id": pg_agent_id}), 200) +def _user_may_pin(conn, agent: dict, user_id: str, shared_with_me: set) -> bool: + """Whether ``user_id`` may pin, and keep reading, ``agent``. + + Pinning follows visibility: the owner, a team grantee, anyone who opened a + still-live share link, and the premade system templates. Without this gate a + pin is an unscoped read of any agent id. + + Args: + conn: Open database connection. + agent: The agent row, needing at least ``id``, ``user_id`` and ``shared``. + user_id: The caller. + shared_with_me: Agent ids the caller reached through a share link. + + Returns: + True if the caller is allowed to see the agent. + """ + owner_id = agent.get("user_id") + if owner_id == user_id: + return True + if owner_id in ("system", "__system__"): + return True + agent_id = str(agent["id"]) + if agent.get("shared") and agent_id in shared_with_me: + return True + return can_access(conn, "agent", agent_id, user_id) + + @agents_ns.route("/pinned_agents") class PinnedAgents(Resource): @api.doc(description="Get pinned agents for the user") @@ -1469,11 +1496,13 @@ class PinnedAgents(Resource): with db_session() as conn: users_repo = UsersRepository(conn) user_doc = users_repo.upsert(user_id) - pinned_ids = ( - user_doc.get("agent_preferences", {}).get("pinned", []) + prefs = ( + user_doc.get("agent_preferences", {}) if isinstance(user_doc.get("agent_preferences"), dict) - else [] + else {} ) + pinned_ids = prefs.get("pinned", []) or [] + shared_with_me = set(prefs.get("shared_with_me", []) or []) if not pinned_ids: return make_response(jsonify([]), 200) @@ -1500,6 +1529,15 @@ class PinnedAgents(Resource): if stale: users_repo.remove_pinned_bulk(user_id, stale) + # A pin is not proof of access: a share or team grant can be + # revoked long after the agent was pinned, so re-check every + # row on read rather than trusting the stored id list. + pinned_agents = [ + agent + for agent in pinned_agents + if _user_may_pin(conn, agent, user_id, shared_with_me) + ] + list_pinned_agents = [] for agent in pinned_agents: source_id = agent.get("source_id") @@ -1528,7 +1566,7 @@ class PinnedAgents(Resource): "last_used_at": agent.get("last_used_at", ""), "key": ( f"{agent['key'][:4]}...{agent['key'][-4:]}" - if agent.get("key") + if agent.get("key") and agent.get("user_id") == user_id else "" ), "pinned": True, @@ -1797,20 +1835,25 @@ class PinAgent(Resource): ) try: with db_session() as conn: - # Any user can pin any agent they can see — including - # shared ones. Use the non-user-scoped lookup so pins - # aren't restricted to owner-only. + # Any user can pin any agent they can *see* — owned, team-shared, + # reached through a share link, or a system template. The lookup + # is deliberately not owner-scoped, so visibility is checked + # separately below. from sqlalchemy import text as _sql_text if looks_like_uuid(agent_id): agent_row = conn.execute( - _sql_text("SELECT id FROM agents WHERE id = CAST(:id AS uuid)"), + _sql_text( + "SELECT id, user_id, shared FROM agents " + "WHERE id = CAST(:id AS uuid)" + ), {"id": agent_id}, ).fetchone() else: agent_row = conn.execute( _sql_text( - "SELECT id FROM agents WHERE legacy_mongo_id = :id" + "SELECT id, user_id, shared FROM agents " + "WHERE legacy_mongo_id = :id" ), {"id": agent_id}, ).fetchone() @@ -1823,15 +1866,28 @@ class PinAgent(Resource): users_repo = UsersRepository(conn) user_doc = users_repo.upsert(user_id) - pinned_list = ( - user_doc.get("agent_preferences", {}).get("pinned", []) + prefs = ( + user_doc.get("agent_preferences", {}) if isinstance(user_doc.get("agent_preferences"), dict) - else [] + else {} ) + pinned_list = prefs.get("pinned", []) or [] if pg_agent_id in pinned_list: + # Unpinning stays open regardless of current access, or a + # revoked share would leave a pin the user can't clear. users_repo.remove_pinned(user_id, pg_agent_id) action = "unpinned" else: + if not _user_may_pin( + conn, + dict(agent_row._mapping), + user_id, + set(prefs.get("shared_with_me", []) or []), + ): + return make_response( + jsonify({"success": False, "message": "Agent not found"}), + 404, + ) users_repo.add_pinned(user_id, pg_agent_id) action = "pinned" except Exception as err: diff --git a/tests/api/user/agents/test_routes_happy.py b/tests/api/user/agents/test_routes_happy.py index 66e6c95f..d850970f 100644 --- a/tests/api/user/agents/test_routes_happy.py +++ b/tests/api/user/agents/test_routes_happy.py @@ -1503,6 +1503,138 @@ class TestPinAgentMore: assert response.status_code == 500 +class TestPinAccessControl: + """Pinning must follow visibility — it is a read of the agent.""" + + def _pin(self, app, pg_conn, agent_id, user): + from docsgpt.api.user.agents.routes import PinAgent + + with _patch_db(pg_conn), app.test_request_context( + f"/api/pin_agent?id={agent_id}", method="POST" + ): + from flask import request + request.decoded_token = {"sub": user} + return PinAgent().post() + + def _pinned(self, app, pg_conn, user): + from docsgpt.api.user.agents.routes import PinnedAgents + + with _patch_db(pg_conn), app.test_request_context("/api/pinned_agents"): + from flask import request + request.decoded_token = {"sub": user} + return PinnedAgents().get() + + def test_stranger_cannot_pin_another_users_agent(self, app, pg_conn): + from docsgpt.storage.db.repositories.users import UsersRepository + + owner, stranger = "u-pin-owner", "u-pin-stranger" + agent = _seed_agent(pg_conn, user=owner, name="Private") + + response = self._pin(app, pg_conn, str(agent["id"]), stranger) + + assert response.status_code == 404 + prefs = UsersRepository(pg_conn).upsert(stranger).get( + "agent_preferences" + ) or {} + assert prefs.get("pinned", []) == [] + + def test_stranger_pinned_list_stays_empty(self, app, pg_conn): + """Even a pin forced into the prefs must not leak the agent.""" + from docsgpt.storage.db.repositories.users import UsersRepository + + owner, stranger = "u-pin-owner2", "u-pin-stranger2" + agent = _seed_agent(pg_conn, user=owner, name="Private") + users_repo = UsersRepository(pg_conn) + users_repo.upsert(stranger) + users_repo.add_pinned(stranger, str(agent["id"])) + + response = self._pinned(app, pg_conn, stranger) + + assert response.status_code == 200 + assert response.json == [] + + def test_share_link_recipient_can_pin(self, app, pg_conn): + from docsgpt.storage.db.repositories.agents import AgentsRepository + from docsgpt.storage.db.repositories.users import UsersRepository + + owner, recipient = "u-pin-sharer", "u-pin-recipient" + agent = _seed_agent(pg_conn, user=owner, name="Shared") + agent_id = str(agent["id"]) + AgentsRepository(pg_conn).update(agent_id, owner, {"shared": True}) + users_repo = UsersRepository(pg_conn) + users_repo.upsert(recipient) + users_repo.add_shared(recipient, agent_id) + + assert self._pin(app, pg_conn, agent_id, recipient).status_code == 200 + listed = self._pinned(app, pg_conn, recipient) + assert [a["id"] for a in listed.json] == [agent_id] + + def test_revoked_share_drops_out_of_the_pinned_list(self, app, pg_conn): + from docsgpt.storage.db.repositories.agents import AgentsRepository + from docsgpt.storage.db.repositories.users import UsersRepository + + owner, recipient = "u-pin-revoke-owner", "u-pin-revoke-recipient" + agent = _seed_agent(pg_conn, user=owner, name="Shared") + agent_id = str(agent["id"]) + repo = AgentsRepository(pg_conn) + repo.update(agent_id, owner, {"shared": True}) + users_repo = UsersRepository(pg_conn) + users_repo.upsert(recipient) + users_repo.add_shared(recipient, agent_id) + self._pin(app, pg_conn, agent_id, recipient) + + repo.update(agent_id, owner, {"shared": False}) + + assert self._pinned(app, pg_conn, recipient).json == [] + + def test_team_grantee_can_pin_but_gets_no_key(self, app, pg_conn): + from docsgpt.storage.db.repositories.agents import AgentsRepository + + owner, member = "u-pin-team-owner", "u-pin-team-member" + agent = _seed_agent(pg_conn, user=owner, name="Team") + agent_id = str(agent["id"]) + AgentsRepository(pg_conn).update(agent_id, owner, {"key": "abcd1234wxyz"}) + + with patch( + "docsgpt.api.user.agents.routes.can_access", return_value=True + ): + assert self._pin(app, pg_conn, agent_id, member).status_code == 200 + listed = self._pinned(app, pg_conn, member) + + assert [a["id"] for a in listed.json] == [agent_id] + assert listed.json[0]["key"] == "" + + def test_owner_still_sees_the_masked_key(self, app, pg_conn): + from docsgpt.storage.db.repositories.agents import AgentsRepository + + owner = "u-pin-key-owner" + agent = _seed_agent(pg_conn, user=owner, name="Mine") + agent_id = str(agent["id"]) + AgentsRepository(pg_conn).update(agent_id, owner, {"key": "abcd1234wxyz"}) + + self._pin(app, pg_conn, agent_id, owner) + + assert self._pinned(app, pg_conn, owner).json[0]["key"] == "abcd...wxyz" + + def test_unpin_survives_lost_access(self, app, pg_conn): + """A revoked share must not strand a pin the user can't remove.""" + from docsgpt.storage.db.repositories.users import UsersRepository + + owner, recipient = "u-unpin-owner", "u-unpin-recipient" + agent = _seed_agent(pg_conn, user=owner, name="Gone") + agent_id = str(agent["id"]) + users_repo = UsersRepository(pg_conn) + users_repo.upsert(recipient) + users_repo.add_pinned(recipient, agent_id) + + response = self._pin(app, pg_conn, agent_id, recipient) + + assert response.status_code == 200 + assert response.json["action"] == "unpinned" + prefs = users_repo.upsert(recipient).get("agent_preferences") or {} + assert agent_id not in prefs.get("pinned", []) + + class TestRegenerateAgentKey: def _seed_published_with_key(self, pg_conn, user, key): from docsgpt.storage.db.repositories.agents import AgentsRepository From 25987c8947b91eacab38ab17609e4fccc8505038 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 11 Sep 2026 11:28:26 +0100 Subject: [PATCH 5/7] fix(agents): correct source selection in the agent form, chat and uploads selectedSourceIdsFromAgent preferred the extra sources over the legacy single source instead of merging them. Since every save writes both fields, opening an agent that held both and saving -- even just a rename -- silently detached the primary source. Retrieval reads both, so merge them. The edit form also reported unsaved changes the moment it loaded: the snapshot taken after the fetch defaulted the retriever to "classic" and left the default model empty, while the effects that run straight after rewrite both. Normalise the snapshot through the same serializer and rules the effects apply. A source attached to a team-shared agent that the caller cannot list -- the owner's private source -- rendered no picker row, so it showed as attached with nothing to switch it off. Append rows for selected ids missing from the list, and memoise the label lookup the trigger and the picker now share. Uploading from the agent builder repointed the open conversation onto the new source and persisted it, because the upload modal always dispatched the chat selection. Gate that behind a prop the builder opts out of. The wiki source is created synchronously and never refreshed the source list, so the builder selected an id the picker could not render and the trigger read "External KB"; refresh on every completion path that skips ingest tracking. Removing the synthetic "Default" source made an empty source list reachable, which the stored-selection reconciler skipped, leaving a deleted source checked forever with no way to clear it. Treat an empty list as a real answer and only skip when the list has not loaded. Finally, finish the picker extraction: the chat input still hand-rolled the item mapping, so team users saw the same sources grouped in the agent builder and flat in chat. Point it at the shared helper, with its own locale keys, and make the sources link a router navigation instead of a full page reload. --- frontend/src/agents/NewAgent.tsx | 79 ++++++++++++------- frontend/src/components/MessageInput.tsx | 20 +++-- .../src/components/SourcesPopoverFooter.tsx | 7 +- frontend/src/locale/de.json | 4 +- frontend/src/locale/en.json | 4 +- frontend/src/locale/es.json | 4 +- frontend/src/locale/jp.json | 4 +- frontend/src/locale/ru.json | 4 +- frontend/src/locale/zh-TW.json | 4 +- frontend/src/locale/zh.json | 4 +- .../src/preferences/preferenceSlice.test.ts | 57 ++++++++++++- frontend/src/preferences/preferenceSlice.ts | 14 ++-- frontend/src/upload/Upload.tsx | 54 +++++++++++-- frontend/src/utils/sourceUtils.test.ts | 8 +- frontend/src/utils/sourceUtils.ts | 20 ++--- 15 files changed, 215 insertions(+), 72 deletions(-) diff --git a/frontend/src/agents/NewAgent.tsx b/frontend/src/agents/NewAgent.tsx index 3f588a9e..72238d7e 100644 --- a/frontend/src/agents/NewAgent.tsx +++ b/frontend/src/agents/NewAgent.tsx @@ -235,32 +235,45 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { // source list; fall back to the owner-resolved name embedded in the agent // payload (source_details) so a team member viewing a shared agent sees the // source name instead of "External KB"; only then show the generic label. - const resolveSourceLabel = (id: string): string => { - const matchedDoc = sourceDocs?.find( - (source) => sourceItemId(source) === id, - ); - if (matchedDoc?.name) return matchedDoc.name; - const detail = agent.source_details?.find((d) => d.id === id); - if (detail?.name) return detail.name; - return t('agents.form.externalKb'); - }; - - const sourceItems = useMemo( - () => - toSourcePickerItems( - sourceDocs, - { - own: t('agents.form.sourcePopup.groupOwn'), - team: t('agents.form.sourcePopup.groupTeam'), - }, - SourceIcon, - ), - [sourceDocs, t], + const resolveSourceLabel = useCallback( + (id: string): string => { + const matchedDoc = sourceDocs?.find( + (source) => sourceItemId(source) === id, + ); + if (matchedDoc?.name) return matchedDoc.name; + const detail = agent.source_details?.find((d) => d.id === id); + if (detail?.name) return detail.name; + return t('agents.form.externalKb'); + }, + [agent.source_details, sourceDocs, t], ); - const selectedSourceNames = Array.from(selectedSourceIds) - .map((id) => resolveSourceLabel(id)) - .filter(Boolean); + const sourceItems = useMemo(() => { + const items = toSourcePickerItems( + sourceDocs, + { + own: t('agents.form.sourcePopup.groupOwn'), + team: t('agents.form.sourcePopup.groupTeam'), + }, + SourceIcon, + ); + // An attached source the caller can't list — an owner's private source on + // a team-shared agent — still needs a row of its own, or it reads as + // selected with nothing to switch it off. + const listed = new Set(items.map((item) => item.id)); + const unlisted = Array.from(selectedSourceIds) + .filter((id) => !listed.has(id)) + .map((id) => ({ id, label: resolveSourceLabel(id), icon: SourceIcon })); + return [...items, ...unlisted]; + }, [resolveSourceLabel, selectedSourceIds, sourceDocs, t]); + + const selectedSourceNames = useMemo( + () => + Array.from(selectedSourceIds) + .map((id) => resolveSourceLabel(id)) + .filter(Boolean), + [resolveSourceLabel, selectedSourceIds], + ); const sourceTriggerLabel = selectedSourceIds.size === 0 ? t('agents.form.placeholders.selectSources') @@ -667,7 +680,8 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { } const data = await response.json(); - setSelectedSourceIds(new Set(selectedSourceIdsFromAgent(data))); + const agentSourceIds = selectedSourceIdsFromAgent(data); + setSelectedSourceIds(new Set(agentSourceIds)); if (data.tool_details) setSelectedTools(data.tool_details); if (data.status === 'draft') setEffectiveMode('draft'); @@ -679,16 +693,22 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { // Backfill required fields so older agents (created before // agent_type / prompt_id / models existed) don't fail // ``isPublishable()`` and leave Save permanently disabled. + // Normalise exactly as the source and model effects below will, or the + // form compares unequal to this snapshot and reports unsaved changes + // before the user has touched anything. + const agentModels: string[] = data.models || []; const normalized = { ...data, agent_type: data.agent_type || 'classic', prompt_id: data.prompt_id || 'default', - retriever: data.retriever || 'classic', + retriever: agentSourceIds.length === 0 ? 'classic' : '', chunks: data.chunks || '2', tools: data.tools || [], - sources: data.sources || [], - models: data.models || [], - default_model_id: data.default_model_id || '', + ...serializeAgentSources(agentSourceIds, sourceDocs), + models: agentModels, + default_model_id: agentModels.includes(data.default_model_id || '') + ? data.default_model_id + : agentModels[0] || '', config: data.config || {}, }; setAgent(normalized); @@ -1468,6 +1488,7 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { renderTab={null} close={() => setUploadModalState('INACTIVE')} onSuccessfulUpload={handleUploadedSource} + selectUploadedDoc={false} /> )} @@ -1460,12 +1461,17 @@ export default function MessageInput({ void _docs; }; - const sourceItems: MultiSelectPopoverItem[] = (sourceDocs || []).map( - (doc) => ({ - id: sourceItemId(doc), - label: doc.name, - icon: SourceIcon, - }), + const sourceItems: MultiSelectPopoverItem[] = useMemo( + () => + toSourcePickerItems( + sourceDocs, + { + own: t('conversation.sources.groupOwn'), + team: t('conversation.sources.groupTeam'), + }, + SourceIcon, + ), + [sourceDocs, t], ); const selectedSourceIds = ( diff --git a/frontend/src/components/SourcesPopoverFooter.tsx b/frontend/src/components/SourcesPopoverFooter.tsx index e918e718..056d47fc 100644 --- a/frontend/src/components/SourcesPopoverFooter.tsx +++ b/frontend/src/components/SourcesPopoverFooter.tsx @@ -1,4 +1,5 @@ import { useTranslation } from 'react-i18next'; +import { Link } from 'react-router-dom'; import RedirectIcon from '../assets/redirect.svg'; import { Button } from './ui/button'; @@ -18,14 +19,14 @@ export default function SourcesPopoverFooter({ return (
- {t('settings.sources.goToSources')} - +