refactor(mcp): Remove unused executeMCPToolAction utility

The  function aimed to abstract the common flow within MCP tool  methods (logging, calling direct function, handling result).

However, the established pattern (e.g., in ) involves the  method directly calling the  function (which handles its own caching via ) and then passing the result to . This pattern is clear, functional, and leverages the core utilities effectively.

Removing the unused  simplifies , eliminates a redundant abstraction layer, and clarifies the standard implementation pattern for MCP tools.
This commit is contained in:
Eyal Toledano
2025-03-30 23:37:24 -04:00
parent a56a3628b3
commit bc9707f813
6 changed files with 107 additions and 91 deletions

View File

@@ -180,95 +180,6 @@ export async function getCachedOrExecute({ cacheKey, actionFn, log }) {
};
}
/**
* Executes a Task Master tool action with standardized error handling, logging, and response formatting.
* Integrates caching logic via getCachedOrExecute if a cacheKeyGenerator is provided.
*
* @param {Object} options - Options for executing the tool action
* @param {Function} options.actionFn - The core action function (e.g., listTasksDirect) to execute. Should return {success, data, error}.
* @param {Object} options.args - Arguments for the action, passed to actionFn and cacheKeyGenerator.
* @param {Object} options.log - Logger object from FastMCP.
* @param {string} options.actionName - Name of the action for logging purposes.
* @param {Function} [options.cacheKeyGenerator] - Optional function to generate a cache key based on args. If provided, caching is enabled.
* @param {Function} [options.processResult=processMCPResponseData] - Optional function to process the result data before returning.
* @returns {Promise<Object>} - Standardized response for FastMCP.
*/
export async function executeMCPToolAction({
actionFn,
args,
log,
actionName,
cacheKeyGenerator, // Note: We decided not to use this for listTasks for now
processResult = processMCPResponseData
}) {
try {
// Log the action start
log.info(`${actionName} with args: ${JSON.stringify(args)}`);
// Normalize project root path - common to almost all tools
const projectRootRaw = args.projectRoot || process.cwd();
const projectRoot = path.isAbsolute(projectRootRaw)
? projectRootRaw
: path.resolve(process.cwd(), projectRootRaw);
log.info(`Using project root: ${projectRoot}`);
const executionArgs = { ...args, projectRoot };
let result;
const cacheKey = cacheKeyGenerator ? cacheKeyGenerator(executionArgs) : null;
if (cacheKey) {
// Use caching utility
log.info(`Caching enabled for ${actionName} with key: ${cacheKey}`);
const cacheWrappedAction = async () => await actionFn(executionArgs, log);
result = await getCachedOrExecute({
cacheKey,
actionFn: cacheWrappedAction,
log
});
} else {
// Execute directly without caching
log.info(`Caching disabled for ${actionName}. Executing directly.`);
// We need to ensure the result from actionFn has a fromCache field
// Let's assume actionFn now consistently returns { success, data/error, fromCache }
// The current listTasksDirect does this if it calls getCachedOrExecute internally.
result = await actionFn(executionArgs, log);
// If the action function itself doesn't determine caching (like our original listTasksDirect refactor attempt),
// we'd set it here:
// result.fromCache = false;
}
// Handle error case
if (!result.success) {
const errorMsg = result.error?.message || `Unknown error during ${actionName.toLowerCase()}`;
// Include fromCache in error logs too, might be useful
log.error(`Error during ${actionName.toLowerCase()}: ${errorMsg}. From cache: ${result.fromCache}`);
return createErrorResponse(errorMsg);
}
// Log success
log.info(`Successfully completed ${actionName.toLowerCase()}. From cache: ${result.fromCache}`);
// Process the result data if needed
const processedData = processResult ? processResult(result.data) : result.data;
// Create a new object that includes both the processed data and the fromCache flag
const responsePayload = {
fromCache: result.fromCache, // Include the flag here
data: processedData // Embed the actual data under a 'data' key
};
// Pass this combined payload to createContentResponse
return createContentResponse(responsePayload);
} catch (error) {
// Handle unexpected errors during the execution wrapper itself
log.error(`Unexpected error during ${actionName.toLowerCase()} execution wrapper: ${error.message}`);
console.error(error.stack); // Log stack for debugging wrapper errors
return createErrorResponse(`Internal server error during ${actionName.toLowerCase()}: ${error.message}`);
}
}
/**
* Recursively removes specified fields from task objects, whether single or in an array.
* Handles common data structures returned by task commands.