Increase Conciseness of the codebase by replacing switch statement with ToolExecutors

[chriscarrollsmith]

chriscarrollsmith@chriscarrollsmith-IdeaPad-3-15ADA05:~/Documents/Software/MCP_Servers/taskqueue-mcp$ cat repomix-output.txt | llm -m gemi
ni-2.0-pro-exp-02-05 "\n\nWhat's the single biggest opportunity to make this codebase more concise without deleting functionality?"
The single biggest opportunity to make this codebase more concise without deleting functionality lies in refactoring the executeToolWithErrorHandling function in src/server/tools.ts. It's a massive switch statement that handles the logic for every single tool. This creates several problems:

1. High Cyclomatic Complexity: The switch statement makes the function very complex and difficult to read, test, and maintain. Adding new tools or modifying existing ones becomes increasingly error-prone.

2. Violation of Open/Closed Principle: Every time a new tool is added, this function must be modified. This violates the Open/Closed Principle, which states that software entities should be open for extension but closed for modification.

3. Redundant Error Handling: The error handling (try-catch block) is wrapped around the entire switch statement. While this catches all errors, it makes it harder to pinpoint the source of an error and potentially handle specific errors differently. It also adds unnecessary nesting.

4. Tight Coupling: The tool execution logic is tightly coupled to the TaskManager instance passed in.

Here's how we can address this and make the code significantly more concise and maintainable:

Refactoring Strategy: Command Pattern + Tool Objects

The ideal solution is to implement the Command Pattern. Each tool would be represented by its own class (or object with methods) responsible for:

• Knowing its name.
• Knowing its schema (already defined).
• Executing its logic, taking the TaskManager and arguments as input.
• Handling its own specific errors.

Here's a conceptual outline of the refactoring (I'll provide a detailed code example below):

1. Create a ToolExecutor Interface/Type: Define a common interface or type that all tool execution objects must implement. This ensures consistency.

   interface ToolExecutor {
     name: string;
     execute(taskManager: TaskManager, args: Record<string, unknown>): Promise<{ content: Array<{ type: "text"; text: string }>; isError?: boolean }>;
   }

2. Create Tool Executor Objects: For each tool in ALL_TOOLS, create a corresponding object that implements ToolExecutor. This object will contain the specific logic for that tool.

   // Example: listProjectsToolExecutor
   const listProjectsToolExecutor: ToolExecutor = {
     name: "list_projects",
     async execute(taskManager: TaskManager, args: Record<string, unknown>) {
       const result = await taskManager.listProjects((args.state as any)); // type cast as the enums cause some small issues
       return { content: [{ type: "text", text: JSON.stringify(result, null, 2) }] };
     },
   };

3. Create a Tool Map: Create a Map or a simple object that maps tool names (strings) to their corresponding ToolExecutor objects.

   const toolExecutorMap: Map<string, ToolExecutor> = new Map([
     ["list_projects", listProjectsToolExecutor],
     // ... add other tool executors here ...
   ]);

4. Refactor executeToolWithErrorHandling: This function now becomes incredibly simple:

   export async function executeToolWithErrorHandling(
     toolName: string,
     args: Record<string, unknown>,
     taskManager: TaskManager
   ): Promise<{ content: Array<{ type: "text"; text: string }>; isError?: boolean }> {
     const executor = toolExecutorMap.get(toolName);
   
     if (!executor) {
       return {
         content: [{ type: "text", text: `Error: Unknown tool: ${toolName}` }],
         isError: true,
       };
     }
   
     try {
       return await executor.execute(taskManager, args);
     } catch (error) {
       const standardError = normalizeError(error);
       return {
         content: [{ type: "text", text: `Error: ${standardError.message}` }],
         isError: true,
       };
     }
   }

Complete Code Example (Illustrative - Not all tools are refactored):

// src/server/tools.ts (Modified)

import { Tool } from "@modelcontextprotocol/sdk/types.js";
import { TaskManager } from "./TaskManager.js";
import { ErrorCode } from "../types/index.js";
import { createError, normalizeError } from "../utils/errors.js";

// ... (Your existing Tool definitions: listProjectsTool, createProjectTool, etc.) ...

// ---------------------- PROJECT TOOLS ----------------------
// ... (Tool Definitions as before) ...

// Tool Executor Interface
interface ToolExecutor {
  name: string;
  execute(taskManager: TaskManager, args: Record<string, unknown>): Promise<{ content: Array<{ type: "text"; text: string }>; isError?: boolean }>;
}
// --- Tool Executors ---

