feat: Refactor codebase, improve types, attempt test fixes

tests/unit/tools/workflow/list.test.ts

@@ -0,0 +1,154 @@
+/**
+ * ListWorkflowsHandler unit tests
+ */
+
+import { describe, it, expect, jest, beforeEach } from '@jest/globals';
+import { ListWorkflowsHandler, getListWorkflowsToolDefinition } from '../../../../src/tools/workflow/list.js'; // Add .js back
+import { createMockWorkflows } from '../../../mocks/n8n-fixtures.js'; // Add .js back
+// Import BaseWorkflowToolHandler to check constructor call, but don't mock it directly
+import { BaseWorkflowToolHandler } from '../../../../src/tools/workflow/base-handler.js'; // Add .js back
+
+// Generate mock data
+const mockWorkflows = createMockWorkflows(); 
+
+// Mock dependencies of the BaseWorkflowToolHandler
+const mockGetWorkflows = jest.fn(); // Define typed mock function variable
+const mockApiService = {
+  getWorkflows: mockGetWorkflows,
+  // Add other methods used by BaseWorkflowToolHandler if necessary
+};
+const mockEnvConfig = { 
+  /* mock necessary env config properties */ 
+  n8nApiUrl: 'http://mock-n8n.com',
+  n8nApiKey: 'mock-key',
+  n8nWebhookUsername: 'test-user', // Added webhook user
+  n8nWebhookPassword: 'test-pass', // Added webhook pass
+};
+
+// Add .js extension back to mock paths
+jest.mock('../../../../src/config/environment.js', () => ({ 
+  getEnvConfig: jest.fn(() => mockEnvConfig),
+}));
+
+jest.mock('../../../../src/api/n8n-client.js', () => ({ 
+  createApiService: jest.fn(() => mockApiService),
+}));
+
+
+import { Workflow, ToolCallResult } from '../../../../src/types/index.js'; // Add .js back
+
+
+describe('ListWorkflows Tool', () => {
+  let handler: ListWorkflowsHandler;
+
+  beforeEach(() => {
+    // Reset mocks before each test
+    jest.clearAllMocks();
+    // Instantiate the handler, passing the mocked apiService (cast to any)
+    handler = new ListWorkflowsHandler(mockApiService as any); 
+    // Check that dependencies were called by the base constructor
+    // Use the mocked functions directly now, casting the require result
+    const configMock = jest.requireMock('../../../../src/config/environment.js') as any; // Add .js back
+    const apiMock = jest.requireMock('../../../../src/api/n8n-client.js') as any; // Add .js back
+    expect(configMock.getEnvConfig).toHaveBeenCalledTimes(1); 
+    expect(apiMock.createApiService).toHaveBeenCalledWith(mockEnvConfig);
+  });
+
+  describe('getListWorkflowsToolDefinition', () => {
+    it('should return the correct tool definition', () => {
+      // Execute
+    const definition = getListWorkflowsToolDefinition();
+    
+    // Assert
+    expect(definition.name).toBe('list_workflows');
+    expect(definition.description).toBeTruthy();
+    expect(definition.inputSchema).toBeDefined();
+    expect(definition.inputSchema.properties).toHaveProperty('active');
+      expect(definition.inputSchema.required).toEqual([]);
+    });
+  });
+
+  describe('execute', () => {
+    it('should fetch all workflows and format them when no filter is provided', async () => {
+      // Arrange
+      // @ts-ignore - Suppress persistent TS2345 error
+      mockGetWorkflows.mockResolvedValue(mockWorkflows); 
+      const expectedFormatted = mockWorkflows.map((wf: Workflow) => ({ // Add type annotation
+        id: wf.id,
+        name: wf.name,
+        active: wf.active,
+        updatedAt: wf.updatedAt,
+      }));
+
+      // Act
+      const result = await handler.execute({});
+
+      // Assert
+      expect(mockApiService.getWorkflows).toHaveBeenCalledTimes(1);
+      // Check the actual result, which uses the real base class formatters
+      expect(result.isError).toBe(false);
+      expect(result.content[0].text).toContain(`Found ${expectedFormatted.length} workflow(s)`);
+      expect(result.content[0].text).toContain(JSON.stringify(expectedFormatted, null, 2)); 
+    });
+
+    it('should filter workflows by active=true when provided', async () => {
+      // Arrange
+      // @ts-ignore - Suppress persistent TS2345 error
+      mockGetWorkflows.mockResolvedValue(mockWorkflows); 
+      const activeWorkflows = mockWorkflows.filter(wf => wf.active === true);
+      const expectedFormatted = activeWorkflows.map((wf: Workflow) => ({ // Add type annotation
+        id: wf.id,
+        name: wf.name,
+        active: wf.active,
+        updatedAt: wf.updatedAt,
+      }));
+
+      // Act
+      const result = await handler.execute({ active: true });
+
+      // Assert
+      expect(mockApiService.getWorkflows).toHaveBeenCalledTimes(1);
+      expect(result.isError).toBe(false);
+      expect(result.content[0].text).toContain(`Found ${expectedFormatted.length} workflow(s) (filtered by active=true)`);
+      expect(result.content[0].text).toContain(JSON.stringify(expectedFormatted, null, 2)); 
+    });
+
+    it('should filter workflows by active=false when provided', async () => {
+      // Arrange
+      // @ts-ignore - Suppress persistent TS2345 error
+      mockGetWorkflows.mockResolvedValue(mockWorkflows); 
+      const inactiveWorkflows = mockWorkflows.filter(wf => wf.active === false);
+      const expectedFormatted = inactiveWorkflows.map((wf: Workflow) => ({ // Add type annotation
+        id: wf.id,
+        name: wf.name,
+        active: wf.active,
+        updatedAt: wf.updatedAt,
+      }));
+
+      // Act
+      const result = await handler.execute({ active: false });
+
+      // Assert
+      expect(mockApiService.getWorkflows).toHaveBeenCalledTimes(1);
+      expect(result.isError).toBe(false);
+      expect(result.content[0].text).toContain(`Found ${expectedFormatted.length} workflow(s) (filtered by active=false)`);
+      expect(result.content[0].text).toContain(JSON.stringify(expectedFormatted, null, 2)); 
+    });
+
+    it('should handle errors during API call', async () => {
+      // Arrange
+      const apiError = new Error('API Failure');
+      // @ts-ignore - Suppress persistent TS2345 error
+      mockGetWorkflows.mockRejectedValue(apiError); 
+
+      // Act
+      const result = await handler.execute({});
+
+      // Assert
+      expect(mockApiService.getWorkflows).toHaveBeenCalledTimes(1);
+      // Check the actual error result formatted by the real base class method
+      expect(result.isError).toBe(true);
+      expect(result.content[0].text).toContain('Error executing workflow tool: API Failure'); 
+    });
+  });
+});

