mcp: recreate the toolset when server instructions change#278141
mcp: recreate the toolset when server instructions change#278141connor4312 merged 1 commit intomainfrom
Conversation
422d15a to
dcbf617
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the issue where toolsets are not recreated when MCP server instructions or metadata change. The changes track server metadata as part of the tool source data and compare it on each autorun iteration to detect when recreation is needed.
Key Changes:
- Modified
mcpServerToSourceDatato accept an optionalIReaderparameter and use.read(reader)instead of.get()to enable dependency tracking on server metadata - Updated the autorun logic in
McpLanguageModelToolContributionto compare previous and current source data (including server instructions) and recreate the toolset when changes are detected - Changed the storage structure from
DisposableStoreto a customRectype that includes both the source data and disposal capability
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/vs/workbench/contrib/mcp/common/mcpTypesUtils.ts | Added optional IReader parameter to mcpServerToSourceData to enable observable dependency tracking on server metadata |
| src/vs/workbench/contrib/mcp/common/mcpLanguageModelToolContribution.ts | Implemented source data comparison logic to detect metadata changes and trigger toolset recreation, with a new Rec type to store source alongside dispose function |
| toDelete.delete(server); | ||
| continue; | ||
| if (!previousRec.source || equals(previousRec.source, mcpServerToSourceData(server, reader))) { | ||
| continue; // same definition, no need to update |
There was a problem hiding this comment.
The condition !previousRec.source will be true if the server has no tools yet and the Lazy hasn't been evaluated. In this case, if server metadata (instructions or serverName) changes before any tools are registered, the toolset won't be recreated because the check will pass and continue on line 61.
Consider eagerly setting rec.source immediately after creating the record, before the Lazy:
const store = new DisposableStore();
const rec: Rec = { source: mcpServerToSourceData(server), dispose: () => store.dispose() };
const toolSet = new Lazy(() => {
const toolSet = store.add(this._toolsService.createToolSet(
rec.source!,
server.definition.id, server.definition.label,
{
icon: Codicon.mcp,
description: localize('mcp.toolset', "{0}: All Tools", server.definition.label)
}
));
return { toolSet, source: rec.source! };
});
Closes #277894