From 465ae252f04064f9b0b0a0e6b1e1bde114df2715 Mon Sep 17 00:00:00 2001 From: Eyal Toledano Date: Fri, 11 Apr 2025 03:44:27 -0400 Subject: [PATCH] refactor(mcp): Enforce projectRoot and centralize path validation This commit refactors how project paths are handled in MCP direct functions to improve reliability, particularly when session context is incomplete or missing. Key changes: 1) Made projectRoot required in MCP tools. 2) Refactored findTasksJsonPath to return {tasksPath, validatedProjectRoot}. 3) Updated all direct functions to pass session to findTasksJsonPath. 4) Updated analyzeTaskComplexityDirect to use the validated root for output path resolution. This ensures operations relying on project context receive an explicitly provided and validated project root directory, resolving errors caused by incorrect path resolution. --- .../core/direct-functions/add-dependency.js | 4 +- .../src/core/direct-functions/add-subtask.js | 4 +- .../src/core/direct-functions/add-task.js | 7 +- .../analyze-task-complexity.js | 63 ++- .../core/direct-functions/clear-subtasks.js | 4 +- .../direct-functions/complexity-report.js | 4 +- .../core/direct-functions/expand-all-tasks.js | 6 +- .../src/core/direct-functions/expand-task.js | 6 +- .../core/direct-functions/fix-dependencies.js | 4 +- .../direct-functions/generate-task-files.js | 4 +- .../initialize-project-direct.js | 3 +- .../src/core/direct-functions/list-tasks.js | 4 +- .../src/core/direct-functions/next-task.js | 4 +- .../direct-functions/remove-dependency.js | 4 +- .../core/direct-functions/remove-subtask.js | 4 +- .../src/core/direct-functions/remove-task.js | 4 +- .../core/direct-functions/set-task-status.js | 4 +- .../src/core/direct-functions/show-task.js | 4 +- .../direct-functions/update-subtask-by-id.js | 6 +- .../direct-functions/update-task-by-id.js | 6 +- .../src/core/direct-functions/update-tasks.js | 6 +- .../direct-functions/validate-dependencies.js | 4 +- mcp-server/src/core/utils/path-utils.js | 401 +++++++++++------- mcp-server/src/tools/analyze.js | 70 ++- mcp-server/src/tools/utils.js | 5 +- scripts/init.js | 2 +- 26 files changed, 348 insertions(+), 289 deletions(-) diff --git a/mcp-server/src/core/direct-functions/add-dependency.js b/mcp-server/src/core/direct-functions/add-dependency.js index 9a34a0ae..2833a603 100644 --- a/mcp-server/src/core/direct-functions/add-dependency.js +++ b/mcp-server/src/core/direct-functions/add-dependency.js @@ -21,7 +21,7 @@ import { * @param {Object} log - Logger object * @returns {Promise} - Result object with success status and data/error information */ -export async function addDependencyDirect(args, log) { +export async function addDependencyDirect(args, log, { session }) { try { log.info(`Adding dependency with args: ${JSON.stringify(args)}`); @@ -47,7 +47,7 @@ export async function addDependencyDirect(args, log) { } // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + const tasksPath = findTasksJsonPath(args, log, session); // Format IDs for the core function const taskId = diff --git a/mcp-server/src/core/direct-functions/add-subtask.js b/mcp-server/src/core/direct-functions/add-subtask.js index 67b2283e..ccddc2fc 100644 --- a/mcp-server/src/core/direct-functions/add-subtask.js +++ b/mcp-server/src/core/direct-functions/add-subtask.js @@ -25,7 +25,7 @@ import { * @param {Object} log - Logger object * @returns {Promise<{success: boolean, data?: Object, error?: string}>} */ -export async function addSubtaskDirect(args, log) { +export async function addSubtaskDirect(args, log, { session }) { try { log.info(`Adding subtask with args: ${JSON.stringify(args)}`); @@ -51,7 +51,7 @@ export async function addSubtaskDirect(args, log) { } // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + const tasksPath = findTasksJsonPath(args, log, session); // Parse dependencies if provided let dependencies = []; diff --git a/mcp-server/src/core/direct-functions/add-task.js b/mcp-server/src/core/direct-functions/add-task.js index 9be18a39..3630094f 100644 --- a/mcp-server/src/core/direct-functions/add-task.js +++ b/mcp-server/src/core/direct-functions/add-task.js @@ -37,13 +37,13 @@ import { * @param {Object} context - Additional context (reportProgress, session) * @returns {Promise} - Result object { success: boolean, data?: any, error?: { code: string, message: string } } */ -export async function addTaskDirect(args, log, context = {}) { +export async function addTaskDirect(args, log, { session }) { try { // Enable silent mode to prevent console logs from interfering with JSON response enableSilentMode(); // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + const tasksPath = findTasksJsonPath(args, log, session); // Check if this is manual task creation or AI-driven task creation const isManualCreation = args.title && args.description; @@ -75,9 +75,6 @@ export async function addTaskDirect(args, log, context = {}) { : []; const priority = args.priority || 'medium'; - // Extract context parameters for advanced functionality - const { session } = context; - let manualTaskData = null; if (isManualCreation) { diff --git a/mcp-server/src/core/direct-functions/analyze-task-complexity.js b/mcp-server/src/core/direct-functions/analyze-task-complexity.js index 1afdd2d0..c36ac061 100644 --- a/mcp-server/src/core/direct-functions/analyze-task-complexity.js +++ b/mcp-server/src/core/direct-functions/analyze-task-complexity.js @@ -3,7 +3,11 @@ */ import { analyzeTaskComplexity } from '../../../../scripts/modules/task-manager.js'; -import { findTasksJsonPath } from '../utils/path-utils.js'; +import { + findTasksJsonPath, + resolveProjectPath, + ensureDirectoryExists +} from '../utils/path-utils.js'; import { enableSilentMode, disableSilentMode, @@ -26,23 +30,33 @@ import path from 'path'; * @param {Object} [context={}] - Context object containing session data * @returns {Promise<{success: boolean, data?: Object, error?: {code: string, message: string}}>} */ -export async function analyzeTaskComplexityDirect(args, log, context = {}) { - const { session } = context; // Only extract session, not reportProgress - +export async function analyzeTaskComplexityDirect(args, log, { session }) { try { log.info(`Analyzing task complexity with args: ${JSON.stringify(args)}`); - // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + // Find the tasks.json path AND get the validated project root + const { tasksPath, validatedProjectRoot } = findTasksJsonPath( + args, + log, + session + ); + log.info( + `Using tasks file: ${tasksPath} located within project root: ${validatedProjectRoot}` + ); - // Determine output path - let outputPath = args.output || 'scripts/task-complexity-report.json'; - if (!path.isAbsolute(outputPath) && args.projectRoot) { - outputPath = path.join(args.projectRoot, outputPath); - } + // Determine and resolve the output path using the VALIDATED root + const relativeOutputPath = + args.output || 'scripts/task-complexity-report.json'; + const absoluteOutputPath = resolveProjectPath( + relativeOutputPath, + validatedProjectRoot, + log + ); - log.info(`Analyzing task complexity from: ${tasksPath}`); - log.info(`Output report will be saved to: ${outputPath}`); + // Ensure the output directory exists + ensureDirectoryExists(path.dirname(absoluteOutputPath), log); + + log.info(`Output report will be saved to: ${absoluteOutputPath}`); if (args.research) { log.info('Using Perplexity AI for research-backed complexity analysis'); @@ -51,7 +65,7 @@ export async function analyzeTaskComplexityDirect(args, log, context = {}) { // Create options object for analyzeTaskComplexity const options = { file: tasksPath, - output: outputPath, + output: absoluteOutputPath, model: args.model, threshold: args.threshold, research: args.research === true @@ -95,7 +109,7 @@ export async function analyzeTaskComplexityDirect(args, log, context = {}) { } // Verify the report file was created - if (!fs.existsSync(outputPath)) { + if (!fs.existsSync(absoluteOutputPath)) { return { success: false, error: { @@ -108,7 +122,7 @@ export async function analyzeTaskComplexityDirect(args, log, context = {}) { // Read the report file let report; try { - report = JSON.parse(fs.readFileSync(outputPath, 'utf8')); + report = JSON.parse(fs.readFileSync(absoluteOutputPath, 'utf8')); // Important: Handle different report formats // The core function might return an array or an object with a complexityAnalysis property @@ -130,8 +144,8 @@ export async function analyzeTaskComplexityDirect(args, log, context = {}) { return { success: true, data: { - message: `Task complexity analysis complete. Report saved to ${outputPath}`, - reportPath: outputPath, + message: `Task complexity analysis complete. Report saved to ${absoluteOutputPath}`, + reportPath: absoluteOutputPath, reportSummary: { taskCount: analysisArray.length, highComplexityTasks, @@ -151,18 +165,23 @@ export async function analyzeTaskComplexityDirect(args, log, context = {}) { }; } } catch (error) { - // Make sure to restore normal logging even if there's an error + // Centralized error catching for issues like invalid root, file not found, core errors etc. if (isSilentMode()) { disableSilentMode(); } - log.error(`Error in analyzeTaskComplexityDirect: ${error.message}`); + log.error(`Error in analyzeTaskComplexityDirect: ${error.message}`, { + code: error.code, + details: error.details, + stack: error.stack + }); return { success: false, error: { - code: 'CORE_FUNCTION_ERROR', + code: error.code || 'ANALYZE_COMPLEXITY_ERROR', message: error.message - } + }, + fromCache: false // Assume errors are not from cache }; } } diff --git a/mcp-server/src/core/direct-functions/clear-subtasks.js b/mcp-server/src/core/direct-functions/clear-subtasks.js index 7e3987b1..3ee682cb 100644 --- a/mcp-server/src/core/direct-functions/clear-subtasks.js +++ b/mcp-server/src/core/direct-functions/clear-subtasks.js @@ -20,7 +20,7 @@ import fs from 'fs'; * @param {Object} log - Logger object * @returns {Promise<{success: boolean, data?: Object, error?: {code: string, message: string}}>} */ -export async function clearSubtasksDirect(args, log) { +export async function clearSubtasksDirect(args, log, { session }) { try { log.info(`Clearing subtasks with args: ${JSON.stringify(args)}`); @@ -37,7 +37,7 @@ export async function clearSubtasksDirect(args, log) { } // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + const tasksPath = findTasksJsonPath(args, log, session); // Check if tasks.json exists if (!fs.existsSync(tasksPath)) { diff --git a/mcp-server/src/core/direct-functions/complexity-report.js b/mcp-server/src/core/direct-functions/complexity-report.js index 9461a113..a4138d3b 100644 --- a/mcp-server/src/core/direct-functions/complexity-report.js +++ b/mcp-server/src/core/direct-functions/complexity-report.js @@ -19,14 +19,14 @@ import path from 'path'; * @param {Object} log - Logger object * @returns {Promise} - Result object with success status and data/error information */ -export async function complexityReportDirect(args, log) { +export async function complexityReportDirect(args, log, { session }) { try { log.info(`Getting complexity report with args: ${JSON.stringify(args)}`); // Get tasks file path to determine project root for the default report location let tasksPath; try { - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } catch (error) { log.warn( `Tasks file not found, using current directory: ${error.message}` diff --git a/mcp-server/src/core/direct-functions/expand-all-tasks.js b/mcp-server/src/core/direct-functions/expand-all-tasks.js index ac9574de..fe757795 100644 --- a/mcp-server/src/core/direct-functions/expand-all-tasks.js +++ b/mcp-server/src/core/direct-functions/expand-all-tasks.js @@ -26,9 +26,7 @@ import fs from 'fs'; * @param {Object} context - Context object containing session * @returns {Promise<{success: boolean, data?: Object, error?: {code: string, message: string}}>} */ -export async function expandAllTasksDirect(args, log, context = {}) { - const { session } = context; // Only extract session, not reportProgress - +export async function expandAllTasksDirect(args, log, { session }) { try { log.info(`Expanding all tasks with args: ${JSON.stringify(args)}`); @@ -37,7 +35,7 @@ export async function expandAllTasksDirect(args, log, context = {}) { try { // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + const tasksPath = findTasksJsonPath(args, log, session); // Parse parameters const numSubtasks = args.num ? parseInt(args.num, 10) : undefined; diff --git a/mcp-server/src/core/direct-functions/expand-task.js b/mcp-server/src/core/direct-functions/expand-task.js index 1efa9db9..2230bea3 100644 --- a/mcp-server/src/core/direct-functions/expand-task.js +++ b/mcp-server/src/core/direct-functions/expand-task.js @@ -27,9 +27,7 @@ import fs from 'fs'; * @param {Object} context - Context object containing session and reportProgress * @returns {Promise} - Task expansion result { success: boolean, data?: any, error?: { code: string, message: string }, fromCache: boolean } */ -export async function expandTaskDirect(args, log, context = {}) { - const { session } = context; - +export async function expandTaskDirect(args, log, { session }) { // Log session root data for debugging log.info( `Session data in expandTaskDirect: ${JSON.stringify({ @@ -53,7 +51,7 @@ export async function expandTaskDirect(args, log, context = {}) { log.info( `[expandTaskDirect] No direct file path provided or file not found at ${args.file}, searching using findTasksJsonPath` ); - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } } catch (error) { log.error( diff --git a/mcp-server/src/core/direct-functions/fix-dependencies.js b/mcp-server/src/core/direct-functions/fix-dependencies.js index 8dc61833..4e40c33c 100644 --- a/mcp-server/src/core/direct-functions/fix-dependencies.js +++ b/mcp-server/src/core/direct-functions/fix-dependencies.js @@ -18,12 +18,12 @@ import fs from 'fs'; * @param {Object} log - Logger object * @returns {Promise<{success: boolean, data?: Object, error?: {code: string, message: string}}>} */ -export async function fixDependenciesDirect(args, log) { +export async function fixDependenciesDirect(args, log, { session }) { try { log.info(`Fixing invalid dependencies in tasks...`); // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + const tasksPath = findTasksJsonPath(args, log, session); // Verify the file exists if (!fs.existsSync(tasksPath)) { diff --git a/mcp-server/src/core/direct-functions/generate-task-files.js b/mcp-server/src/core/direct-functions/generate-task-files.js index d84956ab..739c18ac 100644 --- a/mcp-server/src/core/direct-functions/generate-task-files.js +++ b/mcp-server/src/core/direct-functions/generate-task-files.js @@ -18,14 +18,14 @@ import path from 'path'; * @param {Object} log - Logger object. * @returns {Promise} - Result object with success status and data/error information. */ -export async function generateTaskFilesDirect(args, log) { +export async function generateTaskFilesDirect(args, log, { session }) { try { log.info(`Generating task files with args: ${JSON.stringify(args)}`); // Get tasks file path let tasksPath; try { - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } catch (error) { log.error(`Error finding tasks file: ${error.message}`); return { diff --git a/mcp-server/src/core/direct-functions/initialize-project-direct.js b/mcp-server/src/core/direct-functions/initialize-project-direct.js index 088d124d..71553737 100644 --- a/mcp-server/src/core/direct-functions/initialize-project-direct.js +++ b/mcp-server/src/core/direct-functions/initialize-project-direct.js @@ -16,8 +16,7 @@ import os from 'os'; // Import os module for home directory check * @param {object} context - The context object, must contain { session }. * @returns {Promise<{success: boolean, data?: any, error?: {code: string, message: string}}>} - Standard result object. */ -export async function initializeProjectDirect(args, log, context = {}) { - const { session } = context; +export async function initializeProjectDirect(args, log, { session }) { const homeDir = os.homedir(); let targetDirectory = null; diff --git a/mcp-server/src/core/direct-functions/list-tasks.js b/mcp-server/src/core/direct-functions/list-tasks.js index c6f5e050..d70fbd13 100644 --- a/mcp-server/src/core/direct-functions/list-tasks.js +++ b/mcp-server/src/core/direct-functions/list-tasks.js @@ -18,11 +18,11 @@ import { * @param {Object} log - Logger object. * @returns {Promise} - Task list result { success: boolean, data?: any, error?: { code: string, message: string }, fromCache: boolean }. */ -export async function listTasksDirect(args, log) { +export async function listTasksDirect(args, log, { session }) { let tasksPath; try { // Find the tasks path first - needed for cache key and execution - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } catch (error) { if (error.code === 'TASKS_FILE_NOT_FOUND') { log.error(`Tasks file not found: ${error.message}`); diff --git a/mcp-server/src/core/direct-functions/next-task.js b/mcp-server/src/core/direct-functions/next-task.js index 3286ed69..7e9193ce 100644 --- a/mcp-server/src/core/direct-functions/next-task.js +++ b/mcp-server/src/core/direct-functions/next-task.js @@ -19,11 +19,11 @@ import { * @param {Object} log - Logger object * @returns {Promise} - Next task result { success: boolean, data?: any, error?: { code: string, message: string }, fromCache: boolean } */ -export async function nextTaskDirect(args, log) { +export async function nextTaskDirect(args, log, { session }) { let tasksPath; try { // Find the tasks path first - needed for cache key and execution - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } catch (error) { log.error(`Tasks file not found: ${error.message}`); return { diff --git a/mcp-server/src/core/direct-functions/remove-dependency.js b/mcp-server/src/core/direct-functions/remove-dependency.js index 59ed7f3f..effe0b59 100644 --- a/mcp-server/src/core/direct-functions/remove-dependency.js +++ b/mcp-server/src/core/direct-functions/remove-dependency.js @@ -19,7 +19,7 @@ import { * @param {Object} log - Logger object * @returns {Promise<{success: boolean, data?: Object, error?: {code: string, message: string}}>} */ -export async function removeDependencyDirect(args, log) { +export async function removeDependencyDirect(args, log, { session }) { try { log.info(`Removing dependency with args: ${JSON.stringify(args)}`); @@ -45,7 +45,7 @@ export async function removeDependencyDirect(args, log) { } // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + const tasksPath = findTasksJsonPath(args, log, session); // Format IDs for the core function const taskId = diff --git a/mcp-server/src/core/direct-functions/remove-subtask.js b/mcp-server/src/core/direct-functions/remove-subtask.js index 143a985f..db944142 100644 --- a/mcp-server/src/core/direct-functions/remove-subtask.js +++ b/mcp-server/src/core/direct-functions/remove-subtask.js @@ -20,7 +20,7 @@ import { * @param {Object} log - Logger object * @returns {Promise<{success: boolean, data?: Object, error?: {code: string, message: string}}>} */ -export async function removeSubtaskDirect(args, log) { +export async function removeSubtaskDirect(args, log, { session }) { try { // Enable silent mode to prevent console logs from interfering with JSON response enableSilentMode(); @@ -50,7 +50,7 @@ export async function removeSubtaskDirect(args, log) { } // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + const tasksPath = findTasksJsonPath(args, log, session); // Convert convertToTask to a boolean const convertToTask = args.convert === true; diff --git a/mcp-server/src/core/direct-functions/remove-task.js b/mcp-server/src/core/direct-functions/remove-task.js index 0e98ac92..b44eedb5 100644 --- a/mcp-server/src/core/direct-functions/remove-task.js +++ b/mcp-server/src/core/direct-functions/remove-task.js @@ -17,12 +17,12 @@ import { findTasksJsonPath } from '../utils/path-utils.js'; * @param {Object} log - Logger object * @returns {Promise} - Remove task result { success: boolean, data?: any, error?: { code: string, message: string }, fromCache: false } */ -export async function removeTaskDirect(args, log) { +export async function removeTaskDirect(args, log, { session }) { try { // Find the tasks path first let tasksPath; try { - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } catch (error) { log.error(`Tasks file not found: ${error.message}`); return { diff --git a/mcp-server/src/core/direct-functions/set-task-status.js b/mcp-server/src/core/direct-functions/set-task-status.js index 9f52c115..0a1db6d1 100644 --- a/mcp-server/src/core/direct-functions/set-task-status.js +++ b/mcp-server/src/core/direct-functions/set-task-status.js @@ -18,7 +18,7 @@ import { * @param {Object} log - Logger object. * @returns {Promise} - Result object with success status and data/error information. */ -export async function setTaskStatusDirect(args, log) { +export async function setTaskStatusDirect(args, log, { session }) { try { log.info(`Setting task status with args: ${JSON.stringify(args)}`); @@ -49,7 +49,7 @@ export async function setTaskStatusDirect(args, log) { let tasksPath; try { // The enhanced findTasksJsonPath will now search in parent directories if needed - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); log.info(`Found tasks file at: ${tasksPath}`); } catch (error) { log.error(`Error finding tasks file: ${error.message}`); diff --git a/mcp-server/src/core/direct-functions/show-task.js b/mcp-server/src/core/direct-functions/show-task.js index adbeb391..3f28b1d4 100644 --- a/mcp-server/src/core/direct-functions/show-task.js +++ b/mcp-server/src/core/direct-functions/show-task.js @@ -19,11 +19,11 @@ import { * @param {Object} log - Logger object * @returns {Promise} - Task details result { success: boolean, data?: any, error?: { code: string, message: string }, fromCache: boolean } */ -export async function showTaskDirect(args, log) { +export async function showTaskDirect(args, log, { session }) { let tasksPath; try { // Find the tasks path first - needed for cache key and execution - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } catch (error) { log.error(`Tasks file not found: ${error.message}`); return { diff --git a/mcp-server/src/core/direct-functions/update-subtask-by-id.js b/mcp-server/src/core/direct-functions/update-subtask-by-id.js index f8a235ce..e4416ad8 100644 --- a/mcp-server/src/core/direct-functions/update-subtask-by-id.js +++ b/mcp-server/src/core/direct-functions/update-subtask-by-id.js @@ -22,9 +22,7 @@ import { * @param {Object} context - Context object containing session data. * @returns {Promise} - Result object with success status and data/error information. */ -export async function updateSubtaskByIdDirect(args, log, context = {}) { - const { session } = context; // Only extract session, not reportProgress - +export async function updateSubtaskByIdDirect(args, log, { session }) { try { log.info(`Updating subtask with args: ${JSON.stringify(args)}`); @@ -77,7 +75,7 @@ export async function updateSubtaskByIdDirect(args, log, context = {}) { // Get tasks file path let tasksPath; try { - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } catch (error) { log.error(`Error finding tasks file: ${error.message}`); return { diff --git a/mcp-server/src/core/direct-functions/update-task-by-id.js b/mcp-server/src/core/direct-functions/update-task-by-id.js index 98c368d2..cc9a1015 100644 --- a/mcp-server/src/core/direct-functions/update-task-by-id.js +++ b/mcp-server/src/core/direct-functions/update-task-by-id.js @@ -22,9 +22,7 @@ import { * @param {Object} context - Context object containing session data. * @returns {Promise} - Result object with success status and data/error information. */ -export async function updateTaskByIdDirect(args, log, context = {}) { - const { session } = context; // Only extract session, not reportProgress - +export async function updateTaskByIdDirect(args, log, { session }) { try { log.info(`Updating task with args: ${JSON.stringify(args)}`); @@ -77,7 +75,7 @@ export async function updateTaskByIdDirect(args, log, context = {}) { // Get tasks file path let tasksPath; try { - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } catch (error) { log.error(`Error finding tasks file: ${error.message}`); return { diff --git a/mcp-server/src/core/direct-functions/update-tasks.js b/mcp-server/src/core/direct-functions/update-tasks.js index 7a5e925e..6817ba22 100644 --- a/mcp-server/src/core/direct-functions/update-tasks.js +++ b/mcp-server/src/core/direct-functions/update-tasks.js @@ -22,9 +22,7 @@ import { * @param {Object} context - Context object containing session data. * @returns {Promise} - Result object with success status and data/error information. */ -export async function updateTasksDirect(args, log, context = {}) { - const { session } = context; // Only extract session, not reportProgress - +export async function updateTasksDirect(args, log, { session }) { try { log.info(`Updating tasks with args: ${JSON.stringify(args)}`); @@ -88,7 +86,7 @@ export async function updateTasksDirect(args, log, context = {}) { // Get tasks file path let tasksPath; try { - tasksPath = findTasksJsonPath(args, log); + tasksPath = findTasksJsonPath(args, log, session); } catch (error) { log.error(`Error finding tasks file: ${error.message}`); return { diff --git a/mcp-server/src/core/direct-functions/validate-dependencies.js b/mcp-server/src/core/direct-functions/validate-dependencies.js index 487aa08e..37a6b240 100644 --- a/mcp-server/src/core/direct-functions/validate-dependencies.js +++ b/mcp-server/src/core/direct-functions/validate-dependencies.js @@ -18,12 +18,12 @@ import fs from 'fs'; * @param {Object} log - Logger object * @returns {Promise<{success: boolean, data?: Object, error?: {code: string, message: string}}>} */ -export async function validateDependenciesDirect(args, log) { +export async function validateDependenciesDirect(args, log, { session }) { try { log.info(`Validating dependencies in tasks...`); // Find the tasks.json path - const tasksPath = findTasksJsonPath(args, log); + const tasksPath = findTasksJsonPath(args, log, session); // Verify the file exists if (!fs.existsSync(tasksPath)) { diff --git a/mcp-server/src/core/utils/path-utils.js b/mcp-server/src/core/utils/path-utils.js index 5ede8d1b..ced5dd4c 100644 --- a/mcp-server/src/core/utils/path-utils.js +++ b/mcp-server/src/core/utils/path-utils.js @@ -12,11 +12,11 @@ import path from 'path'; import fs from 'fs'; import { fileURLToPath } from 'url'; import os from 'os'; +// Removed lastFoundProjectRoot as it's not suitable for MCP server +// Assuming getProjectRootFromSession is available +import { getProjectRootFromSession } from '../../tools/utils.js'; -// Store last found project root to improve performance on subsequent calls (primarily for CLI) -export let lastFoundProjectRoot = null; - -// Project marker files that indicate a potential project root +// Project marker files that indicate a potential project root (can be kept for potential future use or logging) export const PROJECT_MARKERS = [ // Task Master specific 'tasks.json', @@ -75,109 +75,142 @@ export function getPackagePath() { } /** - * Finds the absolute path to the tasks.json file based on project root and arguments. + * Finds the absolute path to the tasks.json file and returns the validated project root. + * Determines the project root using args and session, validates it, searches for tasks.json. + * * @param {Object} args - Command arguments, potentially including 'projectRoot' and 'file'. * @param {Object} log - Logger object. - * @returns {string} - Absolute path to the tasks.json file. - * @throws {Error} - If tasks.json cannot be found. + * @param {Object} session - MCP session object. + * @returns {Promise<{tasksPath: string, validatedProjectRoot: string}>} - Object containing absolute path to tasks.json and the validated root. + * @throws {Error} - If a valid project root cannot be determined or tasks.json cannot be found. */ -export function findTasksJsonPath(args, log) { - // PRECEDENCE ORDER for finding tasks.json: - // 1. Explicitly provided `projectRoot` in args (Highest priority, expected in MCP context) - // 2. Previously found/cached `lastFoundProjectRoot` (primarily for CLI performance) - // 3. Search upwards from current working directory (`process.cwd()`) - CLI usage +export function findTasksJsonPath(args, log, session) { + const homeDir = os.homedir(); + let targetDirectory = null; + let rootSource = 'unknown'; - // 1. If project root is explicitly provided (e.g., from MCP session), use it directly - if (args.projectRoot) { - const projectRoot = args.projectRoot; - log.info(`Using explicitly provided project root: ${projectRoot}`); - try { - // This will throw if tasks.json isn't found within this root - return findTasksJsonInDirectory(projectRoot, args.file, log); - } catch (error) { - // Include debug info in error - const debugInfo = { - projectRoot, - currentDir: process.cwd(), - serverDir: path.dirname(process.argv[1]), - possibleProjectRoot: path.resolve( - path.dirname(process.argv[1]), - '../..' - ), - lastFoundProjectRoot, - searchedPaths: error.message - }; - - error.message = `Tasks file not found in any of the expected locations relative to project root "${projectRoot}" (from session).\nDebug Info: ${JSON.stringify(debugInfo, null, 2)}`; - throw error; - } - } - - // --- Fallback logic primarily for CLI or when projectRoot isn't passed --- - - // 2. If we have a last known project root that worked, try it first - if (lastFoundProjectRoot) { - log.info(`Trying last known project root: ${lastFoundProjectRoot}`); - try { - // Use the cached root - const tasksPath = findTasksJsonInDirectory( - lastFoundProjectRoot, - args.file, - log - ); - return tasksPath; // Return if found in cached root - } catch (error) { - log.info( - `Task file not found in last known project root, continuing search.` - ); - // Continue with search if not found in cache - } - } - - // 3. Start search from current directory (most common CLI scenario) - const startDir = process.cwd(); log.info( - `Searching for tasks.json starting from current directory: ${startDir}` + `Finding tasks.json path. Args: ${JSON.stringify(args)}, Session available: ${!!session}` ); - // Try to find tasks.json by walking up the directory tree from cwd + // --- Determine Target Directory --- + if ( + args.projectRoot && + args.projectRoot !== '/' && + args.projectRoot !== homeDir + ) { + log.info(`Using projectRoot directly from args: ${args.projectRoot}`); + targetDirectory = args.projectRoot; + rootSource = 'args.projectRoot'; + } else { + log.warn( + `args.projectRoot ('${args.projectRoot}') is missing or invalid. Attempting to derive from session.` + ); + const sessionDerivedPath = getProjectRootFromSession(session, log); + if ( + sessionDerivedPath && + sessionDerivedPath !== '/' && + sessionDerivedPath !== homeDir + ) { + log.info( + `Using project root derived from session: ${sessionDerivedPath}` + ); + targetDirectory = sessionDerivedPath; + rootSource = 'session'; + } else { + log.error( + `Could not derive a valid project root from session. Session path='${sessionDerivedPath}'` + ); + } + } + + // --- Validate the final targetDirectory --- + if (!targetDirectory) { + const error = new Error( + `Cannot find tasks.json: Could not determine a valid project root directory. Please ensure a workspace/folder is open or specify projectRoot.` + ); + error.code = 'INVALID_PROJECT_ROOT'; + error.details = { + attemptedArgsProjectRoot: args.projectRoot, + sessionAvailable: !!session, + // Add session derived path attempt for better debugging + attemptedSessionDerivedPath: getProjectRootFromSession(session, { + info: () => {}, + warn: () => {}, + error: () => {} + }), // Call again silently for details + finalDeterminedRoot: targetDirectory // Will be null here + }; + log.error(`Validation failed: ${error.message}`, error.details); + throw error; + } + + // --- Verify targetDirectory exists --- + if (!fs.existsSync(targetDirectory)) { + const error = new Error( + `Determined project root directory does not exist: ${targetDirectory}` + ); + error.code = 'PROJECT_ROOT_NOT_FOUND'; + error.details = { + /* ... add details ... */ + }; + log.error(error.message, error.details); + throw error; + } + if (!fs.statSync(targetDirectory).isDirectory()) { + const error = new Error( + `Determined project root path is not a directory: ${targetDirectory}` + ); + error.code = 'PROJECT_ROOT_NOT_A_DIRECTORY'; + error.details = { + /* ... add details ... */ + }; + log.error(error.message, error.details); + throw error; + } + + // --- Search within the validated targetDirectory --- + log.info( + `Validated project root (${rootSource}): ${targetDirectory}. Searching for tasks file.` + ); try { - // This will throw if not found in the CWD tree - return findTasksJsonWithParentSearch(startDir, args.file, log); + const tasksPath = findTasksJsonInDirectory(targetDirectory, args.file, log); + // Return both the tasks path and the validated root + return { tasksPath: tasksPath, validatedProjectRoot: targetDirectory }; } catch (error) { - // If all attempts fail, augment and throw the original error from CWD search - error.message = `${error.message}\n\nPossible solutions:\n1. Run the command from your project directory containing tasks.json\n2. Use --project-root=/path/to/project to specify the project location (if using CLI)\n3. Ensure the project root is correctly passed from the client (if using MCP)\n\nCurrent working directory: ${startDir}\nLast known project root: ${lastFoundProjectRoot}\nProject root from args: ${args.projectRoot}`; + // Augment the error + error.message = `Tasks file not found within validated project root "${targetDirectory}" (source: ${rootSource}). Ensure 'tasks.json' exists at the root or in a 'tasks/' subdirectory.\nOriginal Error: ${error.message}`; + error.details = { + ...(error.details || {}), // Keep original details if any + validatedProjectRoot: targetDirectory, + rootSource: rootSource, + attemptedArgsProjectRoot: args.projectRoot, + sessionAvailable: !!session + }; + log.error(`Search failed: ${error.message}`, error.details); throw error; } } /** - * Check if a directory contains any project marker files or directories - * @param {string} dirPath - Directory to check - * @returns {boolean} - True if the directory contains any project markers - */ -function hasProjectMarkers(dirPath) { - return PROJECT_MARKERS.some((marker) => { - const markerPath = path.join(dirPath, marker); - // Check if the marker exists as either a file or directory - return fs.existsSync(markerPath); - }); -} - -/** - * Search for tasks.json in a specific directory - * @param {string} dirPath - Directory to search in - * @param {string} explicitFilePath - Optional explicit file path relative to dirPath + * Search for tasks.json in a specific directory (now assumes dirPath is a validated project root) + * @param {string} dirPath - The validated project root directory to search in. + * @param {string} explicitFilePath - Optional explicit file path relative to dirPath (e.g., args.file) * @param {Object} log - Logger object * @returns {string} - Absolute path to tasks.json - * @throws {Error} - If tasks.json cannot be found + * @throws {Error} - If tasks.json cannot be found in the standard locations within dirPath. */ function findTasksJsonInDirectory(dirPath, explicitFilePath, log) { const possiblePaths = []; - // 1. If a file is explicitly provided relative to dirPath + // 1. If an explicit file path is provided (relative to dirPath) if (explicitFilePath) { - possiblePaths.push(path.resolve(dirPath, explicitFilePath)); + // Ensure it's treated as relative to the project root if not absolute + const resolvedExplicitPath = path.isAbsolute(explicitFilePath) + ? explicitFilePath + : path.resolve(dirPath, explicitFilePath); + possiblePaths.push(resolvedExplicitPath); + log.info(`Explicit file path provided, checking: ${resolvedExplicitPath}`); } // 2. Check the standard locations relative to dirPath @@ -186,108 +219,152 @@ function findTasksJsonInDirectory(dirPath, explicitFilePath, log) { path.join(dirPath, 'tasks', 'tasks.json') ); - log.info(`Checking potential task file paths: ${possiblePaths.join(', ')}`); + // Deduplicate paths in case explicitFilePath matches a standard location + const uniquePaths = [...new Set(possiblePaths)]; + + log.info( + `Checking for tasks file in validated root ${dirPath}. Potential paths: ${uniquePaths.join(', ')}` + ); // Find the first existing path - for (const p of possiblePaths) { - log.info(`Checking if exists: ${p}`); + for (const p of uniquePaths) { + // log.info(`Checking if exists: ${p}`); // Can reduce verbosity const exists = fs.existsSync(p); - log.info(`Path ${p} exists: ${exists}`); + // log.info(`Path ${p} exists: ${exists}`); // Can reduce verbosity if (exists) { log.info(`Found tasks file at: ${p}`); - // Store the project root for future use - lastFoundProjectRoot = dirPath; + // No need to set lastFoundProjectRoot anymore return p; } } // If no file was found, throw an error const error = new Error( - `Tasks file not found in any of the expected locations relative to ${dirPath}: ${possiblePaths.join(', ')}` + `Tasks file not found in any of the expected locations within directory ${dirPath}: ${uniquePaths.join(', ')}` ); - error.code = 'TASKS_FILE_NOT_FOUND'; + error.code = 'TASKS_FILE_NOT_FOUND_IN_ROOT'; + error.details = { searchedDirectory: dirPath, checkedPaths: uniquePaths }; throw error; } +// Removed findTasksJsonWithParentSearch, hasProjectMarkers, and findTasksWithNpmConsideration +// as the project root is now determined upfront and validated. + +/** + * Resolves a relative path against the project root, ensuring it's within the project. + * @param {string} relativePath - The relative path (e.g., 'scripts/report.json'). + * @param {string} projectRoot - The validated absolute path to the project root. + * @param {Object} log - Logger object. + * @returns {string} - The absolute path. + * @throws {Error} - If the resolved path is outside the project root or resolution fails. + */ +export function resolveProjectPath(relativePath, projectRoot, log) { + if (!projectRoot || !path.isAbsolute(projectRoot)) { + log.error( + `Cannot resolve project path: Invalid projectRoot provided: ${projectRoot}` + ); + throw new Error( + `Internal Error: Cannot resolve project path due to invalid projectRoot: ${projectRoot}` + ); + } + if (!relativePath || typeof relativePath !== 'string') { + log.error( + `Cannot resolve project path: Invalid relativePath provided: ${relativePath}` + ); + throw new Error( + `Internal Error: Cannot resolve project path due to invalid relativePath: ${relativePath}` + ); + } + + // If relativePath is already absolute, check if it's within the project root + if (path.isAbsolute(relativePath)) { + if (!relativePath.startsWith(projectRoot)) { + log.error( + `Path Security Violation: Absolute path \"${relativePath}\" provided is outside the project root \"${projectRoot}\"` + ); + throw new Error( + `Provided absolute path is outside the project directory: ${relativePath}` + ); + } + log.info( + `Provided path is already absolute and within project root: ${relativePath}` + ); + return relativePath; // Return as is if valid absolute path within project + } + + // Resolve relative path against project root + const absolutePath = path.resolve(projectRoot, relativePath); + + // Security check: Ensure the resolved path is still within the project root boundary + // Normalize paths to handle potential .. usages properly before comparison + const normalizedAbsolutePath = path.normalize(absolutePath); + const normalizedProjectRoot = path.normalize(projectRoot + path.sep); // Ensure trailing separator for accurate startsWith check + + if ( + !normalizedAbsolutePath.startsWith(normalizedProjectRoot) && + normalizedAbsolutePath !== path.normalize(projectRoot) + ) { + log.error( + `Path Security Violation: Resolved path \"${normalizedAbsolutePath}\" is outside project root \"${normalizedProjectRoot}\"` + ); + throw new Error( + `Resolved path is outside the project directory: ${relativePath}` + ); + } + + log.info(`Resolved project path: \"${relativePath}\" -> \"${absolutePath}\"`); + return absolutePath; +} + /** - * Recursively search for tasks.json in the given directory and parent directories - * Also looks for project markers to identify potential project roots - * @param {string} startDir - Directory to start searching from - * @param {string} explicitFilePath - Optional explicit file path - * @param {Object} log - Logger object - * @returns {string} - Absolute path to tasks.json - * @throws {Error} - If tasks.json cannot be found in any parent directory + * Ensures a directory exists, creating it if necessary. + * Also verifies that if the path already exists, it is indeed a directory. + * @param {string} dirPath - The absolute path to the directory. + * @param {Object} log - Logger object. */ -function findTasksJsonWithParentSearch(startDir, explicitFilePath, log) { - let currentDir = startDir; - const rootDir = path.parse(currentDir).root; +export function ensureDirectoryExists(dirPath, log) { + // Validate dirPath is an absolute path before proceeding + if (!path.isAbsolute(dirPath)) { + log.error( + `Cannot ensure directory: Path provided is not absolute: ${dirPath}` + ); + throw new Error( + `Internal Error: ensureDirectoryExists requires an absolute path.` + ); + } - // Keep traversing up until we hit the root directory - while (currentDir !== rootDir) { - // First check for tasks.json directly + if (!fs.existsSync(dirPath)) { + log.info(`Directory does not exist, creating recursively: ${dirPath}`); try { - return findTasksJsonInDirectory(currentDir, explicitFilePath, log); + fs.mkdirSync(dirPath, { recursive: true }); + log.info(`Successfully created directory: ${dirPath}`); } catch (error) { - // If tasks.json not found but the directory has project markers, - // log it as a potential project root (helpful for debugging) - if (hasProjectMarkers(currentDir)) { - log.info(`Found project markers in ${currentDir}, but no tasks.json`); - } - - // Move up to parent directory - const parentDir = path.dirname(currentDir); - - // Check if we've reached the root - if (parentDir === currentDir) { - break; - } - - log.info( - `Tasks file not found in ${currentDir}, searching in parent directory: ${parentDir}` + log.error(`Failed to create directory ${dirPath}: ${error.message}`); + // Re-throw the error after logging + throw new Error( + `Could not create directory: ${dirPath}. Reason: ${error.message}` ); - currentDir = parentDir; } - } - - // If we've searched all the way to the root and found nothing - const error = new Error( - `Tasks file not found in ${startDir} or any parent directory.` - ); - error.code = 'TASKS_FILE_NOT_FOUND'; - throw error; -} - -// Note: findTasksWithNpmConsideration is not used by findTasksJsonPath and might be legacy or used elsewhere. -// If confirmed unused, it could potentially be removed in a separate cleanup. -function findTasksWithNpmConsideration(startDir, log) { - // First try our recursive parent search from cwd - try { - return findTasksJsonWithParentSearch(startDir, null, log); - } catch (error) { - // If that fails, try looking relative to the executable location - const execPath = process.argv[1]; - const execDir = path.dirname(execPath); - log.info(`Looking for tasks file relative to executable at: ${execDir}`); - + } else { + // Path exists, verify it's a directory try { - return findTasksJsonWithParentSearch(execDir, null, log); - } catch (secondError) { - // If that also fails, check standard locations in user's home directory - const homeDir = os.homedir(); - log.info(`Looking for tasks file in home directory: ${homeDir}`); - - try { - // Check standard locations in home dir - return findTasksJsonInDirectory( - path.join(homeDir, '.task-master'), - null, - log + const stats = fs.statSync(dirPath); + if (!stats.isDirectory()) { + log.error(`Path exists but is not a directory: ${dirPath}`); + throw new Error( + `Expected directory but found file at path: ${dirPath}` ); - } catch (thirdError) { - // If all approaches fail, throw the original error - throw error; } + log.info(`Directory already exists and is valid: ${dirPath}`); + } catch (error) { + // Handle potential errors from statSync (e.g., permissions) or the explicit throw above + log.error( + `Error checking existing directory ${dirPath}: ${error.message}` + ); + throw new Error( + `Error verifying existing directory: ${dirPath}. Reason: ${error.message}` + ); } } } diff --git a/mcp-server/src/tools/analyze.js b/mcp-server/src/tools/analyze.js index 4f3fa087..33993973 100644 --- a/mcp-server/src/tools/analyze.js +++ b/mcp-server/src/tools/analyze.js @@ -6,8 +6,8 @@ import { z } from 'zod'; import { handleApiResult, - createErrorResponse, - getProjectRootFromSession + createErrorResponse + // getProjectRootFromSession // No longer needed here } from './utils.js'; import { analyzeTaskComplexityDirect } from '../core/task-master-core.js'; @@ -19,19 +19,18 @@ export function registerAnalyzeTool(server) { server.addTool({ name: 'analyze_project_complexity', description: - 'Analyze task complexity and generate expansion recommendations', + 'Analyze task complexity and generate expansion recommendations. Requires the project root path.', parameters: z.object({ + projectRoot: z + .string() + .describe( + 'Required. Absolute path to the root directory of the project being analyzed.' + ), output: z .string() .optional() .describe( - 'Output file path for the report (default: scripts/task-complexity-report.json)' - ), - model: z - .string() - .optional() - .describe( - 'LLM model to use for analysis (defaults to configured model)' + 'Output file path for the report, relative to projectRoot (default: scripts/task-complexity-report.json)' ), threshold: z.coerce .number() @@ -39,62 +38,43 @@ export function registerAnalyzeTool(server) { .max(10) .optional() .describe( - 'Minimum complexity score to recommend expansion (1-10) (default: 5)' - ), - file: z - .string() - .optional() - .describe( - 'Absolute path to the tasks file (default: tasks/tasks.json)' + 'Minimum complexity score to recommend expansion (1-10) (default: 5). If the complexity score is below this threshold, the tool will not recommend adding subtasks.' ), research: z .boolean() .optional() - .describe('Use Perplexity AI for research-backed complexity analysis'), - projectRoot: z - .string() - .optional() - .describe( - 'Root directory of the project (default: current working directory)' - ) + .describe('Use Perplexity AI for research-backed complexity analysis') }), execute: async (args, { log, session }) => { try { log.info( - `Analyzing task complexity with args: ${JSON.stringify(args)}` + `Analyzing task complexity with required projectRoot: ${args.projectRoot}, other args: ${JSON.stringify(args)}` ); - let rootFolder = getProjectRootFromSession(session, log); + const result = await analyzeTaskComplexityDirect(args, log, { + session + }); - if (!rootFolder && args.projectRoot) { - rootFolder = args.projectRoot; - log.info(`Using project root from args as fallback: ${rootFolder}`); - } - - const result = await analyzeTaskComplexityDirect( - { - projectRoot: rootFolder, - ...args - }, - log, - { session } - ); - - if (result.success) { + if (result.success && result.data) { log.info(`Task complexity analysis complete: ${result.data.message}`); log.info( `Report summary: ${JSON.stringify(result.data.reportSummary)}` ); - } else { + } else if (!result.success && result.error) { log.error( - `Failed to analyze task complexity: ${result.error.message}` + `Failed to analyze task complexity: ${result.error.message} (Code: ${result.error.code})` ); } return handleApiResult(result, log, 'Error analyzing task complexity'); } catch (error) { - log.error(`Error in analyze tool: ${error.message}`); - return createErrorResponse(error.message); + log.error( + `Unexpected error in analyze tool execute method: ${error.message}`, + { stack: error.stack } + ); + return createErrorResponse( + `Unexpected error in analyze tool: ${error.message}` + ); } } }); diff --git a/mcp-server/src/tools/utils.js b/mcp-server/src/tools/utils.js index 571030e0..7ef4ccfe 100644 --- a/mcp-server/src/tools/utils.js +++ b/mcp-server/src/tools/utils.js @@ -9,10 +9,7 @@ import fs from 'fs'; import { contextManager } from '../core/context-manager.js'; // Import the singleton // Import path utilities to ensure consistent path resolution -import { - lastFoundProjectRoot, - PROJECT_MARKERS -} from '../core/utils/path-utils.js'; +import { PROJECT_MARKERS } from '../core/utils/path-utils.js'; /** * Get normalized project root path diff --git a/scripts/init.js b/scripts/init.js index c609874b..9bce6f0e 100755 --- a/scripts/init.js +++ b/scripts/init.js @@ -909,7 +909,7 @@ function setupMCPConfiguration(targetDir, projectName) { const newMCPServer = { 'task-master-ai': { command: 'npx', - args: ['-y', '--package', 'task-master-ai', 'task-master-mcp'], + args: ['-y', 'task-master-mcp'], env: { ANTHROPIC_API_KEY: '%ANTHROPIC_API_KEY%', PERPLEXITY_API_KEY: '%PERPLEXITY_API_KEY%',