tests/unit/tools/workflow/list.test.ts.bak

@@ -1,25 +0,0 @@
-/**
- * ListWorkflowsHandler unit tests
- */
-
-import { describe, it, expect, jest } from '@jest/globals';
-import { getListWorkflowsToolDefinition } from '../../../../src/tools/workflow/list.js';
-import { mockApiResponses } from '../../../mocks/n8n-fixtures.js';
-
-// Since this is an integration test, we'll test the definition directly
-// rather than mocking the complex handler implementation
-jest.mock('../../../../src/tools/workflow/base-handler.js');
-
-describe('getListWorkflowsToolDefinition', () => {
-  it('should return the correct tool definition', () => {
-    // Execute
-    const definition = getListWorkflowsToolDefinition();
-    
-    // Assert
-    expect(definition.name).toBe('list_workflows');
-    expect(definition.description).toBeTruthy();
-    expect(definition.inputSchema).toBeDefined();
-    expect(definition.inputSchema.properties).toHaveProperty('active');
-    expect(definition.inputSchema.required).toEqual([]);
-  });
-});

tests/unit/tools/workflow/simple-tool.test.ts

@@ -1,90 +0,0 @@
-/**
- * Simple workflow tool tests without complex dependencies
- */
-
-import { describe, it, expect } from '@jest/globals';
-
-// Mock workflow data
-const mockWorkflows = [
-  {
-    id: '1234abc',
-    name: 'Test Workflow 1',
-    active: true,
-    createdAt: '2025-03-01T12:00:00.000Z',
-    updatedAt: '2025-03-02T14:30:00.000Z',
-    nodes: []
-  },
-  {
-    id: '5678def',
-    name: 'Test Workflow 2',
-    active: false,
-    createdAt: '2025-03-01T12:00:00.000Z',
-    updatedAt: '2025-03-12T10:15:00.000Z',
-    nodes: []
-  }
-];
-
-// Simple function to test tool definition
-function getListWorkflowsToolDefinition() {
-  return {
-    name: 'list_workflows',
-    description: 'List all workflows with optional filtering by status',
-    inputSchema: {
-      type: 'object',
-      properties: {
-        active: {
-          type: 'boolean',
-          description: 'Filter workflows by active status'
-        }
-      },
-      required: []
-    }
-  };
-}
-
-// Simple function to test workflow filtering
-function filterWorkflows(workflows, filter) {
-  if (filter && typeof filter.active === 'boolean') {
-    return workflows.filter(workflow => workflow.active === filter.active);
-  }
-  return workflows;
-}
-
-describe('Workflow Tools', () => {
-  describe('getListWorkflowsToolDefinition', () => {
-    it('should return the correct tool definition', () => {
-      const definition = getListWorkflowsToolDefinition();
-      
-      expect(definition.name).toBe('list_workflows');
-      expect(definition.description).toBeTruthy();
-      expect(definition.inputSchema).toBeDefined();
-      expect(definition.inputSchema.properties).toHaveProperty('active');
-      expect(definition.inputSchema.required).toEqual([]);
-    });
-  });
-  
-  describe('filterWorkflows', () => {
-    it('should return all workflows when no filter is provided', () => {
-      const result = filterWorkflows(mockWorkflows, {});
-      
-      expect(result).toHaveLength(2);
-      expect(result).toEqual(mockWorkflows);
-    });
-    
-    it('should filter workflows by active status when active is true', () => {
-      const result = filterWorkflows(mockWorkflows, { active: true });
-      
-      expect(result).toHaveLength(1);
-      expect(result[0].id).toBe('1234abc');
-      expect(result[0].active).toBe(true);
-    });
-    
-    it('should filter workflows by active status when active is false', () => {
-      const result = filterWorkflows(mockWorkflows, { active: false });
-      
-      expect(result).toHaveLength(1);
-      expect(result[0].id).toBe('5678def');
-      expect(result[0].active).toBe(false);
-    });
-  });
-});