refactor: address review feedback

- Consolidate ProjectSettingsSection state (18 useState → grouped objects + custom hook)
- Extract parseMCPNameCommand helper to dedupe add/edit parsing in registry.ts
- Unify MCP test API (testServer + testCommand → single test endpoint)
- Add localStorage caching for test results with CachedMCPTestResult type
- Show test age in Settings UI (formatRelativeTime)
- Cache successful test results when adding servers

Author: ammar-agent

src/browser/components/Settings/sections/ProjectSettingsSection.tsx

@@ -585,6 +585,25 @@ const newCommandDefinition: SlashCommandDefinition = {
   },
 };
 
+/**
+ * Parse MCP subcommand that takes name + command (add/edit).
+ * Returns { name, command } or null if invalid.
+ */
+function parseMCPNameCommand(
+  subcommand: string,
+  tokens: string[],
+  rawInput: string
+): { name: string; command: string } | null {
+  const name = tokens[1];
+  // Extract command text after "subcommand name"
+  const command = rawInput
+    .trim()
+    .replace(new RegExp(`^${subcommand}\\s+[^\\s]+\\s*`, "i"), "")
+    .trim();
+  if (!name || !command) return null;
+  return { name, command };
+}
+
 const mcpCommandDefinition: SlashCommandDefinition = {
   key: "mcp",
   description: "Manage MCP servers for this project",
@@ -594,28 +613,13 @@ const mcpCommandDefinition: SlashCommandDefinition = {
     }
 
     const sub = cleanRemainingTokens[0];
-    if (sub === "add") {
-      const name = cleanRemainingTokens[1];
-      const commandText = rawInput
-        .trim()
-        .replace(/^add\s+[^\s]+\s*/i, "")
-        .trim();
-      if (!name || !commandText) {
-        return { type: "unknown-command", command: "mcp", subcommand: "add" };
-      }
-      return { type: "mcp-add", name, command: commandText };
-    }
 
-    if (sub === "edit") {
-      const name = cleanRemainingTokens[1];
-      const commandText = rawInput
-        .trim()
-        .replace(/^edit\s+[^\s]+\s*/i, "")
-        .trim();
-      if (!name || !commandText) {
-        return { type: "unknown-command", command: "mcp", subcommand: "edit" };
+    if (sub === "add" || sub === "edit") {
+      const parsed = parseMCPNameCommand(sub, cleanRemainingTokens, rawInput);
+      if (!parsed) {
+        return { type: "unknown-command", command: "mcp", subcommand: sub };
       }
-      return { type: "mcp-edit", name, command: commandText };
+      return { type: sub === "add" ? "mcp-add" : "mcp-edit", ...parsed };
     }
 
     if (sub === "remove") {

src/browser/utils/slashCommands/registry.ts

@@ -48,6 +48,15 @@ export const SELECTED_WORKSPACE_KEY = "selectedWorkspace";
  */
 export const EXPANDED_PROJECTS_KEY = "expandedProjects";
 
+/**
+ * Get the localStorage key for cached MCP server test results (per project)
+ * Format: "mcpTestResults:{projectPath}"
+ * Stores: Record<serverName, CachedMCPTestResult>
+ */
+export function getMCPTestResultsKey(projectPath: string): string {
+  return `mcpTestResults:${projectPath}`;
+}
+
 /**
  * Helper to create a thinking level storage key for a workspace
  * Format: "thinkingLevel:{workspaceId}"

src/common/constants/storage.ts

@@ -44,7 +44,6 @@ export {
   MCPAddParamsSchema,
   MCPRemoveParamsSchema,
   MCPTestParamsSchema,
-  MCPTestCommandParamsSchema,
   MCPTestResultSchema,
 } from "./schemas/mcp";
 

src/common/orpc/schemas.ts

@@ -20,7 +20,6 @@ import {
   MCPRemoveParamsSchema,
   MCPServerMapSchema,
   MCPTestParamsSchema,
-  MCPTestCommandParamsSchema,
   MCPTestResultSchema,
 } from "./mcp";
 
@@ -141,10 +140,6 @@ export const projects = {
       input: MCPTestParamsSchema,
       output: MCPTestResultSchema,
     },
-    testCommand: {
-      input: MCPTestCommandParamsSchema,
-      output: MCPTestResultSchema,
-    },
   },
   secrets: {
     get: {

src/common/orpc/schemas/api.ts

@@ -13,14 +13,14 @@ export const MCPRemoveParamsSchema = z.object({
   name: z.string(),
 });
 
+/**
+ * Unified test params - provide either name (to test configured server) or command (to test arbitrary command).
+ * At least one of name or command must be provided.
+ */
 export const MCPTestParamsSchema = z.object({
   projectPath: z.string(),
-  name: z.string(),
-});
-
-export const MCPTestCommandParamsSchema = z.object({
-  projectPath: z.string(),
-  command: z.string(),
+  name: z.string().optional(),
+  command: z.string().optional(),
 });
 
 export const MCPTestResultSchema = z.discriminatedUnion("success", [

src/common/orpc/schemas/mcp.ts

@@ -6,3 +6,9 @@ export type MCPServerMap = Record<string, string>;
 
 /** Result of testing an MCP server connection */
 export type MCPTestResult = { success: true; tools: string[] } | { success: false; error: string };
+
+/** Cached test result with timestamp for age display */
+export interface CachedMCPTestResult {
+  result: MCPTestResult;
+  testedAt: number; // Unix timestamp ms
+}

src/common/types/mcp.ts

@@ -189,13 +189,7 @@ export const router = (authToken?: string) => {
           .input(schemas.projects.mcp.test.input)
           .output(schemas.projects.mcp.test.output)
           .handler(({ context, input }) =>
-            context.mcpServerManager.testServer(input.projectPath, input.name)
-          ),
-        testCommand: t
-          .input(schemas.projects.mcp.testCommand.input)
-          .output(schemas.projects.mcp.testCommand.output)
-          .handler(({ context, input }) =>
-            context.mcpServerManager.testCommand(input.projectPath, input.command)
+            context.mcpServerManager.test(input.projectPath, input.name, input.command)
           ),
       },
     },

src/node/orpc/router.ts

@@ -80,7 +80,7 @@ export class MCPConfigService {
 
   async removeServer(projectPath: string, name: string): Promise<Result<void>> {
     const cfg = await this.getConfig(projectPath);
-    if (!cfg.servers?.[name]) {
+    if (!cfg.servers[name]) {
       return Err(`Server ${name} not found`);
     }
     delete cfg.servers[name];

src/node/services/mcpConfigService.ts

@@ -242,27 +242,23 @@ export class MCPServerManager {
   }
 
   /**
-   * Test an MCP server configuration by spawning it, fetching tools, then closing.
-   * Used by the Settings UI to verify a server works before relying on it.
+   * Test an MCP server. Provide either:
+   * - `name` to test a configured server by looking up its command
+   * - `command` to test an arbitrary command directly
    */
-  async testServer(projectPath: string, name: string): Promise<MCPTestResult> {
-    const servers = await this.configService.listServers(projectPath);
-    const command = servers[name];
-    if (!command) {
-      return { success: false, error: `Server "${name}" not found in configuration` };
+  async test(projectPath: string, name?: string, command?: string): Promise<MCPTestResult> {
+    if (name) {
+      const servers = await this.configService.listServers(projectPath);
+      const serverCommand = servers[name];
+      if (!serverCommand) {
+        return { success: false, error: `Server "${name}" not found in configuration` };
+      }
+      return runServerTest(serverCommand, projectPath, `server "${name}"`);
     }
-    return runServerTest(command, projectPath, `server "${name}"`);
-  }
-
-  /**
-   * Test an MCP command directly without requiring it to be in config.
-   * Used by Settings UI to validate before adding.
-   */
-  async testCommand(projectPath: string, command: string): Promise<MCPTestResult> {
-    if (!command.trim()) {
-      return { success: false, error: "Command is required" };
+    if (command?.trim()) {
+      return runServerTest(command, projectPath, "command");
     }
-    return runServerTest(command, projectPath, "command");
+    return { success: false, error: "Either name or command is required" };
   }
 
   private collectTools(instances: Map<string, MCPServerInstance>): Record<string, Tool> {