diff --git a/.changeset/olive-clouds-tan.md b/.changeset/olive-clouds-tan.md new file mode 100644 index 00000000..7a2b184e --- /dev/null +++ b/.changeset/olive-clouds-tan.md @@ -0,0 +1,5 @@ +--- +"task-master-ai": patch +--- + +Fix data corruption issues by ensuring project root and tag information is properly passed through all command operations diff --git a/mcp-server/src/core/direct-functions/add-subtask.js b/mcp-server/src/core/direct-functions/add-subtask.js index 9c52d88f..1790bbdd 100644 --- a/mcp-server/src/core/direct-functions/add-subtask.js +++ b/mcp-server/src/core/direct-functions/add-subtask.js @@ -20,6 +20,8 @@ import { * @param {string} [args.status] - Status for new subtask (default: 'pending') * @param {string} [args.dependencies] - Comma-separated list of dependency IDs * @param {boolean} [args.skipGenerate] - Skip regenerating task files + * @param {string} [args.projectRoot] - Project root directory + * @param {string} [args.tag] - Tag for the task * @param {Object} log - Logger object * @returns {Promise<{success: boolean, data?: Object, error?: string}>} */ @@ -34,7 +36,9 @@ export async function addSubtaskDirect(args, log) { details, status, dependencies: dependenciesStr, - skipGenerate + skipGenerate, + projectRoot, + tag } = args; try { log.info(`Adding subtask with args: ${JSON.stringify(args)}`); @@ -96,6 +100,8 @@ export async function addSubtaskDirect(args, log) { // Enable silent mode to prevent console logs from interfering with JSON response enableSilentMode(); + const context = { projectRoot, tag }; + // Case 1: Convert existing task to subtask if (existingTaskId) { log.info(`Converting task ${existingTaskId} to a subtask of ${parentId}`); @@ -104,7 +110,8 @@ export async function addSubtaskDirect(args, log) { parentId, existingTaskId, null, - generateFiles + generateFiles, + context ); // Restore normal logging @@ -135,7 +142,8 @@ export async function addSubtaskDirect(args, log) { parentId, null, newSubtaskData, - generateFiles + generateFiles, + context ); // Restore normal logging diff --git a/mcp-server/src/core/direct-functions/expand-task.js b/mcp-server/src/core/direct-functions/expand-task.js index f0513bec..d231a95c 100644 --- a/mcp-server/src/core/direct-functions/expand-task.js +++ b/mcp-server/src/core/direct-functions/expand-task.js @@ -171,8 +171,8 @@ export async function expandTaskDirect(args, log, context = {}) { task.subtasks = []; } - // Save tasks.json with potentially empty subtasks array - writeJSON(tasksPath, data); + // Save tasks.json with potentially empty subtasks array and proper context + writeJSON(tasksPath, data, projectRoot, tag); // Create logger wrapper using the utility const mcpLog = createLogWrapper(log); diff --git a/mcp-server/src/core/direct-functions/fix-dependencies.js b/mcp-server/src/core/direct-functions/fix-dependencies.js index 65dd407c..7bfceddf 100644 --- a/mcp-server/src/core/direct-functions/fix-dependencies.js +++ b/mcp-server/src/core/direct-functions/fix-dependencies.js @@ -13,12 +13,14 @@ import fs from 'fs'; * Fix invalid dependencies in tasks.json automatically * @param {Object} args - Function arguments * @param {string} args.tasksJsonPath - Explicit path to the tasks.json file. + * @param {string} args.projectRoot - Project root directory + * @param {string} args.tag - Tag for the project * @param {Object} log - Logger object * @returns {Promise<{success: boolean, data?: Object, error?: {code: string, message: string}}>} */ export async function fixDependenciesDirect(args, log) { // Destructure expected args - const { tasksJsonPath } = args; + const { tasksJsonPath, projectRoot, tag } = args; try { log.info(`Fixing invalid dependencies in tasks: ${tasksJsonPath}`); @@ -51,8 +53,10 @@ export async function fixDependenciesDirect(args, log) { // Enable silent mode to prevent console logs from interfering with JSON response enableSilentMode(); - // Call the original command function using the provided path - await fixDependenciesCommand(tasksPath); + // Call the original command function using the provided path and proper context + await fixDependenciesCommand(tasksPath, { + context: { projectRoot, tag } + }); // Restore normal logging disableSilentMode(); @@ -61,7 +65,8 @@ export async function fixDependenciesDirect(args, log) { success: true, data: { message: 'Dependencies fixed successfully', - tasksPath + tasksPath, + tag: tag || 'master' } }; } catch (error) { diff --git a/mcp-server/src/core/direct-functions/remove-task.js b/mcp-server/src/core/direct-functions/remove-task.js index 34200f41..fb1698ae 100644 --- a/mcp-server/src/core/direct-functions/remove-task.js +++ b/mcp-server/src/core/direct-functions/remove-task.js @@ -20,12 +20,13 @@ import { * @param {Object} args - Command arguments * @param {string} args.tasksJsonPath - Explicit path to the tasks.json file. * @param {string} args.id - The ID(s) of the task(s) or subtask(s) to remove (comma-separated for multiple). + * @param {string} [args.tag] - Tag context to operate on (defaults to current active tag). * @param {Object} log - Logger object * @returns {Promise} - Remove task result { success: boolean, data?: any, error?: { code: string, message: string } } */ export async function removeTaskDirect(args, log, context = {}) { // Destructure expected args - const { tasksJsonPath, id, projectRoot } = args; + const { tasksJsonPath, id, projectRoot, tag } = args; const { session } = context; try { // Check if tasksJsonPath was provided @@ -56,17 +57,17 @@ export async function removeTaskDirect(args, log, context = {}) { const taskIdArray = id.split(',').map((taskId) => taskId.trim()); log.info( - `Removing ${taskIdArray.length} task(s) with ID(s): ${taskIdArray.join(', ')} from ${tasksJsonPath}` + `Removing ${taskIdArray.length} task(s) with ID(s): ${taskIdArray.join(', ')} from ${tasksJsonPath}${tag ? ` in tag '${tag}'` : ''}` ); // Validate all task IDs exist before proceeding - const data = readJSON(tasksJsonPath, projectRoot); + const data = readJSON(tasksJsonPath, projectRoot, tag); if (!data || !data.tasks) { return { success: false, error: { code: 'INVALID_TASKS_FILE', - message: `No valid tasks found in ${tasksJsonPath}` + message: `No valid tasks found in ${tasksJsonPath}${tag ? ` for tag '${tag}'` : ''}` } }; } @@ -80,71 +81,51 @@ export async function removeTaskDirect(args, log, context = {}) { success: false, error: { code: 'INVALID_TASK_ID', - message: `The following tasks were not found: ${invalidTasks.join(', ')}` + message: `The following tasks were not found${tag ? ` in tag '${tag}'` : ''}: ${invalidTasks.join(', ')}` } }; } - // Remove tasks one by one - const results = []; - // Enable silent mode to prevent console logs from interfering with JSON response enableSilentMode(); try { - for (const taskId of taskIdArray) { - try { - const result = await removeTask(tasksJsonPath, taskId); - results.push({ - taskId, - success: true, - message: result.message, - removedTask: result.removedTask - }); - log.info(`Successfully removed task: ${taskId}`); - } catch (error) { - results.push({ - taskId, - success: false, - error: error.message - }); - log.error(`Error removing task ${taskId}: ${error.message}`); - } + // Call removeTask with proper context including tag + const result = await removeTask(tasksJsonPath, id, { + projectRoot, + tag + }); + + if (!result.success) { + return { + success: false, + error: { + code: 'REMOVE_TASK_ERROR', + message: result.errors.join('; ') || 'Failed to remove tasks', + details: result.errors + } + }; } + + log.info(`Successfully removed ${result.removedTasks.length} task(s)`); + + return { + success: true, + data: { + totalTasks: taskIdArray.length, + successful: result.removedTasks.length, + failed: result.errors.length, + removedTasks: result.removedTasks, + messages: result.messages, + errors: result.errors, + tasksPath: tasksJsonPath, + tag: data.tag || tag || 'master' + } + }; } finally { // Restore normal logging disableSilentMode(); } - - // Check if all tasks were successfully removed - const successfulRemovals = results.filter((r) => r.success); - const failedRemovals = results.filter((r) => !r.success); - - if (successfulRemovals.length === 0) { - // All removals failed - return { - success: false, - error: { - code: 'REMOVE_TASK_ERROR', - message: 'Failed to remove any tasks', - details: failedRemovals - .map((r) => `${r.taskId}: ${r.error}`) - .join('; ') - } - }; - } - - // At least some tasks were removed successfully - return { - success: true, - data: { - totalTasks: taskIdArray.length, - successful: successfulRemovals.length, - failed: failedRemovals.length, - results: results, - tasksPath: tasksJsonPath - } - }; } catch (error) { // Ensure silent mode is disabled even if an outer error occurs disableSilentMode(); diff --git a/mcp-server/src/core/direct-functions/set-task-status.js b/mcp-server/src/core/direct-functions/set-task-status.js index da0eeb41..aacd94fc 100644 --- a/mcp-server/src/core/direct-functions/set-task-status.js +++ b/mcp-server/src/core/direct-functions/set-task-status.js @@ -20,7 +20,8 @@ import { nextTaskDirect } from './next-task.js'; */ export async function setTaskStatusDirect(args, log, context = {}) { // Destructure expected args, including the resolved tasksJsonPath and projectRoot - const { tasksJsonPath, id, status, complexityReportPath, projectRoot } = args; + const { tasksJsonPath, id, status, complexityReportPath, projectRoot, tag } = + args; const { session } = context; try { log.info(`Setting task status with args: ${JSON.stringify(args)}`); @@ -69,11 +70,17 @@ export async function setTaskStatusDirect(args, log, context = {}) { enableSilentMode(); // Enable silent mode before calling core function try { // Call the core function - await setTaskStatus(tasksPath, taskId, newStatus, { - mcpLog: log, - projectRoot, - session - }); + await setTaskStatus( + tasksPath, + taskId, + newStatus, + { + mcpLog: log, + projectRoot, + session + }, + tag + ); log.info(`Successfully set task ${taskId} status to ${newStatus}`); diff --git a/mcp-server/src/tools/add-subtask.js b/mcp-server/src/tools/add-subtask.js index 6e6fd377..8d1fda44 100644 --- a/mcp-server/src/tools/add-subtask.js +++ b/mcp-server/src/tools/add-subtask.js @@ -52,6 +52,7 @@ export function registerAddSubtaskTool(server) { .describe( 'Absolute path to the tasks file (default: tasks/tasks.json)' ), + tag: z.string().optional().describe('Tag context to operate on'), skipGenerate: z .boolean() .optional() @@ -89,7 +90,8 @@ export function registerAddSubtaskTool(server) { status: args.status, dependencies: args.dependencies, skipGenerate: args.skipGenerate, - projectRoot: args.projectRoot + projectRoot: args.projectRoot, + tag: args.tag }, log, { session } diff --git a/mcp-server/src/tools/fix-dependencies.js b/mcp-server/src/tools/fix-dependencies.js index 34ffcdf5..c46d18bc 100644 --- a/mcp-server/src/tools/fix-dependencies.js +++ b/mcp-server/src/tools/fix-dependencies.js @@ -24,7 +24,8 @@ export function registerFixDependenciesTool(server) { file: z.string().optional().describe('Absolute path to the tasks file'), projectRoot: z .string() - .describe('The directory of the project. Must be an absolute path.') + .describe('The directory of the project. Must be an absolute path.'), + tag: z.string().optional().describe('Tag context to operate on') }), execute: withNormalizedProjectRoot(async (args, { log, session }) => { try { @@ -46,7 +47,9 @@ export function registerFixDependenciesTool(server) { const result = await fixDependenciesDirect( { - tasksJsonPath: tasksJsonPath + tasksJsonPath: tasksJsonPath, + projectRoot: args.projectRoot, + tag: args.tag }, log ); diff --git a/mcp-server/src/tools/remove-task.js b/mcp-server/src/tools/remove-task.js index 81e25f5e..c2b1c60c 100644 --- a/mcp-server/src/tools/remove-task.js +++ b/mcp-server/src/tools/remove-task.js @@ -33,7 +33,13 @@ export function registerRemoveTaskTool(server) { confirm: z .boolean() .optional() - .describe('Whether to skip confirmation prompt (default: false)') + .describe('Whether to skip confirmation prompt (default: false)'), + tag: z + .string() + .optional() + .describe( + 'Specify which tag context to operate on. Defaults to the current active tag.' + ) }), execute: withNormalizedProjectRoot(async (args, { log, session }) => { try { @@ -59,7 +65,8 @@ export function registerRemoveTaskTool(server) { { tasksJsonPath: tasksJsonPath, id: args.id, - projectRoot: args.projectRoot + projectRoot: args.projectRoot, + tag: args.tag }, log, { session } diff --git a/mcp-server/src/tools/set-task-status.js b/mcp-server/src/tools/set-task-status.js index a9cb13a0..ad6edb9b 100644 --- a/mcp-server/src/tools/set-task-status.js +++ b/mcp-server/src/tools/set-task-status.js @@ -47,7 +47,8 @@ export function registerSetTaskStatusTool(server) { ), projectRoot: z .string() - .describe('The directory of the project. Must be an absolute path.') + .describe('The directory of the project. Must be an absolute path.'), + tag: z.string().optional().describe('Optional tag context to operate on') }), execute: withNormalizedProjectRoot(async (args, { log, session }) => { try { @@ -86,7 +87,8 @@ export function registerSetTaskStatusTool(server) { id: args.id, status: args.status, complexityReportPath, - projectRoot: args.projectRoot + projectRoot: args.projectRoot, + tag: args.tag }, log, { session } diff --git a/scripts/modules/task-manager/add-subtask.js b/scripts/modules/task-manager/add-subtask.js index 83d0c9f2..b48a8dc9 100644 --- a/scripts/modules/task-manager/add-subtask.js +++ b/scripts/modules/task-manager/add-subtask.js @@ -1,6 +1,6 @@ import path from 'path'; -import { log, readJSON, writeJSON } from '../utils.js'; +import { log, readJSON, writeJSON, getCurrentTag } from '../utils.js'; import { isTaskDependentOn } from '../task-manager.js'; import generateTaskFiles from './generate-task-files.js'; @@ -25,8 +25,10 @@ async function addSubtask( try { log('info', `Adding subtask to parent task ${parentId}...`); + const currentTag = + context.tag || getCurrentTag(context.projectRoot) || 'master'; // Read the existing tasks with proper context - const data = readJSON(tasksPath, context.projectRoot, context.tag); + const data = readJSON(tasksPath, context.projectRoot, currentTag); if (!data || !data.tasks) { throw new Error(`Invalid or missing tasks file at ${tasksPath}`); } @@ -137,12 +139,12 @@ async function addSubtask( } // Write the updated tasks back to the file with proper context - writeJSON(tasksPath, data, context.projectRoot, context.tag); + writeJSON(tasksPath, data, context.projectRoot, currentTag); // Generate task files if requested if (generateFiles) { log('info', 'Regenerating task files...'); - // await generateTaskFiles(tasksPath, path.dirname(tasksPath), context); + await generateTaskFiles(tasksPath, path.dirname(tasksPath), context); } return newSubtask; diff --git a/scripts/modules/task-manager/add-task.js b/scripts/modules/task-manager/add-task.js index 1e94c05b..0fc29478 100644 --- a/scripts/modules/task-manager/add-task.js +++ b/scripts/modules/task-manager/add-task.js @@ -28,6 +28,7 @@ import { import { generateObjectService } from '../ai-services-unified.js'; import { getDefaultPriority } from '../config-manager.js'; import ContextGatherer from '../utils/contextGatherer.js'; +import generateTaskFiles from './generate-task-files.js'; // Define Zod schema for the expected AI output object const AiTaskDataSchema = z.object({ @@ -553,18 +554,18 @@ async function addTask( report('DEBUG: Writing tasks.json...', 'debug'); // Write the updated raw data back to the file // The writeJSON function will automatically filter out _rawTaggedData - writeJSON(tasksPath, rawData); + writeJSON(tasksPath, rawData, projectRoot, targetTag); report('DEBUG: tasks.json written.', 'debug'); // Generate markdown task files - // report('Generating task files...', 'info'); - // report('DEBUG: Calling generateTaskFiles...', 'debug'); - // // Pass mcpLog if available to generateTaskFiles - // await generateTaskFiles(tasksPath, path.dirname(tasksPath), { - // projectRoot, - // tag: targetTag - // }); - // report('DEBUG: generateTaskFiles finished.', 'debug'); + report('Generating task files...', 'info'); + report('DEBUG: Calling generateTaskFiles...', 'debug'); + // Pass mcpLog if available to generateTaskFiles + await generateTaskFiles(tasksPath, path.dirname(tasksPath), { + projectRoot, + tag: targetTag + }); + report('DEBUG: generateTaskFiles finished.', 'debug'); // Show success message - only for text output (CLI) if (outputFormat === 'text') { diff --git a/scripts/modules/task-manager/generate-task-files.js b/scripts/modules/task-manager/generate-task-files.js index 17498db0..d5dba0d6 100644 --- a/scripts/modules/task-manager/generate-task-files.js +++ b/scripts/modules/task-manager/generate-task-files.js @@ -1,5 +1,5 @@ -import fs from 'fs'; import path from 'path'; +import fs from 'fs'; import chalk from 'chalk'; import { log, readJSON } from '../utils.js'; diff --git a/scripts/modules/task-manager/remove-task.js b/scripts/modules/task-manager/remove-task.js index de4382fa..0406482e 100644 --- a/scripts/modules/task-manager/remove-task.js +++ b/scripts/modules/task-manager/remove-task.js @@ -1,7 +1,6 @@ -import fs from 'fs'; import path from 'path'; - -import { log, readJSON, writeJSON } from '../utils.js'; +import * as fs from 'fs'; +import { readJSON, writeJSON, log, findTaskById } from '../utils.js'; import generateTaskFiles from './generate-task-files.js'; import taskExists from './task-exists.js'; @@ -172,7 +171,7 @@ async function removeTask(tasksPath, taskIds, context = {}) { } // Save the updated raw data structure - writeJSON(tasksPath, fullTaggedData); + writeJSON(tasksPath, fullTaggedData, projectRoot, currentTag); // Delete task files AFTER saving tasks.json for (const taskIdNum of tasksToDeleteFiles) { @@ -195,10 +194,10 @@ async function removeTask(tasksPath, taskIds, context = {}) { // Generate updated task files ONCE, with context try { - // await generateTaskFiles(tasksPath, path.dirname(tasksPath), { - // projectRoot, - // tag: currentTag - // }); + await generateTaskFiles(tasksPath, path.dirname(tasksPath), { + projectRoot, + tag: currentTag + }); results.messages.push('Task files regenerated successfully.'); } catch (genError) { const genErrMsg = `Failed to regenerate task files: ${genError.message}`; diff --git a/scripts/modules/task-manager/set-task-status.js b/scripts/modules/task-manager/set-task-status.js index 9a08fcba..218aad3d 100644 --- a/scripts/modules/task-manager/set-task-status.js +++ b/scripts/modules/task-manager/set-task-status.js @@ -132,7 +132,7 @@ async function setTaskStatus( // Write the updated raw data back to the file // The writeJSON function will automatically filter out _rawTaggedData - writeJSON(tasksPath, rawData); + writeJSON(tasksPath, rawData, options.projectRoot, currentTag); // Validate dependencies after status update log('info', 'Validating dependencies after status update...'); diff --git a/scripts/modules/task-manager/tag-management.js b/scripts/modules/task-manager/tag-management.js index 65a80e35..c17f7068 100644 --- a/scripts/modules/task-manager/tag-management.js +++ b/scripts/modules/task-manager/tag-management.js @@ -145,8 +145,8 @@ async function createTag( } } - // Write the clean data back to file - writeJSON(tasksPath, cleanData); + // Write the clean data back to file with proper context to avoid tag corruption + writeJSON(tasksPath, cleanData, projectRoot); logFn.success(`Successfully created tag "${tagName}"`); @@ -365,8 +365,8 @@ async function deleteTag( } } - // Write the clean data back to file - writeJSON(tasksPath, cleanData); + // Write the clean data back to file with proper context to avoid tag corruption + writeJSON(tasksPath, cleanData, projectRoot); logFn.success(`Successfully deleted tag "${tagName}"`); @@ -485,7 +485,7 @@ async function enhanceTagsWithMetadata(tasksPath, rawData, context = {}) { cleanData[key] = value; } } - writeJSON(tasksPath, cleanData); + writeJSON(tasksPath, cleanData, context.projectRoot); } } catch (error) { // Don't throw - just log and continue @@ -905,8 +905,8 @@ async function renameTag( } } - // Write the clean data back to file - writeJSON(tasksPath, cleanData); + // Write the clean data back to file with proper context to avoid tag corruption + writeJSON(tasksPath, cleanData, projectRoot); // Get task count const tasks = getTasksForTag(rawData, newName); @@ -1062,8 +1062,8 @@ async function copyTag( } } - // Write the clean data back to file - writeJSON(tasksPath, cleanData); + // Write the clean data back to file with proper context to avoid tag corruption + writeJSON(tasksPath, cleanData, projectRoot); logFn.success( `Successfully copied tag from "${sourceName}" to "${targetName}"` diff --git a/tests/unit/mcp/tools/remove-task.test.js b/tests/unit/mcp/tools/remove-task.test.js new file mode 100644 index 00000000..0568f183 --- /dev/null +++ b/tests/unit/mcp/tools/remove-task.test.js @@ -0,0 +1,528 @@ +/** + * Tests for the remove-task MCP tool + * + * Note: This test does NOT test the actual implementation. It tests that: + * 1. The tool is registered correctly with the correct parameters + * 2. Arguments are passed correctly to removeTaskDirect + * 3. Error handling works as expected + * 4. Tag parameter is properly handled and passed through + * + * We do NOT import the real implementation - everything is mocked + */ + +import { jest } from '@jest/globals'; + +// Mock EVERYTHING +const mockRemoveTaskDirect = jest.fn(); +jest.mock('../../../../mcp-server/src/core/task-master-core.js', () => ({ + removeTaskDirect: mockRemoveTaskDirect +})); + +const mockHandleApiResult = jest.fn((result) => result); +const mockWithNormalizedProjectRoot = jest.fn((fn) => fn); +const mockCreateErrorResponse = jest.fn((msg) => ({ + success: false, + error: { code: 'ERROR', message: msg } +})); +const mockFindTasksPath = jest.fn(() => '/mock/project/tasks.json'); + +jest.mock('../../../../mcp-server/src/tools/utils.js', () => ({ + handleApiResult: mockHandleApiResult, + createErrorResponse: mockCreateErrorResponse, + withNormalizedProjectRoot: mockWithNormalizedProjectRoot +})); + +jest.mock('../../../../mcp-server/src/core/utils/path-utils.js', () => ({ + findTasksPath: mockFindTasksPath +})); + +// Mock the z object from zod +const mockZod = { + object: jest.fn(() => mockZod), + string: jest.fn(() => mockZod), + boolean: jest.fn(() => mockZod), + optional: jest.fn(() => mockZod), + describe: jest.fn(() => mockZod), + _def: { + shape: () => ({ + id: {}, + file: {}, + projectRoot: {}, + confirm: {}, + tag: {} + }) + } +}; + +jest.mock('zod', () => ({ + z: mockZod +})); + +// DO NOT import the real module - create a fake implementation +// This is the fake implementation of registerRemoveTaskTool +const registerRemoveTaskTool = (server) => { + // Create simplified version of the tool config + const toolConfig = { + name: 'remove_task', + description: 'Remove a task or subtask permanently from the tasks list', + parameters: mockZod, + + // Create a simplified mock of the execute function + execute: mockWithNormalizedProjectRoot(async (args, context) => { + const { log, session } = context; + + try { + log.info && log.info(`Removing task(s) with ID(s): ${args.id}`); + + // Use args.projectRoot directly (guaranteed by withNormalizedProjectRoot) + let tasksJsonPath; + try { + tasksJsonPath = mockFindTasksPath( + { projectRoot: args.projectRoot, file: args.file }, + log + ); + } catch (error) { + log.error && log.error(`Error finding tasks.json: ${error.message}`); + return mockCreateErrorResponse( + `Failed to find tasks.json: ${error.message}` + ); + } + + log.info && log.info(`Using tasks file path: ${tasksJsonPath}`); + + const result = await mockRemoveTaskDirect( + { + tasksJsonPath: tasksJsonPath, + id: args.id, + projectRoot: args.projectRoot, + tag: args.tag + }, + log, + { session } + ); + + if (result.success) { + log.info && log.info(`Successfully removed task: ${args.id}`); + } else { + log.error && + log.error(`Failed to remove task: ${result.error.message}`); + } + + return mockHandleApiResult( + result, + log, + 'Error removing task', + undefined, + args.projectRoot + ); + } catch (error) { + log.error && log.error(`Error in remove-task tool: ${error.message}`); + return mockCreateErrorResponse(error.message); + } + }) + }; + + // Register the tool with the server + server.addTool(toolConfig); +}; + +describe('MCP Tool: remove-task', () => { + // Create mock server + let mockServer; + let executeFunction; + + // Create mock logger + const mockLogger = { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn() + }; + + // Test data + const validArgs = { + id: '5', + projectRoot: '/mock/project/root', + file: '/mock/project/tasks.json', + confirm: true, + tag: 'feature-branch' + }; + + const multipleTaskArgs = { + id: '5,6.1,7', + projectRoot: '/mock/project/root', + tag: 'master' + }; + + // Standard responses + const successResponse = { + success: true, + data: { + totalTasks: 1, + successful: 1, + failed: 0, + removedTasks: [ + { + id: 5, + title: 'Removed Task', + status: 'pending' + } + ], + messages: ["Successfully removed task 5 from tag 'feature-branch'"], + errors: [], + tasksPath: '/mock/project/tasks.json', + tag: 'feature-branch' + } + }; + + const multipleTasksSuccessResponse = { + success: true, + data: { + totalTasks: 3, + successful: 3, + failed: 0, + removedTasks: [ + { id: 5, title: 'Task 5', status: 'pending' }, + { id: 1, title: 'Subtask 6.1', status: 'done', parentTaskId: 6 }, + { id: 7, title: 'Task 7', status: 'in-progress' } + ], + messages: [ + "Successfully removed task 5 from tag 'master'", + "Successfully removed subtask 6.1 from tag 'master'", + "Successfully removed task 7 from tag 'master'" + ], + errors: [], + tasksPath: '/mock/project/tasks.json', + tag: 'master' + } + }; + + const errorResponse = { + success: false, + error: { + code: 'INVALID_TASK_ID', + message: "The following tasks were not found in tag 'feature-branch': 999" + } + }; + + const pathErrorResponse = { + success: false, + error: { + code: 'PATH_ERROR', + message: 'Failed to find tasks.json: No tasks.json found' + } + }; + + beforeEach(() => { + // Reset all mocks + jest.clearAllMocks(); + + // Create mock server + mockServer = { + addTool: jest.fn((config) => { + executeFunction = config.execute; + }) + }; + + // Setup default successful response + mockRemoveTaskDirect.mockResolvedValue(successResponse); + mockFindTasksPath.mockReturnValue('/mock/project/tasks.json'); + + // Register the tool + registerRemoveTaskTool(mockServer); + }); + + test('should register the tool correctly', () => { + // Verify tool was registered + expect(mockServer.addTool).toHaveBeenCalledWith( + expect.objectContaining({ + name: 'remove_task', + description: 'Remove a task or subtask permanently from the tasks list', + parameters: expect.any(Object), + execute: expect.any(Function) + }) + ); + + // Verify the tool config was passed + const toolConfig = mockServer.addTool.mock.calls[0][0]; + expect(toolConfig).toHaveProperty('parameters'); + expect(toolConfig).toHaveProperty('execute'); + }); + + test('should execute the tool with valid parameters including tag', async () => { + // Setup context + const mockContext = { + log: mockLogger, + session: { workingDirectory: '/mock/dir' } + }; + + // Execute the function + await executeFunction(validArgs, mockContext); + + // Verify findTasksPath was called with correct arguments + expect(mockFindTasksPath).toHaveBeenCalledWith( + { + projectRoot: validArgs.projectRoot, + file: validArgs.file + }, + mockLogger + ); + + // Verify removeTaskDirect was called with correct arguments including tag + expect(mockRemoveTaskDirect).toHaveBeenCalledWith( + expect.objectContaining({ + tasksJsonPath: '/mock/project/tasks.json', + id: validArgs.id, + projectRoot: validArgs.projectRoot, + tag: validArgs.tag // This is the key test - tag parameter should be passed through + }), + mockLogger, + { + session: mockContext.session + } + ); + + // Verify handleApiResult was called + expect(mockHandleApiResult).toHaveBeenCalledWith( + successResponse, + mockLogger, + 'Error removing task', + undefined, + validArgs.projectRoot + ); + }); + + test('should handle multiple task IDs with tag context', async () => { + // Setup multiple tasks response + mockRemoveTaskDirect.mockResolvedValueOnce(multipleTasksSuccessResponse); + + // Setup context + const mockContext = { + log: mockLogger, + session: { workingDirectory: '/mock/dir' } + }; + + // Execute the function + await executeFunction(multipleTaskArgs, mockContext); + + // Verify removeTaskDirect was called with comma-separated IDs and tag + expect(mockRemoveTaskDirect).toHaveBeenCalledWith( + expect.objectContaining({ + id: '5,6.1,7', + tag: 'master' + }), + mockLogger, + expect.any(Object) + ); + + // Verify successful handling of multiple tasks + expect(mockHandleApiResult).toHaveBeenCalledWith( + multipleTasksSuccessResponse, + mockLogger, + 'Error removing task', + undefined, + multipleTaskArgs.projectRoot + ); + }); + + test('should handle missing tag parameter (defaults to current tag)', async () => { + const argsWithoutTag = { + id: '5', + projectRoot: '/mock/project/root' + }; + + // Setup context + const mockContext = { + log: mockLogger, + session: { workingDirectory: '/mock/dir' } + }; + + // Execute the function + await executeFunction(argsWithoutTag, mockContext); + + // Verify removeTaskDirect was called with undefined tag (should default to current tag) + expect(mockRemoveTaskDirect).toHaveBeenCalledWith( + expect.objectContaining({ + id: '5', + projectRoot: '/mock/project/root', + tag: undefined // Should be undefined when not provided + }), + mockLogger, + expect.any(Object) + ); + }); + + test('should handle errors from removeTaskDirect', async () => { + // Setup error response + mockRemoveTaskDirect.mockResolvedValueOnce(errorResponse); + + // Setup context + const mockContext = { + log: mockLogger, + session: { workingDirectory: '/mock/dir' } + }; + + // Execute the function + await executeFunction(validArgs, mockContext); + + // Verify removeTaskDirect was called + expect(mockRemoveTaskDirect).toHaveBeenCalled(); + + // Verify error logging + expect(mockLogger.error).toHaveBeenCalledWith( + "Failed to remove task: The following tasks were not found in tag 'feature-branch': 999" + ); + + // Verify handleApiResult was called with error response + expect(mockHandleApiResult).toHaveBeenCalledWith( + errorResponse, + mockLogger, + 'Error removing task', + undefined, + validArgs.projectRoot + ); + }); + + test('should handle path finding errors', async () => { + // Setup path finding error + mockFindTasksPath.mockImplementationOnce(() => { + throw new Error('No tasks.json found'); + }); + + // Setup context + const mockContext = { + log: mockLogger, + session: { workingDirectory: '/mock/dir' } + }; + + // Execute the function + const result = await executeFunction(validArgs, mockContext); + + // Verify error logging + expect(mockLogger.error).toHaveBeenCalledWith( + 'Error finding tasks.json: No tasks.json found' + ); + + // Verify error response was returned + expect(mockCreateErrorResponse).toHaveBeenCalledWith( + 'Failed to find tasks.json: No tasks.json found' + ); + + // Verify removeTaskDirect was NOT called + expect(mockRemoveTaskDirect).not.toHaveBeenCalled(); + }); + + test('should handle unexpected errors in execute function', async () => { + // Setup unexpected error + mockRemoveTaskDirect.mockImplementationOnce(() => { + throw new Error('Unexpected error'); + }); + + // Setup context + const mockContext = { + log: mockLogger, + session: { workingDirectory: '/mock/dir' } + }; + + // Execute the function + await executeFunction(validArgs, mockContext); + + // Verify error logging + expect(mockLogger.error).toHaveBeenCalledWith( + 'Error in remove-task tool: Unexpected error' + ); + + // Verify error response was returned + expect(mockCreateErrorResponse).toHaveBeenCalledWith('Unexpected error'); + }); + + test('should properly handle withNormalizedProjectRoot wrapper', () => { + // Verify that withNormalizedProjectRoot was called with the execute function + expect(mockWithNormalizedProjectRoot).toHaveBeenCalledWith( + expect.any(Function) + ); + }); + + test('should log appropriate info messages for successful operations', async () => { + // Setup context + const mockContext = { + log: mockLogger, + session: { workingDirectory: '/mock/dir' } + }; + + // Execute the function + await executeFunction(validArgs, mockContext); + + // Verify appropriate logging + expect(mockLogger.info).toHaveBeenCalledWith( + 'Removing task(s) with ID(s): 5' + ); + expect(mockLogger.info).toHaveBeenCalledWith( + 'Using tasks file path: /mock/project/tasks.json' + ); + expect(mockLogger.info).toHaveBeenCalledWith( + 'Successfully removed task: 5' + ); + }); + + test('should handle subtask removal with proper tag context', async () => { + const subtaskArgs = { + id: '5.2', + projectRoot: '/mock/project/root', + tag: 'feature-branch' + }; + + const subtaskSuccessResponse = { + success: true, + data: { + totalTasks: 1, + successful: 1, + failed: 0, + removedTasks: [ + { + id: 2, + title: 'Removed Subtask', + status: 'pending', + parentTaskId: 5 + } + ], + messages: [ + "Successfully removed subtask 5.2 from tag 'feature-branch'" + ], + errors: [], + tasksPath: '/mock/project/tasks.json', + tag: 'feature-branch' + } + }; + + mockRemoveTaskDirect.mockResolvedValueOnce(subtaskSuccessResponse); + + // Setup context + const mockContext = { + log: mockLogger, + session: { workingDirectory: '/mock/dir' } + }; + + // Execute the function + await executeFunction(subtaskArgs, mockContext); + + // Verify removeTaskDirect was called with subtask ID and tag + expect(mockRemoveTaskDirect).toHaveBeenCalledWith( + expect.objectContaining({ + id: '5.2', + tag: 'feature-branch' + }), + mockLogger, + expect.any(Object) + ); + + // Verify successful handling + expect(mockHandleApiResult).toHaveBeenCalledWith( + subtaskSuccessResponse, + mockLogger, + 'Error removing task', + undefined, + subtaskArgs.projectRoot + ); + }); +}); diff --git a/tests/unit/scripts/modules/dependency-manager/fix-dependencies-command.test.js b/tests/unit/scripts/modules/dependency-manager/fix-dependencies-command.test.js new file mode 100644 index 00000000..264e0303 --- /dev/null +++ b/tests/unit/scripts/modules/dependency-manager/fix-dependencies-command.test.js @@ -0,0 +1,190 @@ +/** + * Unit test to ensure fixDependenciesCommand writes JSON with the correct + * projectRoot and tag arguments so that tag data is preserved. + */ + +import { jest } from '@jest/globals'; + +// Mock process.exit to prevent test termination +const mockProcessExit = jest.fn(); +const originalExit = process.exit; +process.exit = mockProcessExit; + +// Mock utils.js BEFORE importing the module under test +jest.unstable_mockModule('../../../../../scripts/modules/utils.js', () => ({ + readJSON: jest.fn(), + writeJSON: jest.fn(), + log: jest.fn(), + findProjectRoot: jest.fn(() => '/mock/project/root'), + getCurrentTag: jest.fn(() => 'master'), + taskExists: jest.fn(() => true), + formatTaskId: jest.fn((id) => id), + findCycles: jest.fn(() => []), + isSilentMode: jest.fn(() => true), + resolveTag: jest.fn(() => 'master'), + getTasksForTag: jest.fn(() => []), + setTasksForTag: jest.fn(), + enableSilentMode: jest.fn(), + disableSilentMode: jest.fn() +})); + +// Mock ui.js +jest.unstable_mockModule('../../../../../scripts/modules/ui.js', () => ({ + displayBanner: jest.fn() +})); + +// Mock task-manager.js +jest.unstable_mockModule( + '../../../../../scripts/modules/task-manager.js', + () => ({ + generateTaskFiles: jest.fn() + }) +); + +// Mock external libraries +jest.unstable_mockModule('chalk', () => ({ + default: { + green: jest.fn((text) => text), + cyan: jest.fn((text) => text), + bold: jest.fn((text) => text) + } +})); + +jest.unstable_mockModule('boxen', () => ({ + default: jest.fn((text) => text) +})); + +// Import the mocked modules +const { readJSON, writeJSON, log, taskExists } = await import( + '../../../../../scripts/modules/utils.js' +); + +// Import the module under test +const { fixDependenciesCommand } = await import( + '../../../../../scripts/modules/dependency-manager.js' +); + +describe('fixDependenciesCommand tag preservation', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockProcessExit.mockClear(); + }); + + afterAll(() => { + // Restore original process.exit + process.exit = originalExit; + }); + + it('calls writeJSON with projectRoot and tag parameters when changes are made', async () => { + const tasksPath = '/mock/tasks.json'; + const projectRoot = '/mock/project/root'; + const tag = 'master'; + + // Mock data WITH dependency issues to trigger writeJSON + const tasksDataWithIssues = { + tasks: [ + { + id: 1, + title: 'Task 1', + dependencies: [999] // Non-existent dependency to trigger fix + }, + { + id: 2, + title: 'Task 2', + dependencies: [] + } + ], + tag: 'master', + _rawTaggedData: { + master: { + tasks: [ + { + id: 1, + title: 'Task 1', + dependencies: [999] + } + ] + } + } + }; + + readJSON.mockReturnValue(tasksDataWithIssues); + taskExists.mockReturnValue(false); // Make dependency invalid to trigger fix + + await fixDependenciesCommand(tasksPath, { + context: { projectRoot, tag } + }); + + // Verify readJSON was called with correct parameters + expect(readJSON).toHaveBeenCalledWith(tasksPath, projectRoot, tag); + + // Verify writeJSON was called (should be triggered by removing invalid dependency) + expect(writeJSON).toHaveBeenCalled(); + + // Check the writeJSON call parameters + const writeJSONCalls = writeJSON.mock.calls; + const lastWriteCall = writeJSONCalls[writeJSONCalls.length - 1]; + const [calledPath, _data, calledProjectRoot, calledTag] = lastWriteCall; + + expect(calledPath).toBe(tasksPath); + expect(calledProjectRoot).toBe(projectRoot); + expect(calledTag).toBe(tag); + + // Verify process.exit was NOT called (meaning the function succeeded) + expect(mockProcessExit).not.toHaveBeenCalled(); + }); + + it('does not call writeJSON when no changes are needed', async () => { + const tasksPath = '/mock/tasks.json'; + const projectRoot = '/mock/project/root'; + const tag = 'master'; + + // Mock data WITHOUT dependency issues (no changes needed) + const cleanTasksData = { + tasks: [ + { + id: 1, + title: 'Task 1', + dependencies: [] // Clean, no issues + } + ], + tag: 'master' + }; + + readJSON.mockReturnValue(cleanTasksData); + taskExists.mockReturnValue(true); // All dependencies exist + + await fixDependenciesCommand(tasksPath, { + context: { projectRoot, tag } + }); + + // Verify readJSON was called + expect(readJSON).toHaveBeenCalledWith(tasksPath, projectRoot, tag); + + // Verify writeJSON was NOT called (no changes needed) + expect(writeJSON).not.toHaveBeenCalled(); + + // Verify process.exit was NOT called + expect(mockProcessExit).not.toHaveBeenCalled(); + }); + + it('handles early exit when no valid tasks found', async () => { + const tasksPath = '/mock/tasks.json'; + + // Mock invalid data to trigger early exit + readJSON.mockReturnValue(null); + + await fixDependenciesCommand(tasksPath, { + context: { projectRoot: '/mock', tag: 'master' } + }); + + // Verify readJSON was called + expect(readJSON).toHaveBeenCalled(); + + // Verify writeJSON was NOT called (early exit) + expect(writeJSON).not.toHaveBeenCalled(); + + // Verify process.exit WAS called due to invalid data + expect(mockProcessExit).toHaveBeenCalledWith(1); + }); +}); diff --git a/tests/unit/scripts/modules/task-manager/add-subtask.test.js b/tests/unit/scripts/modules/task-manager/add-subtask.test.js index 7980778a..eeaf6ef7 100644 --- a/tests/unit/scripts/modules/task-manager/add-subtask.test.js +++ b/tests/unit/scripts/modules/task-manager/add-subtask.test.js @@ -2,308 +2,171 @@ * Tests for the addSubtask function */ import { jest } from '@jest/globals'; -import path from 'path'; -// Mock dependencies -const mockReadJSON = jest.fn(); -const mockWriteJSON = jest.fn(); -const mockGenerateTaskFiles = jest.fn(); -const mockIsTaskDependentOn = jest.fn().mockReturnValue(false); - -// Mock path module -jest.mock('path', () => ({ - dirname: jest.fn() -})); - -// Define test version of the addSubtask function -const testAddSubtask = ( - tasksPath, - parentId, - existingTaskId, - newSubtaskData, - generateFiles = true -) => { - // Read the existing tasks - const data = mockReadJSON(tasksPath); - if (!data || !data.tasks) { - throw new Error(`Invalid or missing tasks file at ${tasksPath}`); - } - - // Convert parent ID to number - const parentIdNum = parseInt(parentId, 10); - - // Find the parent task - const parentTask = data.tasks.find((t) => t.id === parentIdNum); - if (!parentTask) { - throw new Error(`Parent task with ID ${parentIdNum} not found`); - } - - // Initialize subtasks array if it doesn't exist - if (!parentTask.subtasks) { - parentTask.subtasks = []; - } - - let newSubtask; - - // Case 1: Convert an existing task to a subtask - if (existingTaskId !== null) { - const existingTaskIdNum = parseInt(existingTaskId, 10); - - // Find the existing task - const existingTaskIndex = data.tasks.findIndex( - (t) => t.id === existingTaskIdNum - ); - if (existingTaskIndex === -1) { - throw new Error(`Task with ID ${existingTaskIdNum} not found`); - } - - const existingTask = data.tasks[existingTaskIndex]; - - // Check if task is already a subtask - if (existingTask.parentTaskId) { - throw new Error( - `Task ${existingTaskIdNum} is already a subtask of task ${existingTask.parentTaskId}` - ); - } - - // Check for circular dependency - if (existingTaskIdNum === parentIdNum) { - throw new Error(`Cannot make a task a subtask of itself`); - } - - // Check for circular dependency using mockIsTaskDependentOn - if (mockIsTaskDependentOn()) { - throw new Error( - `Cannot create circular dependency: task ${parentIdNum} is already a subtask or dependent of task ${existingTaskIdNum}` - ); - } - - // Find the highest subtask ID to determine the next ID - const highestSubtaskId = - parentTask.subtasks.length > 0 - ? Math.max(...parentTask.subtasks.map((st) => st.id)) - : 0; - const newSubtaskId = highestSubtaskId + 1; - - // Clone the existing task to be converted to a subtask - newSubtask = { - ...existingTask, - id: newSubtaskId, - parentTaskId: parentIdNum - }; - - // Add to parent's subtasks - parentTask.subtasks.push(newSubtask); - - // Remove the task from the main tasks array - data.tasks.splice(existingTaskIndex, 1); - } - // Case 2: Create a new subtask - else if (newSubtaskData) { - // Find the highest subtask ID to determine the next ID - const highestSubtaskId = - parentTask.subtasks.length > 0 - ? Math.max(...parentTask.subtasks.map((st) => st.id)) - : 0; - const newSubtaskId = highestSubtaskId + 1; - - // Create the new subtask object - newSubtask = { - id: newSubtaskId, - title: newSubtaskData.title, - description: newSubtaskData.description || '', - details: newSubtaskData.details || '', - status: newSubtaskData.status || 'pending', - dependencies: newSubtaskData.dependencies || [], - parentTaskId: parentIdNum - }; - - // Add to parent's subtasks - parentTask.subtasks.push(newSubtask); - } else { - throw new Error('Either existingTaskId or newSubtaskData must be provided'); - } - - // Write the updated tasks back to the file - mockWriteJSON(tasksPath, data); - - // Generate task files if requested - if (generateFiles) { - mockGenerateTaskFiles(tasksPath, path.dirname(tasksPath)); - } - - return newSubtask; +// Mock dependencies before importing the module +const mockUtils = { + readJSON: jest.fn(), + writeJSON: jest.fn(), + log: jest.fn(), + getCurrentTag: jest.fn() }; +const mockTaskManager = { + isTaskDependentOn: jest.fn() +}; +const mockGenerateTaskFiles = jest.fn(); + +jest.unstable_mockModule( + '../../../../../scripts/modules/utils.js', + () => mockUtils +); +jest.unstable_mockModule( + '../../../../../scripts/modules/task-manager.js', + () => mockTaskManager +); +jest.unstable_mockModule( + '../../../../../scripts/modules/task-manager/generate-task-files.js', + () => ({ + default: mockGenerateTaskFiles + }) +); + +const addSubtask = ( + await import('../../../../../scripts/modules/task-manager/add-subtask.js') +).default; describe('addSubtask function', () => { - // Reset mocks before each test + const multiTagData = { + master: { + tasks: [{ id: 1, title: 'Master Task', subtasks: [] }], + metadata: { description: 'Master tasks' } + }, + 'feature-branch': { + tasks: [{ id: 1, title: 'Feature Task', subtasks: [] }], + metadata: { description: 'Feature tasks' } + } + }; + beforeEach(() => { jest.clearAllMocks(); + mockTaskManager.isTaskDependentOn.mockReturnValue(false); + }); - // Default mock implementations - mockReadJSON.mockImplementation(() => ({ - tasks: [ - { - id: 1, - title: 'Parent Task', - description: 'This is a parent task', - status: 'pending', - dependencies: [] - }, - { - id: 2, - title: 'Existing Task', - description: 'This is an existing task', - status: 'pending', - dependencies: [] - }, - { - id: 3, - title: 'Another Task', - description: 'This is another task', - status: 'pending', - dependencies: [1] - } - ] - })); - - // Setup success write response - mockWriteJSON.mockImplementation((path, data) => { - return data; + test('should add a new subtask and preserve other tags', async () => { + const context = { projectRoot: '/fake/root', tag: 'feature-branch' }; + const newSubtaskData = { title: 'My New Subtask' }; + mockUtils.readJSON.mockReturnValueOnce({ + tasks: [{ id: 1, title: 'Feature Task', subtasks: [] }], + metadata: { description: 'Feature tasks' } }); - // Set up default behavior for dependency check - mockIsTaskDependentOn.mockReturnValue(false); + await addSubtask('tasks.json', '1', null, newSubtaskData, true, context); + + expect(mockUtils.writeJSON).toHaveBeenCalledWith( + 'tasks.json', + expect.any(Object), + '/fake/root', + 'feature-branch' + ); + const writtenData = mockUtils.writeJSON.mock.calls[0][1]; + const parentTask = writtenData.tasks.find((t) => t.id === 1); + expect(parentTask.subtasks).toHaveLength(1); + expect(parentTask.subtasks[0].title).toBe('My New Subtask'); }); test('should add a new subtask to a parent task', async () => { - // Create new subtask data - const newSubtaskData = { - title: 'New Subtask', - description: 'This is a new subtask', - details: 'Implementation details for the subtask', - status: 'pending', - dependencies: [] - }; - - // Execute the test version of addSubtask - const newSubtask = testAddSubtask( - 'tasks/tasks.json', - 1, + mockUtils.readJSON.mockReturnValueOnce({ + tasks: [{ id: 1, title: 'Parent Task', subtasks: [] }] + }); + const context = {}; + const newSubtask = await addSubtask( + 'tasks.json', + '1', null, - newSubtaskData, - true + { title: 'New Subtask' }, + true, + context ); - - // Verify readJSON was called with the correct path - expect(mockReadJSON).toHaveBeenCalledWith('tasks/tasks.json'); - - // Verify writeJSON was called with the correct path - expect(mockWriteJSON).toHaveBeenCalledWith( - 'tasks/tasks.json', - expect.any(Object) - ); - - // Verify the subtask was created with correct data expect(newSubtask).toBeDefined(); expect(newSubtask.id).toBe(1); - expect(newSubtask.title).toBe('New Subtask'); expect(newSubtask.parentTaskId).toBe(1); - - // Verify generateTaskFiles was called + expect(mockUtils.writeJSON).toHaveBeenCalled(); + const writeCallArgs = mockUtils.writeJSON.mock.calls[0][1]; // data is the second arg now + const parentTask = writeCallArgs.tasks.find((t) => t.id === 1); + expect(parentTask.subtasks).toHaveLength(1); + expect(parentTask.subtasks[0].title).toBe('New Subtask'); expect(mockGenerateTaskFiles).toHaveBeenCalled(); }); test('should convert an existing task to a subtask', async () => { - // Execute the test version of addSubtask to convert task 2 to a subtask of task 1 - const convertedSubtask = testAddSubtask( - 'tasks/tasks.json', - 1, - 2, + mockUtils.readJSON.mockReturnValueOnce({ + tasks: [ + { id: 1, title: 'Parent Task', subtasks: [] }, + { id: 2, title: 'Existing Task 2', subtasks: [] } + ] + }); + const context = {}; + const convertedSubtask = await addSubtask( + 'tasks.json', + '1', + '2', null, - true + true, + context ); - - // Verify readJSON was called with the correct path - expect(mockReadJSON).toHaveBeenCalledWith('tasks/tasks.json'); - - // Verify writeJSON was called - expect(mockWriteJSON).toHaveBeenCalled(); - - // Verify the subtask was created with correct data - expect(convertedSubtask).toBeDefined(); expect(convertedSubtask.id).toBe(1); - expect(convertedSubtask.title).toBe('Existing Task'); expect(convertedSubtask.parentTaskId).toBe(1); - - // Verify generateTaskFiles was called - expect(mockGenerateTaskFiles).toHaveBeenCalled(); + expect(convertedSubtask.title).toBe('Existing Task 2'); + expect(mockUtils.writeJSON).toHaveBeenCalled(); + const writeCallArgs = mockUtils.writeJSON.mock.calls[0][1]; + const parentTask = writeCallArgs.tasks.find((t) => t.id === 1); + expect(parentTask.subtasks).toHaveLength(1); + expect(parentTask.subtasks[0].title).toBe('Existing Task 2'); }); test('should throw an error if parent task does not exist', async () => { - // Create new subtask data - const newSubtaskData = { - title: 'New Subtask', - description: 'This is a new subtask' - }; + mockUtils.readJSON.mockReturnValueOnce({ + tasks: [{ id: 1, title: 'Task 1', subtasks: [] }] + }); + const context = {}; + await expect( + addSubtask( + 'tasks.json', + '99', + null, + { title: 'New Subtask' }, + true, + context + ) + ).rejects.toThrow('Parent task with ID 99 not found'); + }); - // Override mockReadJSON for this specific test case - mockReadJSON.mockImplementationOnce(() => ({ + test('should throw an error if trying to convert a non-existent task', async () => { + mockUtils.readJSON.mockReturnValueOnce({ + tasks: [{ id: 1, title: 'Parent Task', subtasks: [] }] + }); + const context = {}; + await expect( + addSubtask('tasks.json', '1', '99', null, true, context) + ).rejects.toThrow('Task with ID 99 not found'); + }); + + test('should throw an error for circular dependency', async () => { + mockUtils.readJSON.mockReturnValueOnce({ tasks: [ - { - id: 1, - title: 'Task 1', - status: 'pending' - } + { id: 1, title: 'Parent Task', subtasks: [] }, + { id: 2, title: 'Child Task', subtasks: [] } ] - })); - - // Expect an error when trying to add a subtask to a non-existent parent - expect(() => - testAddSubtask('tasks/tasks.json', 999, null, newSubtaskData) - ).toThrow(/Parent task with ID 999 not found/); - - // Verify writeJSON was not called - expect(mockWriteJSON).not.toHaveBeenCalled(); - }); - - test('should throw an error if existing task does not exist', async () => { - // Expect an error when trying to convert a non-existent task - expect(() => testAddSubtask('tasks/tasks.json', 1, 999, null)).toThrow( - /Task with ID 999 not found/ + }); + mockTaskManager.isTaskDependentOn.mockImplementation( + (tasks, parentTask, existingTaskIdNum) => { + return parentTask.id === 1 && existingTaskIdNum === 2; + } ); - - // Verify writeJSON was not called - expect(mockWriteJSON).not.toHaveBeenCalled(); - }); - - test('should throw an error if trying to create a circular dependency', async () => { - // Force the isTaskDependentOn mock to return true for this test only - mockIsTaskDependentOn.mockReturnValueOnce(true); - - // Expect an error when trying to create a circular dependency - expect(() => testAddSubtask('tasks/tasks.json', 3, 1, null)).toThrow( - /circular dependency/ + const context = {}; + await expect( + addSubtask('tasks.json', '1', '2', null, true, context) + ).rejects.toThrow( + 'Cannot create circular dependency: task 1 is already a subtask or dependent of task 2' ); - - // Verify writeJSON was not called - expect(mockWriteJSON).not.toHaveBeenCalled(); - }); - - test('should not regenerate task files if generateFiles is false', async () => { - // Create new subtask data - const newSubtaskData = { - title: 'New Subtask', - description: 'This is a new subtask' - }; - - // Execute the test version of addSubtask with generateFiles = false - testAddSubtask('tasks/tasks.json', 1, null, newSubtaskData, false); - - // Verify writeJSON was called - expect(mockWriteJSON).toHaveBeenCalled(); - - // Verify task files were not regenerated - expect(mockGenerateTaskFiles).not.toHaveBeenCalled(); }); }); diff --git a/tests/unit/scripts/modules/task-manager/add-task.test.js b/tests/unit/scripts/modules/task-manager/add-task.test.js index 4f894729..2aae8777 100644 --- a/tests/unit/scripts/modules/task-manager/add-task.test.js +++ b/tests/unit/scripts/modules/task-manager/add-task.test.js @@ -258,7 +258,9 @@ describe('addTask', () => { }) ]) }) - }) + }), + '/mock/project/root', // projectRoot parameter + 'master' // tag parameter ); expect(result).toEqual( expect.objectContaining({ @@ -299,7 +301,9 @@ describe('addTask', () => { }) ]) }) - }) + }), + '/mock/project/root', // projectRoot parameter + 'master' // tag parameter ); }); @@ -334,7 +338,9 @@ describe('addTask', () => { }) ]) }) - }) + }), + '/mock/project/root', // projectRoot parameter + 'master' // tag parameter ); expect(context.mcpLog.warn).toHaveBeenCalledWith( expect.stringContaining( @@ -366,7 +372,9 @@ describe('addTask', () => { }) ]) }) - }) + }), + '/mock/project/root', // projectRoot parameter + 'master' // tag parameter ); }); @@ -401,7 +409,9 @@ describe('addTask', () => { }) ]) }) - }) + }), + '/mock/project/root', // projectRoot parameter + 'master' // tag parameter ); }); diff --git a/tests/unit/scripts/modules/task-manager/set-task-status.test.js b/tests/unit/scripts/modules/task-manager/set-task-status.test.js index 6cbb8752..6e252b00 100644 --- a/tests/unit/scripts/modules/task-manager/set-task-status.test.js +++ b/tests/unit/scripts/modules/task-manager/set-task-status.test.js @@ -247,7 +247,9 @@ describe('setTaskStatus', () => { expect.objectContaining({ id: 2, status: 'done' }) ]) }) - }) + }), + undefined, + 'master' ); // expect(generateTaskFiles).toHaveBeenCalledWith( // tasksPath, @@ -287,7 +289,9 @@ describe('setTaskStatus', () => { }) ]) }) - }) + }), + undefined, + 'master' ); }); @@ -318,7 +322,9 @@ describe('setTaskStatus', () => { expect.objectContaining({ id: 2, status: 'done' }) ]) }) - }) + }), + undefined, + 'master' ); }); @@ -354,7 +360,9 @@ describe('setTaskStatus', () => { }) ]) }) - }) + }), + undefined, + 'master' ); }); @@ -524,4 +532,45 @@ describe('setTaskStatus', () => { ); expect(result).toBeDefined(); }); + + // Regression test to ensure tag preservation when updating in multi-tag environment + test('should preserve other tags when updating task status', async () => { + // Arrange + const multiTagData = { + master: JSON.parse(JSON.stringify(sampleTasks.master)), + 'feature-branch': { + tasks: [ + { id: 10, title: 'FB Task', status: 'pending', dependencies: [] } + ], + metadata: { description: 'Feature branch tasks' } + } + }; + const tasksPath = '/mock/path/tasks.json'; + + readJSON.mockReturnValue({ + ...multiTagData.master, // resolved view not used + tag: 'master', + _rawTaggedData: multiTagData + }); + + // Act + await setTaskStatus(tasksPath, '1', 'done', { + mcpLog: { info: jest.fn() } + }); + + // Assert: writeJSON should be called with data containing both tags intact + const writeArgs = writeJSON.mock.calls[0]; + expect(writeArgs[0]).toBe(tasksPath); + const writtenData = writeArgs[1]; + expect(writtenData).toHaveProperty('master'); + expect(writtenData).toHaveProperty('feature-branch'); + // master task updated + const updatedTask = writtenData.master.tasks.find((t) => t.id === 1); + expect(updatedTask.status).toBe('done'); + // feature-branch untouched + expect(writtenData['feature-branch'].tasks[0].status).toBe('pending'); + // ensure additional args (projectRoot undefined, tag 'master') present + expect(writeArgs[2]).toBeUndefined(); + expect(writeArgs[3]).toBe('master'); + }); }); diff --git a/tests/unit/task-manager/tag-management.test.js b/tests/unit/task-manager/tag-management.test.js new file mode 100644 index 00000000..8ee0dbdf --- /dev/null +++ b/tests/unit/task-manager/tag-management.test.js @@ -0,0 +1,115 @@ +import fs from 'fs'; +import path from 'path'; +import { + createTag, + deleteTag, + renameTag, + copyTag, + tags as listTags +} from '../../../scripts/modules/task-manager/tag-management.js'; + +const TEMP_DIR = path.join(process.cwd(), '.tmp_tag_management_tests'); +const TASKS_PATH = path.join(TEMP_DIR, 'tasks.json'); + +/** + * Helper to write an initial tagged tasks.json structure + */ +function writeInitialFile() { + const initialData = { + master: { + tasks: [{ id: 1, title: 'Initial Task', status: 'pending' }], + metadata: { + created: new Date().toISOString(), + description: 'Master tag' + } + } + }; + fs.mkdirSync(TEMP_DIR, { recursive: true }); + fs.writeFileSync(TASKS_PATH, JSON.stringify(initialData, null, 2)); +} + +describe('Tag Management – writeJSON context preservation', () => { + beforeEach(() => { + writeInitialFile(); + }); + + afterEach(() => { + fs.rmSync(TEMP_DIR, { recursive: true, force: true }); + }); + + it('createTag should not corrupt other tags', async () => { + await createTag( + TASKS_PATH, + 'feature', + { copyFromCurrent: true }, + { projectRoot: TEMP_DIR }, + 'json' + ); + + const data = JSON.parse(fs.readFileSync(TASKS_PATH, 'utf8')); + expect(data.master).toBeDefined(); + expect(data.feature).toBeDefined(); + }); + + it('renameTag should keep overall structure intact', async () => { + await createTag( + TASKS_PATH, + 'oldtag', + {}, + { projectRoot: TEMP_DIR }, + 'json' + ); + + await renameTag( + TASKS_PATH, + 'oldtag', + 'newtag', + {}, + { projectRoot: TEMP_DIR }, + 'json' + ); + + const data = JSON.parse(fs.readFileSync(TASKS_PATH, 'utf8')); + expect(data.newtag).toBeDefined(); + expect(data.oldtag).toBeUndefined(); + }); + + it('copyTag then deleteTag preserves other tags', async () => { + await createTag( + TASKS_PATH, + 'source', + {}, + { projectRoot: TEMP_DIR }, + 'json' + ); + + await copyTag( + TASKS_PATH, + 'source', + 'copy', + {}, + { projectRoot: TEMP_DIR }, + 'json' + ); + + await deleteTag( + TASKS_PATH, + 'copy', + { yes: true }, + { projectRoot: TEMP_DIR }, + 'json' + ); + + const tagsList = await listTags( + TASKS_PATH, + {}, + { projectRoot: TEMP_DIR }, + 'json' + ); + + const tagNames = tagsList.tags.map((t) => t.name); + expect(tagNames).toContain('master'); + expect(tagNames).toContain('source'); + expect(tagNames).not.toContain('copy'); + }); +});