From 81198d9468db3b1076c6eba5d6ea11b22d73dc3e Mon Sep 17 00:00:00 2001 From: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com> Date: Sat, 19 Apr 2025 23:49:50 +0200 Subject: [PATCH] feat: Add --append flag to parsePRD command - Fixes #207 (#272) * feat: Add --append flag to parsePRD command - Fixes #207 * chore: format * chore: implement tests to core logic and commands * feat: implement MCP for append flag of parse_prd tool * fix: append not considering existing tasks * chore: fix tests --------- Co-authored-by: Kresna Sucandra --- .changeset/cold-bats-fly.md | 5 + .../src/core/direct-functions/parse-prd.js | 23 ++- mcp-server/src/tools/parse-prd.js | 9 +- scripts/modules/commands.js | 24 ++- scripts/modules/task-manager.js | 48 ++++- .../modules/task-manager.js (lines 3036-3084) | 32 ---- tests/fixture/test-tasks.json | 26 +-- tests/unit/commands.test.js | 165 +++++++++++++++++- tests/unit/task-manager.test.js | 129 ++++++++++++-- 9 files changed, 377 insertions(+), 84 deletions(-) create mode 100644 .changeset/cold-bats-fly.md delete mode 100644 scripts/modules/task-manager.js (lines 3036-3084) diff --git a/.changeset/cold-bats-fly.md b/.changeset/cold-bats-fly.md new file mode 100644 index 00000000..b9ceb4b0 --- /dev/null +++ b/.changeset/cold-bats-fly.md @@ -0,0 +1,5 @@ +--- +'task-master-ai': patch +--- + +Enhance the `parsePRD` to include `--append` flag. This flag allows users to append the parsed PRD to an existing file, making it easier to manage multiple PRD files without overwriting existing content. diff --git a/mcp-server/src/core/direct-functions/parse-prd.js b/mcp-server/src/core/direct-functions/parse-prd.js index c3220962..29fdf97a 100644 --- a/mcp-server/src/core/direct-functions/parse-prd.js +++ b/mcp-server/src/core/direct-functions/parse-prd.js @@ -5,9 +5,7 @@ import path from 'path'; import fs from 'fs'; -import os from 'os'; // Import os module for home directory check import { parsePRD } from '../../../../scripts/modules/task-manager.js'; -import { findTasksJsonPath } from '../utils/path-utils.js'; import { enableSilentMode, disableSilentMode @@ -124,8 +122,12 @@ export async function parsePRDDirect(args, log, context = {}) { } } + // Extract the append flag from args + const append = Boolean(args.append) === true; + + // Log key parameters including append flag log.info( - `Preparing to parse PRD from ${inputPath} and output to ${outputPath} with ${numTasks} tasks` + `Preparing to parse PRD from ${inputPath} and output to ${outputPath} with ${numTasks} tasks, append mode: ${append}` ); // Create the logger wrapper for proper logging in the core function @@ -157,7 +159,8 @@ export async function parsePRDDirect(args, log, context = {}) { numTasks, { mcpLog: logWrapper, - session + session, + append }, aiClient, modelConfig @@ -167,16 +170,18 @@ export async function parsePRDDirect(args, log, context = {}) { // to return it to the caller if (fs.existsSync(outputPath)) { const tasksData = JSON.parse(fs.readFileSync(outputPath, 'utf8')); - log.info( - `Successfully parsed PRD and generated ${tasksData.tasks?.length || 0} tasks` - ); + const actionVerb = append ? 'appended' : 'generated'; + const message = `Successfully ${actionVerb} ${tasksData.tasks?.length || 0} tasks from PRD`; + + log.info(message); return { success: true, data: { - message: `Successfully generated ${tasksData.tasks?.length || 0} tasks from PRD`, + message, taskCount: tasksData.tasks?.length || 0, - outputPath + outputPath, + appended: append }, fromCache: false // This operation always modifies state and should never be cached }; diff --git a/mcp-server/src/tools/parse-prd.js b/mcp-server/src/tools/parse-prd.js index 7963f39a..d2892243 100644 --- a/mcp-server/src/tools/parse-prd.js +++ b/mcp-server/src/tools/parse-prd.js @@ -47,6 +47,12 @@ export function registerParsePRDTool(server) { .boolean() .optional() .describe('Allow overwriting an existing tasks.json file.'), + append: z + .boolean() + .optional() + .describe( + 'Append new tasks to existing tasks.json instead of overwriting' + ), projectRoot: z .string() .describe('The directory of the project. Must be absolute path.') @@ -86,7 +92,8 @@ export function registerParsePRDTool(server) { input: prdPath, output: tasksJsonPath, numTasks: args.numTasks, - force: args.force + force: args.force, + append: args.append }, log, { session } diff --git a/scripts/modules/commands.js b/scripts/modules/commands.js index 7a5494f5..c83224c0 100644 --- a/scripts/modules/commands.js +++ b/scripts/modules/commands.js @@ -88,6 +88,10 @@ function registerCommands(programInstance) { .option('-o, --output ', 'Output file path', 'tasks/tasks.json') .option('-n, --num-tasks ', 'Number of tasks to generate', '10') .option('-f, --force', 'Skip confirmation when overwriting existing tasks') + .option( + '--append', + 'Append new tasks to existing tasks.json instead of overwriting' + ) .action(async (file, options) => { // Use input option if file argument not provided const inputFile = file || options.input; @@ -95,10 +99,11 @@ function registerCommands(programInstance) { const numTasks = parseInt(options.numTasks, 10); const outputPath = options.output; const force = options.force || false; + const append = options.append || false; // Helper function to check if tasks.json exists and confirm overwrite async function confirmOverwriteIfNeeded() { - if (fs.existsSync(outputPath) && !force) { + if (fs.existsSync(outputPath) && !force && !append) { const shouldContinue = await confirmTaskOverwrite(outputPath); if (!shouldContinue) { console.log(chalk.yellow('Operation cancelled by user.')); @@ -117,7 +122,7 @@ function registerCommands(programInstance) { if (!(await confirmOverwriteIfNeeded())) return; console.log(chalk.blue(`Generating ${numTasks} tasks...`)); - await parsePRD(defaultPrdPath, outputPath, numTasks); + await parsePRD(defaultPrdPath, outputPath, numTasks, { append }); return; } @@ -138,17 +143,21 @@ function registerCommands(programInstance) { ' -i, --input Path to the PRD file (alternative to positional argument)\n' + ' -o, --output Output file path (default: "tasks/tasks.json")\n' + ' -n, --num-tasks Number of tasks to generate (default: 10)\n' + - ' -f, --force Skip confirmation when overwriting existing tasks\n\n' + + ' -f, --force Skip confirmation when overwriting existing tasks\n' + + ' --append Append new tasks to existing tasks.json instead of overwriting\n\n' + chalk.cyan('Example:') + '\n' + ' task-master parse-prd requirements.txt --num-tasks 15\n' + ' task-master parse-prd --input=requirements.txt\n' + - ' task-master parse-prd --force\n\n' + + ' task-master parse-prd --force\n' + + ' task-master parse-prd requirements_v2.txt --append\n\n' + chalk.yellow('Note: This command will:') + '\n' + ' 1. Look for a PRD file at scripts/prd.txt by default\n' + ' 2. Use the file specified by --input or positional argument if provided\n' + - ' 3. Generate tasks from the PRD and overwrite any existing tasks.json file', + ' 3. Generate tasks from the PRD and either:\n' + + ' - Overwrite any existing tasks.json file (default)\n' + + ' - Append to existing tasks.json if --append is used', { padding: 1, borderColor: 'blue', borderStyle: 'round' } ) ); @@ -160,8 +169,11 @@ function registerCommands(programInstance) { console.log(chalk.blue(`Parsing PRD file: ${inputFile}`)); console.log(chalk.blue(`Generating ${numTasks} tasks...`)); + if (append) { + console.log(chalk.blue('Appending to existing tasks...')); + } - await parsePRD(inputFile, outputPath, numTasks); + await parsePRD(inputFile, outputPath, numTasks, { append }); }); // update command diff --git a/scripts/modules/task-manager.js b/scripts/modules/task-manager.js index 741c244b..2e291ec9 100644 --- a/scripts/modules/task-manager.js +++ b/scripts/modules/task-manager.js @@ -106,7 +106,7 @@ async function parsePRD( aiClient = null, modelConfig = null ) { - const { reportProgress, mcpLog, session } = options; + const { reportProgress, mcpLog, session, append } = options; // Determine output format based on mcpLog presence (simplification) const outputFormat = mcpLog ? 'json' : 'text'; @@ -127,8 +127,30 @@ async function parsePRD( // Read the PRD content const prdContent = fs.readFileSync(prdPath, 'utf8'); + // If appending and tasks.json exists, read existing tasks first + let existingTasks = { tasks: [] }; + let lastTaskId = 0; + if (append && fs.existsSync(tasksPath)) { + try { + existingTasks = readJSON(tasksPath); + if (existingTasks.tasks?.length) { + // Find the highest task ID + lastTaskId = existingTasks.tasks.reduce((maxId, task) => { + const mainId = parseInt(task.id.toString().split('.')[0], 10) || 0; + return Math.max(maxId, mainId); + }, 0); + } + } catch (error) { + report( + `Warning: Could not read existing tasks file: ${error.message}`, + 'warn' + ); + existingTasks = { tasks: [] }; + } + } + // Call Claude to generate tasks, passing the provided AI client if available - const tasksData = await callClaude( + const newTasksData = await callClaude( prdContent, prdPath, numTasks, @@ -138,15 +160,33 @@ async function parsePRD( modelConfig ); + // Update task IDs if appending + if (append && lastTaskId > 0) { + report(`Updating task IDs to continue from ID ${lastTaskId}`, 'info'); + newTasksData.tasks.forEach((task, index) => { + task.id = lastTaskId + index + 1; + }); + } + + // Merge tasks if appending + const tasksData = append + ? { + ...existingTasks, + tasks: [...existingTasks.tasks, ...newTasksData.tasks] + } + : newTasksData; + // Create the directory if it doesn't exist const tasksDir = path.dirname(tasksPath); if (!fs.existsSync(tasksDir)) { fs.mkdirSync(tasksDir, { recursive: true }); } + // Write the tasks to the file writeJSON(tasksPath, tasksData); + const actionVerb = append ? 'appended' : 'generated'; report( - `Successfully generated ${tasksData.tasks.length} tasks from PRD`, + `Successfully ${actionVerb} ${newTasksData.tasks.length} tasks from PRD`, 'success' ); report(`Tasks saved to: ${tasksPath}`, 'info'); @@ -166,7 +206,7 @@ async function parsePRD( console.log( boxen( chalk.green( - `Successfully generated ${tasksData.tasks.length} tasks from PRD` + `Successfully ${actionVerb} ${newTasksData.tasks.length} tasks from PRD` ), { padding: 1, borderColor: 'green', borderStyle: 'round' } ) diff --git a/scripts/modules/task-manager.js (lines 3036-3084) b/scripts/modules/task-manager.js (lines 3036-3084) deleted file mode 100644 index b9b90bb2..00000000 --- a/scripts/modules/task-manager.js (lines 3036-3084) +++ /dev/null @@ -1,32 +0,0 @@ -async function updateSubtaskById(tasksPath, subtaskId, prompt, useResearch = false) { - let loadingIndicator = null; - try { - log('info', `Updating subtask ${subtaskId} with prompt: "${prompt}"`); - - // Validate subtask ID format - if (!subtaskId || typeof subtaskId !== 'string' || !subtaskId.includes('.')) { - throw new Error(`Invalid subtask ID format: ${subtaskId}. Subtask ID must be in format "parentId.subtaskId"`); - } - - // Validate prompt - if (!prompt || typeof prompt !== 'string' || prompt.trim() === '') { - throw new Error('Prompt cannot be empty. Please provide context for the subtask update.'); - } - - // Prepare for fallback handling - let claudeOverloaded = false; - - // Validate tasks file exists - if (!fs.existsSync(tasksPath)) { - throw new Error(`Tasks file not found at path: ${tasksPath}`); - } - - // Read the tasks file - const data = readJSON(tasksPath); - // ... rest of the function - } catch (error) { - // Handle errors - console.error(`Error updating subtask: ${error.message}`); - throw error; - } -} \ No newline at end of file diff --git a/tests/fixture/test-tasks.json b/tests/fixture/test-tasks.json index a1ef13d7..6b99c177 100644 --- a/tests/fixture/test-tasks.json +++ b/tests/fixture/test-tasks.json @@ -1,14 +1,14 @@ { - "tasks": [ - { - "id": 1, - "dependencies": [], - "subtasks": [ - { - "id": 1, - "dependencies": [] - } - ] - } - ] -} + "tasks": [ + { + "id": 1, + "dependencies": [], + "subtasks": [ + { + "id": 1, + "dependencies": [] + } + ] + } + ] +} \ No newline at end of file diff --git a/tests/unit/commands.test.js b/tests/unit/commands.test.js index 54ed9200..da0f9111 100644 --- a/tests/unit/commands.test.js +++ b/tests/unit/commands.test.js @@ -199,16 +199,35 @@ describe('Commands Module', () => { // Use input option if file argument not provided const inputFile = file || options.input; const defaultPrdPath = 'scripts/prd.txt'; + const append = options.append || false; + const force = options.force || false; + const outputPath = options.output || 'tasks/tasks.json'; + + // Mock confirmOverwriteIfNeeded function to test overwrite behavior + const mockConfirmOverwrite = jest.fn().mockResolvedValue(true); + + // Helper function to check if tasks.json exists and confirm overwrite + async function confirmOverwriteIfNeeded() { + if (fs.existsSync(outputPath) && !force && !append) { + return mockConfirmOverwrite(); + } + return true; + } // If no input file specified, check for default PRD location if (!inputFile) { if (fs.existsSync(defaultPrdPath)) { console.log(chalk.blue(`Using default PRD file: ${defaultPrdPath}`)); const numTasks = parseInt(options.numTasks, 10); - const outputPath = options.output; + + // Check if we need to confirm overwrite + if (!(await confirmOverwriteIfNeeded())) return; console.log(chalk.blue(`Generating ${numTasks} tasks...`)); - await mockParsePRD(defaultPrdPath, outputPath, numTasks); + if (append) { + console.log(chalk.blue('Appending to existing tasks...')); + } + await mockParsePRD(defaultPrdPath, outputPath, numTasks, { append }); return; } @@ -221,12 +240,20 @@ describe('Commands Module', () => { } const numTasks = parseInt(options.numTasks, 10); - const outputPath = options.output; + + // Check if we need to confirm overwrite + if (!(await confirmOverwriteIfNeeded())) return; console.log(chalk.blue(`Parsing PRD file: ${inputFile}`)); console.log(chalk.blue(`Generating ${numTasks} tasks...`)); + if (append) { + console.log(chalk.blue('Appending to existing tasks...')); + } - await mockParsePRD(inputFile, outputPath, numTasks); + await mockParsePRD(inputFile, outputPath, numTasks, { append }); + + // Return mock for testing + return { mockConfirmOverwrite }; } beforeEach(() => { @@ -252,7 +279,8 @@ describe('Commands Module', () => { expect(mockParsePRD).toHaveBeenCalledWith( 'scripts/prd.txt', 'tasks/tasks.json', - 10 // Default value from command definition + 10, // Default value from command definition + { append: false } ); }); @@ -290,7 +318,8 @@ describe('Commands Module', () => { expect(mockParsePRD).toHaveBeenCalledWith( testFile, 'tasks/tasks.json', - 10 + 10, + { append: false } ); expect(mockExistsSync).not.toHaveBeenCalledWith('scripts/prd.txt'); }); @@ -313,7 +342,8 @@ describe('Commands Module', () => { expect(mockParsePRD).toHaveBeenCalledWith( testFile, 'tasks/tasks.json', - 10 + 10, + { append: false } ); expect(mockExistsSync).not.toHaveBeenCalledWith('scripts/prd.txt'); }); @@ -331,7 +361,126 @@ describe('Commands Module', () => { }); // Assert - expect(mockParsePRD).toHaveBeenCalledWith(testFile, outputFile, numTasks); + expect(mockParsePRD).toHaveBeenCalledWith( + testFile, + outputFile, + numTasks, + { append: false } + ); + }); + + test('should pass append flag to parsePRD when provided', async () => { + // Arrange + const testFile = 'test/prd.txt'; + + // Act - call the handler directly with append flag + await parsePrdAction(testFile, { + numTasks: '10', + output: 'tasks/tasks.json', + append: true + }); + + // Assert + expect(mockConsoleLog).toHaveBeenCalledWith( + expect.stringContaining('Appending to existing tasks') + ); + expect(mockParsePRD).toHaveBeenCalledWith( + testFile, + 'tasks/tasks.json', + 10, + { append: true } + ); + }); + + test('should bypass confirmation when append flag is true and tasks.json exists', async () => { + // Arrange + const testFile = 'test/prd.txt'; + const outputFile = 'tasks/tasks.json'; + + // Mock that tasks.json exists + mockExistsSync.mockImplementation((path) => { + if (path === outputFile) return true; + if (path === testFile) return true; + return false; + }); + + // Act - call the handler with append flag + const { mockConfirmOverwrite } = + (await parsePrdAction(testFile, { + numTasks: '10', + output: outputFile, + append: true + })) || {}; + + // Assert - confirm overwrite should not be called with append flag + expect(mockConfirmOverwrite).not.toHaveBeenCalled(); + expect(mockParsePRD).toHaveBeenCalledWith(testFile, outputFile, 10, { + append: true + }); + + // Reset mock implementation + mockExistsSync.mockReset(); + }); + + test('should prompt for confirmation when append flag is false and tasks.json exists', async () => { + // Arrange + const testFile = 'test/prd.txt'; + const outputFile = 'tasks/tasks.json'; + + // Mock that tasks.json exists + mockExistsSync.mockImplementation((path) => { + if (path === outputFile) return true; + if (path === testFile) return true; + return false; + }); + + // Act - call the handler without append flag + const { mockConfirmOverwrite } = + (await parsePrdAction(testFile, { + numTasks: '10', + output: outputFile + // append: false (default) + })) || {}; + + // Assert - confirm overwrite should be called without append flag + expect(mockConfirmOverwrite).toHaveBeenCalled(); + expect(mockParsePRD).toHaveBeenCalledWith(testFile, outputFile, 10, { + append: false + }); + + // Reset mock implementation + mockExistsSync.mockReset(); + }); + + test('should bypass confirmation when force flag is true, regardless of append flag', async () => { + // Arrange + const testFile = 'test/prd.txt'; + const outputFile = 'tasks/tasks.json'; + + // Mock that tasks.json exists + mockExistsSync.mockImplementation((path) => { + if (path === outputFile) return true; + if (path === testFile) return true; + return false; + }); + + // Act - call the handler with force flag + const { mockConfirmOverwrite } = + (await parsePrdAction(testFile, { + numTasks: '10', + output: outputFile, + force: true, + append: false + })) || {}; + + // Assert - confirm overwrite should not be called with force flag + expect(mockConfirmOverwrite).not.toHaveBeenCalled(); + expect(mockParsePRD).toHaveBeenCalledWith(testFile, outputFile, 10, { + append: false + }); + + // Reset mock implementation + mockExistsSync.mockReset(); }); }); diff --git a/tests/unit/task-manager.test.js b/tests/unit/task-manager.test.js index 34c4d2ca..feaf71c4 100644 --- a/tests/unit/task-manager.test.js +++ b/tests/unit/task-manager.test.js @@ -134,33 +134,59 @@ jest.mock('../../scripts/modules/task-manager.js', () => { }); // Create a simplified version of parsePRD for testing -const testParsePRD = async (prdPath, outputPath, numTasks) => { +const testParsePRD = async (prdPath, outputPath, numTasks, options = {}) => { + const { append = false } = options; try { + // Handle existing tasks when append flag is true + let existingTasks = { tasks: [] }; + let lastTaskId = 0; + // Check if the output file already exists if (mockExistsSync(outputPath)) { - const confirmOverwrite = await mockPromptYesNo( - `Warning: ${outputPath} already exists. Overwrite?`, - false - ); + if (append) { + // Simulate reading existing tasks.json + existingTasks = { + tasks: [ + { id: 1, title: 'Existing Task 1', status: 'done' }, + { id: 2, title: 'Existing Task 2', status: 'pending' } + ] + }; + lastTaskId = 2; // Highest existing ID + } else { + const confirmOverwrite = await mockPromptYesNo( + `Warning: ${outputPath} already exists. Overwrite?`, + false + ); - if (!confirmOverwrite) { - console.log(`Operation cancelled. ${outputPath} was not modified.`); - return null; + if (!confirmOverwrite) { + console.log(`Operation cancelled. ${outputPath} was not modified.`); + return null; + } } } const prdContent = mockReadFileSync(prdPath, 'utf8'); - const tasks = await mockCallClaude(prdContent, prdPath, numTasks); + // Modify mockCallClaude to accept lastTaskId parameter + let newTasks = await mockCallClaude(prdContent, prdPath, numTasks); + + // Merge tasks if appending + const tasksData = append + ? { + ...existingTasks, + tasks: [...existingTasks.tasks, ...newTasks.tasks] + } + : newTasks; + const dir = mockDirname(outputPath); if (!mockExistsSync(dir)) { mockMkdirSync(dir, { recursive: true }); } - mockWriteJSON(outputPath, tasks); + mockWriteJSON(outputPath, tasksData); await mockGenerateTaskFiles(outputPath, dir); - return tasks; + return tasksData; } catch (error) { console.error(`Error parsing PRD: ${error.message}`); process.exit(1); @@ -628,6 +654,27 @@ describe('Task Manager Module', () => { // Mock the sample PRD content const samplePRDContent = '# Sample PRD for Testing'; + // Mock existing tasks for append test + const existingTasks = { + tasks: [ + { id: 1, title: 'Existing Task 1', status: 'done' }, + { id: 2, title: 'Existing Task 2', status: 'pending' } + ] + }; + + // Mock new tasks with continuing IDs for append test + const newTasksWithContinuedIds = { + tasks: [ + { id: 3, title: 'New Task 3' }, + { id: 4, title: 'New Task 4' } + ] + }; + + // Mock merged tasks for append test + const mergedTasks = { + tasks: [...existingTasks.tasks, ...newTasksWithContinuedIds.tasks] + }; + beforeEach(() => { // Reset all mocks jest.clearAllMocks(); @@ -811,6 +858,66 @@ describe('Task Manager Module', () => { sampleClaudeResponse ); }); + + test('should append new tasks when append option is true', async () => { + // Setup mocks to simulate tasks.json already exists + mockExistsSync.mockImplementation((path) => { + if (path === 'tasks/tasks.json') return true; // Output file exists + if (path === 'tasks') return true; // Directory exists + return false; + }); + + // Mock for reading existing tasks + mockReadJSON.mockReturnValue(existingTasks); + // mockReadJSON = jest.fn().mockReturnValue(existingTasks); + + // Mock callClaude to return new tasks with continuing IDs + mockCallClaude.mockResolvedValueOnce(newTasksWithContinuedIds); + + // Call the function with append option + const result = await testParsePRD( + 'path/to/prd.txt', + 'tasks/tasks.json', + 2, + { append: true } + ); + + // Verify prompt was NOT called (no confirmation needed for append) + expect(mockPromptYesNo).not.toHaveBeenCalled(); + + // Verify the file was written with merged tasks + expect(mockWriteJSON).toHaveBeenCalledWith( + 'tasks/tasks.json', + expect.objectContaining({ + tasks: expect.arrayContaining([ + expect.objectContaining({ id: 1 }), + expect.objectContaining({ id: 2 }), + expect.objectContaining({ id: 3 }), + expect.objectContaining({ id: 4 }) + ]) + }) + ); + + // Verify the result contains merged tasks + expect(result.tasks.length).toBe(4); + }); + + test('should skip prompt and not overwrite when append is true', async () => { + // Setup mocks to simulate tasks.json already exists + mockExistsSync.mockImplementation((path) => { + if (path === 'tasks/tasks.json') return true; // Output file exists + if (path === 'tasks') return true; // Directory exists + return false; + }); + + // Call the function with append option + await testParsePRD('path/to/prd.txt', 'tasks/tasks.json', 3, { + append: true + }); + + // Verify prompt was NOT called with append flag + expect(mockPromptYesNo).not.toHaveBeenCalled(); + }); }); describe.skip('updateTasks function', () => {