// List Projects Executor
const listProjectsToolExecutor: ToolExecutor = {
  name: "list_projects",
  async execute(taskManager: TaskManager, args: Record<string, unknown>) {
    const result = await taskManager.listProjects(args.state as any);
    return { content: [{ type: "text", text: JSON.stringify(result, null, 2) }] };
  },
};

// Create Project Executor
const createProjectToolExecutor: ToolExecutor = {
    name: "create_project",
    async execute(taskManager: TaskManager, args: Record<string, unknown>) {
      const initialPrompt = String(args.initialPrompt || "");
      if (!initialPrompt || !args.tasks || !Array.isArray(args.tasks)) {
        throw createError(
          ErrorCode.MissingParameter,
          "Missing required parameters: initialPrompt and/or tasks"
        );
      }
      const projectPlan = args.projectPlan ? String(args.projectPlan) : undefined;
      const autoApprove = args.autoApprove === true;

      const result = await taskManager.createProject(
        initialPrompt,
        args.tasks,
        projectPlan,
        autoApprove
      );
      return { content: [{ type: "text", text: JSON.stringify(result, null, 2) }] };
    },
  };

//Get next task executor
const getNextTaskExecutor: ToolExecutor = {
    name: "get_next_task",
    async execute(taskManager: TaskManager, args: Record<string, unknown>) {
        const projectId = String(args.projectId);
        if (!projectId) {
          throw createError(
            ErrorCode.MissingParameter,
            "Missing required parameter: projectId"
          );
        }
        const result = await taskManager.getNextTask(projectId);
        
        // Ensure backward compatibility with integration tests
        // by adding a task property that refers to the data
        if (result.status === "next_task" && result.data) {
          return {
            content: [{ type: "text", text: JSON.stringify({
              status: "next_task",
              task: result.data,
              message: result.data.message
            }, null, 2) }],
          };
        }
        
        return {
          content: [{ type: "text", text: JSON.stringify(result, null, 2) }],
        };
    }
}

// ... (Implement other tool executors similarly) ...

// Tool Executor Map
const toolExecutorMap: Map<string, ToolExecutor> = new Map([
  ["list_projects", listProjectsToolExecutor],
  ["create_project", createProjectToolExecutor],
  ["get_next_task", getNextTaskExecutor],
  // ... (Add other tool executors to the map) ...
]);

// Refactored executeToolWithErrorHandling
export async function executeToolWithErrorHandling(
  toolName: string,
  args: Record<string, unknown>,
  taskManager: TaskManager
): Promise<{ content: Array<{ type: "text"; text: string }>; isError?: boolean }> {
  const executor = toolExecutorMap.get(toolName);

  if (!executor) {
    return {
      content: [{ type: "text", text: `Error: Unknown tool: ${toolName}` }],
      isError: true,
    };
  }

  try {
    return await executor.execute(taskManager, args);
  } catch (error) {
    const standardError = normalizeError(error);
    return {
      content: [{ type: "text", text: `Error: ${standardError.message}` }],
      isError: true,
    };
  }
}


// Export all tools as an array
export const ALL_TOOLS: Tool[] = [
  listProjectsTool,
  readProjectTool,
  createProjectTool,
  deleteProjectTool,
  addTasksToProjectTool,
  finalizeProjectTool,

  listTasksTool,
  readTaskTool,
  createTaskTool,
  updateTaskTool,
  deleteTaskTool,
  approveTaskTool,
  getNextTaskTool,
];

Key Changes and Benefits:

• Conciseness: The executeToolWithErrorHandling function is reduced from a massive switch statement to a few lines of code.
• Maintainability: Adding new tools is now trivial: create a new ToolExecutor object and add it to the toolExecutorMap. No modification of the core executeToolWithErrorHandling function is needed.
• Testability: Each ToolExecutor can be tested in isolation, making unit testing much easier and more reliable.
• Readability: The code is much easier to understand, as the logic for each tool is encapsulated in its own object.
• Open/Closed Principle: The code is now open for extension (adding new tools) but closed for modification (no need to change existing code to add new tools).
• Decoupling: The execution logic is now decoupled from the main executeToolWithErrorHandling function.
• Improved Error Handling: Each tool executor can now optionally include more fine-grained error handling if needed. The general try/catch is still present for unexpected errors.

This refactoring significantly improves the structure, maintainability, and testability of the codebase while achieving the goal of making it more concise. This is the most impactful change for conciseness without sacrificing functionality.