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.
This commit is contained in:
Eyal Toledano
2025-05-02 01:54:24 -04:00
parent 70c5097553
commit ad89253e31
8 changed files with 440 additions and 275 deletions

View File

@@ -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<Object>} - 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<Object>} - 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
}
};
}
}

View File

@@ -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}`);
}
}
})
});
}

View File

@@ -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
};