fix(ui): preserve MCPServer argument ordering when creating stdio servers

ui/src/components/mcp/McpServerForm.tsx

@@ -12,6 +12,7 @@ import type { RemoteMCPServer, MCPServer, ToolServerCreateRequest } from "@/type
 import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from "@/components/ui/select";
 import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger } from "@/components/ui/tooltip";
 import { createRFC1123ValidName, isResourceNameValid } from "@/lib/utils";
+import { buildMCPServerArgs } from "@/lib/toolUtils";
 import { NamespaceCombobox } from "@/components/NamespaceCombobox";
 import { Checkbox } from "@/components/ui/checkbox";
 
@@ -21,10 +22,6 @@ export type McpServerFormProps = {
   onCreate: (serverRequest: ToolServerCreateRequest) => Promise<void>;
 };
 
-interface ArgPair {
-  value: string;
-}
-
 interface EnvPair {
   key: string;
   value: string;
@@ -42,10 +39,10 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
   const [commandType, setCommandType] = useState("npx");
   const [commandPrefix, setCommandPrefix] = useState("");
   const [packageName, setPackageName] = useState("");
-  const [argPairs, setArgPairs] = useState<ArgPair[]>([{ value: "" }]);
+  const [argPairs, setArgPairs] = useState<string[]>([""]);
   const [envPairs, setEnvPairs] = useState<EnvPair[]>([{ key: "", value: "" }]);
   const [commandPreview, setCommandPreview] = useState("");
-  
+
 
   // SseServer parameters
   const [url, setUrl] = useState("");
@@ -80,36 +77,36 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
         const urlObj = new URL(url.trim());
         // Convert hostname to RFC 1123 compliant format
         let hostname = urlObj.hostname.toLowerCase();
-        
+
         // Replace invalid characters with hyphens
         hostname = hostname.replace(/[^a-z0-9.-]/g, "-");
-        
+
         // Replace multiple consecutive hyphens with a single hyphen
         hostname = hostname.replace(/-+/g, "-");
-        
+
         // Remove hyphens at the beginning and end
         hostname = hostname.replace(/^-+|-+$/g, "");
-        
+
         // If the hostname starts with a dot, prepend an 'a'
         if (hostname.startsWith(".")) {
           hostname = "a" + hostname;
         }
-        
+
         // If the hostname ends with a dot, append an 'a'
         if (hostname.endsWith(".")) {
           hostname = hostname + "a";
         }
-        
+
         // If it doesn't start with alphanumeric, prepend 'server-'
         if (!/^[a-z0-9]/.test(hostname)) {
           hostname = "server-" + hostname;
         }
-        
+
         // If it doesn't end with alphanumeric, append '-server'
         if (!/[a-z0-9]$/.test(hostname)) {
           hostname = hostname + "-server";
         }
-        
+
         generatedName = hostname;
       } catch {
         // If URL is invalid, use a default name
@@ -138,8 +135,8 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
       }
 
       argPairs.forEach((arg) => {
-        if (arg.value.trim()) {
-          preview += " " + arg.value.trim();
+        if (arg.trim()) {
+          preview += " " + arg.trim();
         }
       });
 
@@ -148,7 +145,7 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
   }, [activeTab, commandType, commandPrefix, packageName, argPairs]);
 
   const addArgPair = () => {
-    setArgPairs([...argPairs, { value: "" }]);
+    setArgPairs([...argPairs, ""]);
   };
 
   const removeArgPair = (index: number) => {
@@ -157,7 +154,7 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
 
   const updateArgPair = (index: number, newValue: string) => {
     const updatedPairs = [...argPairs];
-    updatedPairs[index].value = newValue;
+    updatedPairs[index] = newValue;
     setArgPairs(updatedPairs);
   };
 
@@ -227,44 +224,20 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
         // Create MCPServer for stdio-based server
         let image: string;
         let cmd: string;
-        let args: string[];
 
         if (commandType === "uvx") {
           // Use uvx with the official uv image
           image = "ghcr.io/astral-sh/uv:debian";
           cmd = "uvx";
-
-          // Build args array: [args..., packageName]
-          args = [];
-          if (commandPrefix.trim()) {
-            // Split command prefix and add to args
-            args.push(...commandPrefix.trim().split(/\s+/));
-          }
-          // Add additional arguments first
-          argPairs.filter((arg) => arg.value.trim() !== "").forEach((arg) => {
-            args.push(arg.value.trim());
-          });
-          // Add package name at the end
-          args.push(packageName.trim());
         } else {
           // Use npx with Node.js image
           image = "node:24-alpine3.21";
           cmd = "npx";
-
-          // Build args array: [args..., packageName]
-          args = [];
-          if (commandPrefix.trim()) {
-            // Split command prefix and add to args
-            args.push(...commandPrefix.trim().split(/\s+/));
-          }
-          // Add additional arguments first
-          argPairs.filter((arg) => arg.value.trim() !== "").forEach((arg) => {
-            args.push(arg.value.trim());
-          });
-          // Add package name at the end
-          args.push(packageName.trim());
         }
 
+        // Build args array: [commandPrefix..., packageName, additionalArgs...]
+        const args: string[] = buildMCPServerArgs(commandPrefix, packageName, argPairs);
+
         const mcpServer: MCPServer = {
           metadata: {
             name: finalServerName,
@@ -353,19 +326,19 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
     if (errorMsg.includes("already exists")) {
       return "A server with this name already exists. Please choose a different name.";
     }
-    
+
     if (errorMsg.includes("Failed to create server")) {
       return "Failed to create server. Please check your configuration and try again.";
     }
-    
+
     if (errorMsg.includes("Network error")) {
       return "Network error: Could not connect to the server. Please check your connection and try again.";
     }
-    
+
     if (errorMsg.includes("Request timed out")) {
       return "Request timed out: The server took too long to respond. Please try again later.";
     }
-    
+
     // Return the original error if no specific formatting is needed
     return errorMsg;
   };
@@ -438,10 +411,10 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
                   </Tooltip>
                 </TooltipProvider>
               </div>
-              <Input 
-                id="server-name" 
-                placeholder="e.g., my-tool-server" 
-                value={serverName} 
+              <Input
+                id="server-name"
+                placeholder="e.g., my-tool-server"
+                value={serverName}
                 onChange={handleServerNameChange}
                 className={!isResourceNameValid(serverName) && serverName ? "border-red-300" : ""}
               />
@@ -545,7 +518,7 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
                   <div className="space-y-2">
                     {argPairs.map((pair, index) => (
                       <div key={index} className="flex gap-2 items-center">
-                        <Input placeholder="Argument (e.g., --verbose, --help, ...)" value={pair.value} onChange={(e) => updateArgPair(index, e.target.value)} className="flex-1" />
+                        <Input placeholder="Argument (e.g., --verbose, --help, ...)" value={pair} onChange={(e) => updateArgPair(index, e.target.value)} className="flex-1" />
                         <Button type="button" variant="ghost" size="sm" onClick={() => removeArgPair(index)} disabled={argPairs.length === 1} className="p-1">
                           <Trash2 className="h-4 w-4 text-red-500" />
                         </Button>
@@ -580,7 +553,7 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
                   </div>
                 </div>
 
-                
+
               </TabsContent>
 
               <TabsContent value="url" className="pt-4 space-y-4">
@@ -649,4 +622,4 @@ export function McpServerForm({ supportedToolServerTypes, onCreate }: McpServerF
       </div>
     </form>
   );
-}
+}

ui/src/lib/__tests__/toolUtils.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it, jest, beforeEach, afterEach } from '@jest/globals';
-import { 
-  isMcpTool, 
+import {
+  isMcpTool,
   isAgentTool,
   groupMcpToolsByServer,
   getToolIdentifier,
@@ -16,6 +16,7 @@ import {
   getDiscoveredToolDescription,
   getDiscoveredToolCategory,
   getDiscoveredToolIdentifier,
+  buildMCPServerArgs,
 } from '../toolUtils';
 import { k8sRefUtils } from '../k8sUtils';
 import { Tool, ToolsResponse, DiscoveredTool } from "@/types";
@@ -1027,4 +1028,47 @@ describe('Tool Utility Functions', () => {
       expect(getToolDescription(malformedMcpTool, [])).toBe("No description available");
     });
   });
+
+  describe('buildMCPServerArgs', () => {
+    it('should place the package before additional arguments', () => {
+      const args = buildMCPServerArgs(
+        "-y",
+        "@modelcontextprotocol/server-map",
+        ["--stdio"],
+      );
+      expect(args).toEqual([
+        "-y",
+        "@modelcontextprotocol/server-map",
+        "--stdio",
+      ]);
+    });
+
+    it('should include only non-empty arguments', () => {
+      const args = buildMCPServerArgs(
+        "",
+        "@acme/mcp-tool",
+        ["", "--verbose", ""],
+      );
+      expect(args).toEqual(["@acme/mcp-tool", "--verbose"]);
+    });
+
+    it('should split the command prefix on whitespace', () => {
+      const args = buildMCPServerArgs(
+        "--cache --quiet",
+        "@acme/mcp-tool",
+        ["--help"],
+      );
+      expect(args).toEqual([
+        "--cache",
+        "--quiet",
+        "@acme/mcp-tool",
+        "--help",
+      ]);
+    });
+
+    it('should handle empty prefix and no additional arguments', () => {
+      const args = buildMCPServerArgs("", "@acme/mcp-tool", [""]);
+      expect(args).toEqual(["@acme/mcp-tool"]);
+    });
+  });
 });

