feat: adds ability to add or remove subtasks. Can also turn subtasks into standalone features. Also refactors the task-master.js by deleting 200+ lines of duplicate code. Instead properly imports the commands from commands.js which is the single source of truth for command definitions.

This commit is contained in:
Eyal Toledano
2025-03-24 21:18:49 -04:00
parent 50cbe5419e
commit a26e7adf8c
17 changed files with 1670 additions and 503 deletions

View File

@@ -19,6 +19,7 @@ const mockFormatDependenciesWithStatus = jest.fn();
const mockValidateAndFixDependencies = jest.fn();
const mockReadJSON = jest.fn();
const mockLog = jest.fn();
const mockIsTaskDependentOn = jest.fn().mockReturnValue(false);
// Mock fs module
jest.mock('fs', () => ({
@@ -66,79 +67,8 @@ jest.mock('../../scripts/modules/task-manager.js', () => {
// Return a modified module with our custom implementation of generateTaskFiles
return {
...originalModule,
generateTaskFiles: (tasksPath, outputDir) => {
try {
const data = mockReadJSON(tasksPath);
if (!data || !data.tasks) {
throw new Error(`No valid tasks found in ${tasksPath}`);
}
// Create the output directory if it doesn't exist
if (!mockExistsSync(outputDir)) {
mockMkdirSync(outputDir, { recursive: true });
}
// Validate and fix dependencies before generating files
mockValidateAndFixDependencies(data, tasksPath);
// Generate task files
data.tasks.forEach(task => {
const taskPath = `${outputDir}/task_${task.id.toString().padStart(3, '0')}.txt`;
// Format the content
let content = `# Task ID: ${task.id}\n`;
content += `# Title: ${task.title}\n`;
content += `# Status: ${task.status || 'pending'}\n`;
// Format dependencies with their status
if (task.dependencies && task.dependencies.length > 0) {
content += `# Dependencies: ${mockFormatDependenciesWithStatus(task.dependencies, data.tasks, false)}\n`;
} else {
content += '# Dependencies: None\n';
}
content += `# Priority: ${task.priority || 'medium'}\n`;
content += `# Description: ${task.description || ''}\n`;
// Add more detailed sections
content += '# Details:\n';
content += (task.details || '').split('\n').map(line => line).join('\n');
content += '\n\n';
content += '# Test Strategy:\n';
content += (task.testStrategy || '').split('\n').map(line => line).join('\n');
content += '\n';
// Add subtasks if they exist
if (task.subtasks && task.subtasks.length > 0) {
content += '\n# Subtasks:\n';
task.subtasks.forEach(subtask => {
content += `## ${subtask.id}. ${subtask.title} [${subtask.status || 'pending'}]\n`;
if (subtask.dependencies && subtask.dependencies.length > 0) {
const subtaskDeps = subtask.dependencies.join(', ');
content += `### Dependencies: ${subtaskDeps}\n`;
} else {
content += '### Dependencies: None\n';
}
content += `### Description: ${subtask.description || ''}\n`;
content += '### Details:\n';
content += (subtask.details || '').split('\n').map(line => line).join('\n');
content += '\n\n';
});
}
// Write the file
mockWriteFileSync(taskPath, content);
});
} catch (error) {
mockLog('error', `Error generating task files: ${error.message}`);
console.error(`Error generating task files: ${error.message}`);
process.exit(1);
}
}
generateTaskFiles: mockGenerateTaskFiles,
isTaskDependentOn: mockIsTaskDependentOn
};
});
@@ -166,9 +96,10 @@ const testParsePRD = async (prdPath, outputPath, numTasks) => {
// Import after mocks
import * as taskManager from '../../scripts/modules/task-manager.js';
import { sampleClaudeResponse } from '../fixtures/sample-claude-response.js';
import { sampleTasks, emptySampleTasks } from '../fixtures/sample-tasks.js';
// Destructure the required functions for convenience
const { findNextTask, generateTaskFiles } = taskManager;
const { findNextTask, generateTaskFiles, clearSubtasks } = taskManager;
describe('Task Manager Module', () => {
beforeEach(() => {
@@ -833,42 +764,123 @@ describe('Task Manager Module', () => {
});
});
describe.skip('clearSubtasks function', () => {
describe('clearSubtasks function', () => {
beforeEach(() => {
jest.clearAllMocks();
});
// Test implementation of clearSubtasks that just returns the updated data
const testClearSubtasks = (tasksData, taskIds) => {
// Create a deep copy of the data to avoid modifying the original
const data = JSON.parse(JSON.stringify(tasksData));
let clearedCount = 0;
// Handle multiple task IDs (comma-separated)
const taskIdArray = taskIds.split(',').map(id => id.trim());
taskIdArray.forEach(taskId => {
const id = parseInt(taskId, 10);
if (isNaN(id)) {
return;
}
const task = data.tasks.find(t => t.id === id);
if (!task) {
// Log error for non-existent task
mockLog('error', `Task ${id} not found`);
return;
}
if (!task.subtasks || task.subtasks.length === 0) {
// No subtasks to clear
return;
}
const subtaskCount = task.subtasks.length;
delete task.subtasks;
clearedCount++;
});
return { data, clearedCount };
};
test('should clear subtasks from a specific task', () => {
// This test would verify that:
// 1. The function reads the tasks file correctly
// 2. It finds the target task by ID
// 3. It clears the subtasks array
// 4. It writes the updated tasks back to the file
expect(true).toBe(true);
// Create a deep copy of the sample data
const testData = JSON.parse(JSON.stringify(sampleTasks));
// Execute the test function
const { data, clearedCount } = testClearSubtasks(testData, '3');
// Verify results
expect(clearedCount).toBe(1);
// Verify the task's subtasks were removed
const task = data.tasks.find(t => t.id === 3);
expect(task).toBeDefined();
expect(task.subtasks).toBeUndefined();
});
test('should clear subtasks from multiple tasks when given comma-separated IDs', () => {
// This test would verify that:
// 1. The function handles comma-separated task IDs
// 2. It clears subtasks from all specified tasks
expect(true).toBe(true);
// Setup data with subtasks on multiple tasks
const testData = JSON.parse(JSON.stringify(sampleTasks));
// Add subtasks to task 2
testData.tasks[1].subtasks = [
{
id: 1,
title: "Test Subtask",
description: "A test subtask",
status: "pending",
dependencies: []
}
];
// Execute the test function
const { data, clearedCount } = testClearSubtasks(testData, '2,3');
// Verify results
expect(clearedCount).toBe(2);
// Verify both tasks had their subtasks cleared
const task2 = data.tasks.find(t => t.id === 2);
const task3 = data.tasks.find(t => t.id === 3);
expect(task2.subtasks).toBeUndefined();
expect(task3.subtasks).toBeUndefined();
});
test('should handle tasks with no subtasks', () => {
// This test would verify that:
// 1. The function handles tasks without subtasks gracefully
// 2. It provides appropriate feedback
expect(true).toBe(true);
// Task 1 has no subtasks in the sample data
const testData = JSON.parse(JSON.stringify(sampleTasks));
// Execute the test function
const { clearedCount } = testClearSubtasks(testData, '1');
// Verify no tasks were cleared
expect(clearedCount).toBe(0);
});
test('should handle non-existent task IDs', () => {
// This test would verify that:
// 1. The function handles non-existent task IDs gracefully
// 2. It logs appropriate error messages
expect(true).toBe(true);
const testData = JSON.parse(JSON.stringify(sampleTasks));
// Execute the test function
testClearSubtasks(testData, '99');
// Verify an error was logged
expect(mockLog).toHaveBeenCalledWith('error', expect.stringContaining('Task 99 not found'));
});
test('should regenerate task files after clearing subtasks', () => {
// This test would verify that:
// 1. The function regenerates task files after clearing subtasks
// 2. The new files reflect the changes
expect(true).toBe(true);
test('should handle multiple task IDs including both valid and non-existent IDs', () => {
const testData = JSON.parse(JSON.stringify(sampleTasks));
// Execute the test function
const { data, clearedCount } = testClearSubtasks(testData, '3,99');
// Verify results
expect(clearedCount).toBe(1);
expect(mockLog).toHaveBeenCalledWith('error', expect.stringContaining('Task 99 not found'));
// Verify the valid task's subtasks were removed
const task3 = data.tasks.find(t => t.id === 3);
expect(task3.subtasks).toBeUndefined();
});
});
@@ -916,4 +928,525 @@ describe('Task Manager Module', () => {
expect(true).toBe(true);
});
});
});
// Add test suite for addSubtask function
describe('addSubtask function', () => {
// Reset mocks before each test
beforeEach(() => {
jest.clearAllMocks();
// 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;
});
// Set up default behavior for dependency check
mockIsTaskDependentOn.mockReturnValue(false);
});
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, null, newSubtaskData, true);
// 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(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, null, true);
// 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();
});
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'
};
// Override mockReadJSON for this specific test case
mockReadJSON.mockImplementationOnce(() => ({
tasks: [
{
id: 1,
title: 'Task 1',
status: 'pending'
}
]
}));
// 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/);
// 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/);
// 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();
});
});
// Test suite for removeSubtask function
describe('removeSubtask function', () => {
// Reset mocks before each test
beforeEach(() => {
jest.clearAllMocks();
// Default mock implementations
mockReadJSON.mockImplementation(() => ({
tasks: [
{
id: 1,
title: 'Parent Task',
description: 'This is a parent task',
status: 'pending',
dependencies: [],
subtasks: [
{
id: 1,
title: 'Subtask 1',
description: 'This is subtask 1',
status: 'pending',
dependencies: [],
parentTaskId: 1
},
{
id: 2,
title: 'Subtask 2',
description: 'This is subtask 2',
status: 'in-progress',
dependencies: [1], // Depends on subtask 1
parentTaskId: 1
}
]
},
{
id: 2,
title: 'Another Task',
description: 'This is another task',
status: 'pending',
dependencies: [1]
}
]
}));
// Setup success write response
mockWriteJSON.mockImplementation((path, data) => {
return data;
});
});
test('should remove a subtask from its parent task', async () => {
// Execute the test version of removeSubtask to remove subtask 1.1
testRemoveSubtask('tasks/tasks.json', '1.1', false, true);
// Verify readJSON was called with the correct path
expect(mockReadJSON).toHaveBeenCalledWith('tasks/tasks.json');
// Verify writeJSON was called with updated data
expect(mockWriteJSON).toHaveBeenCalled();
// Verify generateTaskFiles was called
expect(mockGenerateTaskFiles).toHaveBeenCalled();
});
test('should convert a subtask to a standalone task', async () => {
// Execute the test version of removeSubtask to convert subtask 1.1 to a standalone task
const result = testRemoveSubtask('tasks/tasks.json', '1.1', true, true);
// Verify the result is the new task
expect(result).toBeDefined();
expect(result.id).toBe(3);
expect(result.title).toBe('Subtask 1');
expect(result.dependencies).toContain(1);
// Verify writeJSON was called
expect(mockWriteJSON).toHaveBeenCalled();
// Verify generateTaskFiles was called
expect(mockGenerateTaskFiles).toHaveBeenCalled();
});
test('should throw an error if subtask ID format is invalid', async () => {
// Expect an error for invalid subtask ID format
expect(() =>
testRemoveSubtask('tasks/tasks.json', '1', false)
).toThrow(/Invalid subtask ID format/);
// Verify writeJSON was not called
expect(mockWriteJSON).not.toHaveBeenCalled();
});
test('should throw an error if parent task does not exist', async () => {
// Expect an error for non-existent parent task
expect(() =>
testRemoveSubtask('tasks/tasks.json', '999.1', false)
).toThrow(/Parent task with ID 999 not found/);
// Verify writeJSON was not called
expect(mockWriteJSON).not.toHaveBeenCalled();
});
test('should throw an error if subtask does not exist', async () => {
// Expect an error for non-existent subtask
expect(() =>
testRemoveSubtask('tasks/tasks.json', '1.999', false)
).toThrow(/Subtask 1.999 not found/);
// Verify writeJSON was not called
expect(mockWriteJSON).not.toHaveBeenCalled();
});
test('should remove subtasks array if last subtask is removed', async () => {
// Create a data object with just one subtask
mockReadJSON.mockImplementationOnce(() => ({
tasks: [
{
id: 1,
title: 'Parent Task',
description: 'This is a parent task',
status: 'pending',
dependencies: [],
subtasks: [
{
id: 1,
title: 'Last Subtask',
description: 'This is the last subtask',
status: 'pending',
dependencies: [],
parentTaskId: 1
}
]
},
{
id: 2,
title: 'Another Task',
description: 'This is another task',
status: 'pending',
dependencies: [1]
}
]
}));
// Mock the behavior of writeJSON to capture the updated tasks data
const updatedTasksData = { tasks: [] };
mockWriteJSON.mockImplementation((path, data) => {
// Store the data for assertions
updatedTasksData.tasks = [...data.tasks];
return data;
});
// Remove the last subtask
testRemoveSubtask('tasks/tasks.json', '1.1', false, true);
// Verify writeJSON was called
expect(mockWriteJSON).toHaveBeenCalled();
// Verify the subtasks array was removed completely
const parentTask = updatedTasksData.tasks.find(t => t.id === 1);
expect(parentTask).toBeDefined();
expect(parentTask.subtasks).toBeUndefined();
// Verify generateTaskFiles was called
expect(mockGenerateTaskFiles).toHaveBeenCalled();
});
test('should not regenerate task files if generateFiles is false', async () => {
// Execute the test version of removeSubtask with generateFiles = false
testRemoveSubtask('tasks/tasks.json', '1.1', false, false);
// Verify writeJSON was called
expect(mockWriteJSON).toHaveBeenCalled();
// Verify task files were not regenerated
expect(mockGenerateTaskFiles).not.toHaveBeenCalled();
});
});
});
// Define test versions of the addSubtask and removeSubtask functions
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;
};
const testRemoveSubtask = (tasksPath, subtaskId, convertToTask = false, generateFiles = true) => {
// Read the existing tasks
const data = mockReadJSON(tasksPath);
if (!data || !data.tasks) {
throw new Error(`Invalid or missing tasks file at ${tasksPath}`);
}
// Parse the subtask ID (format: "parentId.subtaskId")
if (!subtaskId.includes('.')) {
throw new Error(`Invalid subtask ID format: ${subtaskId}. Expected format: "parentId.subtaskId"`);
}
const [parentIdStr, subtaskIdStr] = subtaskId.split('.');
const parentId = parseInt(parentIdStr, 10);
const subtaskIdNum = parseInt(subtaskIdStr, 10);
// Find the parent task
const parentTask = data.tasks.find(t => t.id === parentId);
if (!parentTask) {
throw new Error(`Parent task with ID ${parentId} not found`);
}
// Check if parent has subtasks
if (!parentTask.subtasks || parentTask.subtasks.length === 0) {
throw new Error(`Parent task ${parentId} has no subtasks`);
}
// Find the subtask to remove
const subtaskIndex = parentTask.subtasks.findIndex(st => st.id === subtaskIdNum);
if (subtaskIndex === -1) {
throw new Error(`Subtask ${subtaskId} not found`);
}
// Get a copy of the subtask before removing it
const removedSubtask = { ...parentTask.subtasks[subtaskIndex] };
// Remove the subtask from the parent
parentTask.subtasks.splice(subtaskIndex, 1);
// If parent has no more subtasks, remove the subtasks array
if (parentTask.subtasks.length === 0) {
delete parentTask.subtasks;
}
let convertedTask = null;
// Convert the subtask to a standalone task if requested
if (convertToTask) {
// Find the highest task ID to determine the next ID
const highestId = Math.max(...data.tasks.map(t => t.id));
const newTaskId = highestId + 1;
// Create the new task from the subtask
convertedTask = {
id: newTaskId,
title: removedSubtask.title,
description: removedSubtask.description || '',
details: removedSubtask.details || '',
status: removedSubtask.status || 'pending',
dependencies: removedSubtask.dependencies || [],
priority: parentTask.priority || 'medium' // Inherit priority from parent
};
// Add the parent task as a dependency if not already present
if (!convertedTask.dependencies.includes(parentId)) {
convertedTask.dependencies.push(parentId);
}
// Add the converted task to the tasks array
data.tasks.push(convertedTask);
}
// Write the updated tasks back to the file
mockWriteJSON(tasksPath, data);
// Generate task files if requested
if (generateFiles) {
mockGenerateTaskFiles(tasksPath, path.dirname(tasksPath));
}
return convertedTask;
};