Skip to content

mcp: replace repeated variable placeholders in config values#299145

Open
bharatvansh wants to merge 1 commit intomicrosoft:mainfrom
bharatvansh:fix/mcp-repeated-placeholder-replacement
Open

mcp: replace repeated variable placeholders in config values#299145
bharatvansh wants to merge 1 commit intomicrosoft:mainfrom
bharatvansh:fix/mcp-repeated-placeholder-replacement

Conversation

@bharatvansh
Copy link
Copy Markdown

Summary

Fixes placeholder interpolation for MCP manifest values when the same variable appears multiple times in a single string.

Previously, interpolation used single-replacement behavior, so only the first {variable} occurrence was converted to ${input:variable}. This left subsequent occurrences unresolved.

Changes

  • Add replaceInputVariableReference(value, variableId) helper in mcpManagementService.ts.
  • Use the helper in all variable interpolation paths:
    • key/value inputs (env + remote headers)
    • positional arguments
    • named arguments
  • Add regression tests for repeated placeholders in:
    • environment values
    • argument values

Example

Bearer {api_token}:{api_token} now becomes:
Bearer ${input:api_token}:${input:api_token}

Validation

  • npm run transpile-client
  • scripts/test.bat --grep "McpManagementService - getMcpServerConfigurationFromManifest" ✅ (37 passing)

Copilot AI review requested due to automatic review settings March 4, 2026 08:18
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes MCP manifest placeholder interpolation so that all occurrences of a {variable} placeholder in a single string are replaced (not just the first), ensuring generated ${input:...} references are complete across MCP configuration parsing.

Changes:

  • Introduces a replaceInputVariableReference(value, variableId) helper to replace repeated {variableId} placeholders.
  • Applies the helper across all interpolation paths (key/value inputs and arguments).
  • Adds regression tests covering repeated placeholders in env values and named argument values.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/vs/platform/mcp/common/mcpManagementService.ts Centralizes placeholder replacement into a helper and uses it in key/value input and argument processing.
src/vs/platform/mcp/test/common/mcpManagementService.test.ts Adds regression tests to ensure repeated placeholders are fully replaced in env and argument strings.

Comment on lines +791 to +795
test('repeated variable placeholders in argument values should all be replaced', () => {
const manifest: IGalleryMcpServerConfiguration = {
packages: [{
registryType: RegistryType.NODE,
identifier: 'test-server',
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new interpolation helper is used for key/value inputs (including remote headers) and positional args too, but the added regression tests only cover env values and a named argument. Please add at least one test that exercises repeated placeholders in a remote header value and one for a positional argument value, so the full intended behavior is protected against regressions.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants