From 43e0025f4c5870a3c56682cbb8fe0348d711953b Mon Sep 17 00:00:00 2001 From: Parthy <52548018+mm-parthy@users.noreply.github.com> Date: Wed, 2 Jul 2025 12:53:12 +0200 Subject: [PATCH] fix: prevent tag corruption in bulk updates (#856) * fix(task-manager): prevent tag corruption in bulk updates and add tag preservation test - Fix writeJSON call in scripts/modules/task-manager/update-tasks.js (line 469) to include projectRoot and tag parameters. - Ensure tagged task lists maintain data integrity during bulk updates, preventing task disappearance in tagged contexts. - Update MCP tools to properly pass tag context through the call chain. - Introduce a comprehensive test case to verify that all tags are preserved when updating tasks, covering both master and feature-branch scenarios. Addresses an issue where bulk updates could corrupt tasks.json in tagged task list structures, reinforcing task management robustness. * style(tests): format task data in update-tasks test --- .changeset/chubby-needles-beam.md | 5 + .../src/core/direct-functions/update-tasks.js | 5 +- mcp-server/src/tools/update.js | 8 +- scripts/modules/task-manager/update-tasks.js | 18 ++- .../modules/task-manager/update-tasks.test.js | 125 +++++++++++++++++- 5 files changed, 147 insertions(+), 14 deletions(-) create mode 100644 .changeset/chubby-needles-beam.md diff --git a/.changeset/chubby-needles-beam.md b/.changeset/chubby-needles-beam.md new file mode 100644 index 00000000..422215c8 --- /dev/null +++ b/.changeset/chubby-needles-beam.md @@ -0,0 +1,5 @@ +--- +"task-master-ai": patch +--- + +Fix bulk update tag corruption in tagged task lists diff --git a/mcp-server/src/core/direct-functions/update-tasks.js b/mcp-server/src/core/direct-functions/update-tasks.js index 1b33eca2..36d1ef4e 100644 --- a/mcp-server/src/core/direct-functions/update-tasks.js +++ b/mcp-server/src/core/direct-functions/update-tasks.js @@ -21,7 +21,7 @@ import { */ export async function updateTasksDirect(args, log, context = {}) { const { session } = context; - const { from, prompt, research, tasksJsonPath, projectRoot } = args; + const { from, prompt, research, tasksJsonPath, projectRoot, tag } = args; // Create the standard logger wrapper const logWrapper = createLogWrapper(log); @@ -75,7 +75,8 @@ export async function updateTasksDirect(args, log, context = {}) { { session, mcpLog: logWrapper, - projectRoot + projectRoot, + tag }, 'json' ); diff --git a/mcp-server/src/tools/update.js b/mcp-server/src/tools/update.js index 8d9785ef..475eb338 100644 --- a/mcp-server/src/tools/update.js +++ b/mcp-server/src/tools/update.js @@ -43,11 +43,12 @@ export function registerUpdateTool(server) { .optional() .describe( 'The directory of the project. (Optional, usually from session)' - ) + ), + tag: z.string().optional().describe('Tag context to operate on') }), execute: withNormalizedProjectRoot(async (args, { log, session }) => { const toolName = 'update'; - const { from, prompt, research, file, projectRoot } = args; + const { from, prompt, research, file, projectRoot, tag } = args; try { log.info( @@ -71,7 +72,8 @@ export function registerUpdateTool(server) { from: from, prompt: prompt, research: research, - projectRoot: projectRoot + projectRoot: projectRoot, + tag: tag }, log, { session } diff --git a/scripts/modules/task-manager/update-tasks.js b/scripts/modules/task-manager/update-tasks.js index 48b1d541..528f81e2 100644 --- a/scripts/modules/task-manager/update-tasks.js +++ b/scripts/modules/task-manager/update-tasks.js @@ -9,7 +9,8 @@ import { readJSON, writeJSON, truncate, - isSilentMode + isSilentMode, + getCurrentTag } from '../utils.js'; import { @@ -222,6 +223,7 @@ function parseUpdatedTasksFromText(text, expectedCount, logFn, isMCP) { * @param {Object} [context.session] - Session object from MCP server. * @param {Object} [context.mcpLog] - MCP logger object. * @param {string} [outputFormat='text'] - Output format ('text' or 'json'). + * @param {string} [tag=null] - Tag associated with the tasks. */ async function updateTasks( tasksPath, @@ -231,7 +233,7 @@ async function updateTasks( context = {}, outputFormat = 'text' // Default to text for CLI ) { - const { session, mcpLog, projectRoot: providedProjectRoot } = context; + const { session, mcpLog, projectRoot: providedProjectRoot, tag } = context; // Use mcpLog if available, otherwise use the imported consoleLog function const logFn = mcpLog || consoleLog; // Flag to easily check which logger type we have @@ -255,8 +257,11 @@ async function updateTasks( throw new Error('Could not determine project root directory'); } - // --- Task Loading/Filtering (Unchanged) --- - const data = readJSON(tasksPath, projectRoot); + // Determine the current tag - prioritize explicit tag, then context.tag, then current tag + const currentTag = tag || getCurrentTag(projectRoot) || 'master'; + + // --- Task Loading/Filtering (Updated to pass projectRoot and tag) --- + const data = readJSON(tasksPath, projectRoot, currentTag); if (!data || !data.tasks) throw new Error(`No valid tasks found in ${tasksPath}`); const tasksToUpdate = data.tasks.filter( @@ -428,7 +433,7 @@ The changes described in the prompt should be applied to ALL tasks in the list.` isMCP ); - // --- Update Tasks Data (Unchanged) --- + // --- Update Tasks Data (Updated writeJSON call) --- if (!Array.isArray(parsedUpdatedTasks)) { // Should be caught by parser, but extra check throw new Error( @@ -467,7 +472,8 @@ The changes described in the prompt should be applied to ALL tasks in the list.` `Applied updates to ${actualUpdateCount} tasks in the dataset.` ); - writeJSON(tasksPath, data); + // Fix: Pass projectRoot and currentTag to writeJSON + writeJSON(tasksPath, data, projectRoot, currentTag); if (isMCP) logFn.info( `Successfully updated ${actualUpdateCount} tasks in ${tasksPath}` diff --git a/tests/unit/scripts/modules/task-manager/update-tasks.test.js b/tests/unit/scripts/modules/task-manager/update-tasks.test.js index 3449b239..5dd51cae 100644 --- a/tests/unit/scripts/modules/task-manager/update-tasks.test.js +++ b/tests/unit/scripts/modules/task-manager/update-tasks.test.js @@ -165,7 +165,11 @@ describe('updateTasks', () => { // Assert // 1. Read JSON called - expect(readJSON).toHaveBeenCalledWith(mockTasksPath, '/mock/path'); + expect(readJSON).toHaveBeenCalledWith( + mockTasksPath, + '/mock/path', + 'master' + ); // 2. AI Service called with correct args expect(generateTextService).toHaveBeenCalledWith(expect.any(Object)); @@ -183,7 +187,9 @@ describe('updateTasks', () => { ]) }) }) - }) + }), + '/mock/path', + 'master' ); // 4. Check return value @@ -228,7 +234,11 @@ describe('updateTasks', () => { ); // Assert - expect(readJSON).toHaveBeenCalledWith(mockTasksPath, '/mock/path'); + expect(readJSON).toHaveBeenCalledWith( + mockTasksPath, + '/mock/path', + 'master' + ); expect(generateTextService).not.toHaveBeenCalled(); expect(writeJSON).not.toHaveBeenCalled(); expect(log).toHaveBeenCalledWith( @@ -239,4 +249,113 @@ describe('updateTasks', () => { // Should return early with no updates expect(result).toBeUndefined(); }); + + test('should preserve all tags when updating tasks in tagged context', async () => { + // Arrange - Simple 2-tag structure to test tag corruption fix + const mockTasksPath = '/mock/path/tasks.json'; + const mockFromId = 1; + const mockPrompt = 'Update master tag tasks'; + + const mockTaggedData = { + master: { + tasks: [ + { + id: 1, + title: 'Master Task', + status: 'pending', + details: 'Old details' + }, + { + id: 2, + title: 'Master Task 2', + status: 'done', + details: 'Done task' + } + ], + metadata: { + created: '2024-01-01T00:00:00.000Z', + description: 'Master tag tasks' + } + }, + 'feature-branch': { + tasks: [ + { + id: 1, + title: 'Feature Task', + status: 'pending', + details: 'Feature work' + } + ], + metadata: { + created: '2024-01-02T00:00:00.000Z', + description: 'Feature branch tasks' + } + } + }; + + const mockUpdatedTasks = [ + { + id: 1, + title: 'Updated Master Task', + status: 'pending', + details: 'Updated details', + description: 'Updated description', + dependencies: [], + priority: 'medium', + testStrategy: 'Test the updated functionality', + subtasks: [] + } + ]; + + // Configure mocks - readJSON returns resolved view for master tag + readJSON.mockReturnValue({ + ...mockTaggedData.master, + tag: 'master', + _rawTaggedData: mockTaggedData + }); + + generateTextService.mockResolvedValue({ + mainResult: JSON.stringify(mockUpdatedTasks), + telemetryData: { commandName: 'update-tasks', totalCost: 0.05 } + }); + + // Act + const result = await updateTasks( + mockTasksPath, + mockFromId, + mockPrompt, + false, // research + { projectRoot: '/mock/project/root', tag: 'master' }, + 'json' + ); + + // Assert - CRITICAL: Both tags must be preserved (this would fail before the fix) + expect(writeJSON).toHaveBeenCalledWith( + mockTasksPath, + expect.objectContaining({ + _rawTaggedData: expect.objectContaining({ + master: expect.objectContaining({ + tasks: expect.arrayContaining([ + expect.objectContaining({ id: 1, title: 'Updated Master Task' }), + expect.objectContaining({ id: 2, title: 'Master Task 2' }) // Unchanged done task + ]) + }), + // CRITICAL: This tag would be missing/corrupted if the bug existed + 'feature-branch': expect.objectContaining({ + tasks: expect.arrayContaining([ + expect.objectContaining({ id: 1, title: 'Feature Task' }) + ]), + metadata: expect.objectContaining({ + description: 'Feature branch tasks' + }) + }) + }) + }), + '/mock/project/root', + 'master' + ); + + expect(result.success).toBe(true); + expect(result.updatedTasks).toEqual(mockUpdatedTasks); + }); });