From 23b49a24f5f4aa79138bf287e3e106cd22e87c89 Mon Sep 17 00:00:00 2001 From: Ralph Khreish <35776126+Crunchyman-ralph@users.noreply.github.com> Date: Tue, 25 Nov 2025 18:26:29 +0100 Subject: [PATCH] chore: apply requested changes --- .changeset/pretty-suits-marry.md | 2 +- .../commands/generate.command.test.ts | 24 ++++--- apps/mcp/src/tools/tasks/generate.tool.ts | 4 -- .../integration/tools/generate.tool.test.ts | 12 +--- mcp-server/src/tools/tool-registry.js | 2 +- .../services/task-file-generator.service.ts | 71 +++++++++++++------ .../modules/tasks/services/task-service.ts | 11 +++ .../tm-core/src/modules/tasks/tasks-domain.ts | 12 +++- packages/tm-core/src/tm-core.ts | 11 +++ 9 files changed, 98 insertions(+), 51 deletions(-) diff --git a/.changeset/pretty-suits-marry.md b/.changeset/pretty-suits-marry.md index b100ec2e..a13f1610 100644 --- a/.changeset/pretty-suits-marry.md +++ b/.changeset/pretty-suits-marry.md @@ -8,4 +8,4 @@ Bring back `task-master generate` as a command and mcp tool (after popular deman - They also follow the markdownlint format making them look like more standard md files - added parameters to generate allowing you to generate with the `--tag` flag - If I am on an active tag and want to generate files from another tag, I can with the tag parameter -- See `--help` on more information on how `task-master generate --help` works +- See `task-master generate --help` for more information. diff --git a/apps/cli/tests/integration/commands/generate.command.test.ts b/apps/cli/tests/integration/commands/generate.command.test.ts index 3a184e32..403c8e66 100644 --- a/apps/cli/tests/integration/commands/generate.command.test.ts +++ b/apps/cli/tests/integration/commands/generate.command.test.ts @@ -181,17 +181,7 @@ describe('generate command', () => { const testData = { master: { tasks: [], metadata: {} }, 'feature-branch': { - tasks: [ - { - id: '1', - title: 'Test Task', - description: 'Test', - status: 'pending', - priority: 'medium', - dependencies: [], - subtasks: [] - } - ], + tasks: [createTask({ id: 1, title: 'Test Task', status: 'pending' })], metadata: {} } }; @@ -219,4 +209,16 @@ describe('generate command', () => { expect(exitCode).toBe(0); expect(output).toContain('.taskmaster/tasks'); }); + + it('should exit with non-zero code for invalid --format value', () => { + const testData = createTasksFile({ + tasks: [createTask({ id: 1, title: 'Test Task', status: 'pending' })] + }); + writeTasks(testData); + + const { output, exitCode } = runGenerate('--format invalid'); + + expect(exitCode).not.toBe(0); + expect(output.toLowerCase()).toMatch(/invalid|error|format/); + }); }); diff --git a/apps/mcp/src/tools/tasks/generate.tool.ts b/apps/mcp/src/tools/tasks/generate.tool.ts index 3ec6a6c3..8a014184 100644 --- a/apps/mcp/src/tools/tasks/generate.tool.ts +++ b/apps/mcp/src/tools/tasks/generate.tool.ts @@ -10,10 +10,6 @@ import type { ToolContext } from '../../shared/types.js'; import type { FastMCP } from 'fastmcp'; const GenerateSchema = z.object({ - file: z - .string() - .optional() - .describe('Absolute path to the tasks file (default: tasks/tasks.json)'), output: z .string() .optional() diff --git a/apps/mcp/tests/integration/tools/generate.tool.test.ts b/apps/mcp/tests/integration/tools/generate.tool.test.ts index bbbca55e..46a2afcb 100644 --- a/apps/mcp/tests/integration/tools/generate.tool.test.ts +++ b/apps/mcp/tests/integration/tools/generate.tool.test.ts @@ -177,17 +177,7 @@ describe('generate MCP tool', () => { const testData = { master: { tasks: [], metadata: {} }, 'feature-branch': { - tasks: [ - { - id: '1', - title: 'Test Task', - description: 'Test', - status: 'pending', - priority: 'medium', - dependencies: [], - subtasks: [] - } - ], + tasks: [createTask({ id: 1, title: 'Test Task', status: 'pending' })], metadata: {} } }; diff --git a/mcp-server/src/tools/tool-registry.js b/mcp-server/src/tools/tool-registry.js index 1888357f..b56ad7a3 100644 --- a/mcp-server/src/tools/tool-registry.js +++ b/mcp-server/src/tools/tool-registry.js @@ -53,7 +53,7 @@ import { } from '@tm/mcp'; /** - * Comprehensive tool registry mapping all 44 tool names to their registration functions + * Comprehensive tool registry mapping tool names to their registration functions * Used for dynamic tool registration and validation */ export const toolRegistry = { diff --git a/packages/tm-core/src/modules/tasks/services/task-file-generator.service.ts b/packages/tm-core/src/modules/tasks/services/task-file-generator.service.ts index 437bc7f1..f6494237 100644 --- a/packages/tm-core/src/modules/tasks/services/task-file-generator.service.ts +++ b/packages/tm-core/src/modules/tasks/services/task-file-generator.service.ts @@ -3,7 +3,7 @@ * Generates individual markdown task files from tasks.json */ -import fs from 'node:fs'; +import fs from 'node:fs/promises'; import path from 'node:path'; import type { Task, Subtask, TaskStatus } from '../../../common/types/index.js'; import type { IStorage } from '../../../common/interfaces/storage.interface.js'; @@ -24,14 +24,16 @@ export interface GenerateTaskFilesOptions { */ export interface GenerateTaskFilesResult { success: boolean; - /** Number of task files generated */ + /** Number of task files successfully generated */ count: number; /** Output directory where files were written */ directory: string; /** Number of orphaned files cleaned up */ orphanedFilesRemoved: number; - /** Error message if generation failed */ + /** Error message if generation failed completely */ error?: string; + /** Individual file write errors (task ID -> error message) */ + fileErrors?: Record; } /** @@ -60,9 +62,7 @@ export class TaskFileGeneratorService { try { // Ensure output directory exists - if (!fs.existsSync(outputDir)) { - fs.mkdirSync(outputDir, { recursive: true }); - } + await fs.mkdir(outputDir, { recursive: true }); // Load tasks from storage const tasks = await this.storage.loadTasks(tag); @@ -77,26 +77,43 @@ export class TaskFileGeneratorService { } // Clean up orphaned task files - const orphanedCount = this.cleanupOrphanedFiles( + const orphanedCount = await this.cleanupOrphanedFiles( outputDir, tasks, tag ); - // Generate task files - for (const task of tasks) { - const content = this.formatTaskContent(task, tasks); - const fileName = this.getTaskFileName(task.id, tag); - const filePath = path.join(outputDir, fileName); + // Generate task files in parallel with individual error handling + // This allows partial success - some files can be written even if others fail + const fileErrors: Record = {}; + const results = await Promise.allSettled( + tasks.map(async (task) => { + const content = this.formatTaskContent(task, tasks); + const fileName = this.getTaskFileName(task.id, tag); + const filePath = path.join(outputDir, fileName); + await fs.writeFile(filePath, content, 'utf-8'); + return task.id; + }) + ); - fs.writeFileSync(filePath, content, 'utf-8'); + // Count successes and collect errors + let successCount = 0; + for (let i = 0; i < results.length; i++) { + const result = results[i]; + if (result.status === 'fulfilled') { + successCount++; + } else { + const taskId = String(tasks[i].id); + fileErrors[taskId] = result.reason?.message || 'Unknown error'; + } } return { - success: true, - count: tasks.length, + success: Object.keys(fileErrors).length === 0, + count: successCount, directory: outputDir, - orphanedFilesRemoved: orphanedCount + orphanedFilesRemoved: orphanedCount, + ...(Object.keys(fileErrors).length > 0 && { fileErrors }) }; } catch (error: any) { return { @@ -132,21 +149,24 @@ export class TaskFileGeneratorService { * Clean up orphaned task files (files for tasks that no longer exist) * @returns Number of files removed */ - private cleanupOrphanedFiles( + private async cleanupOrphanedFiles( outputDir: string, tasks: Task[], tag: string - ): number { + ): Promise { let removedCount = 0; try { - const files = fs.readdirSync(outputDir); + const files = await fs.readdir(outputDir); const validTaskIds = tasks.map((task) => String(task.id)); // Tag-aware file patterns const masterFilePattern = /^task_(\d+)\.md$/; const taggedFilePattern = new RegExp(`^task_(\\d+)_${this.escapeRegExp(tag)}\\.md$`); + // Collect files to remove + const filesToRemove: string[] = []; + for (const file of files) { let match = null; let fileTaskId: string | null = null; @@ -169,12 +189,19 @@ export class TaskFileGeneratorService { // Convert to integer for comparison (removes leading zeros) const normalizedId = String(parseInt(fileTaskId, 10)); if (!validTaskIds.includes(normalizedId)) { - const filePath = path.join(outputDir, file); - fs.unlinkSync(filePath); - removedCount++; + filesToRemove.push(file); } } } + + // Remove files in parallel + await Promise.all( + filesToRemove.map(async (file) => { + const filePath = path.join(outputDir, file); + await fs.unlink(filePath); + }) + ); + removedCount = filesToRemove.length; } catch (error) { // Ignore errors during cleanup - non-critical operation } diff --git a/packages/tm-core/src/modules/tasks/services/task-service.ts b/packages/tm-core/src/modules/tasks/services/task-service.ts index 84350857..ea2a0620 100644 --- a/packages/tm-core/src/modules/tasks/services/task-service.ts +++ b/packages/tm-core/src/modules/tasks/services/task-service.ts @@ -792,4 +792,15 @@ export class TaskService { ); } } + + /** + * Close and cleanup resources + * Releases file locks and other storage resources + */ + async close(): Promise { + if (this.storage) { + await this.storage.close(); + } + this.initialized = false; + } } diff --git a/packages/tm-core/src/modules/tasks/tasks-domain.ts b/packages/tm-core/src/modules/tasks/tasks-domain.ts index df996180..6ccb5645 100644 --- a/packages/tm-core/src/modules/tasks/tasks-domain.ts +++ b/packages/tm-core/src/modules/tasks/tasks-domain.ts @@ -393,7 +393,7 @@ export class TasksDomain { /** * Generate individual task markdown files from tasks.json - * This writes .txt files for each task in the tasks directory. + * This writes .md files for each task in the tasks directory. * * Note: Only applicable for file storage. API storage throws an error. * @@ -417,4 +417,14 @@ export class TasksDomain { return this.taskFileGenerator.generateTaskFiles(options); } + + // ========== Cleanup ========== + + /** + * Close and cleanup resources + * Releases file locks and other storage resources + */ + async close(): Promise { + await this.taskService.close(); + } } diff --git a/packages/tm-core/src/tm-core.ts b/packages/tm-core/src/tm-core.ts index 65362805..4511c1c5 100644 --- a/packages/tm-core/src/tm-core.ts +++ b/packages/tm-core/src/tm-core.ts @@ -203,6 +203,17 @@ export class TmCore { get projectPath(): string { return this._projectPath; } + + /** + * Close and cleanup resources + * Releases file locks and other storage resources + * Should be called when done using TmCore, especially in tests + */ + async close(): Promise { + if (this._tasks) { + await this._tasks.close(); + } + } } /**