From 1fe071d9304a4615eb4942e578836581ae4aaf7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Logro=C3=B1o=20Guerrero?= Date: Wed, 11 Nov 2026 11:08:05 -0500 Subject: [PATCH 1/2] fix(ui): sort MCP tools by name and lead the template page back to the chat MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The tools an MCP server reports are listed alphabetically (numeric-aware) on the MCP servers page and the app detail page, instead of in discovery order. - The chat context panel's Template link reads as a link: it underlines on hover or focus and has a tooltip saying it opens the AgentTemplate. - Opened from that link, the AgentTemplate page's back button says "Back to conversation" and returns to the same chat. A direct visit still says "Back to templates". The origin travels in router state through a small `useBackLink` hook, which only follows in-app paths. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Carlos Logroño Guerrero --- .../chat/AgentContextPanel.test.tsx | 29 +++++++++-- ui/src/components/chat/AgentContextPanel.tsx | 36 +++++++++----- .../components/mcp/ToolServerTools.test.tsx | 29 +++++++++++ ui/src/components/mcp/ToolServerTools.tsx | 14 +++++- ui/src/pages/AgentTemplateDetailsPage.tsx | 8 ++- ui/src/router/backLink.test.tsx | 46 +++++++++++++++++ ui/src/router/backLink.ts | 49 +++++++++++++++++++ 7 files changed, 192 insertions(+), 19 deletions(-) create mode 100644 ui/src/components/mcp/ToolServerTools.test.tsx create mode 100644 ui/src/router/backLink.test.tsx create mode 100644 ui/src/router/backLink.ts diff --git a/ui/src/components/chat/AgentContextPanel.test.tsx b/ui/src/components/chat/AgentContextPanel.test.tsx index caa5512015..2ccff7e1bb 100644 --- a/ui/src/components/chat/AgentContextPanel.test.tsx +++ b/ui/src/components/chat/AgentContextPanel.test.tsx @@ -1,6 +1,7 @@ import { render, screen } from "@testing-library/react"; import { ThemeProvider } from "@emotion/react"; -import { MemoryRouter } from "react-router-dom"; +import { MemoryRouter, Route, Routes, useLocation } from "react-router-dom"; +import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import { themeFor } from "@/theme/theme"; import { AgentContextPanel } from "./AgentContextPanel"; @@ -9,6 +10,11 @@ const {useAgentTemplate, useAgent} = vi.hoisted(() => ({useAgentTemplate: vi.fn( vi.mock("@/api", () => ({ useAgentTemplate, useAgent })); +function LocationState() { + const location = useLocation(); + return 
{JSON.stringify(location.state)}
; +} + function renderPanel(tools: unknown[], modelNamespace?: string) { useAgent.mockReturnValue({data: {resource: {spec: {template: {ref: {name: "assistant"}}, harnessRef: {name: "runner"}}}}}); useAgentTemplate.mockReturnValue({ @@ -26,16 +32,29 @@ function renderPanel(tools: unknown[], modelNamespace?: string) { render( - - + + + } + /> + } /> + , ); } describe("AgentContextPanel", () => { + it("opens the template with a way back to this conversation", async () => { + renderPanel([]); + await userEvent.click(screen.getByTestId("chat-agent-context-template")); + expect(JSON.parse(screen.getByTestId("location-state").textContent ?? "null")).toEqual({ + from: "/agents/abc/chat?mock=ok", + fromLabel: "Back to conversation", + }); + }); + it("renders an MCP binding whose optional tools list is omitted", () => { renderPanel([ { diff --git a/ui/src/components/chat/AgentContextPanel.tsx b/ui/src/components/chat/AgentContextPanel.tsx index 654c5a7219..f56e0c5b1b 100644 --- a/ui/src/components/chat/AgentContextPanel.tsx +++ b/ui/src/components/chat/AgentContextPanel.tsx @@ -1,9 +1,10 @@ import { referenceLabel } from "@/api/domain/agentTemplates"; -import { Alert, Skeleton, Space, Tag, Typography } from "antd"; +import { Alert, Skeleton, Space, Tag, Tooltip, Typography } from "antd"; import { useTheme } from "@emotion/react"; -import { Link } from "react-router-dom"; +import { Link, useLocation } from "react-router-dom"; import { useAgent, useAgentTemplate, bareName, type AgentInstance } from "@/api"; import { buildPath, paths } from "@/router/routes"; +import { backLinkState } from "@/router/backLink"; const { Text, Paragraph } = Typography; @@ -48,6 +49,7 @@ export function AgentContextPanel({ agentRef?: { namespace: string; name?: string }; }) { const theme = useTheme(); + const location = useLocation(); const namespace = instance?.agent?.split("/")[0] ?? agentRef?.namespace ?? ""; const name = instance?.agent ? bareName(instance.agent) : agentRef?.name; @@ -65,17 +67,27 @@ export function AgentContextPanel({
{templateRef && templateNamespace && templateName ? ( - - + + {templateRef} - - + + ) : ( {definition.data?.resource.spec.template.inline ? "Inline" : "Not reported"} )} diff --git a/ui/src/components/mcp/ToolServerTools.test.tsx b/ui/src/components/mcp/ToolServerTools.test.tsx new file mode 100644 index 0000000000..5071b8bb02 --- /dev/null +++ b/ui/src/components/mcp/ToolServerTools.test.tsx @@ -0,0 +1,29 @@ +import { render, screen } from "@testing-library/react"; +import { ThemeProvider } from "@emotion/react"; +import { describe, expect, it } from "vitest"; +import { themeFor } from "@/theme/theme"; +import { ToolServerTools } from "./ToolServerTools"; + +function renderedNames(names: string[]) { + render( + + ({ name, description: "" }))} /> + , + ); + return screen.getAllByTestId("tool-server-tool").map((item) => item.textContent); +} + +describe("ToolServerTools", () => { + it("lists tools alphabetically, whatever order the server reported them in", () => { + expect( + renderedNames(["k8s_get_pods", "K8s_apply", "k8s_describe"]), + ).toEqual(["K8s_apply", "k8s_describe", "k8s_get_pods"]); + }); + + it("orders numbered tools by number rather than character by character", () => { + expect(renderedNames(["tool_10", "tool_2"])).toEqual([ + "tool_2", + "tool_10", + ]); + }); +}); diff --git a/ui/src/components/mcp/ToolServerTools.tsx b/ui/src/components/mcp/ToolServerTools.tsx index 4aaa2ec795..bc605a4849 100644 --- a/ui/src/components/mcp/ToolServerTools.tsx +++ b/ui/src/components/mcp/ToolServerTools.tsx @@ -43,7 +43,7 @@ export function ToolServerTools({ tools, highlight }: ToolServerToolsProps) { gap: theme.space(3), }} > - {tools.map((tool) => ( + {sortToolsByName(tools).map((tool) => (
  • + a.name.localeCompare(b.name, undefined, { numeric: true, sensitivity: "base" }), + ); +} + /** * Wraps occurrences of `term` in a ``. * diff --git a/ui/src/pages/AgentTemplateDetailsPage.tsx b/ui/src/pages/AgentTemplateDetailsPage.tsx index 4278d2b908..21d2571f38 100644 --- a/ui/src/pages/AgentTemplateDetailsPage.tsx +++ b/ui/src/pages/AgentTemplateDetailsPage.tsx @@ -29,6 +29,7 @@ import { type AgentTemplateDraft, } from "@/components/agent-template-form/agentTemplateDraft"; import { agentTemplatesTab } from "@/router/routes"; +import { useBackLink } from "@/router/backLink"; import { apiClient, useAgentConversations, @@ -50,6 +51,9 @@ export function AgentTemplateDetailsPage() { const theme = useTheme(); const { namespace, name } = useParams(); const [searchParams, setSearchParams] = useSearchParams(); + // Back to wherever the reader came from — a conversation's context panel, say — and + // to the list when they opened this page directly. + const back = useBackLink({ to: agentTemplatesTab, label: "Back to templates" }); const template = useAgentTemplate(namespace, name); const invalidateTemplates = useInvalidateAgentTemplates(); @@ -335,7 +339,9 @@ export function AgentTemplateDetailsPage() { description="Agents referencing this template cannot prepare new configuration until it is restored or replaced. Existing conversations and last successful revisions are retained." /> ) : null} - + } > diff --git a/ui/src/router/backLink.test.tsx b/ui/src/router/backLink.test.tsx new file mode 100644 index 0000000000..e11796ac69 --- /dev/null +++ b/ui/src/router/backLink.test.tsx @@ -0,0 +1,46 @@ +import { render, screen } from "@testing-library/react"; +import { MemoryRouter } from "react-router-dom"; +import { describe, expect, it } from "vitest"; +import { backLinkState, useBackLink } from "./backLink"; + +const FALLBACK = { to: "/agents?tab=templates", label: "Back to templates" }; + +function Probe() { + const back = useBackLink(FALLBACK); + return {back.label}; +} + +function renderAt(state: unknown) { + render( + + + , + ); + return screen.getByRole("link"); +} + +describe("useBackLink", () => { + it("leads back to the page that opened this one, search included", () => { + const link = renderAt( + backLinkState({ pathname: "/agents/abc/chat", search: "?mock=ok" }, "Back to conversation"), + ); + expect(link.textContent).toBe("Back to conversation"); + expect(link.getAttribute("href")).toBe("/agents/abc/chat?mock=ok"); + }); + + it("falls back to the list on a direct visit", () => { + const link = renderAt(undefined); + expect(link.textContent).toBe("Back to templates"); + expect(link.getAttribute("href")).toBe(FALLBACK.to); + }); + + it.each([ + { from: "https://example.com/", fromLabel: "Back" }, + { from: "//example.com/", fromLabel: "Back" }, + { from: "/agents", fromLabel: "" }, + { from: 42, fromLabel: "Back" }, + "not an object", + ])("ignores state that is not an in-app origin: %j", (state) => { + expect(renderAt(state).textContent).toBe("Back to templates"); + }); +}); diff --git a/ui/src/router/backLink.ts b/ui/src/router/backLink.ts new file mode 100644 index 0000000000..06f7e7bc75 --- /dev/null +++ b/ui/src/router/backLink.ts @@ -0,0 +1,49 @@ +import { useLocation, type Location } from "react-router-dom"; + +/** + * Where a page was opened from, carried in router `state` rather than the URL. + * + * A detail page reached from somewhere other than its own list should lead back to + * that place, not to the list. State rather than a query parameter, because the origin + * is about this visit and not about the page: a copied or bookmarked URL opens the page + * cold, and a cold visit has nowhere to go back to but the list. + */ +export interface BackLinkState { + /** The path and search of the page to return to. */ + from: string; + /** What the back control says, e.g. "Back to conversation". */ + fromLabel: string; +} + +/** The `state` to pass a `Link` so the page it opens can lead back to the current one. */ +export function backLinkState( + location: Pick, + fromLabel: string, +): BackLinkState { + return { from: location.pathname + location.search, fromLabel }; +} + +function isBackLinkState(value: unknown): value is BackLinkState { + if (typeof value !== "object" || value === null) return false; + const { from, fromLabel } = value as Record; + // Only an in-app path: state is set by our own links, but a path is what makes it + // safe to navigate to whatever is found here. + return ( + typeof from === "string" && + from.startsWith("/") && + !from.startsWith("//") && + typeof fromLabel === "string" && + fromLabel.length > 0 + ); +} + +/** + * The page this one was opened from, or the fallback when it was visited directly. + */ +export function useBackLink(fallback: { to: string; label: string }): { + to: string; + label: string; +} { + const { state } = useLocation(); + return isBackLinkState(state) ? { to: state.from, label: state.fromLabel } : fallback; +} From 3cb4e7b3016110a2b77df4efbd728ed8e9f81a5b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Logro=C3=B1o=20Guerrero?= Date: Wed, 11 Nov 2026 11:12:33 -0500 Subject: [PATCH 2/2] fix(ui): keep the template page's way back across tabs and label it by origin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - The context panel's link labels the way back "Back to agent" when no conversation is open, and its tooltip also opens on keyboard focus. - Switching tabs on the AgentTemplate page carries the router state, so the back button keeps pointing at where the reader came from. Returning replaces the history entry. - Tools whose names differ only in case get a fixed order. - An e2e test covers chat → template → tab switch → back to the same chat. Co-Authored-By: Claude Opus 5.5 Signed-off-by: Carlos Logroño Guerrero --- ui/playwright/tests/chat/panels.spec.ts | 35 +++++++++++++++++++ .../chat/AgentContextPanel.test.tsx | 26 +++++++++++--- ui/src/components/chat/AgentContextPanel.tsx | 14 +++++--- .../components/mcp/ToolServerTools.test.tsx | 8 ++++- ui/src/components/mcp/ToolServerTools.tsx | 7 ++-- ui/src/pages/AgentTemplateDetailsPage.tsx | 13 +++++-- ui/src/router/backLink.ts | 6 +++- 7 files changed, 94 insertions(+), 15 deletions(-) diff --git a/ui/playwright/tests/chat/panels.spec.ts b/ui/playwright/tests/chat/panels.spec.ts index 389e00430a..c6ef3cd5e0 100644 --- a/ui/playwright/tests/chat/panels.spec.ts +++ b/ui/playwright/tests/chat/panels.spec.ts @@ -108,3 +108,38 @@ test("chat: a conversation that cannot be read gives the column back", async ({ await expect(page.getByTestId("chat-context-collapse")).toBeHidden(); await expect(page.getByTestId("chat-context-expand")).toBeHidden(); }); + +/** + * The context panel's template link, and the way back from the template page. + * + * The template page is shared by every conversation with an agent, so reaching it from a + * chat must not strand the reader on a page whose only exit is the template list. + */ +test("chat: the template opened from a conversation leads back to it", async ({ page }) => { + // Wide enough for the context panel, as above. + await page.setViewportSize({ width: 1600, height: 900 }); + await page.goto(AGENT_CHAT); + const link = page.getByTestId("chat-agent-context-template"); + await expect(link).toBeVisible({ timeout: 30_000 }); + const chatUrl = page.url(); + + await test.step("1. the link says where it goes", async () => { + await link.hover(); + await expect(page.getByRole("tooltip")).toContainText("AgentTemplate"); + }); + + await test.step("2. the template page offers the way back, across its tabs", async () => { + await link.click(); + const back = page.getByTestId("template-back"); + await expect(back).toHaveText("Back to conversation", { timeout: 30_000 }); + + await page.getByRole("tab", { name: /Agents/ }).click(); + await expect(back).toHaveText("Back to conversation"); + }); + + await test.step("3. and it returns to the same conversation", async () => { + await page.getByTestId("template-back").click(); + await expect(page).toHaveURL(chatUrl); + await expect(page.getByTestId("chat-agent-context")).toBeVisible({ timeout: 30_000 }); + }); +}); diff --git a/ui/src/components/chat/AgentContextPanel.test.tsx b/ui/src/components/chat/AgentContextPanel.test.tsx index 2ccff7e1bb..4216046a07 100644 --- a/ui/src/components/chat/AgentContextPanel.test.tsx +++ b/ui/src/components/chat/AgentContextPanel.test.tsx @@ -4,18 +4,23 @@ import { MemoryRouter, Route, Routes, useLocation } from "react-router-dom"; import userEvent from "@testing-library/user-event"; import { describe, expect, it, vi } from "vitest"; import { themeFor } from "@/theme/theme"; +import type { AgentInstance } from "@/api"; import { AgentContextPanel } from "./AgentContextPanel"; const {useAgentTemplate, useAgent} = vi.hoisted(() => ({useAgentTemplate: vi.fn(), useAgent: vi.fn()})); -vi.mock("@/api", () => ({ useAgentTemplate, useAgent })); +vi.mock("@/api", () => ({ + useAgentTemplate, + useAgent, + bareName: (ref: string) => ref.split("/").pop(), +})); function LocationState() { const location = useLocation(); return 
    {JSON.stringify(location.state)}
    ; } -function renderPanel(tools: unknown[], modelNamespace?: string) { +function renderPanel(tools: unknown[], modelNamespace?: string, instance?: AgentInstance) { useAgent.mockReturnValue({data: {resource: {spec: {template: {ref: {name: "assistant"}}, harnessRef: {name: "runner"}}}}}); useAgentTemplate.mockReturnValue({ data: { @@ -36,7 +41,12 @@ function renderPanel(tools: unknown[], modelNamespace?: string) { } + element={ + + } /> } /> @@ -47,7 +57,7 @@ function renderPanel(tools: unknown[], modelNamespace?: string) { describe("AgentContextPanel", () => { it("opens the template with a way back to this conversation", async () => { - renderPanel([]); + renderPanel([], undefined, { agent: "kagent/assistant" } as AgentInstance); await userEvent.click(screen.getByTestId("chat-agent-context-template")); expect(JSON.parse(screen.getByTestId("location-state").textContent ?? "null")).toEqual({ from: "/agents/abc/chat?mock=ok", @@ -55,6 +65,14 @@ describe("AgentContextPanel", () => { }); }); + it("says the way back leads to the agent when no conversation is open", async () => { + renderPanel([]); + await userEvent.click(screen.getByTestId("chat-agent-context-template")); + expect(JSON.parse(screen.getByTestId("location-state").textContent ?? "null")).toMatchObject({ + fromLabel: "Back to agent", + }); + }); + it("renders an MCP binding whose optional tools list is omitted", () => { renderPanel([ { diff --git a/ui/src/components/chat/AgentContextPanel.tsx b/ui/src/components/chat/AgentContextPanel.tsx index f56e0c5b1b..ef9393cd51 100644 --- a/ui/src/components/chat/AgentContextPanel.tsx +++ b/ui/src/components/chat/AgentContextPanel.tsx @@ -67,15 +67,21 @@ export function AgentContextPanel({
    {templateRef && templateNamespace && templateName ? ( - + { "tool_10", ]); }); + + it("gives names that differ only in case one fixed order", () => { + const first = renderedNames(["get_pods", "Get_pods"]); + cleanup(); + expect(renderedNames(["Get_pods", "get_pods"])).toEqual(first); + }); }); diff --git a/ui/src/components/mcp/ToolServerTools.tsx b/ui/src/components/mcp/ToolServerTools.tsx index bc605a4849..0099eeb909 100644 --- a/ui/src/components/mcp/ToolServerTools.tsx +++ b/ui/src/components/mcp/ToolServerTools.tsx @@ -90,8 +90,11 @@ export function ToolServerTools({ tools, highlight }: ToolServerToolsProps) { * scanning a long list for one tool. Numeric-aware, so `tool_2` sorts before `tool_10`. */ function sortToolsByName(tools: DiscoveredTool[]): DiscoveredTool[] { - return [...tools].sort((a, b) => - a.name.localeCompare(b.name, undefined, { numeric: true, sensitivity: "base" }), + return [...tools].sort( + (a, b) => + a.name.localeCompare(b.name, undefined, { numeric: true, sensitivity: "base" }) || + // Names that differ only in case still get one fixed order. + a.name.localeCompare(b.name, undefined, { numeric: true }), ); } diff --git a/ui/src/pages/AgentTemplateDetailsPage.tsx b/ui/src/pages/AgentTemplateDetailsPage.tsx index 21d2571f38..0906a98bf2 100644 --- a/ui/src/pages/AgentTemplateDetailsPage.tsx +++ b/ui/src/pages/AgentTemplateDetailsPage.tsx @@ -13,7 +13,7 @@ import { } from "antd"; import type { ColumnsType } from "antd/es/table"; import toast from "react-hot-toast"; -import { Link, useNavigate, useParams, useSearchParams } from "react-router-dom"; +import { Link, useLocation, useNavigate, useParams, useSearchParams } from "react-router-dom"; import { useTheme } from "@emotion/react"; import { Pencil } from "lucide-react"; import { PageFrame } from "@/components/Structure/PageFrame"; @@ -48,6 +48,7 @@ const TAB_PARAM = "tab"; export function AgentTemplateDetailsPage() { const navigate = useNavigate(); + const location = useLocation(); const theme = useTheme(); const { namespace, name } = useParams(); const [searchParams, setSearchParams] = useSearchParams(); @@ -110,7 +111,8 @@ export function AgentTemplateDetailsPage() { const next = new URLSearchParams(searchParams); if (tab === "details") next.delete(TAB_PARAM); else next.set(TAB_PARAM, tab); - setSearchParams(next, { replace: true }); + // Carry the router state across, or switching tabs drops the way back. + setSearchParams(next, { replace: true, state: location.state }); }; const missing = template.error !== undefined && isNotFound(template.error); @@ -339,7 +341,12 @@ export function AgentTemplateDetailsPage() { description="Agents referencing this template cannot prepare new configuration until it is restored or replaced. Existing conversations and last successful revisions are retained." /> ) : null} - diff --git a/ui/src/router/backLink.ts b/ui/src/router/backLink.ts index 06f7e7bc75..34dd3458a2 100644 --- a/ui/src/router/backLink.ts +++ b/ui/src/router/backLink.ts @@ -43,7 +43,11 @@ function isBackLinkState(value: unknown): value is BackLinkState { export function useBackLink(fallback: { to: string; label: string }): { to: string; label: string; + /** True when `to` is the page this one was opened from, rather than the fallback. */ + isReturn: boolean; } { const { state } = useLocation(); - return isBackLinkState(state) ? { to: state.from, label: state.fromLabel } : fallback; + return isBackLinkState(state) + ? { to: state.from, label: state.fromLabel, isReturn: true } + : { ...fallback, isReturn: false }; }