diff --git a/apps/mcp/src/shared/utils.spec.ts b/apps/mcp/src/shared/utils.spec.ts new file mode 100644 index 0000000000..3104338a74 --- /dev/null +++ b/apps/mcp/src/shared/utils.spec.ts @@ -0,0 +1,94 @@ +/** + * Tests for handleApiResult tag field behavior + * + * Regression test for https://github.com/eyaltoledano/claude-task-master/issues/1638 + * Bug: handleApiResult returns the active tag from state.json instead of the + * resolved target tag when a tag is explicitly provided to the operation. + */ + +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { handleApiResult } from './utils.js'; + +// Mock fs and path to control getCurrentTag behavior +vi.mock('node:fs', () => ({ + default: { + existsSync: vi.fn(() => true), + readFileSync: vi.fn(() => + JSON.stringify({ currentTag: 'active-tag-from-state' }) + ) + } +})); + +// Mock the package.json import +vi.mock('../../../../package.json', () => ({ + default: { version: '0.0.0-test', name: 'task-master-ai' } +})); + +describe('handleApiResult tag field', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should use provided tag instead of reading from state.json', async () => { + const result = await handleApiResult({ + result: { + success: true, + data: { taskId: 5, message: 'Success' } + }, + projectRoot: '/mock/project', + tag: 'target-tag' + }); + + const responseText = (result.content[0] as { type: 'text'; text: string }).text; + const parsed = JSON.parse(responseText); + + // The tag field should be the explicitly provided tag, not the active tag + expect(parsed.tag).toBe('target-tag'); + }); + + it('should fall back to state.json tag when no tag is provided', async () => { + const result = await handleApiResult({ + result: { + success: true, + data: { taskId: 5, message: 'Success' } + }, + projectRoot: '/mock/project' + }); + + const responseText = (result.content[0] as { type: 'text'; text: string }).text; + const parsed = JSON.parse(responseText); + + // Without explicit tag, it should read from state.json + expect(parsed.tag).toBe('active-tag-from-state'); + }); + + it('should not include tag field when neither tag nor projectRoot is provided', async () => { + const result = await handleApiResult({ + result: { + success: true, + data: { taskId: 5, message: 'Success' } + } + }); + + const responseText = (result.content[0] as { type: 'text'; text: string }).text; + const parsed = JSON.parse(responseText); + + expect(parsed.tag).toBeUndefined(); + }); + + it('should use provided tag in error responses too', async () => { + const result = await handleApiResult({ + result: { + success: false, + error: { message: 'Something went wrong' } + }, + projectRoot: '/mock/project', + tag: 'target-tag' + }); + + // Error responses include tag in text format + const responseText = (result.content[0] as { type: 'text'; text: string }).text; + expect(responseText).toContain('Current Tag: target-tag'); + expect(responseText).not.toContain('active-tag-from-state'); + }); +}); diff --git a/packages/tm-core/src/common/types/index.ts b/packages/tm-core/src/common/types/index.ts index ac86f30c52..5a3b970ac1 100644 --- a/packages/tm-core/src/common/types/index.ts +++ b/packages/tm-core/src/common/types/index.ts @@ -119,7 +119,7 @@ export interface TaskImplementationMetadata { * Placeholder task interface for temporary/minimal task objects */ export interface PlaceholderTask { - id: string; + id: number | string; title: string; status: TaskStatus; priority: TaskPriority; @@ -129,7 +129,7 @@ export interface PlaceholderTask { * Base task interface */ export interface Task extends TaskImplementationMetadata { - id: string; + id: number | string; title: string; description: string; status: TaskStatus; @@ -171,7 +171,7 @@ export interface Task extends TaskImplementationMetadata { */ export interface Subtask extends Omit { id: number | string; - parentId: string; + parentId: number | string; subtasks?: never; // Subtasks cannot have their own subtasks } @@ -225,7 +225,7 @@ export type CreateTask = Omit< * Type for updating a task (all fields optional except ID) */ export type UpdateTask = Partial> & { - id: string; + id: number | string; }; /** diff --git a/packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts b/packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts index 9f6d7f4c04..46a8302081 100644 --- a/packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts +++ b/packages/tm-core/src/modules/storage/adapters/file-storage/file-storage.ts @@ -282,19 +282,19 @@ export class FileStorage implements IStorage { } /** - * Normalize task IDs - keep Task IDs as strings, Subtask IDs as numbers + * Normalize task IDs - Task IDs and Subtask IDs are both numbers for file storage * Note: Uses spread operator to preserve all task properties including user-defined metadata */ private normalizeTaskIds(tasks: Task[]): Task[] { return tasks.map((task) => ({ ...task, - id: String(task.id), // Task IDs are strings + id: Number(task.id), // Task IDs are numbers dependencies: task.dependencies?.map((dep) => String(dep)) || [], subtasks: task.subtasks?.map((subtask) => ({ ...subtask, id: Number(subtask.id), // Subtask IDs are numbers - parentId: String(subtask.parentId) // Parent ID is string (Task ID) + parentId: Number(subtask.parentId) // Parent ID is number (Task ID) })) || [] })); } @@ -404,7 +404,7 @@ export class FileStorage implements IStorage { ...existingTask, ...updates, ...(mergedSubtasks && { subtasks: mergedSubtasks }), - id: String(taskId) // Keep consistent with normalizeTaskIds + id: Number(taskId) // Keep consistent with normalizeTaskIds }; await this.saveTasks(tasks, tag); } diff --git a/packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts b/packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts index 2e072fa167..218d10309b 100644 --- a/packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts +++ b/packages/tm-core/src/modules/storage/adapters/file-storage/format-handler.ts @@ -217,18 +217,18 @@ export class FormatHandler { } /** - * Normalize task IDs - keep Task IDs as strings, Subtask IDs as numbers + * Normalize task IDs - Task IDs and Subtask IDs are both numbers for file storage */ private normalizeTasks(tasks: Task[]): Task[] { return tasks.map((task) => ({ ...task, - id: String(task.id), // Task IDs are strings + id: Number(task.id), // Task IDs are numbers dependencies: task.dependencies?.map((dep) => String(dep)) || [], subtasks: task.subtasks?.map((subtask) => ({ ...subtask, id: Number(subtask.id), // Subtask IDs are numbers - parentId: String(subtask.parentId) // Parent ID is string (Task ID) + parentId: Number(subtask.parentId) // Parent ID is number (Task ID) })) || [] })); } diff --git a/packages/tm-core/src/testing/task-fixtures.ts b/packages/tm-core/src/testing/task-fixtures.ts index efff72be26..5155b5965c 100644 --- a/packages/tm-core/src/testing/task-fixtures.ts +++ b/packages/tm-core/src/testing/task-fixtures.ts @@ -54,7 +54,7 @@ export function createTask( overrides: Partial> & { id: number | string; title: string } ): Task { return { - id: String(overrides.id), + id: Number(overrides.id), title: overrides.title, description: overrides.description ?? overrides.title, status: overrides.status ?? 'pending',