Merge pull request #47 from chriscarrollsmith/44-bring-error-handling-in-line-with-mcp-spec

Overhaul error handling and test patterns

Author: chriscarrollsmith

.cursor/rules/errors.mdc

@@ -0,0 +1,57 @@
+---
+description: 
+globs: **/errors.ts
+alwaysApply: false
+---
+# Error Flow
+
+```mermaid
+graph TD
+    subgraph Core_Logic
+        FS[FileSystemService: e.g., FileReadError] --> TM[TaskManager: Throws App Errors, e.g., ProjectNotFound, TaskNotDone]
+        TM -->|App Error with code ERR_xxxx| CLI_Handler["cli.ts Command Handler"]
+        TM -->|App Error with code ERR_xxxx| ToolExec["toolExecutors.ts: execute"]
+    end
+
+    subgraph CLI_Path
+        CLI_Handler -->|App Error| CLI_Catch["cli.ts catch block"]
+        CLI_Catch -->|Error Object| FormatCLI["client errors.ts formatCliError"]
+        FormatCLI -->|"Error [ERR_xxxx]: message"| ConsoleOut["console.error Output"]
+    end
+
+    subgraph MCP_Server_Path
+        subgraph Validation_Layer
+            ToolExecVal["toolExecutors.ts Validation"] -->|App Error, e.g., MissingParameter| ExecToolErrHandler
+        end
+        
+        subgraph App_Execution
+            ToolExec -->|App Error with code ERR_xxxx| ExecToolErrHandler["tools.ts executeToolAndHandleErrors catch block"]
+            ExecToolErrHandler -->|Map AppError to Protocol Error or Tool Result| ErrorMapping
+            ErrorMapping -->|"If validation error (ERR_1xxx)"| McpError["Create McpError with appropriate ErrorCode"]
+            ErrorMapping -->|"If business logic error (ERR_2xxx+)"| FormatResult["Format as isError true result"]
+
+            McpError -->|Throw| SDKHandler["server index.ts SDK Handler"]
+            FormatResult -->|"{ content: [{ text: Error [ERR_xxxx]: message }], isError: true }"| SDKHandler
+        end
+
+        SDKHandler -- Protocol Error --> SDKFormatError["SDK Formats as JSON-RPC Error Response"]
+        SDKHandler -- Tool Result --> SDKFormatResult["SDK Formats as JSON-RPC Success Response"]
+
+        SDKFormatError -->|"{ error: { code: -326xx, message: ... } }"| MCPClient["MCP Client"]
+        SDKFormatResult -->|"{ result: { content: [...], isError: true } }"| MCPClient
+    end
+```
+
+**Explanation of Updated Error Flow and Transformations:**
+
+Errors are consistently through a unified `AppError` system:
+
+1. **Validation Errors** (`ERR_1xxx` series)
+   - Used for validation issues (e.g., MissingParameter, InvalidArgument)
+   - Thrown by tool executors during parameter validation
+   - Mapped to protocol-level McpErrors in `executeToolAndHandleErrors`
+
+2. **Business Logic Errors** (`ERR_2xxx` and higher)
+   - Used for all business logic and application-specific errors
+   - Include specific error codes
+   - Returned as serialized CallToolResults with `isError: true`

.cursor/rules/general.mdc

@@ -0,0 +1,6 @@
+---
+description: 
+globs: 
+alwaysApply: true
+---
+Work step-by-step. If presented with an implementation plan, implement the plan exactly. If the plan presents more than one implementation option, consult with the human user to decide between options. If you are tempted to embellish or imporve upon the plan, consult with the human user. Always complete the current task and wait for human review before proceeding to the next task.

.cursor/rules/tests.mdc

@@ -3,197 +3,4 @@ description: Writing unit tests with `jest`
 globs: tests/**/*
 alwaysApply: false
 ---
