diff --git a/.gitignore b/.gitignore index f9e72c46..27013d3b 100644 --- a/.gitignore +++ b/.gitignore @@ -211,3 +211,7 @@ bench/ # Python package build output (uv build) dist/ + +# Personal agent exports/imports kept at the repo root (may hold chat ids, +# webhook targets and other per-user config) +/*.agent.yaml 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..2ddcda5d 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", @@ -480,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( @@ -500,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) @@ -661,20 +654,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 +1169,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 " @@ -1497,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") @@ -1510,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) @@ -1541,11 +1529,18 @@ 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") - if not source_id and not agent.get("retriever"): - continue list_pinned_agents.append( { "id": str(agent["id"]), @@ -1571,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, @@ -1840,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() @@ -1866,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/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..72238d7e 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,8 @@ 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'); const [selectedSourceIds, setSelectedSourceIds] = useState>( new Set(), ); @@ -199,11 +215,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 = () => { @@ -220,15 +235,76 @@ 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) => - source.id === id || source.name === id || source.retriever === id, + 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 sourceItems = useMemo(() => { + const items = toSourcePickerItems( + sourceDocs, + { + own: t('agents.form.sourcePopup.groupOwn'), + team: t('agents.form.sourcePopup.groupTeam'), + }, + SourceIcon, ); - 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'); + // 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') + : selectedSourceIds.size === 1 + ? selectedSourceNames[0] + : t('conversation.sources.selectedCount', { + count: selectedSourceIds.size, + }); + + const handleUploadClick = () => { + 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) => { + const { source, sources } = serializeAgentSources( + selectedSourceIds, + sourceDocs, + ); + formData.append('source', source); + formData.append('sources', JSON.stringify(sources)); }; const handleUpload = useCallback((files: File[]) => { @@ -261,39 +337,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 +430,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,17 +670,6 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { validateAndSetFolder(); }, [folderIdFromUrl, agentFolders, token, dispatch]); - // Auto-select default source if none selected - 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]); - useEffect(() => { if ((mode === 'edit' || mode === 'draft') && agentId) { const getAgent = async () => { @@ -679,31 +680,8 @@ 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])); - } + const agentSourceIds = selectedSourceIdsFromAgent(data); + setSelectedSourceIds(new Set(agentSourceIds)); if (data.tool_details) setSelectedTools(data.tool_details); if (data.status === 'draft') setEffectiveMode('draft'); @@ -715,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); @@ -763,56 +747,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 +945,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 +1480,17 @@ export default function NewAgent({ mode }: { mode: 'new' | 'edit' | 'draft' }) { onClose={() => setShareModalOpen(false)} /> )} + {uploadModalState === 'ACTIVE' && ( + setUploadModalState('INACTIVE')} + onSuccessfulUpload={handleUploadedSource} + selectUploadedDoc={false} + /> + )} ({ 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..ddff0c3f 100644 --- a/frontend/src/components/MessageInput.tsx +++ b/frontend/src/components/MessageInput.tsx @@ -3,6 +3,7 @@ import { useCallback, useEffect, useLayoutEffect, + useMemo, useRef, useState, } from 'react'; @@ -55,6 +56,7 @@ import { partitionAttachmentFiles, } from '../constants/fileUpload'; import { UserToolType } from '../settings/types'; +import { sourceItemId, toSourcePickerItems } from '../utils/sourceUtils'; import { isChatToolVisible } from '../utils/toolUtils'; const generateId = (): string => @@ -1459,15 +1461,17 @@ 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), - 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 new file mode 100644 index 00000000..056d47fc --- /dev/null +++ b/frontend/src/components/SourcesPopoverFooter.tsx @@ -0,0 +1,40 @@ +import { useTranslation } from 'react-i18next'; +import { Link } from 'react-router-dom'; + +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..74848817 100644 --- a/frontend/src/locale/de.json +++ b/frontend/src/locale/de.json @@ -1088,7 +1088,10 @@ "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", + "groupOwn": "Deine Quellen", + "groupTeam": "Mit dem Team geteilt" }, "attachments": { "attach": "Anhängen", @@ -1248,7 +1251,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..af03d4a2 100644 --- a/frontend/src/locale/en.json +++ b/frontend/src/locale/en.json @@ -1093,7 +1093,10 @@ "text": "Choose Your Sources", "link": "Source link", "view_more": "{{count}} more sources", - "noSourcesAvailable": "No sources available" + "noSourcesAvailable": "No sources available", + "selectedCount": "{{count}} sources selected", + "groupOwn": "Your sources", + "groupTeam": "Shared with team" }, "attachments": { "attach": "Attach", @@ -1258,7 +1261,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..575e127c 100644 --- a/frontend/src/locale/es.json +++ b/frontend/src/locale/es.json @@ -1088,7 +1088,10 @@ "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", + "groupOwn": "Tus fuentes", + "groupTeam": "Compartidas con el equipo" }, "attachments": { "attach": "Adjuntar", @@ -1248,7 +1251,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..70dd85d1 100644 --- a/frontend/src/locale/jp.json +++ b/frontend/src/locale/jp.json @@ -1088,7 +1088,10 @@ "text": "ソーステキスト", "link": "ソースリンク", "view_more": "さらに{{count}}個のソース", - "noSourcesAvailable": "利用可能なソースがありません" + "noSourcesAvailable": "利用可能なソースがありません", + "selectedCount": "{{count}} 件のソースを選択中", + "groupOwn": "自分のソース", + "groupTeam": "チームと共有" }, "attachments": { "attach": "添付", @@ -1248,7 +1251,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..92b6f533 100644 --- a/frontend/src/locale/ru.json +++ b/frontend/src/locale/ru.json @@ -1088,7 +1088,10 @@ "text": "Выберите ваши источники", "link": "Ссылка на источник", "view_more": "ещё {{count}} источников", - "noSourcesAvailable": "Нет доступных источников" + "noSourcesAvailable": "Нет доступных источников", + "selectedCount": "Выбрано источников: {{count}}", + "groupOwn": "Ваши источники", + "groupTeam": "Общие с командой" }, "attachments": { "attach": "Прикрепить", @@ -1248,7 +1251,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..54de2c92 100644 --- a/frontend/src/locale/zh-TW.json +++ b/frontend/src/locale/zh-TW.json @@ -1088,7 +1088,10 @@ "text": "來源文字", "link": "來源連結", "view_more": "查看更多 {{count}} 個來源", - "noSourcesAvailable": "沒有可用的來源" + "noSourcesAvailable": "沒有可用的來源", + "selectedCount": "已選擇 {{count}} 個來源", + "groupOwn": "我的來源", + "groupTeam": "團隊共享" }, "attachments": { "attach": "附件", @@ -1248,7 +1251,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..ccf4556f 100644 --- a/frontend/src/locale/zh.json +++ b/frontend/src/locale/zh.json @@ -1088,7 +1088,10 @@ "text": "来源文本", "link": "来源链接", "view_more": "还有{{count}}个来源", - "noSourcesAvailable": "没有可用的来源" + "noSourcesAvailable": "没有可用的来源", + "selectedCount": "已选择 {{count}} 个来源", + "groupOwn": "我的来源", + "groupTeam": "团队共享" }, "attachments": { "attach": "附件", @@ -1248,7 +1251,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.test.ts b/frontend/src/preferences/preferenceSlice.test.ts index cf9480e5..9ac8cb3a 100644 --- a/frontend/src/preferences/preferenceSlice.test.ts +++ b/frontend/src/preferences/preferenceSlice.test.ts @@ -1,12 +1,16 @@ -import { describe, expect, it } from 'vitest'; +import { configureStore } from '@reduxjs/toolkit'; +import { beforeEach, describe, expect, it } from 'vitest'; +import type { Doc } from '../models/misc'; import type { RootState } from '../store'; import reducer, { clearRoles, + prefListenerMiddleware, selectIsAdmin, selectRoles, selectRolesResolved, setRoles, + setSourceDocs, } from './preferenceSlice'; const baseState = () => reducer(undefined, { type: '@@INIT' }); @@ -47,3 +51,54 @@ describe('roles selectors', () => { expect(selectRolesResolved(stateWith(['user'], false))).toBe(false); }); }); + +const sourceDoc = (overrides: Partial): Doc => ({ + name: 'doc', + date: '2026-01-01', + model: 'm', + ...overrides, +}); + +const storeWithListener = () => + configureStore({ + reducer: { preference: reducer }, + middleware: (getDefault) => + getDefault().prepend(prefListenerMiddleware.middleware), + }); + +describe('setSourceDocs reconciler', () => { + beforeEach(() => { + localStorage.clear(); + }); + + it('prunes a stored selection when the source list comes back empty', () => { + const stored = [sourceDoc({ id: 'gone', name: 'Deleted' })]; + localStorage.setItem('DocsGPTRecentDocs', JSON.stringify(stored)); + const store = storeWithListener(); + + store.dispatch(setSourceDocs([])); + + expect(store.getState().preference.selectedDocs).toEqual([]); + expect(localStorage.getItem('DocsGPTRecentDocs')).toBeNull(); + }); + + it('keeps a stored selection that is still in the list', () => { + const kept = sourceDoc({ id: 'kept', name: 'Kept' }); + localStorage.setItem('DocsGPTRecentDocs', JSON.stringify([kept])); + const store = storeWithListener(); + + store.dispatch(setSourceDocs([kept, sourceDoc({ id: 'other' })])); + + expect(store.getState().preference.selectedDocs).toEqual([kept]); + }); + + it('leaves the selection alone while the list has not loaded', () => { + const stored = [sourceDoc({ id: 'pending' })]; + localStorage.setItem('DocsGPTRecentDocs', JSON.stringify(stored)); + const store = storeWithListener(); + + store.dispatch(setSourceDocs(null)); + + expect(localStorage.getItem('DocsGPTRecentDocs')).not.toBeNull(); + }); +}); diff --git a/frontend/src/preferences/preferenceSlice.ts b/frontend/src/preferences/preferenceSlice.ts index 37cb7d4b..e12a144b 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, @@ -223,14 +214,12 @@ prefListenerMiddleware.startListening({ effect: (_action, listenerApi) => { const state = listenerApi.getState() as RootState; const sourceDocs = state.preference.sourceDocs; - if (sourceDocs && sourceDocs.length > 0) { - const validatedDocs = getLocalRecentDocs(sourceDocs); - if (validatedDocs !== null) { - listenerApi.dispatch(setSelectedDocs(validatedDocs)); - } else { - listenerApi.dispatch(setSelectedDocs([])); - } - } + // ``null`` means the list hasn't loaded; an empty array is a real answer + // and still has to prune a stored selection, or a deleted last source + // stays checked forever. + if (!sourceDocs) return; + const validatedDocs = getLocalRecentDocs(sourceDocs); + listenerApi.dispatch(setSelectedDocs(validatedDocs ?? [])); }, }); 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/upload/Upload.tsx b/frontend/src/upload/Upload.tsx index 3355f1d2..2c0858a5 100644 --- a/frontend/src/upload/Upload.tsx +++ b/frontend/src/upload/Upload.tsx @@ -27,7 +27,6 @@ import { ActiveState, Doc } from '../models/misc'; import { getDocs } from '../preferences/preferenceApi'; import { selectSelectedDocs, - selectSourceDocs, selectToken, setSelectedDocs, setSourceDocs, @@ -61,13 +60,21 @@ function Upload({ renderTab = null, close, onSuccessfulUpload = () => undefined, + selectUploadedDoc = true, }: { receivedFile: File[]; setModalState: (state: ActiveState) => void; isOnboarding: boolean; renderTab: string | null; close: () => void; - onSuccessfulUpload?: () => void; + /** Fires once the upload is ingested, with the id of the source it created. */ + onSuccessfulUpload?: (sourceId?: string) => void; + /** + * Whether a finished upload also becomes the chat's selected source. Callers + * that only need the new source's id (the agent builder) pass ``false`` so + * uploading never repoints the conversation the user left open. + */ + selectUploadedDoc?: boolean; }) { const token = useSelector(selectToken); const selectedDocs = useSelector(selectSelectedDocs); @@ -403,8 +410,6 @@ function Upload({ heading: schema.heading, })); - const sourceDocs = useSelector(selectSourceDocs); - const resetUploaderState = useCallback(() => { setIngestor({ type: null, name: '', config: {} }); setfiles([]); @@ -430,6 +435,32 @@ function Upload({ [dispatch], ); + /** Re-read the source list into the store so a new source is pickable. */ + const refreshSourceDocs = useCallback( + () => + getDocs(token).then((docs) => { + dispatch(setSourceDocs(docs)); + return docs; + }), + [dispatch, token], + ); + + /** + * Finish an upload that never got an ingest task (the wiki source is created + * synchronously). The list still has to be refreshed, or the caller selects + * an id no picker can render. + */ + const finishUntrackedUpload = useCallback( + (sourceId?: string) => { + refreshSourceDocs() + .catch((err) => { + console.error('Post-upload source-list refresh failed:', err); + }) + .finally(() => onSuccessfulUpload?.(sourceId)); + }, + [onSuccessfulUpload, refreshSourceDocs], + ); + /** * Wait for the source.ingest.* SSE pipeline to flip this task into a * terminal state, then run the post-completion side effects: refresh @@ -442,22 +473,20 @@ 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; - getDocs(token) + refreshSourceDocs() .then((docs) => { - dispatch(setSourceDocs(docs)); - if (Array.isArray(docs)) { - const existingDocIds = new Set( - (Array.isArray(sourceDocs) ? sourceDocs : []) - .map((doc: Doc) => doc?.id) - .filter((id): id is string => Boolean(id)), - ); - const newDoc = docs.find( - (doc: Doc) => doc.id && !existingDocIds.has(doc.id), - ); + if (selectUploadedDoc && Array.isArray(docs) && sourceId) { + // Match the id this upload returned. Diffing against the list as + // it looked before would pick up any source that appeared + // meanwhile — another upload finishing, or a new team share. + const newDoc = docs.find((doc: Doc) => doc.id === sourceId); if (newDoc) { // If only one doc is selected, replace it completely // If multiple docs are selected, append the new doc @@ -468,7 +497,7 @@ function Upload({ } } } - onSuccessfulUpload?.(); + onSuccessfulUpload?.(sourceId); }) .catch((err) => { console.error( @@ -483,7 +512,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 +523,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') { @@ -536,7 +565,14 @@ function Upload({ } }); }, - [dispatch, onSuccessfulUpload, selectedDocs, sourceDocs, store, token], + [ + dispatch, + onSuccessfulUpload, + refreshSourceDocs, + selectedDocs, + selectUploadedDoc, + store, + ], ); const onDrop = useCallback( @@ -626,7 +662,7 @@ function Upload({ updates: { status: 'completed', progress: 100 }, }), ); - onSuccessfulUpload?.(); + finishUntrackedUpload(parsed.source_id); } } catch (error) { handleTaskFailure(clientTaskId); @@ -762,7 +798,7 @@ function Upload({ updates: { status: 'completed', progress: 100 }, }), ); - onSuccessfulUpload?.(); + finishUntrackedUpload(response.source_id); } } catch (error) { handleTaskFailure(clientTaskId); @@ -813,7 +849,7 @@ function Upload({ }, }), ); - onSuccessfulUpload?.(); + finishUntrackedUpload(data.source_id); }) .catch(() => handleTaskFailure(clientTaskId)); }; diff --git a/frontend/src/utils/sourceUtils.test.ts b/frontend/src/utils/sourceUtils.test.ts new file mode 100644 index 00000000..16ffe547 --- /dev/null +++ b/frontend/src/utils/sourceUtils.test.ts @@ -0,0 +1,150 @@ +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('merges the single source with the sources list', () => { + expect( + selectedSourceIdsFromAgent({ source: 'x', sources: ['a', 'b'] }), + ).toEqual(['x', 'a', 'b']); + }); + + it('keeps a source named in both fields once', () => { + expect( + selectedSourceIdsFromAgent({ source: 'a', 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..8a8ae39b --- /dev/null +++ b/frontend/src/utils/sourceUtils.ts @@ -0,0 +1,75 @@ +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: the legacy single ``source`` and the + * ``sources`` list merged, because retrieval reads both. Preferring one over + * the other would drop the omitted field on the next save. The legacy + * ``"default"`` placeholder means no source. + */ +export function selectedSourceIdsFromAgent(agent: { + source?: string; + sources?: string[]; +}): string[] { + const ids = [ + ...(agent.source ? [agent.source] : []), + ...(agent.sources ?? []), + ]; + return Array.from(new Set(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..d850970f 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 @@ -782,12 +799,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 +842,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 +879,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 +1094,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 @@ -1438,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 @@ -1645,6 +1842,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 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 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