feat(cache): Implement caching for listTasks MCP endpoint

Implemented LRU caching for the  function to improve performance for repeated requests.

Key changes include:
- Added  dependency.
- Introduced a reusable  utility function in  leveraging a .
- Refactored  in  to use the caching utility with a key based on task path, filter, and subtask flag.
- Modified  to include the  boolean flag in the final JSON response structure, nesting the original data under a  key.
- Added  function and corresponding MCP tool () for monitoring cache performance.
- Improved error handling in  for cases where  is not found.

This addresses the previous issue of the empty task list likely caused by stale cache entries and provides clear visibility into whether a response is served from the cache.

Relates to #23.9
This commit is contained in:
Eyal Toledano
2025-03-30 02:25:24 -04:00
parent cd4f4e66d7
commit 42585519d3
10 changed files with 712 additions and 80 deletions

View File

@@ -0,0 +1,85 @@
import { jest } from '@jest/globals';
import { ContextManager } from '../context-manager.js';
describe('ContextManager', () => {
let contextManager;
beforeEach(() => {
contextManager = new ContextManager({
maxCacheSize: 10,
ttl: 1000, // 1 second for testing
maxContextSize: 1000
});
});
describe('getContext', () => {
it('should create a new context when not in cache', async () => {
const context = await contextManager.getContext('test-id', { test: true });
expect(context.id).toBe('test-id');
expect(context.metadata.test).toBe(true);
expect(contextManager.stats.misses).toBe(1);
expect(contextManager.stats.hits).toBe(0);
});
it('should return cached context when available', async () => {
// First call creates the context
await contextManager.getContext('test-id', { test: true });
// Second call should hit cache
const context = await contextManager.getContext('test-id', { test: true });
expect(context.id).toBe('test-id');
expect(context.metadata.test).toBe(true);
expect(contextManager.stats.hits).toBe(1);
expect(contextManager.stats.misses).toBe(1);
});
it('should respect TTL settings', async () => {
// Create context
await contextManager.getContext('test-id', { test: true });
// Wait for TTL to expire
await new Promise(resolve => setTimeout(resolve, 1100));
// Should create new context
await contextManager.getContext('test-id', { test: true });
expect(contextManager.stats.misses).toBe(2);
expect(contextManager.stats.hits).toBe(0);
});
});
describe('updateContext', () => {
it('should update existing context metadata', async () => {
await contextManager.getContext('test-id', { initial: true });
const updated = await contextManager.updateContext('test-id', { updated: true });
expect(updated.metadata.initial).toBe(true);
expect(updated.metadata.updated).toBe(true);
});
});
describe('invalidateContext', () => {
it('should remove context from cache', async () => {
await contextManager.getContext('test-id', { test: true });
contextManager.invalidateContext('test-id', { test: true });
// Should be a cache miss
await contextManager.getContext('test-id', { test: true });
expect(contextManager.stats.invalidations).toBe(1);
expect(contextManager.stats.misses).toBe(2);
});
});
describe('getStats', () => {
it('should return current cache statistics', async () => {
await contextManager.getContext('test-id', { test: true });
const stats = contextManager.getStats();
expect(stats.hits).toBe(0);
expect(stats.misses).toBe(1);
expect(stats.invalidations).toBe(0);
expect(stats.size).toBe(1);
expect(stats.maxSize).toBe(10);
expect(stats.ttl).toBe(1000);
});
});
});

View File