ui/src/lib/toolUtils.ts

@@ -1,6 +1,31 @@
 import { k8sRefUtils } from "@/lib/k8sUtils";
-import type{ Tool, McpServerTool, ToolsResponse, DiscoveredTool, TypedLocalReference, AgentResponse } from "@/types";
+import type { Tool, McpServerTool, ToolsResponse, DiscoveredTool, TypedLocalReference, AgentResponse } from "@/types";
 
+/**
+ * Build the `args` array for a stdio MCPServer deployment.
+ *
+ * The executor (`cmd`) is supplied separately; this helper returns the
+ * remaining arguments in the same order shown in the command preview:
+ *   [commandPrefix...] <packageName> [additionalArgs...]
+ *
+ * `packageName` is required and is always included; callers must validate
+ * that it is non-empty before invoking this helper.
+ */
+export const buildMCPServerArgs = (
+  commandPrefix: string,
+  packageName: string,
+  argPairs: string[],
+): string[] => {
+  const args: string[] = [];
+  if (commandPrefix.trim()) {
+    args.push(...commandPrefix.trim().split(/\s+/));
+  }
+  args.push(packageName.trim());
+  argPairs
+    .filter((arg) => arg.trim() !== "")
+    .forEach((arg) => args.push(arg.trim()));
+  return args;
+};
 
 // Constants for MCP server types and defaults
 const DEFAULT_API_GROUP = "kagent.dev";