-# Testing Guidelines for TypeScript + ES Modules + Jest
-
-This guide contains cumulative in-context learnings about working with this project's testing stack.
-
-## Unit vs. Integration Tests
-
-**Never Mix Test Types**: Separate integration tests from unit tests into different files:
-   - Simple unit tests without mocks for validating rules (like state transitions)
-   - Integration tests with mocks for filesystem and external dependencies
-
-## File Path Handling in Tests
-
-1. **Environment Variables**:
-   - Use `process.env.TASK_MANAGER_FILE_PATH` for configuring file paths in tests
-   - Set this in `beforeEach` and clean up in `afterEach`:
-     ```typescript
-     beforeEach(async () => {
-       tempDir = path.join(os.tmpdir(), `test-${Date.now()}`);
-       await fs.mkdir(tempDir, { recursive: true });
-       tasksFilePath = path.join(tempDir, "test-tasks.json");
-       process.env.TASK_MANAGER_FILE_PATH = tasksFilePath;
-     });
-
-     afterEach(async () => {
-       await fs.rm(tempDir, { recursive: true, force: true });
-       delete process.env.TASK_MANAGER_FILE_PATH;
-     });
-     ```
-
-2. **Temporary Files**:
-   - Create unique temp directories for each test run
-   - Use `os.tmpdir()` for platform-independent temp directories
-   - Include timestamps in directory names to prevent conflicts
-   - Always clean up temp files in `afterEach`
-
-## Jest ESM Mocking, Step-by-Step
-
-1. **Type-Only Import:**  
-   Import types for static analysis without actually executing the module code:
-   ```typescript
-   import type { MyService as MyServiceType } from 'path/to/MyService.js';
-   import type { readFile as ReadFileType } from 'node:fs/promises';
-   ```
-
-2. **Register Mock:**  
-   Use `jest.unstable_mockModule` to replace the real module:
-   ```typescript
-   jest.unstable_mockModule('node:fs/promises', () => ({
-     __esModule: true,
-     readFile: jest.fn(),
-   }));
-   ```
-
-3. **Set Default Mock Implementations, Then Dynamically Import Modules:**  
-   You must dynamically import the modules to be mocked and/or tested *after* registering mocks and setting any mock implementations. This ensures that when `MyService` attempts to import `node:fs/promises`, it gets your mocked version. Depending how you want to scope your mock implementations, you can do this in `beforeAll`, `beforeEach`, or at the top of each test.
-   ```typescript
-   let MyService: typeof MyServiceType;
-   let readFile: jest.MockedFunction<ReadFileType>;
-
-   beforeAll(async () => {
-     const fsPromisesMock = await import('node:fs/promises');
-     readFile = fsPromisesMock.readFile as jest.MockedFunction<ReadFileType>;
-
-     // Set default implementation
-     readFile.mockResolvedValue('default mocked content');
-
-     const serviceModule = await import('path/to/MyService.js');
-     MyService = serviceModule.MyService;
-   });
-   ```
-
-4. **Setup in `beforeEach`:**  
-   Reset mocks and set default behaviors before each test:
-   ```typescript
-   beforeEach(() => {
-     jest.clearAllMocks();
-     readFile.mockResolvedValue('');
-   });
-   ```
-
-5. **Write a Test:**  
-   Now you can test your service with the mocked `readFile`:
-   ```typescript
-   describe('MyService', () => {
-     let myServiceInstance: MyServiceType;
-
-     beforeEach(() => {
-       myServiceInstance = new MyService('somePath');
-     });
-
-     it('should do something', async () => {
-       readFile.mockResolvedValueOnce('some data');
-       const result = await myServiceInstance.someMethod();
-       expect(result).toBe('expected result');
-       expect(readFile).toHaveBeenCalledWith('somePath', 'utf-8');
-     });
-   });
-   ```
-
-### Mocking a Class with Methods
-
-If you have a class `MyClass` that has both instance methods and static methods, you can mock it in an **ES Modules + TypeScript** setup using the same pattern. For instance:
-
-```typescript
-// 1. Create typed jest mock functions using the original types
-type InitResult = { data: string };
-
-const mockInit = jest.fn() as jest.MockedFunction<MyClass['init']>;
-const mockDoWork = jest.fn() as jest.MockedFunction<MyClass['doWork']>;
-const mockStaticHelper = jest.fn() as jest.MockedFunction<typeof MyClass.staticHelper>;
-
-// 2. Use jest.unstable_mockModule with an ES6 class in the factory
-jest.unstable_mockModule('path/to/MyClass.js', () => {
-  class MockMyClass {
-    // Instance methods
-    init = mockInit;
-    doWork = mockDoWork;
-
-    // Static method
-    static staticHelper = mockStaticHelper;
-  }
-
-  return {
-    __esModule: true,
-    MyClass: MockMyClass, // same name/structure as real export
-  };
-});
-
-// 3. Import your class after mocking
-let MyClass: typeof import('path/to/MyClass.js')['MyClass'];
-
-beforeAll(async () => {
-  const myClassModule = await import('path/to/MyClass.js');
-  MyClass = myClassModule.MyClass;
-});
-
-// 4. Write tests and reset mocks
-beforeEach(() => {
-  jest.clearAllMocks();
-  mockInit.mockResolvedValue({ data: 'default' });
-  mockStaticHelper.mockReturnValue(42);
-});
-
-describe('MyClass', () => {
-  it('should call init', async () => {
-    const instance = new MyClass();
-    const result = await instance.init();
-    expect(result).toEqual({ data: 'default' });
-    expect(mockInit).toHaveBeenCalledTimes(1);
-  });
-
-  it('should call the static helper', () => {
-    const val = MyClass.staticHelper();
-    expect(val).toBe(42);
-    expect(mockStaticHelper).toHaveBeenCalledTimes(1);
-  });
-});
-```
-
-### Best Practice: **Type** Your Mocked Functions
-
-By default, `jest.fn()` is very generic and doesn't enforce parameter or return types. This can cause TypeScript errors like:
-
-> `Argument of type 'undefined' is not assignable to parameter of type 'never'`
-
-or
-
-> `Type 'Promise<SomeType>' is not assignable to type 'FunctionLike'`
-
-To avoid these, **use the original type with `jest.MockedFunction`**. For example, if your real function is:
-
-```typescript
-async function loadStuff(id: string): Promise<string[]> {
-  // ...
-}
-```
-
-then you should type the mock as:
-
-```typescript
-const mockLoadStuff = jest.fn() as jest.MockedFunction<typeof loadStuff>;
-```
-
-For class methods, use the class type to get the method signature:
-
-```typescript
-const mockClassMethod = jest.fn() as jest.MockedFunction<YourClass['classMethod']>;
-```
-
-This helps TypeScript catch mistakes if you:
-- call the function with the wrong argument types
-- use `mockResolvedValue` with the wrong shape
-
-Once typed properly, your `mockResolvedValue(...)`, `mockImplementation(...)`, etc. calls will be fully type-safe.
+Make use of the helpers in tests/mcp/test-helpers.ts.

