-
Notifications
You must be signed in to change notification settings - Fork 2.1k
feat: Save large MCP responses to files to prevent context overflow #7043
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Added McpResponseHandler utility class to handle large MCP responses - Automatically saves responses over 50KB (configurable) to temporary files - Provides preview and file path in context instead of full response - Integrated into useMcpToolTool and accessMcpResourceTool - Added mcpResponseSizeThreshold configuration option - Added comprehensive tests for the new functionality Fixes #7042
// Generate unique filename | ||
const timestamp = new Date().toISOString().replace(/[:.]/g, "-") | ||
const hash = crypto.createHash("md5").update(response).digest("hex").substring(0, 8) | ||
const filename = `mcp-response-${serverName}-${toolOrResourceName}-${timestamp}-${hash}.txt` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider sanitizing the generated filename components (serverName and toolOrResourceName) to remove or escape characters such as slashes that may cause unintended directory paths.
const filename = `mcp-response-${serverName}-${toolOrResourceName}-${timestamp}-${hash}.txt` | |
const filename = `mcp-response-${serverName.replace(/[^a-zA-Z0-9-_]/g, '_')}-${toolOrResourceName.replace(/[^a-zA-Z0-9-_]/g, '_')}-${timestamp}-${hash}.txt` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote code that saves files to prevent overflow, but forgot to actually use the config that controls when to save them.
@@ -0,0 +1,241 @@ | |||
import { describe, it, expect, vi, beforeEach, afterEach, beforeAll } from "vitest" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test file should be in src/utils/tests/mcpResponseHandler.test.ts to follow the project's testing convention. All other test files in this project are located in tests subdirectories.
@@ -135,13 +136,18 @@ async function executeToolAndProcessResult( | |||
const outputText = processToolContent(toolResult) | |||
|
|||
if (outputText) { | |||
// Check if response is large and should be saved to file | |||
const processedResponse = await defaultMcpResponseHandler.processResponse(outputText, serverName, toolName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mcpResponseSizeThreshold configuration value is never passed to the handler. The defaultMcpResponseHandler singleton always uses the default 50KB threshold. Consider passing the configured value to the handler.
@@ -81,6 +82,16 @@ export async function accessMcpResourceTool( | |||
} | |||
}) | |||
|
|||
// Check if response is large and should be saved to file | |||
const processedResponse = await defaultMcpResponseHandler.processResponse( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same issue here - the configured threshold isn't being used. The handler should respect the user's mcpResponseSizeThreshold setting instead of always using the default.
} | ||
|
||
// Export a function to get default instance with default configuration | ||
export const getDefaultMcpResponseHandler = (() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The singleton pattern here means configuration changes won't take effect until the extension restarts. Consider allowing the handler to be reconfigured or recreated when settings change.
await fs.mkdir(this.config.responseDirectory, { recursive: true }) | ||
|
||
// Save response to file using safeWriteJson for structured data | ||
if (typeof responseData === "object") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing error handling here. If safeWriteJson throws, it won't be caught. Consider wrapping in try-catch like in processResponse.
/** | ||
* Clean up old response files (optional maintenance method) | ||
*/ | ||
async cleanupOldFiles(maxAgeHours: number = 24): Promise<number> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cleanupOldFiles method is implemented but never called. Should this be integrated into the extension lifecycle or run periodically to prevent accumulation of old response files?
Very interested to see where this goes - I've long thought this was an interesting approach conceptually, especially when you know the client in question is capable of file read operations out of the box. |
Closing this PR to allow for a fresh implementation with proper dynamic context awareness. The issue will be re-scoped to provide clearer requirements for the next attempt. |
Fixes #7042
Summary
This PR implements automatic file saving for large MCP responses to prevent context window overflow issues. When MCP tools or resources return responses larger than a configurable threshold (default 50KB), the response is automatically saved to a temporary file and a preview with file path is provided in the context instead.
Changes
McpResponseHandler
utility class to handle large MCP responsesuseMcpToolTool
andaccessMcpResourceTool
mcpResponseSizeThreshold
configuration option to customize the thresholdBenefits
Testing
McpResponseHandler
classConfiguration
Users can configure the response size threshold in their settings:
The default is 50KB (50000 characters).
Important
Introduces
McpResponseHandler
to save large MCP responses to files, preventing context overflow, with integration into existing tools and comprehensive testing.McpResponseHandler
to save large MCP responses to files, preventing context overflow.useMcpToolTool
andaccessMcpResourceTool
to handle large responses.mcpResponseSizeThreshold
inglobal-settings.ts
for configurable response size limit.McpResponseHandler
inmcpResponseHandler.test.ts
.mcpResponseSizeThreshold
.ClineProvider.ts
to support new configuration and response handling.This description was created by
for 098ff6d. You can customize this summary. It will automatically update as commits are pushed.