Fix token expansion and lazy folder creation for Agent Plugins 1.0 MCP definitions

[revallion]

Summary
Fixes #335006 .
${PLUGIN_ROOT} and ${PLUGIN_DATA} are not expanded.

Changes:
Token Expansion: Updated toPluginMcpServerDefinition in PluginMcpDiscovery to substitute ${PLUGIN_ROOT} with the plugin's source root URI and ${PLUGIN_DATA} with the plugin's storage path.
Lazy folder creation for the data directory. ( Right now, using globalStorageHome/agentPlugin/data/hash, not sure whats the convention, hash or should it be the agent-plugin name?)

Verification
Ran ./scripts/test.sh --run src/vs/workbench/contrib/mcp/test/common/pluginMcpDiscovery.test.ts successfully.
Manually verified that a canonical Agent Plugin 1.0 ($schema: https://agent-plugins.org/schemas/1.0.0/plugin.schema.json) with ${PLUGIN_ROOT} and ${PLUGIN_DATA} in mcp.json correctly expands variables.

Edit: Redid the manual test after all the copilot suggestion fixes.

[revallion]

@microsoft-github-policy-service agree

[Copilot]

Copilot review overview

🟡 Changes recommended

MCP definitions become stale after edits, profile changes can reuse old storage paths, and directory-creation failures are not safely handled.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 3

Open (3)

• Plugin data directory remains stale after profile switches · New
• Avoid creating data directories for HTTP-only MCP servers · New
• MCP definitions stop updating after non-empty edits · New

What changed in this PR

Fixes Agent Plugins 1.0 MCP placeholder expansion and provisions persistent plugin data directories.

Changes:

• Expands ${PLUGIN_ROOT} and ${PLUGIN_DATA} for stdio MCP configurations.
• Assigns profile-scoped plugin data directories.
• Adds MCP conversion and directory-creation tests.

File	Description
agentPluginService.ts	Adds the plugin data-directory contract.
agentPluginServiceImpl.ts	Computes data directories and injects profile services.
pluginMcpDiscovery.ts	Resolves placeholders and creates data directories.
pluginMcpDiscovery.test.ts	Tests expansion and directory creation.
configuredAgentPluginDiscovery.test.ts	Updates constructor setup.
agentPluginFormatDetection.test.ts	Updates test discovery construction.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Copilot review overview

🟡 Changes recommended

Remote path handling, profile switching, rejected folder creation, and reactive definition updates currently produce incorrect or stale MCP registrations.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 3

Open (4)

• Resolve remote plugin paths and data storage for the target OS · New
• Handle createFolder failure and remove stale collection placeholder · New
• MCP definitions stop updating after non-empty edits
• Plugin data directory remains stale after profile switches

Resolved since last review (1)

• Avoid creating data directories for HTTP-only MCP servers

[Copilot]

Copilot review overview

🟡 Changes recommended

Remote fallback, reactive configuration updates, and literal path substitution currently have correctness defects.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 · 2

Open (3)

• Defer remote registration when environment is unavailable · New
• MCP definitions stop updating after non-empty edits
• Plugin data directory remains stale after profile switches

Resolved since last review (2)

• Resolve remote plugin paths and data storage for the target OS
• Handle createFolder failure and remove stale collection placeholder

Previously missed (1)

In code that hasn't changed since last review

Use function replacer to preserve literal filesystem paths

src/​vs/​workbench/​contrib/​mcp/​common/​discovery/​pluginMcpDiscovery.ts:96

String.replaceAll interprets $&, $$, $\`` and $'in a string replacement. A valid plugin/data path containing one of those sequences is therefore corrupted when substituted (for example,/plugins/a$&bturns into/plugins/a${PLUGIN_ROOT}b`). Use a single-pass function replacer so filesystem paths are inserted literally and inserted token-like text is not processed again.

[Copilot]

Copilot review overview

🟡 Changes recommended

Token replacement can corrupt valid paths, live MCP edits become stale, and 32-bit directory identifiers can collide.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2 · 2

Open (4)

• 32-bit plugin hashes can collide on local data directories · New
• Remote plugin data paths use collision-prone hashes · New
• MCP definitions stop updating after non-empty edits
• Plugin data directory remains stale after profile switches

Resolved since last review (1)

• Defer remote registration when environment is unavailable

Previously missed (2)

In code that hasn't changed since last review

Replacement patterns corrupt plugin paths

src/​vs/​workbench/​contrib/​mcp/​common/​discovery/​pluginMcpDiscovery.ts:98

These paths are used as replacement strings, so JavaScript interprets replacement patterns such as $&, $\`` and $'; the second call can also re-expand token text inserted by the first. For example, a plugin directory ending in $&leaves${PLUGIN_ROOT}` in the launched argument. Replace both tokens in one callback-based pass so filesystem paths are inserted literally and never rescanned.

Plugin definition changes do not update the registry

src/​vs/​workbench/​contrib/​mcp/​common/​discovery/​pluginMcpDiscovery.ts:240

This snapshots a file-backed observable and then exposes the result as a constant. mcpServerDefinitions is re-read when mcp.json changes, but the outer autorun only recreates an existing collection when dataDirKey changes. Therefore, editing a plugin from one non-empty definition list to another leaves the registry serving the old definitions; preserve a reactive definition pipeline or explicitly recreate the collection when the definitions change.

[Copilot]

Copilot review overview

🟡 Changes recommended

Token replacement, path containment, update persistence, and a failing hash expectation need correction.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 2

Open (2)

• Use stable plugin identity for persistent plugin data directories · New
• Use production SHA-1 data directory naming in the test · New

Resolved since last review (4)

• Remote plugin data paths use collision-prone hashes
• 32-bit plugin hashes can collide on local data directories
• MCP definitions stop updating after non-empty edits
• Plugin data directory remains stale after profile switches

Previously missed (1)

In code that hasn't changed since last review

Prevent recursive and replacement-string placeholder expansion

src/​vs/​workbench/​contrib/​mcp/​common/​discovery/​pluginMcpDiscovery.ts:98

Placeholder expansion must be a single, non-recursive pass. These chained replacements rescan text inserted for PLUGIN_ROOT, so a valid root containing the literal ${PLUGIN_DATA} is incorrectly rewritten; replacement-string sequences such as $& in a legal filesystem path are also interpreted by replaceAll. Use one callback-based replacement so inserted path text stays literal.

This issue also appears on line 154 of the same file.

[Copilot]

Copilot review overview

🟢 Approval recommended

The implementation matches the canonical format’s field restrictions and includes focused coverage for local, remote, and update-stability behavior.

Review effort: Balanced
Findings: None

Resolved since last review (2)

• Use production SHA-1 data directory naming in the test
• Use stable plugin identity for persistent plugin data directories