From bdd0aed1147d51ef5f725b1e46a0c74538780ffe Mon Sep 17 00:00:00 2001 From: Thanh Nguyen Date: Mon, 10 Aug 2026 00:36:59 +0700 Subject: [PATCH 1/4] feat(onboarding): enhance tool selection logic to merge server tools and improve review display Signed-off-by: Thanh Nguyen --- .../onboarding/steps/ReviewStep.tsx | 26 ++++-- .../onboarding/steps/ToolSelectionStep.tsx | 79 ++++++++++++++-- .../steps/__tests__/ReviewStep.test.tsx | 63 +++++++++++++ .../__tests__/ToolSelectionStep.test.tsx | 92 +++++++++++++++++++ 4 files changed, 245 insertions(+), 15 deletions(-) create mode 100644 ui/src/components/onboarding/steps/__tests__/ReviewStep.test.tsx create mode 100644 ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx diff --git a/ui/src/components/onboarding/steps/ReviewStep.tsx b/ui/src/components/onboarding/steps/ReviewStep.tsx index c95cf026f..218a94ac2 100644 --- a/ui/src/components/onboarding/steps/ReviewStep.tsx +++ b/ui/src/components/onboarding/steps/ReviewStep.tsx @@ -6,6 +6,7 @@ import { ScrollArea } from "@/components/ui/scroll-area"; import { Separator } from "@/components/ui/separator"; import { Badge } from "@/components/ui/badge"; import { Loader2, FunctionSquare } from 'lucide-react'; +import { isMcpTool, isAgentTool } from "@/lib/toolUtils"; import type { Tool } from "@/types"; interface OnboardingDataForReview { @@ -74,12 +75,25 @@ export function ReviewStep({ onboardingData, isLoading, onBack, onSubmit }: Revi {onboardingData.selectedTools && onboardingData.selectedTools.length > 0 ? (
- {onboardingData.selectedTools.map((tool, index) => ( - - - {tool.mcpServer?.name} - - ))} + {onboardingData.selectedTools.flatMap((tool, toolIndex) => { + if (isMcpTool(tool)) { + return tool.mcpServer.toolNames.map((toolName) => ( + + + {toolName} + + )); + } + if (isAgentTool(tool)) { + return [( + + + {tool.agent.name} + + )]; + } + return []; + })}
) : ( diff --git a/ui/src/components/onboarding/steps/ToolSelectionStep.tsx b/ui/src/components/onboarding/steps/ToolSelectionStep.tsx index 0ccebc47e..86256992b 100644 --- a/ui/src/components/onboarding/steps/ToolSelectionStep.tsx +++ b/ui/src/components/onboarding/steps/ToolSelectionStep.tsx @@ -7,7 +7,7 @@ import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert"; import { Info, ChevronDown, ChevronRight, FunctionSquare, Search } from 'lucide-react'; import { LoadingState } from "@/components/LoadingState"; import { ErrorState } from "@/components/ErrorState"; -import { getToolResponseDisplayName, getToolResponseDescription, getToolResponseIdentifier, getToolResponseCategory, toolResponseToAgentTool } from "@/lib/toolUtils"; +import { getToolResponseDisplayName, getToolResponseDescription, getToolResponseIdentifier, getToolResponseCategory, toolResponseToAgentTool, isMcpTool, serverNamesMatch } from "@/lib/toolUtils"; import type { Tool, ToolsResponse } from "@/types"; import { Input } from "@/components/ui/input"; @@ -39,7 +39,7 @@ export function ToolSelectionStep({ if (tool.type === "Agent" && tool.agent) { return false; // Agents don't match ToolResponse objects } else if (tool.type === "McpServer" && tool.mcpServer) { - return tool.mcpServer.name === toolResponse.server_name && + return serverNamesMatch(tool.mcpServer.name, toolResponse.server_name) && tool.mcpServer.toolNames.includes(toolResponse.id); } return false; @@ -125,12 +125,31 @@ export function ToolSelectionStep({ "k8s_get_resources", ]; + // Merge desired tools that share a server into one entry instead of + // pushing one per tool, so preselection doesn't itself produce + // duplicate-looking rows for the same server. const initialSelection: Tool[] = []; availableTools.forEach((tool) => { const toolId = getToolResponseDisplayName(tool); - if (desiredIds.includes(toolId)) { + if (!desiredIds.includes(toolId)) { + return; + } + + const existingIndex = initialSelection.findIndex( + (t) => isMcpTool(t) && serverNamesMatch(t.mcpServer.name, tool.server_name) + ); + if (existingIndex === -1) { initialSelection.push(toolResponseToAgentTool(tool, tool.server_name)); + return; } + const existing = initialSelection[existingIndex] as Tool; + initialSelection[existingIndex] = { + ...existing, + mcpServer: { + ...existing.mcpServer!, + toolNames: [...existing.mcpServer!.toolNames, tool.id], + }, + }; }); if (initialSelection.length > 0) { @@ -141,10 +160,42 @@ export function ToolSelectionStep({ }, [availableTools, initialSelectedTools, selectedTools.length]); const handleToolToggle = (toolResponse: ToolsResponse) => { - const agentTool = toolResponseToAgentTool(toolResponse, toolResponse.server_name); setSelectedTools(prev => { const isSelected = prev.some(t => toolResponseMatchesTool(toolResponse, t)); - return isSelected ? prev.filter(t => !toolResponseMatchesTool(toolResponse, t)) : [...prev, agentTool]; + + if (isSelected) { + // Drop just this tool id from whichever entry holds it, and drop + // the entry entirely once it references no tools. + return prev + .map(t => { + if (!isMcpTool(t) || !serverNamesMatch(t.mcpServer.name, toolResponse.server_name)) { + return t; + } + const remainingToolNames = t.mcpServer.toolNames.filter((id) => id !== toolResponse.id); + return remainingToolNames.length > 0 + ? { ...t, mcpServer: { ...t.mcpServer, toolNames: remainingToolNames } } + : null; + }) + .filter((t): t is Tool => t !== null); + } + + // Merge into the existing entry for this server instead of adding a + // second entry for the same server, which otherwise produced + // duplicate-looking rows downstream (Review step, agent edit page) + // and duplicate live MCP connections at agent runtime. + const existingIndex = prev.findIndex(t => isMcpTool(t) && serverNamesMatch(t.mcpServer.name, toolResponse.server_name)); + if (existingIndex === -1) { + return [...prev, toolResponseToAgentTool(toolResponse, toolResponse.server_name)]; + } + const existing = prev[existingIndex] as Tool; + const merged: Tool = { + ...existing, + mcpServer: { + ...existing.mcpServer!, + toolNames: [...existing.mcpServer!.toolNames, toolResponse.id], + }, + }; + return prev.map((t, i) => (i === existingIndex ? merged : t)); }); }; @@ -217,11 +268,20 @@ export function ToolSelectionStep({ {expandedCategories[category] && (
- {categoryTools.map((tool: ToolsResponse) => ( -
+ {categoryTools.map((tool: ToolsResponse) => { + const selected = isToolSelected(tool); + return ( +
handleToolToggle(tool)} className="mt-1" /> @@ -233,7 +293,8 @@ export function ToolSelectionStep({

{getToolResponseDescription(tool)}

- ))} + ); + })}
)} diff --git a/ui/src/components/onboarding/steps/__tests__/ReviewStep.test.tsx b/ui/src/components/onboarding/steps/__tests__/ReviewStep.test.tsx new file mode 100644 index 000000000..233e19c02 --- /dev/null +++ b/ui/src/components/onboarding/steps/__tests__/ReviewStep.test.tsx @@ -0,0 +1,63 @@ +/** + * @jest-environment jsdom + * + * Bug: the "Selected Tools" badges rendered `tool.mcpServer?.name` (the MCP + * server name, e.g. "kagent-tool-server") - one badge per array entry. Now + * that ToolSelectionStep merges same-server picks into a single Tool entry's + * mcpServer.toolNames array, this collapsed to a single, unhelpful badge no + * matter how many/which tools were actually selected. + * + * Fix: flatMap over each entry's toolNames (for MCP tools) so Review shows + * one badge per selected tool name instead of per array entry. + */ +import React from "react"; +import { describe, it, expect } from "@jest/globals"; +import { render, screen } from "@testing-library/react"; +import { ReviewStep } from "@/components/onboarding/steps/ReviewStep"; +import type { Tool } from "@/types"; + +describe("ReviewStep Selected Tools", () => { + it("shows one badge per tool name, not one per (merged) server entry", () => { + const mergedServerTool: Tool = { + type: "McpServer", + mcpServer: { + kind: "RemoteMCPServer", + apiGroup: "kagent.dev", + name: "kagent-tool-server", + namespace: "kagent", + toolNames: ["k8s_get_pods", "k8s_get_events"], + }, + }; + + render( + {}} + onSubmit={() => {}} + />, + ); + + expect(screen.getByText("k8s_get_pods")).toBeInTheDocument(); + expect(screen.getByText("k8s_get_events")).toBeInTheDocument(); + expect(screen.queryByText("kagent-tool-server")).not.toBeInTheDocument(); + }); + + it("shows the agent name for Agent-type selections", () => { + const agentTool: Tool = { + type: "Agent", + agent: { name: "researcher", namespace: "kagent" }, + }; + + render( + {}} + onSubmit={() => {}} + />, + ); + + expect(screen.getByText("researcher")).toBeInTheDocument(); + }); +}); diff --git a/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx b/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx new file mode 100644 index 000000000..6845d449a --- /dev/null +++ b/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx @@ -0,0 +1,92 @@ +/** + * @jest-environment jsdom + * + * Bug: handleToolToggle pushed a brand-new Tool object per checkbox click + * instead of merging into an existing entry for the same MCP server (unlike + * SelectToolsDialog.handleAddItem, which merges same-server picks into one + * mcpServer.toolNames array). Selecting multiple tools from the same server + * during onboarding produced several Tool entries that all shared the same + * server identity - the data shape that made downstream duplicate-entry + * rendering bugs possible (see ToolsSection's duplicate-key issue). + * + * Fix: handleToolToggle now merges same-server picks into a single entry on + * select, and removes just that tool id (dropping the entry only once it + * references no tools) on deselect. + */ +import React from "react"; +import { describe, it, expect, jest } from "@jest/globals"; +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { ToolSelectionStep } from "@/components/onboarding/steps/ToolSelectionStep"; +import type { Tool, ToolsResponse } from "@/types"; + +const makeTool = (id: string): ToolsResponse => ({ + id, + server_name: "kagent/kagent-tool-server", + created_at: "", + updated_at: "", + deleted_at: "", + description: `${id} description`, + group_kind: "Tool", +}); + +const renderStep = (tools: ToolsResponse[], onNext = jest.fn<(tools: Tool[]) => void>()) => { + render( + , + ); + return { onNext }; +}; + +describe("ToolSelectionStep duplicate-selection prevention", () => { + it("merges multiple tool picks from the same server into a single Tool entry", async () => { + const user = userEvent.setup(); + const tools = [makeTool("k8s_get_pods"), makeTool("k8s_get_events")]; + const { onNext } = renderStep(tools); + + await user.click(await screen.findByRole("checkbox", { name: /k8s_get_pods/i })); + await user.click(screen.getByRole("checkbox", { name: /k8s_get_events/i })); + await user.click(screen.getByRole("button", { name: /next: review/i })); + + expect(onNext).toHaveBeenCalledTimes(1); + const submitted = onNext.mock.calls[0][0]; + expect(submitted).toHaveLength(1); + expect([...submitted[0]!.mcpServer!.toolNames].sort()).toEqual([ + "k8s_get_events", + "k8s_get_pods", + ]); + }); + + it("unchecking one of two same-server selections keeps the other tool selected", async () => { + const user = userEvent.setup(); + const tools = [makeTool("k8s_get_pods"), makeTool("k8s_get_events")]; + const { onNext } = renderStep(tools); + + await user.click(await screen.findByRole("checkbox", { name: /k8s_get_pods/i })); + await user.click(screen.getByRole("checkbox", { name: /k8s_get_events/i })); + await user.click(screen.getByRole("checkbox", { name: /k8s_get_pods/i })); + await user.click(screen.getByRole("button", { name: /next: review/i })); + + const submitted = onNext.mock.calls[0][0]; + expect(submitted).toHaveLength(1); + expect(submitted[0]!.mcpServer!.toolNames).toEqual(["k8s_get_events"]); + }); + + it("unchecking the only selected tool for a server removes the entry entirely", async () => { + const user = userEvent.setup(); + const tools = [makeTool("k8s_get_pods")]; + const { onNext } = renderStep(tools); + + await user.click(await screen.findByRole("checkbox", { name: /k8s_get_pods/i })); + await user.click(screen.getByRole("checkbox", { name: /k8s_get_pods/i })); + await user.click(screen.getByRole("button", { name: /next: review/i })); + + expect(onNext).toHaveBeenCalledWith([]); + }); +}); From 134cdb7a9357254779a920b057c6308d436d3ccc Mon Sep 17 00:00:00 2001 From: Thanh Nguyen Date: Tue, 11 Aug 2026 15:25:54 +0700 Subject: [PATCH 2/4] feat(onboarding): improve tool selection logic to handle server-specific entries and prevent merging Signed-off-by: Thanh Nguyen --- .../onboarding/steps/ReviewStep.tsx | 6 +-- .../onboarding/steps/ToolSelectionStep.tsx | 51 ++++++++----------- .../steps/__tests__/ReviewStep.test.tsx | 37 ++++++++++++++ .../__tests__/ToolSelectionStep.test.tsx | 34 ++++++++++++- 4 files changed, 93 insertions(+), 35 deletions(-) diff --git a/ui/src/components/onboarding/steps/ReviewStep.tsx b/ui/src/components/onboarding/steps/ReviewStep.tsx index 218a94ac2..23bcad6df 100644 --- a/ui/src/components/onboarding/steps/ReviewStep.tsx +++ b/ui/src/components/onboarding/steps/ReviewStep.tsx @@ -75,10 +75,10 @@ export function ReviewStep({ onboardingData, isLoading, onBack, onSubmit }: Revi {onboardingData.selectedTools && onboardingData.selectedTools.length > 0 ? (
- {onboardingData.selectedTools.flatMap((tool, toolIndex) => { + {onboardingData.selectedTools.flatMap((tool) => { if (isMcpTool(tool)) { return tool.mcpServer.toolNames.map((toolName) => ( - + {toolName} @@ -86,7 +86,7 @@ export function ReviewStep({ onboardingData, isLoading, onBack, onSubmit }: Revi } if (isAgentTool(tool)) { return [( - + {tool.agent.name} diff --git a/ui/src/components/onboarding/steps/ToolSelectionStep.tsx b/ui/src/components/onboarding/steps/ToolSelectionStep.tsx index 86256992b..520b8c369 100644 --- a/ui/src/components/onboarding/steps/ToolSelectionStep.tsx +++ b/ui/src/components/onboarding/steps/ToolSelectionStep.tsx @@ -45,6 +45,24 @@ export function ToolSelectionStep({ return false; }; + const mergeToolIntoServerEntry = (tools: Tool[], toolResponse: ToolsResponse): Tool[] => { + const existingIndex = tools.findIndex( + (t) => isMcpTool(t) && serverNamesMatch(t.mcpServer.name, toolResponse.server_name) + ); + if (existingIndex === -1) { + return [...tools, toolResponseToAgentTool(toolResponse, toolResponse.server_name)]; + } + const existing = tools[existingIndex] as Tool; + const merged: Tool = { + ...existing, + mcpServer: { + ...existing.mcpServer!, + toolNames: Array.from(new Set([...existing.mcpServer!.toolNames, toolResponse.id])), + }, + }; + return tools.map((t, i) => (i === existingIndex ? merged : t)); + }; + const toolsByCategory = useMemo(() => { if (!availableTools) return {} as Record; @@ -128,28 +146,13 @@ export function ToolSelectionStep({ // Merge desired tools that share a server into one entry instead of // pushing one per tool, so preselection doesn't itself produce // duplicate-looking rows for the same server. - const initialSelection: Tool[] = []; + let initialSelection: Tool[] = []; availableTools.forEach((tool) => { const toolId = getToolResponseDisplayName(tool); if (!desiredIds.includes(toolId)) { return; } - - const existingIndex = initialSelection.findIndex( - (t) => isMcpTool(t) && serverNamesMatch(t.mcpServer.name, tool.server_name) - ); - if (existingIndex === -1) { - initialSelection.push(toolResponseToAgentTool(tool, tool.server_name)); - return; - } - const existing = initialSelection[existingIndex] as Tool; - initialSelection[existingIndex] = { - ...existing, - mcpServer: { - ...existing.mcpServer!, - toolNames: [...existing.mcpServer!.toolNames, tool.id], - }, - }; + initialSelection = mergeToolIntoServerEntry(initialSelection, tool); }); if (initialSelection.length > 0) { @@ -183,19 +186,7 @@ export function ToolSelectionStep({ // second entry for the same server, which otherwise produced // duplicate-looking rows downstream (Review step, agent edit page) // and duplicate live MCP connections at agent runtime. - const existingIndex = prev.findIndex(t => isMcpTool(t) && serverNamesMatch(t.mcpServer.name, toolResponse.server_name)); - if (existingIndex === -1) { - return [...prev, toolResponseToAgentTool(toolResponse, toolResponse.server_name)]; - } - const existing = prev[existingIndex] as Tool; - const merged: Tool = { - ...existing, - mcpServer: { - ...existing.mcpServer!, - toolNames: [...existing.mcpServer!.toolNames, toolResponse.id], - }, - }; - return prev.map((t, i) => (i === existingIndex ? merged : t)); + return mergeToolIntoServerEntry(prev, toolResponse); }); }; diff --git a/ui/src/components/onboarding/steps/__tests__/ReviewStep.test.tsx b/ui/src/components/onboarding/steps/__tests__/ReviewStep.test.tsx index 233e19c02..ce14af680 100644 --- a/ui/src/components/onboarding/steps/__tests__/ReviewStep.test.tsx +++ b/ui/src/components/onboarding/steps/__tests__/ReviewStep.test.tsx @@ -43,6 +43,43 @@ describe("ReviewStep Selected Tools", () => { expect(screen.queryByText("kagent-tool-server")).not.toBeInTheDocument(); }); + it("shows tools from two different servers as separate entries, not merged", () => { + const serverATool: Tool = { + type: "McpServer", + mcpServer: { + kind: "RemoteMCPServer", + apiGroup: "kagent.dev", + name: "kagent-tool-server", + namespace: "kagent", + toolNames: ["k8s_get_pods"], + }, + }; + const serverBTool: Tool = { + type: "McpServer", + mcpServer: { + kind: "RemoteMCPServer", + apiGroup: "kagent.dev", + name: "context-forge", + namespace: "kagent", + toolNames: ["argocd-get-application"], + }, + }; + + render( + {}} + onSubmit={() => {}} + />, + ); + + expect(screen.getByText("k8s_get_pods")).toBeInTheDocument(); + expect(screen.getByText("argocd-get-application")).toBeInTheDocument(); + expect(screen.queryByText("kagent-tool-server")).not.toBeInTheDocument(); + expect(screen.queryByText("context-forge")).not.toBeInTheDocument(); + }); + it("shows the agent name for Agent-type selections", () => { const agentTool: Tool = { type: "Agent", diff --git a/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx b/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx index 6845d449a..b22f113f1 100644 --- a/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx +++ b/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx @@ -20,9 +20,12 @@ import userEvent from "@testing-library/user-event"; import { ToolSelectionStep } from "@/components/onboarding/steps/ToolSelectionStep"; import type { Tool, ToolsResponse } from "@/types"; -const makeTool = (id: string): ToolsResponse => ({ +const makeTool = ( + id: string, + serverName = "kagent/kagent-tool-server", +): ToolsResponse => ({ id, - server_name: "kagent/kagent-tool-server", + server_name: serverName, created_at: "", updated_at: "", deleted_at: "", @@ -89,4 +92,31 @@ describe("ToolSelectionStep duplicate-selection prevention", () => { expect(onNext).toHaveBeenCalledWith([]); }); + + it("keeps tools from different servers as separate Tool entries instead of merging them", async () => { + // Both server names contain "kagent-tool-server" so ToolSelectionStep's + // own K8s-only filter (server_name?.includes("kagent-tool-server")) + // lets both through - but they have different parsed names, so + // serverNamesMatch must NOT treat them as the same server. + const user = userEvent.setup(); + const tools = [ + makeTool("k8s_get_pods", "kagent/kagent-tool-server"), + makeTool("k8s_get_events", "kagent/kagent-tool-server-extra"), + ]; + const { onNext } = renderStep(tools); + + await user.click(await screen.findByRole("checkbox", { name: /k8s_get_pods/i })); + await user.click(screen.getByRole("checkbox", { name: /k8s_get_events/i })); + await user.click(screen.getByRole("button", { name: /next: review/i })); + + expect(onNext).toHaveBeenCalledTimes(1); + const submitted = onNext.mock.calls[0][0]; + expect(submitted).toHaveLength(2); + + const byServer = new Map( + submitted.map((t) => [t!.mcpServer!.name, t!.mcpServer!.toolNames]), + ); + expect(byServer.get("kagent-tool-server")).toEqual(["k8s_get_pods"]); + expect(byServer.get("kagent-tool-server-extra")).toEqual(["k8s_get_events"]); + }); }); From 6bdc1d7d583c13778ec0300cd2768e0c0f44efbd Mon Sep 17 00:00:00 2001 From: Thanh Nguyen Date: Tue, 11 Aug 2026 15:34:16 +0700 Subject: [PATCH 3/4] feat(onboarding): refine tool merging logic to prevent duplicate entries for same MCP server Signed-off-by: Thanh Nguyen --- .../onboarding/steps/ToolSelectionStep.tsx | 15 +++++++-------- .../steps/__tests__/ToolSelectionStep.test.tsx | 1 + 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/ui/src/components/onboarding/steps/ToolSelectionStep.tsx b/ui/src/components/onboarding/steps/ToolSelectionStep.tsx index 520b8c369..15f42262d 100644 --- a/ui/src/components/onboarding/steps/ToolSelectionStep.tsx +++ b/ui/src/components/onboarding/steps/ToolSelectionStep.tsx @@ -46,21 +46,20 @@ export function ToolSelectionStep({ }; const mergeToolIntoServerEntry = (tools: Tool[], toolResponse: ToolsResponse): Tool[] => { - const existingIndex = tools.findIndex( - (t) => isMcpTool(t) && serverNamesMatch(t.mcpServer.name, toolResponse.server_name) - ); - if (existingIndex === -1) { + const existing = tools + .filter(isMcpTool) + .find((t) => serverNamesMatch(t.mcpServer.name, toolResponse.server_name)); + if (!existing) { return [...tools, toolResponseToAgentTool(toolResponse, toolResponse.server_name)]; } - const existing = tools[existingIndex] as Tool; const merged: Tool = { ...existing, mcpServer: { - ...existing.mcpServer!, - toolNames: Array.from(new Set([...existing.mcpServer!.toolNames, toolResponse.id])), + ...existing.mcpServer, + toolNames: Array.from(new Set([...existing.mcpServer.toolNames, toolResponse.id])), }, }; - return tools.map((t, i) => (i === existingIndex ? merged : t)); + return tools.map((t) => (t === existing ? merged : t)); }; const toolsByCategory = useMemo(() => { diff --git a/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx b/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx index b22f113f1..e9c8a126a 100644 --- a/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx +++ b/ui/src/components/onboarding/steps/__tests__/ToolSelectionStep.test.tsx @@ -76,6 +76,7 @@ describe("ToolSelectionStep duplicate-selection prevention", () => { await user.click(screen.getByRole("checkbox", { name: /k8s_get_pods/i })); await user.click(screen.getByRole("button", { name: /next: review/i })); + expect(onNext).toHaveBeenCalledTimes(1); const submitted = onNext.mock.calls[0][0]; expect(submitted).toHaveLength(1); expect(submitted[0]!.mcpServer!.toolNames).toEqual(["k8s_get_events"]); From 237694dbf2b4b1a8679773512a700c389d96661d Mon Sep 17 00:00:00 2001 From: Thanh Nguyen Date: Tue, 11 Aug 2026 20:56:35 +0700 Subject: [PATCH 4/4] feat(onboarding): implement mergeToolIntoServerEntry for improved tool selection handling Signed-off-by: Thanh Nguyen --- .../components/create/SelectToolsDialog.tsx | 34 ++----------------- .../onboarding/steps/ToolSelectionStep.tsx | 19 +---------- ui/src/lib/toolUtils.ts | 22 ++++++++++++ 3 files changed, 25 insertions(+), 50 deletions(-) diff --git a/ui/src/components/create/SelectToolsDialog.tsx b/ui/src/components/create/SelectToolsDialog.tsx index 2a5265130..04cf7f491 100644 --- a/ui/src/components/create/SelectToolsDialog.tsx +++ b/ui/src/components/create/SelectToolsDialog.tsx @@ -36,7 +36,7 @@ import { isAgentTool, isAgentResponse, isMcpTool, - toolResponseToAgentTool, + mergeToolIntoServerEntry, groupMcpToolsByServer, serverNamesMatch, } from "@/lib/toolUtils"; @@ -326,37 +326,7 @@ export const SelectToolsDialog: React.FC = ({ setLocalSelectedTools((prev) => [...prev, toolToAdd]); } else { const tool = item as ToolsResponse; - - const existingServerToolIndex = localSelectedTools.findIndex( - (t) => - isMcpTool(t) && - serverNamesMatch(t.mcpServer?.name || "", tool.server_name), - ); - - if (existingServerToolIndex >= 0) { - const existingTool = localSelectedTools[existingServerToolIndex]; - - if (existingTool.mcpServer?.toolNames?.includes(tool.id)) { - return; - } - - const updatedTool = { - ...existingTool, - mcpServer: { - ...existingTool.mcpServer!, - toolNames: [...(existingTool.mcpServer!.toolNames || []), tool.id], - }, - }; - - setLocalSelectedTools((prev) => - prev.map((t, idx) => - idx === existingServerToolIndex ? updatedTool : t, - ), - ); - } else { - toolToAdd = toolResponseToAgentTool(tool, tool.server_name); - setLocalSelectedTools((prev) => [...prev, toolToAdd]); - } + setLocalSelectedTools((prev) => mergeToolIntoServerEntry(prev, tool)); } }; diff --git a/ui/src/components/onboarding/steps/ToolSelectionStep.tsx b/ui/src/components/onboarding/steps/ToolSelectionStep.tsx index 15f42262d..8b4c94418 100644 --- a/ui/src/components/onboarding/steps/ToolSelectionStep.tsx +++ b/ui/src/components/onboarding/steps/ToolSelectionStep.tsx @@ -7,7 +7,7 @@ import { Alert, AlertDescription, AlertTitle } from "@/components/ui/alert"; import { Info, ChevronDown, ChevronRight, FunctionSquare, Search } from 'lucide-react'; import { LoadingState } from "@/components/LoadingState"; import { ErrorState } from "@/components/ErrorState"; -import { getToolResponseDisplayName, getToolResponseDescription, getToolResponseIdentifier, getToolResponseCategory, toolResponseToAgentTool, isMcpTool, serverNamesMatch } from "@/lib/toolUtils"; +import { getToolResponseDisplayName, getToolResponseDescription, getToolResponseIdentifier, getToolResponseCategory, isMcpTool, serverNamesMatch, mergeToolIntoServerEntry } from "@/lib/toolUtils"; import type { Tool, ToolsResponse } from "@/types"; import { Input } from "@/components/ui/input"; @@ -45,23 +45,6 @@ export function ToolSelectionStep({ return false; }; - const mergeToolIntoServerEntry = (tools: Tool[], toolResponse: ToolsResponse): Tool[] => { - const existing = tools - .filter(isMcpTool) - .find((t) => serverNamesMatch(t.mcpServer.name, toolResponse.server_name)); - if (!existing) { - return [...tools, toolResponseToAgentTool(toolResponse, toolResponse.server_name)]; - } - const merged: Tool = { - ...existing, - mcpServer: { - ...existing.mcpServer, - toolNames: Array.from(new Set([...existing.mcpServer.toolNames, toolResponse.id])), - }, - }; - return tools.map((t) => (t === existing ? merged : t)); - }; - const toolsByCategory = useMemo(() => { if (!availableTools) return {} as Record; diff --git a/ui/src/lib/toolUtils.ts b/ui/src/lib/toolUtils.ts index 485e69169..40cd44879 100644 --- a/ui/src/lib/toolUtils.ts +++ b/ui/src/lib/toolUtils.ts @@ -276,3 +276,25 @@ export const getDiscoveredToolCategory = (tool: DiscoveredTool, serverRef: strin export const getDiscoveredToolIdentifier = (tool: DiscoveredTool, serverRef: string): string => { return `${serverRef}-${tool.name}`; }; + +// Adds toolResponse to an existing entry for its server (deduping +// toolNames), or appends a new entry if none exists yet. +export const mergeToolIntoServerEntry = (tools: Tool[], toolResponse: ToolsResponse): Tool[] => { + const existing = tools + .filter(isMcpTool) + .find((t) => serverNamesMatch(t.mcpServer.name, toolResponse.server_name)); + if (!existing) { + return [...tools, toolResponseToAgentTool(toolResponse, toolResponse.server_name)]; + } + const merged: Tool = { + ...existing, + mcpServer: { + ...existing.mcpServer, + // Callers may seed this from an existing agent's persisted tools + // (e.g. hand-edited YAML), which isn't guaranteed to satisfy + // toolNames: string[] at runtime despite the type - fall back to []. + toolNames: Array.from(new Set([...(existing.mcpServer.toolNames || []), toolResponse.id])), + }, + }; + return tools.map((t) => (t === existing ? merged : t)); +}; \ No newline at end of file