Skip to content

MCP diagnostics are process-global, so one project's discovery erases another's #1211

Description

@sahrizvi

Summary

The four MCP diagnostic records are module-level singletons keyed by server name alone, with no notion of which project they belong to. In a process serving more than one project — altimate serve, which is how the extension and every hosted user reach the agent — a second project's discovery erases the first's diagnostics, and two projects that reuse a server name overwrite each other.

record file
_unresolvedEnv packages/opencode/src/mcp/discover.ts:53
_drift packages/opencode/src/mcp/discover.ts:83
_discoveredSource packages/opencode/src/mcp/discover.ts:156
_blankedEnv packages/opencode/src/config/variable.ts:37

Reproduction

Two temp projects, each with a .vscode/mcp.json whose server has one unresolved {env:VAR}. Run against d00931b5e6 (current main):

after A:  unresolvedEnvVars('alpha') = ["ALTIMATE_REPRO_VAR_A"]
after B:  unresolvedEnvVars('alpha') = []                          ← erased
after B:  unresolvedEnvVars('beta')  = ["ALTIMATE_REPRO_VAR_B"]

And with both projects using the same server name:

shared name: unresolvedEnvVars('datamate') = ["ALTIMATE_REPRO_VAR_B"]   ← A's answer gone

datamate is not a hypothetical collision — it is the name the extension sync writes into every project.

Why it happens

discoverExternalMcp(projectDir) clears all three of its records at the top of the run and repopulates them afterwards. That was deliberate — it is what stops a variable that has since been fixed from being reported forever (#1121) — but the clear is global, so it takes the other project's entries with it. _blankedEnv has the same shape: blankedEnvVars() returns every config source ever parsed in the process, not the active project's.

The clear also sits before the first await while the writes happen after several, so concurrent discovery can interleave one project's clear with another's writes.

Impact

mcp list, mcp status and /mcps are the surfaces people use when a server will not connect. Under altimate serve — the path users actually run, not a niche debugging mode — they can report:

  • nothing, for a project whose diagnostics another project cleared
  • another project's variable names, under a shared server name
  • drift attributed to a file belonging to a different project

A wrong answer here is worse than none, because the whole point of #1121/#701/#790/#878 was to stop people guessing.

Suggested fix

Key each record by projectDir and take the project as a parameter:

export function unresolvedEnvVars(server: string, projectDir: string): string[]
export function configDrift(projectDir: string): { server: string; source: string; fields: string[] }[]
export function discoveredSource(server: string, projectDir: string): string | undefined
export function blankedEnvVars(projectDir: string): { source: string; names: string[] }[]

A discovery run then clears only its own project's entries, which keeps the staleness fix from #1121 while making the clear harmless to everyone else. This is the InstanceState convention the rest of the codebase already follows for per-directory state.

Call sites to update: cli/cmd/mcp.ts (reportConfigDiagnostics), session/prompt.ts (/mcps), and config/config.ts (drift recording).

Provenance

Flagged independently by cubic, kilo, and the harness bot across #1159 and #1160, and deliberately deferred from both as too broad for those PRs.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions