From f658515ca3e9330a37e667d06db5eb69d6212545 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 22 Sep 2026 00:42:17 +0100 Subject: [PATCH] Address review findings on the agents navigation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Opening a folder dropped the active filter. agentsListPath() always built off the bare /agents/manage root, so from /agents/manage/mine it navigated to /agents/manage?folder=..., and filterFromPath() then read back `all`. A regression from making the filters routable: they used to be component state, which a replace-navigation left alone. The helper now takes the filter and builds off agentsFilterPath(), which also fixes the other half — switching filter while inside a folder re-navigates to the filtered path with the folder kept, instead of the sync effect pulling it back to the unfiltered URL. - Both manage-agents handlers cleared the selected agent and closed the mobile nav before returning early for a modifier-click, so opening the list in a new tab mutated the tab you stayed on. The modifier check and preventDefault() now run first. - The detail breadcrumbs had the name precedence inverted. Tools.tsx shows `customName || displayName` throughout, so a renamed tool kept its old name in the breadcrumb alone. ToolConfig now matches, and RemoteDeviceConfig leads with device?.name as its own heading does. --- frontend/src/Navigation.tsx | 4 ++-- frontend/src/agents/AgentsList.tsx | 17 ++++++++--------- frontend/src/agents/paths.test.ts | 8 ++++++++ frontend/src/agents/paths.ts | 20 ++++++++++++++------ frontend/src/settings/RemoteDeviceConfig.tsx | 4 +++- frontend/src/settings/ToolConfig.tsx | 2 +- 6 files changed, 36 insertions(+), 19 deletions(-) diff --git a/frontend/src/Navigation.tsx b/frontend/src/Navigation.tsx index 59b7ea11..87367af8 100644 --- a/frontend/src/Navigation.tsx +++ b/frontend/src/Navigation.tsx @@ -615,11 +615,11 @@ export default function Navigation({ navOpen, setNavOpen }: NavigationProps) { to={AGENTS_MANAGE_ROOT} end onClick={(event) => { - dispatch(setSelectedAgent(null)); - closeNavOnMobile(); if (event.metaKey || event.ctrlKey || event.shiftKey) return; event.preventDefault(); + dispatch(setSelectedAgent(null)); + closeNavOnMobile(); goToLevel(AGENTS_MANAGE_ROOT); }} className={({ isActive }) => diff --git a/frontend/src/agents/AgentsList.tsx b/frontend/src/agents/AgentsList.tsx index 079d7a56..ec4ad597 100644 --- a/frontend/src/agents/AgentsList.tsx +++ b/frontend/src/agents/AgentsList.tsx @@ -61,6 +61,10 @@ export default function AgentsList() { const [showAgentTypeModal, setShowAgentTypeModal] = useState(false); const [modalFolderId, setModalFolderId] = useState(null); + // The list's filter is a route, so it is linkable and survives a reload; + // the sidebar nav (and the pill row below `lg`) does the navigating. + const activeFilter = filterFromPath(location.pathname); + // Sync folder path with URL useEffect(() => { const currentFolderInUrl = searchParams.get('folder'); @@ -68,19 +72,14 @@ export default function AgentsList() { folderPath.length > 0 ? folderPath[folderPath.length - 1] : null; if (currentFolderId !== currentFolderInUrl) { - const newUrl = currentFolderId - ? agentsListPath(currentFolderId) - : agentsListPath(); - navigate(newUrl, { replace: true }); + navigate(agentsListPath(currentFolderId, activeFilter), { + replace: true, + }); } - }, [folderPath, searchParams, navigate]); + }, [folderPath, searchParams, navigate, activeFilter]); const { isLoading, refetchFolders, refetchUserAgents } = useAgentsFetch(); - // The list's filter is a route, so it is linkable and survives a reload; - // the sidebar nav (and the pill row below `lg`) does the navigating. - const activeFilter = filterFromPath(location.pathname); - const { searchQuery, setSearchQuery, diff --git a/frontend/src/agents/paths.test.ts b/frontend/src/agents/paths.test.ts index b45bcb1d..146594c3 100644 --- a/frontend/src/agents/paths.test.ts +++ b/frontend/src/agents/paths.test.ts @@ -31,6 +31,14 @@ describe('agentsListPath', () => { expect(agentsListPath('a b')).toBe('/agents/manage?folder=a%20b'); expect(agentsListPath(null)).toBe('/agents/manage'); }); + + it('keeps the active filter, so opening a folder does not widen the list', () => { + expect(agentsListPath('f1', 'user')).toBe('/agents/manage/mine?folder=f1'); + expect(agentsListPath(null, 'user')).toBe('/agents/manage/mine'); + expect(filterFromPath(agentsListPath('f1', 'team').split('?')[0])).toBe( + 'team', + ); + }); }); describe('agentEditPathFor', () => { diff --git a/frontend/src/agents/paths.ts b/frontend/src/agents/paths.ts index 1194206b..31ad7023 100644 --- a/frontend/src/agents/paths.ts +++ b/frontend/src/agents/paths.ts @@ -32,18 +32,26 @@ const SLUG_FILTERS = Object.fromEntries( export const isWorkflowAgent = (agent: Pick): boolean => agent.agent_type === 'workflow'; -/** The agent list, optionally scoped to a folder. */ -export const agentsListPath = (folderId?: string | null): string => - folderId - ? `${AGENTS_MANAGE_ROOT}?folder=${encodeURIComponent(folderId)}` - : AGENTS_MANAGE_ROOT; - /** The list narrowed to one filter; each filter is its own linkable route. */ export const agentsFilterPath = (filter: AgentFilterTab): string => filter === 'all' ? AGENTS_MANAGE_ROOT : `${AGENTS_MANAGE_ROOT}/${FILTER_SLUGS[filter]}`; +/** + * The agent list, optionally scoped to a folder and to a filter. The filter + * has to be carried explicitly: it lives in the path now, so building a + * folder URL off the bare root would silently widen the list back to every + * section the moment someone opened a folder from a filtered view. + */ +export const agentsListPath = ( + folderId?: string | null, + filter: AgentFilterTab = 'all', +): string => { + const base = agentsFilterPath(filter); + return folderId ? `${base}?folder=${encodeURIComponent(folderId)}` : base; +}; + /** Which filter a list route selects; `all` for anything unrecognised. */ export const filterFromPath = (pathname: string): AgentFilterTab => { const slug = pathname.startsWith(`${AGENTS_MANAGE_ROOT}/`) diff --git a/frontend/src/settings/RemoteDeviceConfig.tsx b/frontend/src/settings/RemoteDeviceConfig.tsx index b5ac1238..afc04833 100644 --- a/frontend/src/settings/RemoteDeviceConfig.tsx +++ b/frontend/src/settings/RemoteDeviceConfig.tsx @@ -197,7 +197,9 @@ export default function RemoteDeviceConfig({ tool, handleGoBack }: Props) {