@@ -0,0 +1,170 @@
/**
* context-manager.js
* Context and cache management for Task Master MCP Server
*/
import { FastMCP } from 'fastmcp';
import { LRUCache } from 'lru-cache';
/**
* Configuration options for the ContextManager
* @typedef {Object} ContextManagerConfig
* @property {number} maxCacheSize - Maximum number of items in the cache
* @property {number} ttl - Time to live for cached items in milliseconds
* @property {number} maxContextSize - Maximum size of context window in tokens
*/
export class ContextManager {
/**
* Create a new ContextManager instance
* @param {ContextManagerConfig} config - Configuration options
*/
constructor(config = {}) {
this.config = {
maxCacheSize: config.maxCacheSize || 1000,
ttl: config.ttl || 1000 * 60 * 5, // 5 minutes default
maxContextSize: config.maxContextSize || 4000
};
// Initialize LRU cache for context data
this.cache = new LRUCache({
max: this.config.maxCacheSize,
ttl: this.config.ttl,
updateAgeOnGet: true
});
// Cache statistics
this.stats = {
hits: 0,
misses: 0,
invalidations: 0
};
}
/**
* Create a new context or retrieve from cache
* @param {string} contextId - Unique identifier for the context
* @param {Object} metadata - Additional metadata for the context
* @returns {Object} Context object with metadata
*/
async getContext(contextId, metadata = {}) {
const cacheKey = this._getCacheKey(contextId, metadata);
// Try to get from cache first
const cached = this.cache.get(cacheKey);
if (cached) {
this.stats.hits++;
return cached;
}
this.stats.misses++;
// Create new context if not in cache
const context = {
id: contextId,
metadata: {
...metadata,
created: new Date().toISOString()
}
};
// Cache the new context
this.cache.set(cacheKey, context);
return context;
}
/**
* Update an existing context
* @param {string} contextId - Context identifier
* @param {Object} updates - Updates to apply to the context
* @returns {Object} Updated context
*/
async updateContext(contextId, updates) {
const context = await this.getContext(contextId);
// Apply updates to context
Object.assign(context.metadata, updates);
// Update cache
const cacheKey = this._getCacheKey(contextId, context.metadata);
this.cache.set(cacheKey, context);
return context;
}
/**
* Invalidate a context in the cache
* @param {string} contextId - Context identifier
* @param {Object} metadata - Metadata used in the cache key
*/
invalidateContext(contextId, metadata = {}) {
const cacheKey = this._getCacheKey(contextId, metadata);
this.cache.delete(cacheKey);
this.stats.invalidations++;
}
/**
* Get cached data associated with a specific key.
* Increments cache hit stats if found.
* @param {string} key - The cache key.
* @returns {any | undefined} The cached data or undefined if not found/expired.
*/
getCachedData(key) {
const cached = this.cache.get(key);
if (cached !== undefined) { // Check for undefined specifically, as null/false might be valid cached values
this.stats.hits++;
return cached;
}
this.stats.misses++;
return undefined;
}
/**
* Set data in the cache with a specific key.
* @param {string} key - The cache key.
* @param {any} data - The data to cache.
*/
setCachedData(key, data) {
this.cache.set(key, data);
}
/**
* Invalidate a specific cache key.
* Increments invalidation stats.
* @param {string} key - The cache key to invalidate.
*/
invalidateCacheKey(key) {
this.cache.delete(key);
this.stats.invalidations++;
}
/**
* Get cache statistics
* @returns {Object} Cache statistics
*/
getStats() {
return {
hits: this.stats.hits,
misses: this.stats.misses,
invalidations: this.stats.invalidations,
size: this.cache.size,
maxSize: this.config.maxCacheSize,
ttl: this.config.ttl
};
}
/**
* Generate a cache key from context ID and metadata
* @private
* @deprecated No longer used for direct cache key generation outside the manager.
* Prefer generating specific keys in calling functions.
*/
_getCacheKey(contextId, metadata) {
// Kept for potential backward compatibility or internal use if needed later.
return `${contextId}:${JSON.stringify(metadata)}`;
}
}
// Export a singleton instance with default config
export const contextManager = new ContextManager();

View File

