From 2852149a470c15f44bf4e33978c38f9d761b35d3 Mon Sep 17 00:00:00 2001 From: Parthy <52548018+mm-parthy@users.noreply.github.com> Date: Wed, 2 Jul 2025 21:45:10 +0200 Subject: [PATCH] fix: Critical writeJSON Context Fixes - Prevent Tag Corruption (#910) * feat(tasks): Fix critical tag corruption bug in task management - Fixed missing context parameters in writeJSON calls across add-task, remove-task, and add-subtask functions - Added projectRoot and tag parameters to prevent data corruption in multi-tag environments - Re-enabled generateTaskFiles calls to ensure markdown files are updated after operations - Enhanced add_subtask MCP tool with tag parameter support - Refactored addSubtaskDirect function to properly pass context to core logic - Streamlined codebase by removing deprecated functionality This resolves the critical bug where task operations in one tag context would corrupt or delete tasks from other tags in tasks.json. * feat(task-manager): Enhance addSubtask with current tag support - Added `getCurrentTag` utility to retrieve the current tag context for task operations. - Updated `addSubtask` to use the current tag when reading and writing tasks, ensuring proper context handling. - Refactored tests to accommodate changes in the `addSubtask` function, ensuring accurate mock implementations and expectations. - Cleaned up test cases for better readability and maintainability. This improves task management by preventing tag-related data corruption and enhances the overall functionality of the task manager. * feat(remove-task): Add tag support for task removal and enhance error handling - Introduced `tag` parameter in `removeTaskDirect` to specify context for task operations, improving multi-tag support. - Updated logging to include tag context in messages for better traceability. - Refactored task removal logic to streamline the process and improve error reporting. - Added comprehensive unit tests to validate tag handling and ensure robust error management. This enhancement prevents task data corruption across different tags and improves the overall reliability of the task management system. * feat(add-task): Add projectRoot and tag parameters to addTask tests - Updated `addTask` unit tests to include `projectRoot` and `tag` parameters for better context handling. - Enhanced test cases to ensure accurate expectations and improve overall test coverage. This change aligns with recent enhancements in task management, ensuring consistency across task operations. * feat(set-task-status): Add tag parameter support and enhance task status handling - Introduced `tag` parameter in `setTaskStatusDirect` and related functions to improve context management in multi-tag environments. - Updated `writeJSON` calls to ensure task data integrity across different tags. - Enhanced unit tests to validate tag preservation during task status updates, ensuring robust functionality. This change aligns with recent improvements in task management, preventing data corruption and enhancing overall reliability. * feat(tag-management): Enhance writeJSON calls to preserve tag context - Updated `writeJSON` calls in `createTag`, `deleteTag`, `renameTag`, `copyTag`, and `enhanceTagsWithMetadata` to include `projectRoot` for better context management and to prevent tag corruption. - Added comprehensive unit tests for tag management functions to ensure data integrity and proper tag handling during operations. This change improves the reliability of tag management by ensuring that operations do not corrupt existing tags and maintains the overall structure of the task data. * feat(expand-task): Update writeJSON to include projectRoot and tag context - Modified `writeJSON` call in `expandTaskDirect` to pass `projectRoot` and `tag` parameters, ensuring proper context management when saving tasks.json. - This change aligns with recent enhancements in task management, preventing potential data corruption and improving overall reliability. * feat(fix-dependencies): Add projectRoot and tag parameters for enhanced context management - Updated `fixDependenciesDirect` and `registerFixDependenciesTool` to include `projectRoot` and `tag` parameters, improving context handling during dependency fixes. - Introduced a new unit test for `fixDependenciesCommand` to ensure proper preservation of projectRoot and tag data in JSON outputs. This change enhances the reliability of dependency management by ensuring that context is maintained across operations, preventing potential data issues. * fix(context): propagate projectRoot and tag through dependency, expansion, status-update and tag-management commands to prevent cross-tag data corruption * test(fix-dependencies): Enhance unit tests for fixDependenciesCommand - Refactored tests to use unstable mocks for utils, ui, and task-manager modules, improving isolation and reliability. - Added checks for process.exit to ensure proper handling of invalid data scenarios. - Updated test cases to verify writeJSON calls with projectRoot and tag parameters, ensuring accurate context preservation during dependency fixes. This change strengthens the test suite for dependency management, ensuring robust functionality and preventing potential data issues. * chore(plan): remove outdated fix plan for `writeJSON` context parameters --- .changeset/olive-clouds-tan.md | 5 + .../src/core/direct-functions/add-subtask.js | 14 +- .../src/core/direct-functions/expand-task.js | 4 +- .../core/direct-functions/fix-dependencies.js | 13 +- .../src/core/direct-functions/remove-task.js | 93 ++- .../core/direct-functions/set-task-status.js | 19 +- mcp-server/src/tools/add-subtask.js | 4 +- mcp-server/src/tools/fix-dependencies.js | 7 +- mcp-server/src/tools/remove-task.js | 11 +- mcp-server/src/tools/set-task-status.js | 6 +- scripts/modules/task-manager/add-subtask.js | 10 +- scripts/modules/task-manager/add-task.js | 19 +- .../task-manager/generate-task-files.js | 2 +- scripts/modules/task-manager/remove-task.js | 15 +- .../modules/task-manager/set-task-status.js | 2 +- .../modules/task-manager/tag-management.js | 18 +- tests/unit/mcp/tools/remove-task.test.js | 528 ++++++++++++++++++ .../fix-dependencies-command.test.js | 190 +++++++ .../modules/task-manager/add-subtask.test.js | 403 +++++-------- .../modules/task-manager/add-task.test.js | 20 +- .../task-manager/set-task-status.test.js | 57 +- .../unit/task-manager/tag-management.test.js | 115 ++++ 22 files changed, 1166 insertions(+), 389 deletions(-) create mode 100644 .changeset/olive-clouds-tan.md create mode 100644 tests/unit/mcp/tools/remove-task.test.js create mode 100644 tests/unit/scripts/modules/dependency-manager/fix-dependencies-command.test.js create mode 100644 tests/unit/task-manager/tag-management.test.js 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'); + }); +});