From 0fe059db12790a0f63dbcf8909274bed83ad1eb4 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 9 Sep 2026 18:39:09 +0100 Subject: [PATCH] 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