.github/workflows/npm-publish.yml

@@ -13,7 +13,6 @@ jobs:
           node-version: 'latest'
       - run: npm ci
       - run: npm install -g tsx
-      - run: npm run build
       - run: npm test
 
   publish:

README.md

@@ -40,14 +40,14 @@ This will show the available commands and options.
 The task manager supports multiple LLM providers for generating project plans. You can configure one or more of the following environment variables depending on which providers you want to use:
 
 - `OPENAI_API_KEY`: Required for using OpenAI models (e.g., GPT-4)
-- `GEMINI_API_KEY`: Required for using Google's Gemini models
+- `GOOGLE_GENERATIVE_AI_API_KEY`: Required for using Google's Gemini models
 - `DEEPSEEK_API_KEY`: Required for using Deepseek models
 
 To generate project plans using the CLI, set these environment variables in your shell:
 
 ```bash
 export OPENAI_API_KEY="your-api-key"
-export GEMINI_API_KEY="your-api-key"
+export GOOGLE_GENERATIVE_AI_API_KEY="your-api-key"
 export DEEPSEEK_API_KEY="your-api-key"
 ```
 
@@ -61,7 +61,7 @@ Or you can include them in your MCP client configuration to generate project pla
       "args": ["-y", "taskqueue-mcp"],
       "env": {
         "OPENAI_API_KEY": "your-api-key",
-        "GEMINI_API_KEY": "your-api-key",
+        "GOOGLE_GENERATIVE_AI_API_KEY": "your-api-key",
         "DEEPSEEK_API_KEY": "your-api-key"
       }
     }

babel.config.cjs

@@ -0,0 +1,6 @@
+module.exports = {
+  presets: [
+    ['@babel/preset-env', { targets: { node: 'current' } }],
+    '@babel/preset-typescript',
+  ],
+}; 

jest.config.cjs

@@ -1,14 +1,7 @@
-const { createDefaultEsmPreset } = require('ts-jest');
-
-const presetConfig = createDefaultEsmPreset({
-  useESM: true,
-});
-
 module.exports = {
-  ...presetConfig,
   testEnvironment: 'node',
   moduleNameMapper: {
-    '^(\\.{1,2}/.*)\\.js$': '$1',
+    '^(\\.{1,2}/.*)\\.js$': '$1'
   },
   modulePathIgnorePatterns: ['<rootDir>/dist/'],
   // Force Jest to exit after all tests have completed