From ad89253e313a395637aa48b9f92cc39b1ef94ad8 Mon Sep 17 00:00:00 2001 From: Eyal Toledano Date: Fri, 2 May 2025 01:54:24 -0400 Subject: [PATCH] refactor(mcp): introduce withNormalizedProjectRoot HOF for path normalization Added HOF to mcp tools utils to normalize projectRoot from args/session. Refactored get-task tool to use HOF. Updated relevant documentation. --- .changeset/curvy-candies-eat.md | 9 + .cursor/rules/architecture.mdc | 5 +- .cursor/rules/mcp.mdc | 92 +++--- .cursor/rules/new_features.mdc | 29 +- .cursor/rules/utilities.mdc | 79 ++++-- .../src/core/direct-functions/show-task.js | 175 +++++------- mcp-server/src/tools/get-task.js | 64 ++--- mcp-server/src/tools/utils.js | 262 ++++++++++++++---- 8 files changed, 440 insertions(+), 275 deletions(-) create mode 100644 .changeset/curvy-candies-eat.md diff --git a/.changeset/curvy-candies-eat.md b/.changeset/curvy-candies-eat.md new file mode 100644 index 00000000..9b935715 --- /dev/null +++ b/.changeset/curvy-candies-eat.md @@ -0,0 +1,9 @@ +--- +'task-master-ai': patch +--- + +Better support for file paths on Windows, Linux & WSL. + +- Standardizes handling of different path formats (URI encoded, Windows, Linux, WSL). +- Ensures tools receive a clean, absolute path suitable for the server OS. +- Simplifies tool implementation by centralizing normalization logic. diff --git a/.cursor/rules/architecture.mdc b/.cursor/rules/architecture.mdc index d0224fe7..68f32ab5 100644 --- a/.cursor/rules/architecture.mdc +++ b/.cursor/rules/architecture.mdc @@ -65,8 +65,9 @@ alwaysApply: false - **[`mcp-server/`](mdc:mcp-server/): MCP Server Integration** - **Purpose**: Provides MCP interface using FastMCP. - **Responsibilities** (See also: [`mcp.mdc`](mdc:.cursor/rules/mcp.mdc)): - - Registers tools (`mcp-server/src/tools/*.js`). - - Tool `execute` methods call **direct function wrappers** (`mcp-server/src/core/direct-functions/*.js`). + - Registers tools (`mcp-server/src/tools/*.js`). Tool `execute` methods **should be wrapped** with the `withNormalizedProjectRoot` HOF (from `tools/utils.js`) to ensure consistent path handling. + - The HOF provides a normalized `args.projectRoot` to the `execute` method. + - Tool `execute` methods call **direct function wrappers** (`mcp-server/src/core/direct-functions/*.js`), passing the normalized `projectRoot` and other args. - Direct functions use path utilities (`mcp-server/src/core/utils/`) to resolve paths based on `projectRoot` from session. - Direct functions implement silent mode, logger wrappers, and call core logic functions from `scripts/modules/`. - Manages MCP caching and response formatting. diff --git a/.cursor/rules/mcp.mdc b/.cursor/rules/mcp.mdc index 896fed28..ebacd578 100644 --- a/.cursor/rules/mcp.mdc +++ b/.cursor/rules/mcp.mdc @@ -188,58 +188,70 @@ execute: async (args, { log, session }) => { - **args**: Validated parameters. - **context**: Contains `{ log, session }` from FastMCP. (Removed `reportProgress`). -### Standard Tool Execution Pattern +### Standard Tool Execution Pattern with Path Normalization (Updated) -The `execute` method within each MCP tool (in `mcp-server/src/tools/*.js`) should follow this standard pattern: +To ensure consistent handling of project paths across different client environments (Windows, macOS, Linux, WSL) and input formats (e.g., `file:///...`, URI encoded paths), all MCP tool `execute` methods that require access to the project root **MUST** be wrapped with the `withNormalizedProjectRoot` Higher-Order Function (HOF). -1. **Log Entry**: Log the start of the tool execution with relevant arguments. -2. **Get Project Root**: Use the `getProjectRootFromSession(session, log)` utility (from [`tools/utils.js`](mdc:mcp-server/src/tools/utils.js)) to extract the project root path from the client session. Fall back to `args.projectRoot` if the session doesn't provide a root. -3. **Call Direct Function**: Invoke the corresponding `*Direct` function wrapper (e.g., `listTasksDirect` from [`task-master-core.js`](mdc:mcp-server/src/core/task-master-core.js)), passing an updated `args` object that includes the resolved `projectRoot`. Crucially, the third argument (context) passed to the direct function should **only include `{ log, session }`**. **Do NOT pass `reportProgress`**. - ```javascript - // Example call (applies to both AI and non-AI direct functions now) - const result = await someDirectFunction( - { ...args, projectRoot }, // Args including resolved root - log, // MCP logger - { session } // Context containing session - ); - ``` -4. **Handle Result**: Receive the result object (`{ success, data/error, fromCache }`) from the `*Direct` function. -5. **Format Response**: Pass this result object to the `handleApiResult` utility (from [`tools/utils.js`](mdc:mcp-server/src/tools/utils.js)) for standardized MCP response formatting and error handling. -6. **Return**: Return the formatted response object provided by `handleApiResult`. +This HOF, defined in [`mcp-server/src/tools/utils.js`](mdc:mcp-server/src/tools/utils.js), performs the following before calling the tool's core logic: + +1. **Determines the Raw Root:** It prioritizes `args.projectRoot` if provided by the client, otherwise it calls `getRawProjectRootFromSession` to extract the path from the session. +2. **Normalizes the Path:** It uses the `normalizeProjectRoot` helper to decode URIs, strip `file://` prefixes, fix potential Windows drive letter prefixes (e.g., `/C:/`), convert backslashes (`\`) to forward slashes (`/`), and resolve the path to an absolute path suitable for the server's OS. +3. **Injects Normalized Path:** It updates the `args` object by replacing the original `projectRoot` (or adding it) with the normalized, absolute path. +4. **Executes Original Logic:** It calls the original `execute` function body, passing the updated `args` object. + +**Implementation Example:** ```javascript -// Example execute method structure for a tool calling an AI-based direct function -import { getProjectRootFromSession, handleApiResult, createErrorResponse } from './utils.js'; -import { someAIDirectFunction } from '../core/task-master-core.js'; +// In mcp-server/src/tools/your-tool.js +import { + handleApiResult, + createErrorResponse, + withNormalizedProjectRoot // <<< Import HOF +} from './utils.js'; +import { yourDirectFunction } from '../core/task-master-core.js'; +import { findTasksJsonPath } from '../core/utils/path-utils.js'; // If needed -// ... inside server.addTool({...}) -execute: async (args, { log, session }) => { // Note: reportProgress is omitted here - try { - log.info(`Starting AI tool execution with args: ${JSON.stringify(args)}`); +export function registerYourTool(server) { + server.addTool({ + name: "your_tool", + description: "...". + parameters: z.object({ + // ... other parameters ... + projectRoot: z.string().optional().describe('...') // projectRoot is optional here, HOF handles fallback + }), + // Wrap the entire execute function + execute: withNormalizedProjectRoot(async (args, { log, session }) => { + // args.projectRoot is now guaranteed to be normalized and absolute + const { /* other args */, projectRoot } = args; - // 1. Get Project Root - let rootFolder = getProjectRootFromSession(session, log); - if (!rootFolder && args.projectRoot) { // Fallback if needed - rootFolder = args.projectRoot; - log.info(`Using project root from args as fallback: ${rootFolder}`); - } + try { + log.info(`Executing your_tool with normalized root: ${projectRoot}`); - // 2. Call AI-Based Direct Function (passing only log and session in context) - const result = await someAIDirectFunction({ - ...args, - projectRoot: rootFolder // Ensure projectRoot is explicitly passed - }, log, { session }); // Pass session here, NO reportProgress + // Resolve paths using the normalized projectRoot + let tasksPath = findTasksJsonPath({ projectRoot, file: args.file }, log); - // 3. Handle and Format Response - return handleApiResult(result, log); + // Call direct function, passing normalized projectRoot if needed by direct func + const result = await yourDirectFunction( + { + /* other args */, + projectRoot // Pass it if direct function needs it + }, + log, + { session } + ); - } catch (error) { - log.error(`Error during AI tool execution: ${error.message}`); - return createErrorResponse(error.message); - } + return handleApiResult(result, log); + } catch (error) { + log.error(`Error in your_tool: ${error.message}`); + return createErrorResponse(error.message); + } + }) // End HOF wrap + }); } ``` +By using this HOF, the core logic within the `execute` method and any downstream functions (like `findTasksJsonPath` or direct functions) can reliably expect `args.projectRoot` to be a clean, absolute path suitable for the server environment. + ### Project Initialization Tool The `initialize_project` tool allows integrated clients like Cursor to set up a new Task Master project: diff --git a/.cursor/rules/new_features.mdc b/.cursor/rules/new_features.mdc index ff9d2bf3..f6a696f1 100644 --- a/.cursor/rules/new_features.mdc +++ b/.cursor/rules/new_features.mdc @@ -523,14 +523,24 @@ Integrating Task Master commands with the MCP server (for use by tools like Curs 4. **Create MCP Tool (`mcp-server/src/tools/`)**: - Create a new file (e.g., `your-command.js`) using **kebab-case**. - - Import `zod`, `handleApiResult`, `createErrorResponse`, **`getProjectRootFromSession`**, and your `yourCommandDirect` function. + - Import `zod`, `handleApiResult`, **`withNormalizedProjectRoot` HOF**, and your `yourCommandDirect` function. - Implement `registerYourCommandTool(server)`. - - Define the tool `name` using **snake_case** (e.g., `your_command`). - - Define the `parameters` using `zod`. **Crucially, define `projectRoot` as optional**: `projectRoot: z.string().optional().describe(...)`. Include `file` if applicable. - - Implement the standard `async execute(args, { log, reportProgress, session })` method: - - Get `rootFolder` using `getProjectRootFromSession` (with fallback to `args.projectRoot`). - - Call `yourCommandDirect({ ...args, projectRoot: rootFolder }, log)`. - - Pass the result to `handleApiResult(result, log, 'Error Message')`. + - **Define parameters**: Make `projectRoot` optional (`z.string().optional().describe(...)`) as the HOF handles fallback. + - Consider if this operation should run in the background using `AsyncOperationManager`. + - Implement the standard `execute` method **wrapped with `withNormalizedProjectRoot`**: + ```javascript + execute: withNormalizedProjectRoot(async (args, { log, session }) => { + // args.projectRoot is now normalized + const { projectRoot /*, other args */ } = args; + // ... resolve tasks path if needed using normalized projectRoot ... + const result = await yourCommandDirect( + { /* other args */, projectRoot /* if needed by direct func */ }, + log, + { session } + ); + return handleApiResult(result, log); + }) + ``` 5. **Register Tool**: Import and call `registerYourCommandTool` in `mcp-server/src/tools/index.js`. @@ -618,8 +628,3 @@ When implementing project initialization commands: }); } ``` - - } - }); - } - ``` diff --git a/.cursor/rules/utilities.mdc b/.cursor/rules/utilities.mdc index 72b72942..90b0be31 100644 --- a/.cursor/rules/utilities.mdc +++ b/.cursor/rules/utilities.mdc @@ -428,36 +428,69 @@ Taskmaster configuration (excluding API keys) is primarily managed through the ` ## MCP Server Tool Utilities (`mcp-server/src/tools/utils.js`) -- **Purpose**: These utilities specifically support the MCP server tools ([`mcp-server/src/tools/*.js`](mdc:mcp-server/src/tools/*.js)), handling MCP communication patterns, response formatting, caching integration, and the CLI fallback mechanism. -- **Refer to [`mcp.mdc`](mdc:.cursor/rules/mcp.mdc)** for detailed usage patterns within the MCP tool `execute` methods and direct function wrappers. +These utilities specifically support the implementation and execution of MCP tools. -- **`getProjectRootFromSession(session, log)`**: - - ✅ **DO**: Call this utility **within the MCP tool's `execute` method** to extract the project root path from the `session` object. - - Decodes the `file://` URI and handles potential errors. - - Returns the project path string or `null`. - - The returned path should then be passed in the `args` object when calling the corresponding `*Direct` function (e.g., `yourDirectFunction({ ...args, projectRoot: rootFolder }, log)`). +- **`normalizeProjectRoot(rawPath, log)`**: + - **Purpose**: Takes a raw project root path (potentially URI encoded, with `file://` prefix, Windows slashes) and returns a normalized, absolute path suitable for the server's OS. + - **Logic**: Decodes URI, strips `file://`, handles Windows drive prefix (`/C:/`), replaces `\` with `/`, uses `path.resolve()`. + - **Usage**: Used internally by `withNormalizedProjectRoot` HOF. + +- **`getRawProjectRootFromSession(session, log)`**: + - **Purpose**: Extracts the *raw* project root URI string from the session object (`session.roots[0].uri` or `session.roots.roots[0].uri`) without performing normalization. + - **Usage**: Used internally by `withNormalizedProjectRoot` HOF as a fallback if `args.projectRoot` isn't provided. + +- **`withNormalizedProjectRoot(executeFn)`**: + - **Purpose**: A Higher-Order Function (HOF) designed to wrap a tool's `execute` method. + - **Logic**: + 1. Determines the raw project root (from `args.projectRoot` or `getRawProjectRootFromSession`). + 2. Normalizes the raw path using `normalizeProjectRoot`. + 3. Injects the normalized, absolute path back into the `args` object as `args.projectRoot`. + 4. Calls the original `executeFn` with the updated `args`. + - **Usage**: Should wrap the `execute` function of *every* MCP tool that needs a reliable, normalized project root path. + - **Example**: + ```javascript + // In mcp-server/src/tools/your-tool.js + import { withNormalizedProjectRoot } from './utils.js'; + + export function registerYourTool(server) { + server.addTool({ + // ... name, description, parameters ... + execute: withNormalizedProjectRoot(async (args, context) => { + // args.projectRoot is now normalized here + const { projectRoot /*, other args */ } = args; + // ... rest of tool logic using normalized projectRoot ... + }) + }); + } + ``` - **`handleApiResult(result, log, errorPrefix, processFunction)`**: - - ✅ **DO**: Call this from the MCP tool's `execute` method after receiving the result from the `*Direct` function wrapper. - - Takes the standard `{ success, data/error, fromCache }` object. - - Formats the standard MCP success or error response, including the `fromCache` flag. - - Uses `processMCPResponseData` by default to filter response data. - -- **`executeTaskMasterCommand(command, log, args, projectRootRaw)`**: - - Executes a Task Master CLI command as a child process. - - Handles fallback between global `task-master` and local `node scripts/dev.js`. - - ❌ **DON'T**: Use this as the primary method for MCP tools. Prefer direct function calls via `*Direct` wrappers. - -- **`processMCPResponseData(taskOrData, fieldsToRemove)`**: - - Filters task data (e.g., removing `details`, `testStrategy`) before sending to the MCP client. Called by `handleApiResult`. + - **Purpose**: Standardizes the formatting of responses returned by direct functions (`{ success, data/error, fromCache }`) into the MCP response format. + - **Usage**: Call this at the end of the tool's `execute` method, passing the result from the direct function call. - **`createContentResponse(content)` / `createErrorResponse(errorMessage)`**: - - Formatters for standard MCP success/error responses. + - **Purpose**: Helper functions to create the basic MCP response structure for success or error messages. + - **Usage**: Used internally by `handleApiResult` and potentially directly for simple responses. + +- **`createLogWrapper(log)`**: + - **Purpose**: Creates a logger object wrapper with standard methods (`info`, `warn`, `error`, `debug`, `success`) mapping to the passed MCP `log` object's methods. Ensures compatibility when passing loggers to core functions. + - **Usage**: Used within direct functions before passing the `log` object down to core logic that expects the standard method names. - **`getCachedOrExecute({ cacheKey, actionFn, log })`**: - - ✅ **DO**: Use this utility *inside direct function wrappers* to implement caching. - - Checks cache, executes `actionFn` on miss, stores result. - - Returns standard `{ success, data/error, fromCache: boolean }`. + - **Purpose**: Utility for implementing caching within direct functions. Checks cache for `cacheKey`; if miss, executes `actionFn`, caches successful result, and returns. + - **Usage**: Wrap the core logic execution within a direct function call. + +- **`processMCPResponseData(taskOrData, fieldsToRemove)`**: + - **Purpose**: Utility to filter potentially sensitive or large fields (like `details`, `testStrategy`) from task objects before sending the response back via MCP. + - **Usage**: Passed as the default `processFunction` to `handleApiResult`. + +- **`getProjectRootFromSession(session, log)`**: + - **Purpose**: Legacy function to extract *and normalize* the project root from the session. Replaced by the HOF pattern but potentially still used. + - **Recommendation**: Prefer using the `withNormalizedProjectRoot` HOF in tools instead of calling this directly. + +- **`executeTaskMasterCommand(...)`**: + - **Purpose**: Executes `task-master` CLI command as a fallback. + - **Recommendation**: Deprecated for most uses; prefer direct function calls. ## Export Organization diff --git a/mcp-server/src/core/direct-functions/show-task.js b/mcp-server/src/core/direct-functions/show-task.js index 37513352..a662a86b 100644 --- a/mcp-server/src/core/direct-functions/show-task.js +++ b/mcp-server/src/core/direct-functions/show-task.js @@ -3,151 +3,100 @@ * Direct function implementation for showing task details */ -import { findTaskById } from '../../../../scripts/modules/utils.js'; -import { readJSON } from '../../../../scripts/modules/utils.js'; +import { findTaskById, readJSON } from '../../../../scripts/modules/utils.js'; import { getCachedOrExecute } from '../../tools/utils.js'; import { enableSilentMode, disableSilentMode } from '../../../../scripts/modules/utils.js'; +import { findTasksJsonPath } from '../utils/path-utils.js'; /** - * Direct function wrapper for showing task details with error handling and caching. + * Direct function wrapper for getting task details. * - * @param {Object} args - Command arguments - * @param {string} args.tasksJsonPath - Explicit path to the tasks.json file. - * @param {string} args.id - The ID of the task or subtask to show. + * @param {Object} args - Command arguments. + * @param {string} args.id - Task ID to show. + * @param {string} [args.file] - Optional path to the tasks file (passed to findTasksJsonPath). * @param {string} [args.status] - Optional status to filter subtasks by. - * @param {Object} log - Logger object - * @returns {Promise} - Task details result { success: boolean, data?: any, error?: { code: string, message: string }, fromCache: boolean } + * @param {string} args.projectRoot - Absolute path to the project root directory (already normalized by tool). + * @param {Object} log - Logger object. + * @param {Object} context - Context object containing session data. + * @returns {Promise} - Result object with success status and data/error information. */ -export async function showTaskDirect(args, log) { - // Destructure expected args - const { tasksJsonPath, id, status } = args; +export async function showTaskDirect(args, log, context = {}) { + // Destructure session from context if needed later, otherwise ignore + // const { session } = context; + // Destructure projectRoot and other args. projectRoot is assumed normalized. + const { id, file, status, projectRoot } = args; - if (!tasksJsonPath) { - log.error('showTaskDirect called without tasksJsonPath'); + log.info( + `Showing task direct function. ID: ${id}, File: ${file}, Status Filter: ${status}, ProjectRoot: ${projectRoot}` + ); + + // --- Path Resolution using the passed (already normalized) projectRoot --- + let tasksJsonPath; + try { + // Use the projectRoot passed directly from args + tasksJsonPath = findTasksJsonPath( + { projectRoot: projectRoot, file: file }, + log + ); + log.info(`Resolved tasks path: ${tasksJsonPath}`); + } catch (error) { + log.error(`Error finding tasks.json: ${error.message}`); return { success: false, error: { - code: 'MISSING_ARGUMENT', - message: 'tasksJsonPath is required' - }, - fromCache: false + code: 'TASKS_FILE_NOT_FOUND', + message: `Failed to find tasks.json: ${error.message}` + } }; } + // --- End Path Resolution --- - // Validate task ID - const taskId = id; - if (!taskId) { - log.error('Task ID is required'); - return { - success: false, - error: { - code: 'INPUT_VALIDATION_ERROR', - message: 'Task ID is required' - }, - fromCache: false - }; - } - - // Generate cache key using the provided task path, ID, and status filter - const cacheKey = `showTask:${tasksJsonPath}:${taskId}:${status || 'all'}`; - - // Define the action function to be executed on cache miss - const coreShowTaskAction = async () => { - try { - // Enable silent mode to prevent console logs from interfering with JSON response - enableSilentMode(); - - log.info( - `Retrieving task details for ID: ${taskId} from ${tasksJsonPath}${status ? ` (filtering by status: ${status})` : ''}` - ); - - // Read tasks data using the provided path - const data = readJSON(tasksJsonPath); - if (!data || !data.tasks) { - disableSilentMode(); // Disable before returning - return { - success: false, - error: { - code: 'INVALID_TASKS_FILE', - message: `No valid tasks found in ${tasksJsonPath}` - } - }; - } - - // Find the specific task, passing the status filter - const { task, originalSubtaskCount } = findTaskById( - data.tasks, - taskId, - status - ); - - if (!task) { - disableSilentMode(); // Disable before returning - return { - success: false, - error: { - code: 'TASK_NOT_FOUND', - message: `Task with ID ${taskId} not found${status ? ` or no subtasks match status '${status}'` : ''}` - } - }; - } - - // Restore normal logging - disableSilentMode(); - - // Return the task data, the original subtask count (if applicable), - // and the full tasks array for reference (needed for formatDependenciesWithStatus function in UI) - log.info( - `Successfully found task ${taskId}${status ? ` (with status filter: ${status})` : ''}` - ); + // --- Rest of the function remains the same, using tasksJsonPath --- + try { + const tasksData = readJSON(tasksJsonPath); + if (!tasksData || !tasksData.tasks) { return { - success: true, - data: { - task, - originalSubtaskCount, - allTasks: data.tasks - } + success: false, + error: { code: 'INVALID_TASKS_DATA', message: 'Invalid tasks data' } }; - } catch (error) { - // Make sure to restore normal logging even if there's an error - disableSilentMode(); + } - log.error(`Error showing task: ${error.message}`); + const { task, originalSubtaskCount } = findTaskById( + tasksData.tasks, + id, + status + ); + + if (!task) { return { success: false, error: { - code: 'CORE_FUNCTION_ERROR', - message: error.message || 'Failed to show task details' + code: 'TASK_NOT_FOUND', + message: `Task or subtask with ID ${id} not found` } }; } - }; - // Use the caching utility - try { - const result = await getCachedOrExecute({ - cacheKey, - actionFn: coreShowTaskAction, - log - }); - log.info(`showTaskDirect completed. From cache: ${result.fromCache}`); - return result; // Returns { success, data/error, fromCache } + log.info(`Successfully retrieved task ${id}.`); + + const returnData = { ...task }; + if (originalSubtaskCount !== null) { + returnData._originalSubtaskCount = originalSubtaskCount; + returnData._subtaskFilter = status; + } + + return { success: true, data: returnData }; } catch (error) { - // Catch unexpected errors from getCachedOrExecute itself - disableSilentMode(); - log.error( - `Unexpected error during getCachedOrExecute for showTask: ${error.message}` - ); + log.error(`Error showing task ${id}: ${error.message}`); return { success: false, error: { - code: 'UNEXPECTED_ERROR', + code: 'TASK_OPERATION_ERROR', message: error.message - }, - fromCache: false + } }; } } diff --git a/mcp-server/src/tools/get-task.js b/mcp-server/src/tools/get-task.js index 9f530b4a..f2f95332 100644 --- a/mcp-server/src/tools/get-task.js +++ b/mcp-server/src/tools/get-task.js @@ -7,7 +7,7 @@ import { z } from 'zod'; import { handleApiResult, createErrorResponse, - getProjectRootFromSession + withNormalizedProjectRoot } from './utils.js'; import { showTaskDirect } from '../core/task-master-core.js'; import { findTasksJsonPath } from '../core/utils/path-utils.js'; @@ -21,8 +21,10 @@ function processTaskResponse(data) { if (!data) return data; // If we have the expected structure with task and allTasks - if (data.task) { - // Return only the task object, removing the allTasks array + if (typeof data === 'object' && data !== null && data.id && data.title) { + // If the data itself looks like the task object, return it + return data; + } else if (data.task) { return data.task; } @@ -44,44 +46,33 @@ export function registerShowTaskTool(server) { .string() .optional() .describe("Filter subtasks by status (e.g., 'pending', 'done')"), - file: z.string().optional().describe('Absolute path to the tasks file'), + file: z + .string() + .optional() + .describe('Path to the tasks file relative to project root'), projectRoot: z .string() - .describe('The directory of the project. Must be an absolute path.') + .optional() + .describe( + 'Absolute path to the project root directory (Optional, usually from session)' + ) }), - execute: async (args, { log, session }) => { - // Log the session right at the start of execute - log.info( - `Session object received in execute: ${JSON.stringify(session)}` - ); // Use JSON.stringify for better visibility + execute: withNormalizedProjectRoot(async (args, { log, session }) => { + const { id, file, status, projectRoot } = args; try { log.info( - `Getting task details for ID: ${args.id}${args.status ? ` (filtering subtasks by status: ${args.status})` : ''}` + `Getting task details for ID: ${id}${status ? ` (filtering subtasks by status: ${status})` : ''} in root: ${projectRoot}` ); - // Get project root from args or session - const rootFolder = - args.projectRoot || getProjectRootFromSession(session, log); - - // Ensure project root was determined - if (!rootFolder) { - return createErrorResponse( - 'Could not determine project root. Please provide it explicitly or ensure your session contains valid root information.' - ); - } - - log.info(`Attempting to use project root: ${rootFolder}`); // Log the final resolved root - - log.info(`Root folder: ${rootFolder}`); // Log the final resolved root - - // Resolve the path to tasks.json + // Resolve the path to tasks.json using the NORMALIZED projectRoot from args let tasksJsonPath; try { tasksJsonPath = findTasksJsonPath( - { projectRoot: rootFolder, file: args.file }, + { projectRoot: projectRoot, file: file }, log ); + log.info(`Resolved tasks path: ${tasksJsonPath}`); } catch (error) { log.error(`Error finding tasks.json: ${error.message}`); return createErrorResponse( @@ -89,15 +80,16 @@ export function registerShowTaskTool(server) { ); } - log.info(`Attempting to use tasks file path: ${tasksJsonPath}`); - + // Call the direct function, passing the normalized projectRoot const result = await showTaskDirect( { tasksJsonPath: tasksJsonPath, - id: args.id, - status: args.status + id: id, + status: status, + projectRoot: projectRoot }, - log + log, + { session } ); if (result.success) { @@ -108,7 +100,7 @@ export function registerShowTaskTool(server) { log.error(`Failed to get task: ${result.error.message}`); } - // Use our custom processor function to remove allTasks from the response + // Use our custom processor function return handleApiResult( result, log, @@ -116,9 +108,9 @@ export function registerShowTaskTool(server) { processTaskResponse ); } catch (error) { - log.error(`Error in get-task tool: ${error.message}\n${error.stack}`); // Add stack trace + log.error(`Error in get-task tool: ${error.message}\n${error.stack}`); return createErrorResponse(`Failed to get task: ${error.message}`); } - } + }) }); } diff --git a/mcp-server/src/tools/utils.js b/mcp-server/src/tools/utils.js index 71b439f3..327a02d2 100644 --- a/mcp-server/src/tools/utils.js +++ b/mcp-server/src/tools/utils.js @@ -83,10 +83,10 @@ function getProjectRoot(projectRootRaw, log) { } /** - * Extracts the project root path from the FastMCP session object. - * @param {Object} session - The FastMCP session object. - * @param {Object} log - Logger object. - * @returns {string|null} - The absolute path to the project root, or null if not found. + * Extracts and normalizes the project root path from the MCP session object. + * @param {Object} session - The MCP session object. + * @param {Object} log - The MCP logger object. + * @returns {string|null} - The normalized absolute project root path or null if not found/invalid. */ function getProjectRootFromSession(session, log) { try { @@ -107,68 +107,87 @@ function getProjectRootFromSession(session, log) { })}` ); - // ALWAYS ensure we return a valid path for project root + let rawRootPath = null; + let decodedPath = null; + let finalPath = null; + + // Check primary location + if (session?.roots?.[0]?.uri) { + rawRootPath = session.roots[0].uri; + log.info(`Found raw root URI in session.roots[0].uri: ${rawRootPath}`); + } + // Check alternate location + else if (session?.roots?.roots?.[0]?.uri) { + rawRootPath = session.roots.roots[0].uri; + log.info( + `Found raw root URI in session.roots.roots[0].uri: ${rawRootPath}` + ); + } + + if (rawRootPath) { + // Decode URI and strip file:// protocol + decodedPath = rawRootPath.startsWith('file://') + ? decodeURIComponent(rawRootPath.slice(7)) + : rawRootPath; // Assume non-file URI is already decoded? Or decode anyway? Let's decode. + if (!rawRootPath.startsWith('file://')) { + decodedPath = decodeURIComponent(rawRootPath); // Decode even if no file:// + } + + // Handle potential Windows drive prefix after stripping protocol (e.g., /C:/...) + if ( + decodedPath.startsWith('/') && + /[A-Za-z]:/.test(decodedPath.substring(1, 3)) + ) { + decodedPath = decodedPath.substring(1); // Remove leading slash if it's like /C:/... + } + + log.info(`Decoded path: ${decodedPath}`); + + // Normalize slashes and resolve + const normalizedSlashes = decodedPath.replace(/\\/g, '/'); + finalPath = path.resolve(normalizedSlashes); // Resolve to absolute path for current OS + + log.info(`Normalized and resolved session path: ${finalPath}`); + return finalPath; + } + + // Fallback Logic (remains the same) + log.warn('No project root URI found in session. Attempting fallbacks...'); const cwd = process.cwd(); - // If we have a session with roots array - if (session?.roots?.[0]?.uri) { - const rootUri = session.roots[0].uri; - log.info(`Found rootUri in session.roots[0].uri: ${rootUri}`); - const rootPath = rootUri.startsWith('file://') - ? decodeURIComponent(rootUri.slice(7)) - : rootUri; - log.info(`Decoded rootPath: ${rootPath}`); - return rootPath; - } - - // If we have a session with roots.roots array (different structure) - if (session?.roots?.roots?.[0]?.uri) { - const rootUri = session.roots.roots[0].uri; - log.info(`Found rootUri in session.roots.roots[0].uri: ${rootUri}`); - const rootPath = rootUri.startsWith('file://') - ? decodeURIComponent(rootUri.slice(7)) - : rootUri; - log.info(`Decoded rootPath: ${rootPath}`); - return rootPath; - } - - // Get the server's location and try to find project root -- this is a fallback necessary in Cursor IDE - const serverPath = process.argv[1]; // This should be the path to server.js, which is in mcp-server/ + // Fallback 1: Use server path deduction (Cursor IDE) + const serverPath = process.argv[1]; if (serverPath && serverPath.includes('mcp-server')) { - // Find the mcp-server directory first const mcpServerIndex = serverPath.indexOf('mcp-server'); if (mcpServerIndex !== -1) { - // Get the path up to mcp-server, which should be the project root - const projectRoot = serverPath.substring(0, mcpServerIndex - 1); // -1 to remove trailing slash + const projectRoot = path.dirname( + serverPath.substring(0, mcpServerIndex) + ); // Go up one level - // Verify this looks like our project root by checking for key files/directories if ( fs.existsSync(path.join(projectRoot, '.cursor')) || fs.existsSync(path.join(projectRoot, 'mcp-server')) || fs.existsSync(path.join(projectRoot, 'package.json')) ) { - log.info(`Found project root from server path: ${projectRoot}`); - return projectRoot; + log.info( + `Using project root derived from server path: ${projectRoot}` + ); + return projectRoot; // Already absolute } } } - // ALWAYS ensure we return a valid path as a last resort + // Fallback 2: Use CWD log.info(`Using current working directory as ultimate fallback: ${cwd}`); - return cwd; + return cwd; // Already absolute } catch (e) { - // If we have a server path, use it as a basis for project root - const serverPath = process.argv[1]; - if (serverPath && serverPath.includes('mcp-server')) { - const mcpServerIndex = serverPath.indexOf('mcp-server'); - return mcpServerIndex !== -1 - ? serverPath.substring(0, mcpServerIndex - 1) - : process.cwd(); - } - - // Only use cwd if it's not "/" + log.error(`Error in getProjectRootFromSession: ${e.message}`); + // Attempt final fallback to CWD on error const cwd = process.cwd(); - return cwd !== '/' ? cwd : '/'; + log.warn( + `Returning CWD (${cwd}) due to error during session root processing.` + ); + return cwd; } } @@ -474,6 +493,148 @@ function createLogWrapper(log) { }; } +/** + * Resolves and normalizes a project root path from various formats. + * Handles URI encoding, Windows paths, and file protocols. + * @param {string | undefined | null} rawPath - The raw project root path. + * @param {object} [log] - Optional logger object. + * @returns {string | null} Normalized absolute path or null if input is invalid/empty. + */ +function normalizeProjectRoot(rawPath, log) { + if (!rawPath) return null; + try { + let pathString = Array.isArray(rawPath) ? rawPath[0] : String(rawPath); + if (!pathString) return null; + + // 1. Decode URI Encoding + // Use try-catch for decoding as malformed URIs can throw + try { + pathString = decodeURIComponent(pathString); + } catch (decodeError) { + if (log) + log.warn( + `Could not decode URI component for path "${rawPath}": ${decodeError.message}. Proceeding with raw string.` + ); + // Proceed with the original string if decoding fails + pathString = Array.isArray(rawPath) ? rawPath[0] : String(rawPath); + } + + // 2. Strip file:// prefix (handle 2 or 3 slashes) + if (pathString.startsWith('file:///')) { + pathString = pathString.slice(7); // Slice 7 for file:///, may leave leading / on Windows + } else if (pathString.startsWith('file://')) { + pathString = pathString.slice(7); // Slice 7 for file:// + } + + // 3. Handle potential Windows leading slash after stripping prefix (e.g., /C:/...) + // This checks if it starts with / followed by a drive letter C: D: etc. + if ( + pathString.startsWith('/') && + /[A-Za-z]:/.test(pathString.substring(1, 3)) + ) { + pathString = pathString.substring(1); // Remove the leading slash + } + + // 4. Normalize backslashes to forward slashes + pathString = pathString.replace(/\\/g, '/'); + + // 5. Resolve to absolute path using server's OS convention + const resolvedPath = path.resolve(pathString); + return resolvedPath; + } catch (error) { + if (log) { + log.error( + `Error normalizing project root path "${rawPath}": ${error.message}` + ); + } + return null; // Return null on error + } +} + +/** + * Extracts the raw project root path from the session (without normalization). + * Used as a fallback within the HOF. + * @param {Object} session - The MCP session object. + * @param {Object} log - The MCP logger object. + * @returns {string|null} The raw path string or null. + */ +function getRawProjectRootFromSession(session, log) { + try { + // Check primary location + if (session?.roots?.[0]?.uri) { + return session.roots[0].uri; + } + // Check alternate location + else if (session?.roots?.roots?.[0]?.uri) { + return session.roots.roots[0].uri; + } + return null; // Not found in expected session locations + } catch (e) { + log.error(`Error accessing session roots: ${e.message}`); + return null; + } +} + +/** + * Higher-order function to wrap MCP tool execute methods. + * Ensures args.projectRoot is present and normalized before execution. + * @param {Function} executeFn - The original async execute(args, context) function. + * @returns {Function} The wrapped async execute function. + */ +function withNormalizedProjectRoot(executeFn) { + return async (args, context) => { + const { log, session } = context; + let normalizedRoot = null; + let rootSource = 'unknown'; + + try { + // Determine raw root: prioritize args, then session + let rawRoot = args.projectRoot; + if (!rawRoot) { + rawRoot = getRawProjectRootFromSession(session, log); + rootSource = 'session'; + } else { + rootSource = 'args'; + } + + if (!rawRoot) { + log.error('Could not determine project root from args or session.'); + return createErrorResponse( + 'Could not determine project root. Please provide projectRoot argument or ensure session contains root info.' + ); + } + + // Normalize the determined raw root + normalizedRoot = normalizeProjectRoot(rawRoot, log); + + if (!normalizedRoot) { + log.error( + `Failed to normalize project root obtained from ${rootSource}: ${rawRoot}` + ); + return createErrorResponse( + `Invalid project root provided or derived from ${rootSource}: ${rawRoot}` + ); + } + + // Inject the normalized root back into args + const updatedArgs = { ...args, projectRoot: normalizedRoot }; + + // Execute the original function with normalized root in args + return await executeFn(updatedArgs, context); + } catch (error) { + log.error( + `Error within withNormalizedProjectRoot HOF (Normalized Root: ${normalizedRoot}): ${error.message}` + ); + // Add stack trace if available and debug enabled + if (error.stack && log.debug) { + log.debug(error.stack); + } + // Return a generic error or re-throw depending on desired behavior + return createErrorResponse(`Operation failed: ${error.message}`); + } + }; +} + // Ensure all functions are exported export { getProjectRoot, @@ -484,5 +645,8 @@ export { processMCPResponseData, createContentResponse, createErrorResponse, - createLogWrapper + createLogWrapper, + normalizeProjectRoot, + getRawProjectRootFromSession, + withNormalizedProjectRoot };