@@ -19,17 +44,17 @@ export const isAgentTool = (value: unknown): value is { type: "Agent"; agent: Ty
 // Compare server names it handles both "namespace/name" refs and plain names
 export const serverNamesMatch = (serverName1: string, serverName2: string): boolean => {
   if (!serverName1 || !serverName2) return false;
-  
+
   if (serverName1 === serverName2) return true;
-  
+
   try {
-    const name1 = k8sRefUtils.isValidRef(serverName1) 
-      ? k8sRefUtils.fromRef(serverName1).name 
+    const name1 = k8sRefUtils.isValidRef(serverName1)
+      ? k8sRefUtils.fromRef(serverName1).name
       : serverName1;
-    const name2 = k8sRefUtils.isValidRef(serverName2) 
-      ? k8sRefUtils.fromRef(serverName2).name 
+    const name2 = k8sRefUtils.isValidRef(serverName2)
+      ? k8sRefUtils.fromRef(serverName2).name
       : serverName2;
-    
+
     return name1 === name2;
   } catch {
     return false;
@@ -100,10 +125,10 @@ export const groupMcpToolsByServer = (tools: Tool[]): {
   // Convert to Tool objects- preserve original kind, apiGroup, and namespace from the first tool of each server
   const groupedMcpTools = Array.from(mcpToolsByServer.entries()).map(([serverNameRef, toolNamesSet]) => {
     // Find the first tool from this server to get its kind, apiGroup, and namespace
-    const originalTool = tools.find(tool => 
+    const originalTool = tools.find(tool =>
       isMcpTool(tool) && tool.mcpServer?.name === serverNameRef
     );
-    
+
     const originalMcpServer = originalTool?.mcpServer;
     const rawApprovals = mcpApprovalsByServer.get(serverNameRef) || new Set<string>();
     const mergedApprovals = Array.from(rawApprovals).filter((n) => toolNamesSet.has(n));
@@ -213,7 +238,7 @@ export const getToolResponseCategory = (tool: ToolsResponse | undefined | null):
       return parts[0];
     } else {
       return (tool as ToolsResponse).id;
-    } 
+    }
   }
   return (tool as ToolsResponse).server_name;
 };
@@ -226,17 +251,17 @@ export const getToolResponseIdentifier = (tool: ToolsResponse | undefined | null
 // Convert DiscoveredTool to Tool for agent creation
 export const toolResponseToAgentTool = (tool: ToolsResponse, serverRef: string): Tool => {
   const { apiGroup, kind } = parseGroupKind(tool.group_kind);
-  
+
   // Parse namespace and name from serverRef if it's in namespace/name format
   let name = serverRef;
   let namespace: string | undefined;
-  
+
   if (k8sRefUtils.isValidRef(serverRef)) {
     const parsed = k8sRefUtils.fromRef(serverRef);
     name = parsed.name;
     namespace = parsed.namespace;
   }
-  
+
   return {
     type: MCP_SERVER_TYPE,
     mcpServer: {
@@ -266,7 +291,7 @@ export const getDiscoveredToolCategory = (tool: DiscoveredTool, serverRef: strin
       return parts[0];
     } else {
       return tool.name;
-    } 
+    }
   }
   return serverRef;
 };