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
This commit is contained in:
5
.changeset/chubby-needles-beam.md
Normal file
5
.changeset/chubby-needles-beam.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"task-master-ai": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix bulk update tag corruption in tagged task lists
|
||||||
@@ -21,7 +21,7 @@ import {
|
|||||||
*/
|
*/
|
||||||
export async function updateTasksDirect(args, log, context = {}) {
|
export async function updateTasksDirect(args, log, context = {}) {
|
||||||
const { session } = context;
|
const { session } = context;
|
||||||
const { from, prompt, research, tasksJsonPath, projectRoot } = args;
|
const { from, prompt, research, tasksJsonPath, projectRoot, tag } = args;
|
||||||
|
|
||||||
// Create the standard logger wrapper
|
// Create the standard logger wrapper
|
||||||
const logWrapper = createLogWrapper(log);
|
const logWrapper = createLogWrapper(log);
|
||||||
@@ -75,7 +75,8 @@ export async function updateTasksDirect(args, log, context = {}) {
|
|||||||
{
|
{
|
||||||
session,
|
session,
|
||||||
mcpLog: logWrapper,
|
mcpLog: logWrapper,
|
||||||
projectRoot
|
projectRoot,
|
||||||
|
tag
|
||||||
},
|
},
|
||||||
'json'
|
'json'
|
||||||
);
|
);
|
||||||
|
|||||||
@@ -43,11 +43,12 @@ export function registerUpdateTool(server) {
|
|||||||
.optional()
|
.optional()
|
||||||
.describe(
|
.describe(
|
||||||
'The directory of the project. (Optional, usually from session)'
|
'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 }) => {
|
execute: withNormalizedProjectRoot(async (args, { log, session }) => {
|
||||||
const toolName = 'update';
|
const toolName = 'update';
|
||||||
const { from, prompt, research, file, projectRoot } = args;
|
const { from, prompt, research, file, projectRoot, tag } = args;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
log.info(
|
log.info(
|
||||||
@@ -71,7 +72,8 @@ export function registerUpdateTool(server) {
|
|||||||
from: from,
|
from: from,
|
||||||
prompt: prompt,
|
prompt: prompt,
|
||||||
research: research,
|
research: research,
|
||||||
projectRoot: projectRoot
|
projectRoot: projectRoot,
|
||||||
|
tag: tag
|
||||||
},
|
},
|
||||||
log,
|
log,
|
||||||
{ session }
|
{ session }
|
||||||
|
|||||||
@@ -9,7 +9,8 @@ import {
|
|||||||
readJSON,
|
readJSON,
|
||||||
writeJSON,
|
writeJSON,
|
||||||
truncate,
|
truncate,
|
||||||
isSilentMode
|
isSilentMode,
|
||||||
|
getCurrentTag
|
||||||
} from '../utils.js';
|
} from '../utils.js';
|
||||||
|
|
||||||
import {
|
import {
|
||||||
@@ -222,6 +223,7 @@ function parseUpdatedTasksFromText(text, expectedCount, logFn, isMCP) {
|
|||||||
* @param {Object} [context.session] - Session object from MCP server.
|
* @param {Object} [context.session] - Session object from MCP server.
|
||||||
* @param {Object} [context.mcpLog] - MCP logger object.
|
* @param {Object} [context.mcpLog] - MCP logger object.
|
||||||
* @param {string} [outputFormat='text'] - Output format ('text' or 'json').
|
* @param {string} [outputFormat='text'] - Output format ('text' or 'json').
|
||||||
|
* @param {string} [tag=null] - Tag associated with the tasks.
|
||||||
*/
|
*/
|
||||||
async function updateTasks(
|
async function updateTasks(
|
||||||
tasksPath,
|
tasksPath,
|
||||||
@@ -231,7 +233,7 @@ async function updateTasks(
|
|||||||
context = {},
|
context = {},
|
||||||
outputFormat = 'text' // Default to text for CLI
|
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
|
// Use mcpLog if available, otherwise use the imported consoleLog function
|
||||||
const logFn = mcpLog || consoleLog;
|
const logFn = mcpLog || consoleLog;
|
||||||
// Flag to easily check which logger type we have
|
// 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');
|
throw new Error('Could not determine project root directory');
|
||||||
}
|
}
|
||||||
|
|
||||||
// --- Task Loading/Filtering (Unchanged) ---
|
// Determine the current tag - prioritize explicit tag, then context.tag, then current tag
|
||||||
const data = readJSON(tasksPath, projectRoot);
|
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)
|
if (!data || !data.tasks)
|
||||||
throw new Error(`No valid tasks found in ${tasksPath}`);
|
throw new Error(`No valid tasks found in ${tasksPath}`);
|
||||||
const tasksToUpdate = data.tasks.filter(
|
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
|
isMCP
|
||||||
);
|
);
|
||||||
|
|
||||||
// --- Update Tasks Data (Unchanged) ---
|
// --- Update Tasks Data (Updated writeJSON call) ---
|
||||||
if (!Array.isArray(parsedUpdatedTasks)) {
|
if (!Array.isArray(parsedUpdatedTasks)) {
|
||||||
// Should be caught by parser, but extra check
|
// Should be caught by parser, but extra check
|
||||||
throw new Error(
|
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.`
|
`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)
|
if (isMCP)
|
||||||
logFn.info(
|
logFn.info(
|
||||||
`Successfully updated ${actualUpdateCount} tasks in ${tasksPath}`
|
`Successfully updated ${actualUpdateCount} tasks in ${tasksPath}`
|
||||||
|
|||||||
@@ -165,7 +165,11 @@ describe('updateTasks', () => {
|
|||||||
|
|
||||||
// Assert
|
// Assert
|
||||||
// 1. Read JSON called
|
// 1. Read JSON called
|
||||||
expect(readJSON).toHaveBeenCalledWith(mockTasksPath, '/mock/path');
|
expect(readJSON).toHaveBeenCalledWith(
|
||||||
|
mockTasksPath,
|
||||||
|
'/mock/path',
|
||||||
|
'master'
|
||||||
|
);
|
||||||
|
|
||||||
// 2. AI Service called with correct args
|
// 2. AI Service called with correct args
|
||||||
expect(generateTextService).toHaveBeenCalledWith(expect.any(Object));
|
expect(generateTextService).toHaveBeenCalledWith(expect.any(Object));
|
||||||
@@ -183,7 +187,9 @@ describe('updateTasks', () => {
|
|||||||
])
|
])
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
})
|
}),
|
||||||
|
'/mock/path',
|
||||||
|
'master'
|
||||||
);
|
);
|
||||||
|
|
||||||
// 4. Check return value
|
// 4. Check return value
|
||||||
@@ -228,7 +234,11 @@ describe('updateTasks', () => {
|
|||||||
);
|
);
|
||||||
|
|
||||||
// Assert
|
// Assert
|
||||||
expect(readJSON).toHaveBeenCalledWith(mockTasksPath, '/mock/path');
|
expect(readJSON).toHaveBeenCalledWith(
|
||||||
|
mockTasksPath,
|
||||||
|
'/mock/path',
|
||||||
|
'master'
|
||||||
|
);
|
||||||
expect(generateTextService).not.toHaveBeenCalled();
|
expect(generateTextService).not.toHaveBeenCalled();
|
||||||
expect(writeJSON).not.toHaveBeenCalled();
|
expect(writeJSON).not.toHaveBeenCalled();
|
||||||
expect(log).toHaveBeenCalledWith(
|
expect(log).toHaveBeenCalledWith(
|
||||||
@@ -239,4 +249,113 @@ describe('updateTasks', () => {
|
|||||||
// Should return early with no updates
|
// Should return early with no updates
|
||||||
expect(result).toBeUndefined();
|
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);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user