@@ -21,6 +21,10 @@ import {
// We'll import more functions as we continue implementation
} from '../../../scripts/modules/task-manager.js';
// Import context manager
import { contextManager } from './context-manager.js';
import { getCachedOrExecute } from '../tools/utils.js'; // Import the utility here
/**
* Finds the absolute path to the tasks.json file based on project root and arguments.
* @param {Object} args - Command arguments, potentially including 'projectRoot' and 'file'.
@@ -58,52 +62,95 @@ function findTasksJsonPath(args, log) {
}
// If no file was found, throw an error
throw new Error(`Tasks file not found in any of the expected locations relative to ${projectRoot}: ${possiblePaths.join(', ')}`);
const error = new Error(`Tasks file not found in any of the expected locations relative to ${projectRoot}: ${possiblePaths.join(', ')}`);
error.code = 'TASKS_FILE_NOT_FOUND';
throw error;
}
/**
* Direct function wrapper for listTasks with error handling
*
* @param {Object} args - Command arguments (projectRoot is expected to be resolved)
* @param {Object} log - Logger object
* @returns {Object} - Task list result
* Direct function wrapper for listTasks with error handling and caching.
*
* @param {Object} args - Command arguments (projectRoot is expected to be resolved).
* @param {Object} log - Logger object.
* @returns {Promise<Object>} - Task list result { success: boolean, data?: any, error?: { code: string, message: string }, fromCache: boolean }.
*/
export async function listTasksDirect(args, log) {
let tasksPath;
try {
log.info(`Listing tasks with args: ${JSON.stringify(args)}`);
// Use the helper function to find the tasks file path
const tasksPath = findTasksJsonPath(args, log);
// Extract other arguments
const statusFilter = args.status || null;
const withSubtasks = args.withSubtasks || false;
log.info(`Using tasks file at: ${tasksPath}`);
log.info(`Status filter: ${statusFilter}, withSubtasks: ${withSubtasks}`);
// Call listTasks with json format
const result = listTasks(tasksPath, statusFilter, withSubtasks, 'json');
if (!result || !result.tasks) {
throw new Error('Invalid or empty response from listTasks function');
// Find the tasks path first - needed for cache key and execution
tasksPath = findTasksJsonPath(args, log);
} catch (error) {
if (error.code === 'TASKS_FILE_NOT_FOUND') {
log.error(`Tasks file not found: ${error.message}`);
// Return the error structure expected by the calling tool/handler
return { success: false, error: { code: error.code, message: error.message }, fromCache: false };
}
log.info(`Successfully retrieved ${result.tasks.length} tasks`);
// Return the raw result directly
log.error(`Unexpected error finding tasks file: ${error.message}`);
// Re-throw for outer catch or return structured error
return { success: false, error: { code: 'FIND_TASKS_PATH_ERROR', message: error.message }, fromCache: false };
}
// Generate cache key *after* finding tasksPath
const statusFilter = args.status || 'all';
const withSubtasks = args.withSubtasks || false;
const cacheKey = `listTasks:${tasksPath}:${statusFilter}:${withSubtasks}`;
// Define the action function to be executed on cache miss
const coreListTasksAction = async () => {
try {
log.info(`Executing core listTasks function for path: ${tasksPath}, filter: ${statusFilter}, subtasks: ${withSubtasks}`);
const resultData = listTasks(tasksPath, statusFilter, withSubtasks, 'json');
if (!resultData || !resultData.tasks) {
log.error('Invalid or empty response from listTasks core function');
return { success: false, error: { code: 'INVALID_CORE_RESPONSE', message: 'Invalid or empty response from listTasks core function' } };
}
log.info(`Core listTasks function retrieved ${resultData.tasks.length} tasks`);
return { success: true, data: resultData };
} catch (error) {
log.error(`Core listTasks function failed: ${error.message}`);
return { success: false, error: { code: 'LIST_TASKS_CORE_ERROR', message: error.message || 'Failed to list tasks' } };
}
};
// Use the caching utility
try {
const result = await getCachedOrExecute({
cacheKey,
actionFn: coreListTasksAction,
log
});
log.info(`listTasksDirect completed. From cache: ${result.fromCache}`);
return result; // Returns { success, data/error, fromCache }
} catch(error) {
// Catch unexpected errors from getCachedOrExecute itself (though unlikely)
log.error(`Unexpected error during getCachedOrExecute for listTasks: ${error.message}`);
console.error(error.stack);
return { success: false, error: { code: 'CACHE_UTIL_ERROR', message: error.message }, fromCache: false };
}
}
/**
* Get cache statistics for monitoring
* @param {Object} args - Command arguments
* @param {Object} log - Logger object
* @returns {Object} - Cache statistics
*/
export async function getCacheStatsDirect(args, log) {
try {
log.info('Retrieving cache statistics');
const stats = contextManager.getStats();
return {
success: true,
data: result // Return the actual object, not a stringified version
data: stats
};
} catch (error) {
log.error(`Error in listTasksDirect: ${error.message}`);
// Ensure we always return a properly structured error object
log.error(`Error getting cache stats: ${error.message}`);
return {
success: false,
error: {
code: error.code || 'LIST_TASKS_ERROR',
code: 'CACHE_STATS_ERROR',
message: error.message || 'Unknown error occurred'
}
};
@@ -115,5 +162,6 @@ export async function listTasksDirect(args, log) {
*/
export const directFunctions = {
list: listTasksDirect,
cacheStats: getCacheStatsDirect,
// Add more functions as we implement them
};

View File

@@ -5,6 +5,7 @@
import { spawnSync } from "child_process";
import path from "path";
import { contextManager } from '../core/context-manager.js'; // Import the singleton
/**
* Get normalized project root path
@@ -36,15 +37,25 @@ export function getProjectRoot(projectRootRaw, log) {
export function handleApiResult(result, log, errorPrefix = 'API error', processFunction = processMCPResponseData) {
if (!result.success) {
const errorMsg = result.error?.message || `Unknown ${errorPrefix}`;
log.error(`${errorPrefix}: ${errorMsg}`);
// Include cache status in error logs
log.error(`${errorPrefix}: ${errorMsg}. From cache: ${result.fromCache}`); // Keep logging cache status on error
return createErrorResponse(errorMsg);
}
// Process the result data if needed and if we have a processor function
// Process the result data if needed
const processedData = processFunction ? processFunction(result.data) : result.data;
// Return formatted response
return createContentResponse(processedData);
// Log success including cache status
log.info(`Successfully completed operation. From cache: ${result.fromCache}`); // Add success log with cache status
// Create the response payload including the fromCache flag
const responsePayload = {
fromCache: result.fromCache, // Get the flag from the original 'result'
data: processedData // Nest the processed data under a 'data' key
};
// Pass this combined payload to createContentResponse
return createContentResponse(responsePayload);
}
/**
@@ -121,56 +132,140 @@ export function executeTaskMasterCommand(
}
/**
* Executes a Task Master tool action with standardized error handling, logging, and response formatting
* Checks cache for a result using the provided key. If not found, executes the action function,
* caches the result upon success, and returns the result.
*
* @param {Object} options - Configuration options.
* @param {string} options.cacheKey - The unique key for caching this operation's result.
* @param {Function} options.actionFn - The async function to execute if the cache misses.
* Should return an object like { success: boolean, data?: any, error?: { code: string, message: string } }.
* @param {Object} options.log - The logger instance.
* @returns {Promise<Object>} - An object containing the result, indicating if it was from cache.
* Format: { success: boolean, data?: any, error?: { code: string, message: string }, fromCache: boolean }
*/
export async function getCachedOrExecute({ cacheKey, actionFn, log }) {
// Check cache first
const cachedResult = contextManager.getCachedData(cacheKey);
if (cachedResult !== undefined) {
log.info(`Cache hit for key: ${cacheKey}`);
// Return the cached data in the same structure as a fresh result
return {
...cachedResult, // Spread the cached result to maintain its structure
fromCache: true // Just add the fromCache flag
};
}
log.info(`Cache miss for key: ${cacheKey}. Executing action function.`);
// Execute the action function if cache missed
const result = await actionFn();
// If the action was successful, cache the result (but without fromCache flag)
if (result.success && result.data !== undefined) {
log.info(`Action successful. Caching result for key: ${cacheKey}`);
// Cache the entire result structure (minus the fromCache flag)
const { fromCache, ...resultToCache } = result;
contextManager.setCachedData(cacheKey, resultToCache);
} else if (!result.success) {
log.warn(`Action failed for cache key ${cacheKey}. Result not cached. Error: ${result.error?.message}`);
} else {
log.warn(`Action for cache key ${cacheKey} succeeded but returned no data. Result not cached.`);
}
// Return the fresh result, indicating it wasn't from cache
return {
...result,
fromCache: false
};
}
/**
* 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 to execute (must return {success, data, error})
* @param {Object} options.args - Arguments for the action
* @param {Object} options.log - Logger object from FastMCP
* @param {string} options.actionName - Name of the action for logging purposes
* @param {Function} options.processResult - Optional function to process the result before returning
* @returns {Promise<Object>} - Standardized response for FastMCP
* @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}`);
// Execute the core action function with normalized arguments
const result = await actionFn({...args, projectRoot}, log);
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()}`;
log.error(`Error during ${actionName.toLowerCase()}: ${errorMsg}`);
// 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()}`);
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
};
// Return formatted response
return createContentResponse(processedData);
// Pass this combined payload to createContentResponse
return createContentResponse(responsePayload);
} catch (error) {
// Handle unexpected errors
log.error(`Unexpected error during ${actionName.toLowerCase()}: ${error.message}`);
return createErrorResponse(error.message);
// 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}`);
}
}