From 4b784c51d598a07fc805a7c8075ef4befd217b7c Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Tue, 22 Sep 2026 19:11:10 -0700 Subject: [PATCH 01/11] docs(mcp): add user MCP servers design and implementation plan Agent Code has no way to add or toggle third-party MCP servers. The design settles on launch-time injection through the existing built-in MCP path rather than writing provider config, the de facto mcpServers shape with ${input:id} secret references, and per-provider defaults plus per-agent overrides. The evidence behind each decision is recorded in the spec so implementation does not re-derive it. Refs #1143 Refs #244 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../plans/2026-09-22-user-mcp-servers.md | 188 ++++++++ .../2026-09-22-user-mcp-servers-design.md | 442 ++++++++++++++++++ 2 files changed, 630 insertions(+) create mode 100644 docs/superpowers/plans/2026-09-22-user-mcp-servers.md create mode 100644 docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md diff --git a/docs/superpowers/plans/2026-09-22-user-mcp-servers.md b/docs/superpowers/plans/2026-09-22-user-mcp-servers.md new file mode 100644 index 000000000..6ace958c3 --- /dev/null +++ b/docs/superpowers/plans/2026-09-22-user-mcp-servers.md @@ -0,0 +1,188 @@ +# User MCP Servers: Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Let users add any MCP server, set its secrets, and choose which providers get it (Claude Code, Codex). They can override it per agent and turn it on or off from Settings → MCP and the command palette. Servers are delivered at launch through the same path Agent Code's built-in MCP already uses, and provider config files are never written. + +**Architecture:** +- Storage and secrets: + - A main-owned store (`STATE_DIR/mcp-servers.json`, 0600) holds entries in the de facto `mcpServers` shape. Entries use VS Code-style `${input:id}` secret references. + - Secret values live in `safeStorage` blobs and never reach the renderer. +- IPC and renderer: IPC and a broadcast follow the provider-enablement pattern (#1126). A non-persisted zustand mirror sits in the renderer. +- Launch flow: + - The renderer resolves attached server **ids** from the provider defaults plus the per-pane `userMcpOverrides`, and sends them with spawn/recover options. + - Main validates them, resolves secrets, and hands `ResolvedUserMcpServer[]` to the provider launchers. + - The Claude launcher adds them to the existing private `--mcp-config` file. + - The Codex launcher adds `-c mcp_servers.*` overrides, with secrets passed through the environment. + - A bad server is dropped with a notice. It never fails the launch. + +**Spec:** `docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md`. Read its Evidence and Decisions sections before starting any task; every "why" is there. + +**Issue:** #1143 (Refs #244). + +**Working tree:** `.worktrees/user-mcp-servers`, branch `feat/user-mcp-servers`, based on `origin/main` `672d0941`. Setup: `git submodule update --init` and `ln -s ../../node_modules node_modules` (see the memory note on worktree setup). + +**Conventions:** +- Thick WHY comments (AGENTS.md); each non-obvious decision in the spec's Decisions table gets a comment at the code site that enforces it. +- Commit messages use Conventional Commits with scope `mcp`. +- Tests protect behavior. Fixtures are the **real** published Beeper snippets (quoted in the spec), not invented shapes. +- Verification: `npx tsc -b` plus the `unit`/`renderer` vitest projects. Run the full suite once at the end, not per task. +- Never launch the app. + +--- + +### Task 1: Shared contracts and pure validation + +**Files:** +- Create: `src/shared/types/userMcp.ts` (types from the spec's Contracts section, `USER_MCP_PROVIDERS`, reserved names) +- Create: `src/shared/userMcp/validate.ts` and `validate.test.ts` +- Create: `src/shared/userMcp/inputs.ts` and `inputs.test.ts` (`${input:id}` scan and substitute) + +- [ ] Write failing tests: + - Name rules: `^[A-Za-z0-9_-]{1,64}$`; `agent_code`, `AGENT_CODE` and `agent-code-control` are rejected; a duplicate name among servers raises `duplicate-name`. + - Entry validation: + - stdio needs a `command`; http and sse need an absolute `http(s)` url. + - A `type`-less entry with a `url` normalizes to `http`. + - A mixed `command` + `url` entry is `invalid-entry`. + - `${input:x}` is allowed in `env` and `headers` values, and gives `secret-in-forbidden-field` in `command`, `args` or `url`. An undefined input id gives `unknown-input`. + - Support matrix: sse → `codex: { ok:false, reason: 'Codex does not support SSE MCP servers' }`. + - Unknown extra entry keys are preserved through `coerceUserMcpDocument`. + - Coercion: malformed servers are kept but flagged, never silently deleted; a non-object document becomes `{version:1, servers:[]}`. +- [ ] Implement until the tests are green. The WHY comments cover the charset intersection, why names are not prefixed, and why only `env`/`headers` may carry secrets. +- [ ] Commit `feat(mcp): add user MCP server contracts and validation`. + +### Task 2: Import parser + +**Files:** `src/shared/userMcp/importConfig.ts` and `importConfig.test.ts` + +- [ ] Fixtures (verbatim from developers.beeper.com, as quoted in the spec): + - `{"mcpServers":{"beeper":{"url":"http://localhost:23373/v0/mcp","headers":{"Authorization":"Bearer YOUR_TOKEN_HERE"}}}}` + - The `@beeper/mcp-remote` stdio snippet with `env.ACCESS_TOKEN`. + - The VS Code `{"servers":{"beeper":{"type":"http",…}}}` form. + - A VS Code form with `inputs:[{type:'promptString',id,password:true}]`. + - A bare `{name: entry}` map, and a single bare entry. +- [ ] Assert that every literal env or header value becomes `${input:-}`, with its pasted value returned separately as `pendingSecrets`. The resulting entry must contain no literal token. +- [ ] Assert that malformed JSON returns a typed error, not a throw. +- [ ] Commit `feat(mcp): import MCP server configs from standard snippets`. + +### Task 3: Main store, secrets, IPC, broadcast + +**Files:** +- Create: `src/main/userMcp/store.ts`, which loads and coerces `STATE_DIR/mcp-servers.json` and does atomic 0600 writes through a temp file plus rename. Mutations run through one serialized queue. +- Create: `src/main/userMcp/secrets.ts`, a `safeStorage` blob per `/.bin` following the `src/main/dictation/apiKeyStore.ts` pattern. It returns only `{set, hint}`, and deleting a server deletes its secrets. +- Create: `src/main/userMcp/service.ts`, which builds the snapshot (`UserMcpServerView[]` with problems and support), handles mutations (return snapshot + emit), and provides `resolveForLaunch(ids, provider, cwd)`. That resolver is called in Task 5. +- Create: `src/main/ipc/userMcp.ts`, registered in `src/main/ipc/index.ts`, with channels per the spec and argument validation mirroring `ipc/providerEnablement.ts`. +- Create: `src/preload/api/userMcp.ts`, and expose it in the preload API types. +- Wire the service in `src/main/index.ts` next to the provider enablement construction. + +- [ ] Tests: + - Store round trip keeps unknown keys, and the file mode is 0600. + - A corrupt file is preserved (renamed `.corrupt-`), and the store starts empty with a visible problem. Silently resetting would lose the user's config. + - A secret set or clear never appears in the snapshot, only `hint`. + - Mocking `safeStorage` as unavailable surfaces a `secret-missing` problem, not a crash. +- [ ] Commit `feat(mcp): persist user MCP servers and secrets in main`. + +### Task 4: Provider launch translators (pure) + +**Files:** `src/providers/shared/runtime/userMcpLaunch.ts` and `userMcpLaunch.test.ts`; modify `builtInMcpLaunch.ts` + +- [ ] `generatedSecretVar(serverName, key, taken)` is deterministic: `AGENT_CODE_USER_MCP__`, sanitized, with a numeric suffix on collision. A test asserts it is stable when another server is added or removed. Comment the Claude OAuth-key reason (spec, "Secret variable naming"). +- [ ] `claudeUserMcpEntries(servers) → { entries, env }` and `codexUserMcpLaunchConfig(servers, args, env) → { dropped }`: + - Codex stdio: + - Emit `command`, `args` (TOML array), `cwd` and `env_vars=[…]`, with the values placed in `env`. + - If two attached stdio servers use the same env key with different values, drop the second with a reason. + - Codex http: every header goes through `env_http_headers`. + - Unknown keys: + - Claude: passed through verbatim. + - Codex: ignored, and their names are returned for the UI note. +- [ ] Change `createPrivateClaudeMcpConfig(builtIns, userEntries)` so it writes one file when either list is non-empty. The existing callers (`claudeSession.ts:1189-1202`) keep a single `--mcp-config` as the last flag. +- [ ] Golden tests using the Beeper fixtures: + - Claude file JSON. + - Codex argv, with the assertion **no token substring appears in args**. + - The stdio `mcp-remote` case on both providers. +- [ ] Commit `feat(mcp): translate user MCP servers into Claude and Codex launch config`. + +### Task 5: Spawn and recover wiring in main + +**Files:** +- `src/shared/types/session.ts`: add `userMcpServerIds?: string[]` to the spawn and recover options, and add the observed `userMcpServerIds` to `SessionInfo`. The comment makes the same "observed, not requested" point as `builtInMcpDomains`. +- `src/main/sessionManager.ts`: + - At the `builtInMcpServers` assembly (~:2875), call `userMcp.resolveForLaunch`, which: + - drops unknown, disabled, unsupported, problem, missing-secret and native-collision servers; + - applies the Claude `managed-mcp.json` lock. + - Pass `userMcpServers` into `createSession`. + - Emit `user-mcp-unavailable` for dropped servers (mirror `reportSkillsUnavailable` at :2724), and forward it to the renderer the same way. + - Recovery that adopts an existing process keeps that process's recorded ids. +- `src/providers/claude/runtime/claudeSession.ts` and `src/providers/codex/runtime/codexSession.ts`: accept `userMcpServers`, call the Task 4 translators, and put the generated variables into the spawn env. +- Native collision check (Codex): `src/main/userMcp/nativeCodexServers.ts` parses the `mcp_servers` keys from `${CODEX_HOME:-~/.codex}/config.toml` and `/.codex/config.toml` with `@iarna/toml`. It is read-only, and a parse failure means "no collisions known", which it logs. + +- [ ] Tests: + - A spawn with a missing secret still spawns, and emits unavailable. + - A Codex collision drops only Codex. + - `SessionInfo.userMcpServerIds` equals what was actually attached. + - The token is absent from the recorded argv (there is an existing spawn-args capture harness in the codex/claude session tests; reuse it). +- [ ] Commit `feat(mcp): attach user MCP servers when agents launch`. + +### Task 6: Renderer mirror, resolution, per-pane overrides + +**Files:** +- Create: `src/renderer/src/features/userMcp/store.ts`, a mirror plus `useUserMcpSync()`, mounted once in `App.tsx` next to `useProviderEnablementSync`. +- Create: `src/renderer/src/workspace/userMcp.ts` with `resolveSessionUserMcpServerIds` and `normalizeUserMcpOverrides`. Test it in `userMcp.test.ts`, covering the master switch beating an override, provider default, override on/off, and an unsupported provider. +- Modify `src/renderer/src/workspace/types.ts` to add `userMcpOverrides` and `userMcpServerIds` beside the built-in fields (:283-286). +- Modify `workspace/mcpDomains.ts`: `clonedMcpOverrides` also copies `userMcpOverrides`. +- Modify `workspace/hook/actions/session.ts` (the fresh spawn at ~:378 and the replacement/reload at ~:1258) to send `userMcpServerIds`. +- Modify `workspace/builtInMcpReload.ts`, which gains the user-override variant through the same reload path. Don't fork a second reload implementation. +- Modify the workspace persistence coercion so the new pane fields survive save and load. +- Show the `user-mcp-unavailable` notice where `managed-skills-unavailable` is shown today. + +- [ ] Commit `feat(mcp): resolve user MCP servers per agent in the workspace`. + +### Task 7: Settings → MCP + +**Files:** +- `settingsCategories.ts`: add category `mcp` ("MCP", "Servers your agents can use, and Agent Code's own MCP tools."). +- `settingsRegistry.ts`: + - Add the `user-mcp-servers` marker row (`storage: 'external-files'`, `apply: 'new-session'`). + - Move the built-in default MCP rows and `external-control` into the new category, changing the category only. +- `SettingsList.tsx`: dispatch the new row. +- Create: `src/renderer/src/features/userMcp/ui/UserMcpServersRow.tsx` and `UserMcpServerDialog.tsx`: + - List, master switch, and per-provider checkboxes, showing only providers enabled in `useEnabledAgentProviderKinds()`. + - Problem and support chips, Edit, Delete, and Sign in… for HTTP servers. + - The dialog has a name field, a JSON entry editor, and masked secret fields derived from the `${input:*}` references. Validation comes from main. + - Paste-import supports multiple servers. +- Sign in…: + - Codex: open a terminal pane running `codex mcp login ` with the same `-c` url override (the spec notes that login reads the effective config). + - Claude: show inline guidance to run `/mcp` in an agent that has the server attached. +- `ui.openSettings(category?)`: add the optional argument (`command-palette/types.ts:202`) and its implementation. + +- [ ] Renderer tests: + - The row hides the Codex column when Codex is disabled in Providers. + - An SSE server's Codex checkbox is disabled, with the reason shown. + - The dialog never renders a secret value. +- [ ] Commit `feat(mcp): add MCP settings with user server management`. + +### Task 8: Commands + +**Files:** a new `src/renderer/src/features/userMcp/commands.ts` registered in `command-palette/catalog.ts`; `sessionCommands.ts` (`use-global-mcp-settings` also clears user overrides); and `catalog.test.ts` (baseline ids, counts, approved-additions list), plus `taxonomy.test.ts` if tiers apply. + +- [ ] Add `user-mcp-servers` (`MCP Servers…`), `add-user-mcp-server` (`Add MCP Server…`) and `agent-user-mcp-servers` (`Agent MCP Servers…`) as the spec's command table describes. Follow `docs/command-style.md`: descriptions use the "What it does / Use when / Notes" format, and the session command re-checks the provider inside `run`. +- [ ] Commit `feat(mcp): add MCP server commands`. + +### Task 9: Documentation and final verification + +- [ ] Update the README "What you can do with it" section with an **MCP servers** bullet, and update the `ARCHITECTURE.md` provider-integration section to cover user servers delivered at launch. +- [ ] Run `npx tsc -b` and `npm test`, once, and compare any failures against the known local failures noted in memory. +- [ ] Run a real-binary check, read-only and with no app launch: + - Codex: run `codex mcp list` with the generated `-c` overrides for the Beeper fixture, and confirm it parses and lists `beeper` with the header env var. + - Claude: run `claude mcp list --mcp-config ` if it honors the flag; if not, note that in the PR. +- [ ] Open a PR with the title `feat(mcp): manage user MCP servers per provider and per agent`, with `Fixes #1143` and `Refs #244`. Do not merge. + +--- + +## Out of scope (follow-up issues) + +- Read-only "Also loaded natively" list. +- Add from MCP Registry. +- OpenCode and Grok. +- User servers in workflow subagents. +- #244 hosted extension servers. diff --git a/docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md b/docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md new file mode 100644 index 000000000..917fefd69 --- /dev/null +++ b/docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md @@ -0,0 +1,442 @@ +# User MCP servers — design + +Status: draft for review · Date: 2026-09-22 · Branch: `feat/user-mcp-servers` +Issue: #1143 (Refs #244) + +## Problem + +Agent Code cannot add, configure, or switch off a third-party MCP server. The +only MCP servers it knows about are its own: the `agent_code` built-in host +(TLDR, Goal, orchestration, …) and the reserved `agent-code-control` external +operator server. A user who wants, say, the Beeper Desktop MCP server attached to +their agents today has to leave the app and hand-edit `~/.claude.json` or +`~/.codex/config.toml` (or run `claude mcp add` / `codex mcp add`). They then +get it on *every* agent of that provider, can't switch it off for one agent, +can't see it anywhere in Agent Code, and have to repeat the work per provider. + +Wanted: a single place in Agent Code to add **any** MCP server (stdio, +Streamable HTTP, SSE; with secrets; OAuth-capable), choose which providers it +attaches to, override it per agent, and flip it on/off from Settings and the +command palette. The scope is **Claude Code and Codex**. Beeper Desktop is the +worked example and acceptance fixture, not a special case. + +This complements the provider enablement feature (#1126, Settings → +Providers). Enablement decides which providers exist in the app, and this +feature decides which user MCP servers each of them gets. + +## Evidence this design rests on + +The research ran on 2026-09-22 and is summarized here so the next session +doesn't have to redo it. + +### What Agent Code does today + +- **Built-in MCP.** One loopback Streamable-HTTP host + (`src/mcp/runtime/BuiltInMcpHttpHost.ts`) mints a per-session bearer and + returns one `BuiltInMcpServerConfig` named `agent_code`. + `SessionManager.spawn` (`src/main/sessionManager.ts:2875`) passes + `builtInMcpServers` into the provider's `createSession`. +- **Per-launch injection, no durable writes** + (`src/providers/shared/runtime/builtInMcpLaunch.ts`): + - **Claude** gets a mode-0600 private temp `mcp.json`, passed as + `--mcp-config ` and deleted on stop or rollback + (`claudeSession.ts:1189-1202`). + - **Codex** gets `--config mcp_servers..url=…` plus + `env_http_headers.="AGENT_CODE_MCP_i_j"`. Values go in the child env, + never in argv. + - **OpenCode** gets `OPENCODE_CONFIG_CONTENT` with `{env:…}` references. +- **User-native servers already load.** `--strict-mcp-config` is deliberately + *not* used. Claude gets a targeted `deniedMcpServers` instead, inside the + **single** `--settings` value built by `excludeExternalControlFromClaude` + (`externalControlExclusion.ts:47-60`). Codex keeps the user's `CODEX_HOME`. + So whatever the user put in `~/.claude.json`, `.mcp.json` or + `config.toml` still loads, but Agent Code never reads, shows or edits it. +- **Settings model for built-ins.** + - Global `settings.defaultBuiltInMcpDomains` lives in renderer + localStorage. + - Per-pane `builtInMcpOverrides` (absent means inherit, `false` means + explicit off) is resolved by `resolveSessionBuiltInMcpDomains` + (`src/renderer/src/workspace/mcpDomains.ts`). + - The pane's `builtInMcpDomains` is the *observed* launched set, not a + choice. + - Changes apply on the next spawn or reload through + `reloadSessionWithBuiltInMcpOverrides`. +- **Codex `config.toml` writer precedent.** The only writer into provider-owned + config is `src/main/settings/externalCodexConfig.ts`: a hash-stamped managed + block, deep-equal proof that nothing else changed, refusal to touch a + same-name unmanaged server, and observed-compare atomic replace. It is about + 130 lines of safety for **one** server. That cost is why this design does + not write provider config. +- **Provider enablement (#1126)** is the store/IPC/broadcast pattern to copy: + - main-owned state with a coerce-on-load setter; + - `provider-enablement:get|set|reset` IPC plus a + `provider-enablement:changed` broadcast; + - a non-persisted zustand mirror in the renderer + (`features/providers/store.ts`) with a snapshot getter and a hook, synced + once from `App.tsx`; + - a self-subscribing settings "marker row" (`ProviderEnablementRow.tsx`). +- **Secrets precedent.** `src/main/dictation/apiKeyStore.ts` uses Electron + `safeStorage` per-file blobs with no auth prompt; the renderer only sees a + last-4 hint. The key vault (`src/main/keyVault`) gates every read behind + Touch ID, which is wrong for secrets resolved during automatic restore. +- **Open issue #244** ("Host user-authored MCP servers as Agent Code + extensions") is broader: Agent Code *hosting* author-written servers through + a manifest. This design is the configuration layer #244 needs anyway. A + hosted extension server can later appear as one more entry in the same list. + +### How the CLIs load MCP servers (vendor source + installed claude 2.1.280 / codex-cli 0.155.1) + +| | Claude Code | Codex | +|---|---|---| +| Per-launch injection | `--mcp-config …` (variadic, later wins, overrides all file scopes) | `-c mcp_servers..*=` (TOML-valued, deep-merged SessionFlags layer, precedence 30) | +| Launch vs user entry, same name | Whole entry replaced | **Deep-merged key by key.** A user `command` plus our `url` becomes an invalid mixed-transport config, and the launch fails | +| Transports | stdio, http, sse (deprecated), ws | stdio, streamable HTTP. **No SSE** | +| Secret indirection | `${VAR}` / `${VAR:-d}` expanded in command, args, env, url, headers | No expansion. Use `env_vars=[names]` (stdio), `bearer_token_env_var`, `env_http_headers` | +| stdio child env | Inherits Claude's full env plus `env` | Fixed allowlist (HOME, PATH, …) plus `env_vars` plus literal `env` | +| Name charset | `[A-Za-z0-9_-]` | `[A-Za-z0-9_\-:@/.]`, but `-c` splits paths on `.` naively and never unquotes | +| OAuth | Keychain `mcpOAuth`, key `name\|sha256({type,url,headers})[:16]`. `/mcp` login works for `--mcp-config` servers | Keyring "Codex MCP Credentials", key `name + url`. `codex mcp login` searches the *effective* config, so `-c` servers work | +| Persistent per-server off | `disabledMcpServers` in `~/.claude.json` `projects[gitRoot]`, checked **by name for every scope including `--mcp-config`** | `enabled=false` in config.toml | +| Invalid entry | Process exits 1 | Config load fails, so the launch fails | +| Enterprise lock | `managed-mcp.json` present: non-sdk `--mcp-config` entries are rejected and the process exits | `requirements.toml` allowlist can disable | +| Live reload in TUI | Only on `/clear` or `/reload-plugins` | Not reachable from the TUI | + +### Beeper Desktop MCP (worked example) + +- **Built in.** Beeper Desktop has its own MCP server (enable it under + Settings → Developers / Integrations). It runs as Streamable HTTP at + `http://localhost:23373/v0/mcp`. +- **Auth.** OAuth 2.0 + PKCE is the default. Alternatively + `Authorization: Bearer ` with a token from Settings → Integrations + → Approved connections, which bypasses OAuth. +- **Official snippets:** `claude mcp add beeper http://localhost:23373/v0/mcp -t http`, + `codex mcp add beeper --url http://localhost:23373/v0/mcp [--bearer-token-env-var X]`, + and the generic `{"mcpServers":{"beeper":{"url":…,"headers":{"Authorization":"Bearer …"}}}}`. +- **stdio alternatives:** `npx -y @beeper/mcp-remote` or `@beeper/desktop-mcp` + (env `BEEPER_ACCESS_TOKEN`). + +### Open-source prior art (what to build on) + +- **De facto config shape.** Most server READMEs publish + `{"mcpServers": {"": {command,args,env} | {type,url,headers}}}`. + Claude Code, Claude Desktop and Cursor use it. VS Code differs: it uses + `servers` and has `inputs` with `password: true`, referenced as + `${input:id}`, prompted once and stored in a secret store. Claude Desktop's + MCPB manifest converged on the same idea (`user_config` with + `sensitive: true`). +- **`add-mcp` (Apache-2.0, neon-solutions).** Its per-client + `transformConfig(name, cfg)` is the right shape, and ~30 lines per client. + It isn't worth depending on: it has CLI deps, and its Codex transform is + wrong (SSE, `type` key, no env indirection). We copy the idea, not the + package. +- **Aggregators (MetaMCP, 1MCP, MCPHub, ToolHive, Docker gateway): rejected.** + - A supervised runtime to bundle. + - Breaks each CLI's native OAuth. + - Rewrites tool names (breaking permission rules and our transcript + rendering). + - A single point of failure for all servers. + - Their one advantage, hot-swap without restart, doesn't outweigh these. +- **MCP Registry** (registry.modelcontextprotocol.io, v0.1 API frozen, still + *preview*). `server.json` `packages[]` / `remotes[]` with + `isSecret`-flagged env and headers maps directly onto our inputs model. It + is a good optional "Add from registry" source later, but not v1. Beeper + isn't listed. + +## Decisions + +| Question | Decision | Why | +| --- | --- | --- | +| Deliver by writing provider config, or at launch? | **At launch only.** Never write `~/.claude.json`, `.mcp.json` or `config.toml`. | Reuses the proven built-in path. There's no clobber race with live Claude processes (which rewrite `~/.claude.json` constantly), no `codex mcp add` table rewrite, and a per-agent subset comes free. The price, "needs an agent reload", already holds for built-in MCP and for both CLIs' own config. | +| Canonical storage format | **The de facto `mcpServers` entry shape**, plus VS Code-style `${input:id}` secret references and a little Agent Code metadata. | Users paste README snippets unchanged. The format is already understood by every MCP author, so there is no bespoke schema to learn or document. | +| Where state lives | **Main-owned** `STATE_DIR/mcp-servers.json` (0600), not renderer localStorage. | Main resolves secrets and builds launch material. Multiple windows need one owner. This matches provider enablement. | +| Secrets | `safeStorage` per-file blobs under `STATE_DIR/mcp-secrets/`, no auth prompt, never sent to the renderer (hint only). | A Touch ID-gated vault would prompt during automatic restore or reload. This is the dictation key precedent. | +| Where may `${input:id}` appear? | **Only in `env` values and `headers` values.** Rejected in `command`, `args` and `url`. | Codex doesn't expand variables, so a secret in args or url lands in argv / `ps`. Servers that need a token on their command line (e.g. `mcp-remote --header "…${X}"`) already expand their *own* env, so the user writes `${X}` in args and puts the secret in `env.X`. | +| Toggle semantics | Three levels. **`enabled`** is a master switch ("off" means off everywhere, even with a per-agent on). **`providers.{claude,codex}`** is the default attachment for agents of that provider. **Per-agent overrides** add or remove a server for one agent. | Mirrors the built-in MCP defaults-plus-overrides model users already know, plus Cursor's "toggle without deleting". | +| Provider scope | Claude and Codex. The data model is keyed by provider, so OpenCode or Grok is an additive task later. | User scope. OpenCode's launcher already has an inline-config path to extend. | +| Transport support matrix | stdio: Claude + Codex. http: Claude + Codex. sse: **Claude only** (the Codex column is disabled with a reason). | Codex has no SSE. | +| Name rules | Must match `^[A-Za-z0-9_-]{1,64}$`. `agent_code` and `agent-code-control` are rejected (case-insensitive). No automatic prefixing. | The charset is the intersection of both CLIs and `-c` path parsing. A prefix would uglify every tool name (`mcp__ac-beeper__send_message`) and break the permission rules users copy from docs. | +| Same name as a user-native server | **Codex: skip injecting that server for this spawn and warn.** Claude: inject (launch replaces the native entry), and show a notice in Settings. | The Codex deep merge can create an invalid mixed-transport entry that kills the whole launch. Claude replacement is well-defined. | +| OAuth | **Delegated to each CLI.** Settings offers "Sign in…" for HTTP servers. For Codex it opens a terminal pane running `codex mcp login ` with the same `-c` overrides. For Claude it tells the user to run `/mcp` in an agent that has the server attached. | Both CLIs already store OAuth tokens securely and key them stably. Building our own OAuth client would duplicate that and break the CLIs' own refresh logic. | +| Aggregator/proxy | Rejected. | See prior art above. | +| Registry / catalog | Out of v1. Follow-up issue. | Preview API; paste already covers "any server". | +| Workflows (Codex SDK subagents) | Out of scope. They keep their private `CODEX_HOME` without user servers. | Replay safety (`inheritedMcpServers: 'unknown'`). | +| Settings location | New **MCP** category holding the user-servers row, **plus** the existing built-in MCP default rows and the External operator MCP row moved from Agents. | One place for MCP. The move is a category change on existing registry entries (opportunistic cleanup within blast radius). | +| Editing UI | **One JSON editor per server** (the standard entry shape), plus masked secret fields generated from the `${input:*}` references it contains. No field-by-field form builder. | "Any MCP server" means any shape a README publishes. A form either restricts that or grows forever. The user explicitly asked for a lean UI. | + +## Contracts + +### Stored document (`STATE_DIR/mcp-servers.json`, mode 0600) + +```ts +// src/shared/types/userMcp.ts +export const USER_MCP_PROVIDERS = ['claude', 'codex'] as const +export type UserMcpProvider = (typeof USER_MCP_PROVIDERS)[number] + +/** The de facto `mcpServers` entry. Kept structurally identical to what + * READMEs publish so paste → store → re-export is lossless. */ +export type UserMcpServerEntry = + | { type?: 'stdio'; command: string; args?: string[]; env?: Record; cwd?: string } + | { type: 'http' | 'sse'; url: string; headers?: Record } + +export type UserMcpInput = { + id: string // referenced as ${input:}; ^[A-Za-z0-9_-]{1,64}$ + description: string // shown next to the masked field +} + +export type UserMcpServer = { + id: string // stable uuid; overrides key on this, never on name + name: string // provider-visible name, see name rules + enabled: boolean // master switch + providers: Record // default attachment per provider + entry: UserMcpServerEntry + inputs: UserMcpInput[] // secret definitions only; values live in safeStorage +} + +export type UserMcpDocument = { version: 1; servers: UserMcpServer[] } +``` + +A pasted entry that has no `type` but does have `url` is normalized to +`type: 'http'`. That is Cursor/Claude Desktop behavior, and Streamable HTTP is +the current transport. Unknown extra keys in `entry` are **preserved**, not +dropped (see "What would make this wrong"). + +### Renderer-visible snapshot (IPC, no secret values) + +```ts +export type UserMcpServerView = UserMcpServer & { + secrets: Record + problems: UserMcpProblem[] // validation + readiness, computed in main + support: Record +} +export type UserMcpProblem = + | { kind: 'invalid-name' | 'reserved-name' | 'invalid-entry'; message: string } + | { kind: 'secret-in-forbidden-field'; field: string } + | { kind: 'unknown-input'; inputId: string } // ${input:x} with no definition + | { kind: 'secret-missing'; inputId: string } + | { kind: 'duplicate-name'; otherId: string } +``` + +IPC channels are `user-mcp:get`, `user-mcp:save-server` (upsert by id), +`user-mcp:delete-server`, `user-mcp:set-enabled`, `user-mcp:set-provider`, +`user-mcp:set-secret`, `user-mcp:clear-secret` and `user-mcp:import` (parse +only, returns candidate servers), plus the broadcast `user-mcp:changed`. Every +mutation returns the new full snapshot, as provider enablement does. + +### Per-agent state (renderer pane metadata, beside `builtInMcpOverrides`) + +```ts +userMcpOverrides?: Record // absent = inherit +userMcpServerIds?: string[] // OBSERVED: what the running process was launched with +``` + +`resolveSessionUserMcpServerIds({ provider, servers, overrides })` returns +enabled servers whose provider default is on, plus overrides set to `true`, +minus overrides set to `false`. It then filters out servers whose `enabled` is +false and servers that don't support that provider. It is a pure function in +`src/renderer/src/workspace/userMcp.ts`. + +`clonedMcpOverrides` copies `userMcpOverrides` too, because a duplicate +should keep its tools. `Use Global MCP Settings` clears both override maps. + +### Spawn contract (renderer → main) + +`SessionSpawnOptions` / `SessionRecoverOptions` gain +`userMcpServerIds?: string[]`. Main is the authority: + +1. It re-reads its store and drops unknown ids, disabled servers, servers + unsupported on this provider, servers with problems, and servers with a + missing secret. +2. It resolves secrets and builds `ResolvedUserMcpServer[]`. +3. It passes that list into `createSession({ …, userMcpServers })`. +4. `SessionInfo.userMcpServerIds` reports what was actually attached. + +For every dropped id, main emits `user-mcp-unavailable +{ sessionId, servers: [{ name, reason }] }`, shown the same way as +`managed-skills-unavailable` (`sessionManager.ts:220, 2724`). **A bad user +server never fails an agent launch.** + +The renderer sends ids, not configs, because secrets never leave main, and +because a stale renderer snapshot must not be able to inject a server the user +has since deleted. + +### Launch material (main, per provider) + +`src/providers/shared/runtime/userMcpLaunch.ts`. Each translator is a pure +function (`servers → { claudeEntries | codexArgs, env }`), in the spirit of +add-mcp's `transformConfig`. + +**Secret variable naming.** Each `env`/`headers` value that contains +`${input:…}` is fully substituted in main. The result goes in a generated +variable named +`AGENT_CODE_USER_MCP__`: uppercased, non-alphanumerics mapped to +`_`, and made collision-free with a numeric suffix. The name is +**deterministic across spawns**. That is required because Claude keys stored +OAuth tokens on `hash(type,url,headers)`, and headers carry these variable +names, so an index-based name that shifted when another server was toggled +would silently discard the user's OAuth login. + +**Claude.** User entries merge into the **same** private 0600 file that +`createPrivateClaudeMcpConfig` already writes, so there's still one +`--mcp-config`, still last among the flags: + +- stdio: `{type:'stdio', command, args, env:{K: '${AGENT_CODE_USER_MCP_…}' | literal}}`. +- http/sse: `{type, url, headers:{H: '${AGENT_CODE_USER_MCP_…}' | literal}}`. +- The generated variables go into the Claude process env, and Claude expands + them. +- The file is created even when no built-in servers are enabled. Today the + function returns `null` for an empty list, so the signature widens. + +**Codex.** Extend beside `addCodexBuiltInMcpLaunchConfig`: + +- stdio: + - `mcp_servers..command="…"` + - `mcp_servers..args=[…]` (TOML array via JSON-compatible literal) + - `mcp_servers..cwd="…"` + - `mcp_servers..env_vars=["K1","K2"]`, where each `K` is set to its + substituted value in the Codex process env. Codex passes env to stdio + children only through its allowlist plus `env_vars`, and a literal `env` + would put values in argv. + - **Collision:** if two attached stdio servers need the same env key with + different values, the second is dropped with a reason. `env_vars` can't + rename. +- http: + - `mcp_servers..url="…"`. + - Every header goes through `env_http_headers.=""`, + exactly like the built-ins. That includes `Authorization`: Codex rejects + literal `bearer_token`, and `env_http_headers` covers it without special + casing. +- Name collision with a user-native Codex server: before building args, main + parses `${CODEX_HOME:-~/.codex}/config.toml` `mcp_servers` keys with + `@iarna/toml` (already a dependency). On a hit, it skips and warns. A + project `.codex/config.toml` in the cwd is checked the same way. + +**Claude enterprise lock.** If `managed-mcp.json` exists (macOS: +`/Library/Application Support/ClaudeCode/managed-mcp.json`), user servers are +not injected for Claude, and each gets a `managed-policy` reason. Injecting +them would make the process exit. + +### Import parser (`user-mcp:import`) + +It accepts, in order: + +1. `{"mcpServers": {…}}`, the Claude, Cursor and Claude Desktop form. +2. `{"servers": {…}, "inputs": [...]}`, the VS Code form. `inputs` with + `password: true` become our inputs, and non-password `promptString` inputs + become literal placeholders the user must fill. +3. A bare `{"": {command|url…}}` map. +4. A single bare entry `{command|url…}`, for which the user supplies a name. + +The import rule is that every literal `env` value and `headers` value becomes +a generated secret input (`-`), with its value pre-filled from the +paste and stored in safeStorage. The JSON keeps only `${input:…}`. The user can +edit a value back to a literal if it isn't sensitive. We deliberately don't +guess sensitivity from key names (same reasoning as the built-in launcher's +`env_http_headers` WHY comment): a missed guess leaks a token into a +plaintext file. + +## UX + +### Settings → MCP (new category) + +1. **Your MCP servers** (marker row `user-mcp-servers`). Each server shows: + - name, and a transport summary (`http · localhost:23373/v0/mcp`, + `stdio · npx -y @beeper/mcp-remote`); + - a master switch; + - one checkbox per provider that is **enabled in Settings → Providers**, + so a disabled provider's column is hidden (tying into #1126); + - status chips from `problems` and support reasons (for example + "Claude only: Codex has no SSE", "Secret not set", "Name collides with + your Codex config"); + - Edit, Delete and, for HTTP servers, Sign in…. + - Footer: **Add server…** and "Changes apply to new agents and on agent + reload." +2. **Add / Edit dialog.** A name field, a JSON editor holding the entry (paste + anything from a README), and a live list of masked secret fields, one per + `${input:id}` found. Validation errors show inline and come from the same + main-side validator. "Paste config" imports multiple servers at once. +3. The built-in MCP default rows and External operator MCP move here + unchanged. + +### Command palette + +Per `docs/command-style.md`: + +| id | Title | Surface | Behavior | +|---|---|---|---| +| `user-mcp-servers` | `MCP Servers…` | app | Picker of all user servers with toggle state, plus "Add server…" and "Open MCP settings" rows. Selecting a server flips its master switch; `keepPaletteOpen`. | +| `add-user-mcp-server` | `Add MCP Server…` | app | Opens the Add dialog with the paste box focused. | +| `agent-user-mcp-servers` | `Agent MCP Servers…` | session | Picker for the focused Claude or Codex agent. Each row shows attached, inherited or overridden state. Selecting one writes the override and reloads the agent through the existing reload path, one reload per toggle, like the built-in MCP commands. | +| `use-global-mcp-settings` | (existing) | session | Also clears `userMcpOverrides`. | + +The catalog is context-free, so it can't generate one command per server. +Hence pickers. `ui.openSettings()` gains an optional category argument so +"Open MCP settings" deep-links. + +## What would make this wrong (invariants) + +- **Secrets never reach** argv, the renderer, `mcp-servers.json`, logs or + incident bundles. Secrets *do* reach the agent's own process env and the + private 0600 Claude file, so the agent's shell tools can read them. That is + equally true of the existing built-in bearer and of the CLIs' own config, + and it is accepted and stated in the UI copy. +- **A bad user server never fails an agent launch.** Main validates and + drops. The CLIs exit on invalid config (see evidence), so passing something + unvalidated through is a fleet-wide outage. +- **Only one `--settings` for Claude.** This feature adds no Claude settings + fragment. If it ever needs one (for example to hide a native server), it + must merge into `excludeExternalControlFromClaude`. +- **Unknown entry keys survive a round trip.** READMEs use client-specific + keys (`oauth`, `headersHelper`, `timeout`). For Claude they pass through + untouched. For Codex, only the known keys are translated, and the rest are + ignored with a visible "Ignored by Codex: …" note, never silently dropped + from storage. +- **The renderer never decides what gets attached.** It proposes ids, and main + disposes. The launched set on the pane is observed (`userMcpServerIds`), and + recovery adopts the running process's set, exactly like + `builtInMcpDomains`. +- **The master switch really means off.** No per-agent override may resurrect + a server whose `enabled` is false. + +## Known limitations (stated, not solved) + +- A config change needs an agent reload. Neither TUI reloads MCP config + (Claude only on `/clear` or `/reload-plugins`). +- Claude's persistent `disabledMcpServers` (set via `/mcp` in a repo) applies + by name to injected servers too. Toggling an injected server inside `/mcp` + writes that state back into `~/.claude.json`. We document this and don't + fight it. +- Native servers (from `~/.claude.json`, `.mcp.json` or `config.toml`) still + load and aren't shown. The read-only "Also loaded natively" list is a + follow-up. +- Codex can't use SSE-only servers. +- Workflow subagents don't get user servers. + +## Follow-ups (separate issues after v1) + +1. Read-only "Also loaded natively" list, with "Copy into Agent Code". +2. "Add from registry" via the MCP Registry v0.1 API (preview). +3. OpenCode and Grok support (the data model and translators are additive). +4. #244: Agent Code-hosted extension servers register into the same list. + +## Acceptance (Beeper as the fixture) + +1. Paste Beeper's official token snippet → + - the result is one server, `beeper`, of type http with a + `beeper-authorization` secret; + - the JSON on disk contains no token. +2. With Claude and Codex both ticked, a new Claude agent's private + `mcp.json` has `beeper` with an `${AGENT_CODE_USER_MCP_BEEPER_AUTHORIZATION}` + header. +3. A new Codex agent's argv has `mcp_servers.beeper.url` and + `env_http_headers.Authorization="AGENT_CODE_USER_MCP_BEEPER_AUTHORIZATION"`, + and no token appears in argv. +4. `Agent MCP Servers…` → Beeper off → after the reload the agent has no + beeper tools, and `userMcpServerIds` excludes it. +5. The master switch off → no new agent gets it, even one with a per-agent + on override. +6. A stdio server (`npx -y @beeper/mcp-remote`) works on both providers. +7. An SSE server shows the Codex column disabled with a reason. +8. A server named `agent_code` is rejected. A server colliding with a + `[mcp_servers.X]` in the user's `config.toml` is skipped for Codex with a + visible reason, and the Claude launch is unaffected. +9. A deleted secret → the agent still launches, without that server, and + shows the unavailable notice. From fb50235b142fe4716ad020359735adc861b2d3c9 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Tue, 22 Sep 2026 19:20:00 -0700 Subject: [PATCH 02/11] docs(mcp): revise MCP design to one interface for built-in and user servers The user approved a unified interface: built-in and user servers share one Settings grid with per-provider columns, and one staged per-agent modal replaces the individual Enable * MCP commands. Recorded as a Revision 2 section so the superseded decisions stay visible. Refs #1143 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../plans/2026-09-22-user-mcp-servers.md | 275 +++++++----------- .../2026-09-22-user-mcp-servers-design.md | 76 +++++ 2 files changed, 179 insertions(+), 172 deletions(-) diff --git a/docs/superpowers/plans/2026-09-22-user-mcp-servers.md b/docs/superpowers/plans/2026-09-22-user-mcp-servers.md index 6ace958c3..d2f39df41 100644 --- a/docs/superpowers/plans/2026-09-22-user-mcp-servers.md +++ b/docs/superpowers/plans/2026-09-22-user-mcp-servers.md @@ -1,188 +1,119 @@ -# User MCP Servers: Implementation Plan +# MCP Servers: Implementation Plan -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task by task. Steps use checkbox (`- [ ]`) syntax for tracking. -**Goal:** Let users add any MCP server, set its secrets, and choose which providers get it (Claude Code, Codex). They can override it per agent and turn it on or off from Settings → MCP and the command palette. Servers are delivered at launch through the same path Agent Code's built-in MCP already uses, and provider config files are never written. +**Goal:** One MCP interface for every MCP server. Users can: +- add any MCP server by pasting its README snippet; +- keep its secrets encrypted; +- choose per provider which servers new agents get, for Agent Code's built-in servers and their own alike; +- override those choices per agent with one staged reload; +- see and copy in the servers the CLIs already load directly. -**Architecture:** -- Storage and secrets: - - A main-owned store (`STATE_DIR/mcp-servers.json`, 0600) holds entries in the de facto `mcpServers` shape. Entries use VS Code-style `${input:id}` secret references. - - Secret values live in `safeStorage` blobs and never reach the renderer. -- IPC and renderer: IPC and a broadcast follow the provider-enablement pattern (#1126). A non-persisted zustand mirror sits in the renderer. -- Launch flow: - - The renderer resolves attached server **ids** from the provider defaults plus the per-pane `userMcpOverrides`, and sends them with spawn/recover options. - - Main validates them, resolves secrets, and hands `ResolvedUserMcpServer[]` to the provider launchers. - - The Claude launcher adds them to the existing private `--mcp-config` file. - - The Codex launcher adds `-c mcp_servers.*` overrides, with secrets passed through the environment. - - A bad server is dropped with a notice. It never fails the launch. +Agents receive user servers when they launch, through the path the built-in MCP servers already use. Provider config files are never written. -**Spec:** `docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md`. Read its Evidence and Decisions sections before starting any task; every "why" is there. +**Spec:** `docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md`. Its **Revision 2** section overrides anything in the rest of the spec that conflicts with it. Read Evidence, Decisions and Revision 2 before starting any task. -**Issue:** #1143 (Refs #244). +**Issue:** #1143 (Refs #244). **Status:** user-approved 2026-09-22, including auto-approval of this plan. -**Working tree:** `.worktrees/user-mcp-servers`, branch `feat/user-mcp-servers`, based on `origin/main` `672d0941`. Setup: `git submodule update --init` and `ln -s ../../node_modules node_modules` (see the memory note on worktree setup). +**Working tree:** `.worktrees/user-mcp-servers`, branch `feat/user-mcp-servers`, based on `origin/main` `672d0941`. Submodules are initialized and `node_modules` is symlinked. Use Node 24 for vitest, because Node 25 breaks happy-dom. **Conventions:** -- Thick WHY comments (AGENTS.md); each non-obvious decision in the spec's Decisions table gets a comment at the code site that enforces it. -- Commit messages use Conventional Commits with scope `mcp`. -- Tests protect behavior. Fixtures are the **real** published Beeper snippets (quoted in the spec), not invented shapes. -- Verification: `npx tsc -b` plus the `unit`/`renderer` vitest projects. Run the full suite once at the end, not per task. +- Write thick WHY comments at every code site that enforces a decision. +- Use Conventional Commits with scope `mcp`. +- Test fixtures are the real published Beeper snippets. +- Verify with `npx tsc -b` and vitest once at the end. - Never launch the app. --- -### Task 1: Shared contracts and pure validation - -**Files:** -- Create: `src/shared/types/userMcp.ts` (types from the spec's Contracts section, `USER_MCP_PROVIDERS`, reserved names) -- Create: `src/shared/userMcp/validate.ts` and `validate.test.ts` -- Create: `src/shared/userMcp/inputs.ts` and `inputs.test.ts` (`${input:id}` scan and substitute) - -- [ ] Write failing tests: - - Name rules: `^[A-Za-z0-9_-]{1,64}$`; `agent_code`, `AGENT_CODE` and `agent-code-control` are rejected; a duplicate name among servers raises `duplicate-name`. - - Entry validation: - - stdio needs a `command`; http and sse need an absolute `http(s)` url. - - A `type`-less entry with a `url` normalizes to `http`. - - A mixed `command` + `url` entry is `invalid-entry`. - - `${input:x}` is allowed in `env` and `headers` values, and gives `secret-in-forbidden-field` in `command`, `args` or `url`. An undefined input id gives `unknown-input`. - - Support matrix: sse → `codex: { ok:false, reason: 'Codex does not support SSE MCP servers' }`. - - Unknown extra entry keys are preserved through `coerceUserMcpDocument`. - - Coercion: malformed servers are kept but flagged, never silently deleted; a non-object document becomes `{version:1, servers:[]}`. -- [ ] Implement until the tests are green. The WHY comments cover the charset intersection, why names are not prefixed, and why only `env`/`headers` may carry secrets. -- [ ] Commit `feat(mcp): add user MCP server contracts and validation`. - -### Task 2: Import parser - -**Files:** `src/shared/userMcp/importConfig.ts` and `importConfig.test.ts` - -- [ ] Fixtures (verbatim from developers.beeper.com, as quoted in the spec): - - `{"mcpServers":{"beeper":{"url":"http://localhost:23373/v0/mcp","headers":{"Authorization":"Bearer YOUR_TOKEN_HERE"}}}}` - - The `@beeper/mcp-remote` stdio snippet with `env.ACCESS_TOKEN`. - - The VS Code `{"servers":{"beeper":{"type":"http",…}}}` form. - - A VS Code form with `inputs:[{type:'promptString',id,password:true}]`. - - A bare `{name: entry}` map, and a single bare entry. -- [ ] Assert that every literal env or header value becomes `${input:-}`, with its pasted value returned separately as `pendingSecrets`. The resulting entry must contain no literal token. -- [ ] Assert that malformed JSON returns a typed error, not a throw. -- [ ] Commit `feat(mcp): import MCP server configs from standard snippets`. - -### Task 3: Main store, secrets, IPC, broadcast - -**Files:** -- Create: `src/main/userMcp/store.ts`, which loads and coerces `STATE_DIR/mcp-servers.json` and does atomic 0600 writes through a temp file plus rename. Mutations run through one serialized queue. -- Create: `src/main/userMcp/secrets.ts`, a `safeStorage` blob per `/.bin` following the `src/main/dictation/apiKeyStore.ts` pattern. It returns only `{set, hint}`, and deleting a server deletes its secrets. -- Create: `src/main/userMcp/service.ts`, which builds the snapshot (`UserMcpServerView[]` with problems and support), handles mutations (return snapshot + emit), and provides `resolveForLaunch(ids, provider, cwd)`. That resolver is called in Task 5. -- Create: `src/main/ipc/userMcp.ts`, registered in `src/main/ipc/index.ts`, with channels per the spec and argument validation mirroring `ipc/providerEnablement.ts`. -- Create: `src/preload/api/userMcp.ts`, and expose it in the preload API types. -- Wire the service in `src/main/index.ts` next to the provider enablement construction. - -- [ ] Tests: - - Store round trip keeps unknown keys, and the file mode is 0600. - - A corrupt file is preserved (renamed `.corrupt-`), and the store starts empty with a visible problem. Silently resetting would lose the user's config. - - A secret set or clear never appears in the snapshot, only `hint`. - - Mocking `safeStorage` as unavailable surfaces a `secret-missing` problem, not a crash. -- [ ] Commit `feat(mcp): persist user MCP servers and secrets in main`. - -### Task 4: Provider launch translators (pure) - -**Files:** `src/providers/shared/runtime/userMcpLaunch.ts` and `userMcpLaunch.test.ts`; modify `builtInMcpLaunch.ts` - -- [ ] `generatedSecretVar(serverName, key, taken)` is deterministic: `AGENT_CODE_USER_MCP__`, sanitized, with a numeric suffix on collision. A test asserts it is stable when another server is added or removed. Comment the Claude OAuth-key reason (spec, "Secret variable naming"). -- [ ] `claudeUserMcpEntries(servers) → { entries, env }` and `codexUserMcpLaunchConfig(servers, args, env) → { dropped }`: - - Codex stdio: - - Emit `command`, `args` (TOML array), `cwd` and `env_vars=[…]`, with the values placed in `env`. - - If two attached stdio servers use the same env key with different values, drop the second with a reason. - - Codex http: every header goes through `env_http_headers`. - - Unknown keys: - - Claude: passed through verbatim. - - Codex: ignored, and their names are returned for the UI note. -- [ ] Change `createPrivateClaudeMcpConfig(builtIns, userEntries)` so it writes one file when either list is non-empty. The existing callers (`claudeSession.ts:1189-1202`) keep a single `--mcp-config` as the last flag. -- [ ] Golden tests using the Beeper fixtures: - - Claude file JSON. - - Codex argv, with the assertion **no token substring appears in args**. - - The stdio `mcp-remote` case on both providers. -- [ ] Commit `feat(mcp): translate user MCP servers into Claude and Codex launch config`. - -### Task 5: Spawn and recover wiring in main - -**Files:** -- `src/shared/types/session.ts`: add `userMcpServerIds?: string[]` to the spawn and recover options, and add the observed `userMcpServerIds` to `SessionInfo`. The comment makes the same "observed, not requested" point as `builtInMcpDomains`. -- `src/main/sessionManager.ts`: - - At the `builtInMcpServers` assembly (~:2875), call `userMcp.resolveForLaunch`, which: - - drops unknown, disabled, unsupported, problem, missing-secret and native-collision servers; - - applies the Claude `managed-mcp.json` lock. - - Pass `userMcpServers` into `createSession`. - - Emit `user-mcp-unavailable` for dropped servers (mirror `reportSkillsUnavailable` at :2724), and forward it to the renderer the same way. - - Recovery that adopts an existing process keeps that process's recorded ids. -- `src/providers/claude/runtime/claudeSession.ts` and `src/providers/codex/runtime/codexSession.ts`: accept `userMcpServers`, call the Task 4 translators, and put the generated variables into the spawn env. -- Native collision check (Codex): `src/main/userMcp/nativeCodexServers.ts` parses the `mcp_servers` keys from `${CODEX_HOME:-~/.codex}/config.toml` and `/.codex/config.toml` with `@iarna/toml`. It is read-only, and a parse failure means "no collisions known", which it logs. - -- [ ] Tests: - - A spawn with a missing secret still spawns, and emits unavailable. - - A Codex collision drops only Codex. - - `SessionInfo.userMcpServerIds` equals what was actually attached. - - The token is absent from the recorded argv (there is an existing spawn-args capture harness in the codex/claude session tests; reuse it). -- [ ] Commit `feat(mcp): attach user MCP servers when agents launch`. - -### Task 6: Renderer mirror, resolution, per-pane overrides - -**Files:** -- Create: `src/renderer/src/features/userMcp/store.ts`, a mirror plus `useUserMcpSync()`, mounted once in `App.tsx` next to `useProviderEnablementSync`. -- Create: `src/renderer/src/workspace/userMcp.ts` with `resolveSessionUserMcpServerIds` and `normalizeUserMcpOverrides`. Test it in `userMcp.test.ts`, covering the master switch beating an override, provider default, override on/off, and an unsupported provider. -- Modify `src/renderer/src/workspace/types.ts` to add `userMcpOverrides` and `userMcpServerIds` beside the built-in fields (:283-286). -- Modify `workspace/mcpDomains.ts`: `clonedMcpOverrides` also copies `userMcpOverrides`. -- Modify `workspace/hook/actions/session.ts` (the fresh spawn at ~:378 and the replacement/reload at ~:1258) to send `userMcpServerIds`. -- Modify `workspace/builtInMcpReload.ts`, which gains the user-override variant through the same reload path. Don't fork a second reload implementation. -- Modify the workspace persistence coercion so the new pane fields survive save and load. -- Show the `user-mcp-unavailable` notice where `managed-skills-unavailable` is shown today. - -- [ ] Commit `feat(mcp): resolve user MCP servers per agent in the workspace`. - -### Task 7: Settings → MCP - -**Files:** -- `settingsCategories.ts`: add category `mcp` ("MCP", "Servers your agents can use, and Agent Code's own MCP tools."). -- `settingsRegistry.ts`: - - Add the `user-mcp-servers` marker row (`storage: 'external-files'`, `apply: 'new-session'`). - - Move the built-in default MCP rows and `external-control` into the new category, changing the category only. -- `SettingsList.tsx`: dispatch the new row. -- Create: `src/renderer/src/features/userMcp/ui/UserMcpServersRow.tsx` and `UserMcpServerDialog.tsx`: - - List, master switch, and per-provider checkboxes, showing only providers enabled in `useEnabledAgentProviderKinds()`. - - Problem and support chips, Edit, Delete, and Sign in… for HTTP servers. - - The dialog has a name field, a JSON entry editor, and masked secret fields derived from the `${input:*}` references. Validation comes from main. - - Paste-import supports multiple servers. -- Sign in…: - - Codex: open a terminal pane running `codex mcp login ` with the same `-c` url override (the spec notes that login reads the effective config). - - Claude: show inline guidance to run `/mcp` in an agent that has the server attached. -- `ui.openSettings(category?)`: add the optional argument (`command-palette/types.ts:202`) and its implementation. - -- [ ] Renderer tests: - - The row hides the Codex column when Codex is disabled in Providers. - - An SSE server's Codex checkbox is disabled, with the reason shown. - - The dialog never renders a secret value. -- [ ] Commit `feat(mcp): add MCP settings with user server management`. - -### Task 8: Commands - -**Files:** a new `src/renderer/src/features/userMcp/commands.ts` registered in `command-palette/catalog.ts`; `sessionCommands.ts` (`use-global-mcp-settings` also clears user overrides); and `catalog.test.ts` (baseline ids, counts, approved-additions list), plus `taxonomy.test.ts` if tiers apply. - -- [ ] Add `user-mcp-servers` (`MCP Servers…`), `add-user-mcp-server` (`Add MCP Server…`) and `agent-user-mcp-servers` (`Agent MCP Servers…`) as the spec's command table describes. Follow `docs/command-style.md`: descriptions use the "What it does / Use when / Notes" format, and the session command re-checks the provider inside `run`. -- [ ] Commit `feat(mcp): add MCP server commands`. - -### Task 9: Documentation and final verification - -- [ ] Update the README "What you can do with it" section with an **MCP servers** bullet, and update the `ARCHITECTURE.md` provider-integration section to cover user servers delivered at launch. -- [ ] Run `npx tsc -b` and `npm test`, once, and compare any failures against the known local failures noted in memory. -- [ ] Run a real-binary check, read-only and with no app launch: - - Codex: run `codex mcp list` with the generated `-c` overrides for the Beeper fixture, and confirm it parses and lists `beeper` with the header env var. - - Claude: run `claude mcp list --mcp-config ` if it honors the flag; if not, note that in the PR. -- [ ] Open a PR with the title `feat(mcp): manage user MCP servers per provider and per agent`, with `Fixes #1143` and `Refs #244`. Do not merge. - ---- +### Task 1: Shared model (`src/shared/userMcp/`) +- [ ] `types.ts`: + - `UserMcpServer`, `UserMcpServerEntry`, `UserMcpInput` and `UserMcpDocument`. + - The view and problem types. + - `USER_MCP_PROVIDERS` (`claude`, `codex`) and the reserved names. + - `userMcpOverrideKey(id)` and `userMcpOverridesFrom(map)`. +- [ ] `validate.ts`: + - Name rules, entry validation and type normalization. + - The rule that `${input:…}` may appear only in `env` and `headers` values. + - The support matrix (SSE is Claude-only). + - `coerceUserMcpDocument`, which keeps unknown keys and flags malformed servers instead of dropping them. +- [ ] `inputs.ts`: scan and substitute `${input:id}`. +- [ ] `importConfig.ts`: + - Accepts the `mcpServers`, VS Code `servers`/`inputs`, bare-map and bare-entry forms. + - Moves every literal env and header value into a secret input, returned as `pendingSecrets`. +- [ ] Tests for each of the above, using the Beeper fixtures. + +### Task 2: Launch translators (`src/providers/shared/runtime/userMcpLaunch.ts`) +- [ ] `userMcpSecretVariable`: deterministic variable names (needed so Claude's OAuth key stays stable). +- [ ] `claudeUserMcpEntries`: builds Claude's config entries. +- [ ] `addCodexUserMcpLaunchConfig`: builds the Codex arguments, and drops a server whose `env_vars` collide with another's. +- [ ] Widen `createPrivateClaudeMcpConfig(builtIns, userEntries)`. +- [ ] Golden tests, including one asserting that no secret appears in argv. + +### Task 3: Main service (`src/main/userMcp/`) +- [ ] `store.ts`: an atomic write of `STATE_DIR/mcp-servers.json` with mode 0600. A corrupt file is preserved rather than overwritten. +- [ ] `secrets.ts`: `safeStorage` blobs, with only a hint ever returned. +- [ ] `nativeServers.ts`: + - Lists the CLIs' own user-scope servers for Claude and Codex. + - Collects the Codex names used for the collision check. + - Detects Claude's managed-policy lock. +- [ ] `service.ts`: + - Snapshot and mutations, run through a serialized queue. + - `resolveForLaunch(provider, overrides, cwd)`, which returns `{ servers, attachedIds, dropped }`. + - A change emitter. +- [ ] IPC in `src/main/ipc/userMcp.ts`, the preload API in `src/preload/api/userMcp.ts`, and wiring in `src/main/index.ts`. +- [ ] Tests for the store, secrets, resolution and native parsing. + +### Task 4: Session wiring (main and providers) +- [ ] Add `userMcpOverrides` to the spawn and recover options. +- [ ] Add `userMcpServerIds` to the snapshot, spawn result and recover result. +- [ ] `SessionManager` resolves user servers beside `builtInMcpServers` and records the attached ids per session. + - The Codex replacement restore reuses the recorded overrides. + - It emits `user-mcp-unavailable`, which the forwarder broadcasts. +- [ ] Claude and Codex sessions accept `userMcpServers`. +- [ ] Tests: a missing secret still spawns; the token is absent from argv. + +### Task 5: Renderer model +- [ ] Per-provider `defaultBuiltInMcpDomains`, covering: + - type and coercion; + - the resolver picking the provider's list; + - the refs input type; + - the `store.ts` comment. +- [ ] `normalizeBuiltInMcpOverrides` keeps `user:` keys. +- [ ] `clonedMcpOverrides` keeps them too. +- [ ] Every renderer spawn and recover call site sends `userMcpOverrides`, and the pane meta stores `userMcpServerIds`. +- [ ] `features/mcp/store.ts`: a mirror of main's snapshot plus a sync hook, mounted in `App.tsx`. +- [ ] Global toast for `user-mcp-unavailable`. + +### Task 6: Settings → MCP +- [ ] Add the `mcp` category. +- [ ] Add the `mcp-servers` marker row, which replaces the eight built-in default toggle rows. +- [ ] Move `external-control` into the new category. +- [ ] `McpServersRow`: + - The grid of built-in and user servers, with a column per enabled provider. + - Master switches, problem chips, and the ⋯ actions. + - The native section with Copy in. +- [ ] `McpServerDialog`: + - Add and edit, with paste import and a JSON editor. + - Masked secret fields. + - Sign in… launches `codex mcp login` in a new terminal pane with the same `-c` URL, and shows Claude `/mcp` guidance. +- [ ] `ui.openSettings(category?)`. + +### Task 7: Commands and the per-agent modal +- [ ] `AgentMcpServersModal` and its surface: staged toggles, one reload, a reset row, and Root Management going through its confirmation dialog. +- [ ] New commands: `mcp-servers`, `add-mcp-server` and `agent-mcp-servers`. +- [ ] Retire `use-global-mcp-settings` and the eight `enable-*-mcp` toggles. +- [ ] Update the control references, `catalog.test.ts`, `taxonomy.test.ts` and the affected renderer tests. + +### Task 8: Documentation, verification and PR +- [ ] Update README and ARCHITECTURE. +- [ ] Run `npx tsc -b` and vitest. +- [ ] Real-binary check of the generated Codex `-c` arguments with `codex mcp list`. +- [ ] Open the PR `feat(mcp): manage MCP servers per provider and per agent` with `Fixes #1143` and `Refs #244`. Run two orchestrated reviewers, fix the valid findings, wait for CI, and do not merge. ## Out of scope (follow-up issues) - -- Read-only "Also loaded natively" list. - Add from MCP Registry. -- OpenCode and Grok. -- User servers in workflow subagents. +- OpenCode and Grok user servers. +- User servers for workflow subagents. +- Project-scope native server listing. - #244 hosted extension servers. diff --git a/docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md b/docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md index 917fefd69..ef4d501ec 100644 --- a/docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md +++ b/docs/superpowers/specs/2026-09-22-user-mcp-servers-design.md @@ -3,6 +3,82 @@ Status: draft for review · Date: 2026-09-22 · Branch: `feat/user-mcp-servers` Issue: #1143 (Refs #244) +## Revision 2: one interface for every MCP server (user-approved 2026-09-22) + +The user reviewed ASCII mockups and approved a unified interface. It also +covers Agent Code's own built-in MCP servers. Where this section conflicts +with anything below, **this section wins**. + +1. **One grid for all servers.** Settings → MCP lists Agent Code's built-in + servers (TLDR, Goal, Goal Loop, Orchestration, Agent Transcripts, Agent + Management, AI Workspace, Workflows) and the user's servers in **one grid** + with the same per-provider columns. + - The rule for every cell is: *new agents of this provider get it*. + - `—` marks a server that provider can't use, with the reason shown. + - Root Management is listed as per-agent only; it is never a default. + - A provider column appears only when that provider is enabled in + Settings → Providers. + - Built-in cells cover all four providers, because every launcher carries + built-ins. User-server cells for OpenCode and Grok show `—` "Not + supported yet", which is honest and keeps the grid rectangular. +2. **Per-provider built-in defaults.** `settings.defaultBuiltInMcpDomains` + changes from one flat list to + `Record`. + - Coercion copies a legacy flat list to every provider, so nobody's + current behavior changes on upgrade. + - The resolver still accepts a flat list, meaning "every provider". That + keeps orchestration, control and test callers that pass an explicit list + valid without a parallel API. +3. **One per-agent override map.** Per-agent choices for user servers live in + the **same** `builtInMcpOverrides` map, under namespaced keys + `user:`. + - That map already travels through spawn, replace, reload, recovery, + undo-close, provider switch, duplicate and the control API. Giving user + servers a second parallel map would mean re-threading every one of those + paths, and missing one silently drops a choice. + - The field name stays as it is, because renaming a persisted workspace + field needs a migration. + - The type is widened, and a WHY comment at the type explains it. +4. **Main decides which user servers attach.** The renderer sends only the + `user:` subset of the pane's overrides (`userMcpOverrides`) with + spawn/recover. Main applies its own store's defaults (the master switch + and per-provider flags), overrides, readiness and support. + - This replaces the earlier "renderer resolves ids" contract. Main owns + the store and the secrets, so letting a stale renderer snapshot decide + would add a second authority for no benefit. + - Main records the attached ids per session and reports them as + `userMcpServerIds` on the session snapshot, spawn result and recover + result, exactly like `builtInMcpDomains`. +5. **One `Agent MCP Servers…` modal.** It lists built-in and user servers for + the focused agent, **stages** the toggles, and applies them with **one** + reload. "Reset to MCP settings defaults" clears every override. + - Root Management in this modal still goes through its confirmation + dialog. Applying a staged Root Management grant opens that dialog, and + the dialog performs the reload with all the staged choices. + - It **retires** `use-global-mcp-settings` and the eight + `enable-*-mcp` toggles: AI Workspace, Orchestration, Agent Transcripts, + Agent Management, TLDR, Goal, Goal Loop and Workflow. + - `enable-root-agent-code-management` (a confirmation-gated command whose + id other code refers to) and the debug-only `enable-built-in-mcp-ping` + stay. +6. **Other commands.** + - `MCP Servers` opens Settings on the MCP category. `ui.openSettings` + gains an optional category. + - `Add MCP Server…` opens the add dialog. +7. **Read-only list of servers the CLIs load directly, now in v1.** It shows + user-scope `mcpServers` from `~/.claude.json` (honoring + `CLAUDE_CONFIG_DIR`) and `[mcp_servers]` from + `${CODEX_HOME:-~/.codex}/config.toml`, each with a **Copy in** action + that imports it as a managed server. + - Project-scope files depend on the cwd, so Settings (which has no cwd) + doesn't list them. + - The Codex collision check still reads both the user file and + `/.codex/config.toml` at spawn. +8. **Unavailable notice.** A user server that can't attach is broadcast as + `user-mcp-unavailable { servers: [{ name, reason }] }` and shown by the + global toast, with the same repeat window as `managed-skills-unavailable`. +9. **External operator MCP** moves unchanged into the MCP category. + ## Problem Agent Code cannot add, configure, or switch off a third-party MCP server. The From 6c3d4a59b1e78eba62152066e75d5aab686cef1a Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Tue, 22 Sep 2026 19:25:23 -0700 Subject: [PATCH 03/11] feat(mcp): add user MCP server model, import and launch translators The model stores servers in the de facto mcpServers entry shape so a README snippet round-trips losslessly, with VS Code style ${input:id} secret references that may only appear where they can travel through the environment. The translators turn resolved servers into Claude's private --mcp-config entries and Codex -c overrides without putting a secret on argv or on disk, and drop a server rather than fail a launch. Refs #1143 Co-Authored-By: Claude Opus 5.5 (1M context) --- .../shared/runtime/builtInMcpLaunch.ts | 38 ++- .../shared/runtime/userMcpLaunch.test.ts | 153 ++++++++++ src/providers/shared/runtime/userMcpLaunch.ts | 209 +++++++++++++ src/shared/userMcp/importConfig.ts | 157 ++++++++++ src/shared/userMcp/inputs.ts | 40 +++ src/shared/userMcp/types.ts | 221 ++++++++++++++ src/shared/userMcp/userMcp.test.ts | 169 +++++++++++ src/shared/userMcp/validate.ts | 277 ++++++++++++++++++ 8 files changed, 1251 insertions(+), 13 deletions(-) create mode 100644 src/providers/shared/runtime/userMcpLaunch.test.ts create mode 100644 src/providers/shared/runtime/userMcpLaunch.ts create mode 100644 src/shared/userMcp/importConfig.ts create mode 100644 src/shared/userMcp/inputs.ts create mode 100644 src/shared/userMcp/types.ts create mode 100644 src/shared/userMcp/userMcp.test.ts create mode 100644 src/shared/userMcp/validate.ts diff --git a/src/providers/shared/runtime/builtInMcpLaunch.ts b/src/providers/shared/runtime/builtInMcpLaunch.ts index 9839101a5..0a70e4f5f 100644 --- a/src/providers/shared/runtime/builtInMcpLaunch.ts +++ b/src/providers/shared/runtime/builtInMcpLaunch.ts @@ -115,24 +115,36 @@ export function addOpencodeBuiltInMcpLaunchConfig( */ export async function createPrivateClaudeMcpConfig( servers: readonly BuiltInMcpServerConfig[], + // User MCP servers (#1143) share this one file rather than a second + // `--mcp-config`: the flag is variadic and swallows positional arguments that + // follow it, so one occurrence kept last is the only placement that cannot + // eat `--resume`. Their secret values are already `${VAR}` references (see + // userMcpLaunch.ts), so nothing here can put a credential on disk that the + // built-in path would not. + userEntries: Readonly>> = {}, ): Promise { - if (servers.length === 0) return null + if (servers.length === 0 && Object.keys(userEntries).length === 0) return null const directory = await mkdtemp(join(tmpdir(), 'agent-code-mcp-')) const path = join(directory, 'mcp.json') const document = { - mcpServers: Object.fromEntries(servers.map(server => [ - server.name, - { - type: 'http', - url: server.url, - headers: { - ...server.headers, - ...(server.bearerToken === undefined - ? {} - : { Authorization: `Bearer ${server.bearerToken}` }), + mcpServers: Object.fromEntries([ + // User entries first so a built-in entry can never be shadowed. Reserved + // names are rejected upstream; this ordering is the second fence. + ...Object.entries(userEntries), + ...servers.map(server => [ + server.name, + { + type: 'http', + url: server.url, + headers: { + ...server.headers, + ...(server.bearerToken === undefined + ? {} + : { Authorization: `Bearer ${server.bearerToken}` }), + }, }, - }, - ])), + ]), + ]), } try { await writeFile(path, `${JSON.stringify(document)}\n`, { encoding: 'utf8', mode: 0o600 }) diff --git a/src/providers/shared/runtime/userMcpLaunch.test.ts b/src/providers/shared/runtime/userMcpLaunch.test.ts new file mode 100644 index 000000000..7c30c5010 --- /dev/null +++ b/src/providers/shared/runtime/userMcpLaunch.test.ts @@ -0,0 +1,153 @@ +import { readFile } from 'node:fs/promises' + +import { describe, expect, it } from 'vitest' + +import { createPrivateClaudeMcpConfig } from './builtInMcpLaunch.js' +import { + addCodexUserMcpLaunchConfig, + claudeUserMcpEntries, + userMcpSecretVariable, + type ResolvedUserMcpServer, +} from './userMcpLaunch.js' + +const TOKEN = 'bpr_live_9f3a1c' + +// The Beeper Desktop HTTP server as it looks after importing its official +// snippet (https://developers.beeper.com/desktop-api/mcp/): the token has been +// lifted into a secret input and only the reference remains in the entry. +const beeperHttp: ResolvedUserMcpServer = { + id: 'b1', + name: 'beeper', + entry: { + type: 'http', + url: 'http://localhost:23373/v0/mcp', + headers: { Authorization: 'Bearer ${input:beeper-authorization}' }, + }, + secrets: { 'beeper-authorization': TOKEN }, +} + +// The official stdio alternative (`npx -y @beeper/mcp-remote`), where the +// server expands its own ${ACCESS_TOKEN} from the environment we give it. +const beeperStdio: ResolvedUserMcpServer = { + id: 'b2', + name: 'beeper-stdio', + entry: { + command: 'npx', + args: ['-y', '@beeper/mcp-remote', '--header', 'Authorization: Bearer ${ACCESS_TOKEN}'], + env: { ACCESS_TOKEN: '${input:beeper-access_token}', LOG_LEVEL: 'info' }, + }, + secrets: { 'beeper-access_token': TOKEN }, +} + +describe('claudeUserMcpEntries', () => { + it('replaces secret header values with env references and moves the value to the environment', () => { + const { entries, env, dropped } = claudeUserMcpEntries([beeperHttp]) + expect(dropped).toEqual([]) + expect(entries).toEqual({ + beeper: { + type: 'http', + url: 'http://localhost:23373/v0/mcp', + headers: { Authorization: '${AGENT_CODE_USER_MCP_BEEPER_AUTHORIZATION}' }, + }, + }) + expect(env).toEqual({ AGENT_CODE_USER_MCP_BEEPER_AUTHORIZATION: `Bearer ${TOKEN}` }) + }) + + it('keeps literal env values and the server-expanded args of a stdio server', () => { + const { entries, env } = claudeUserMcpEntries([beeperStdio]) + expect(entries['beeper-stdio']).toEqual({ + command: 'npx', + args: ['-y', '@beeper/mcp-remote', '--header', 'Authorization: Bearer ${ACCESS_TOKEN}'], + env: { ACCESS_TOKEN: '${AGENT_CODE_USER_MCP_BEEPER_STDIO_ACCESS_TOKEN}', LOG_LEVEL: 'info' }, + }) + expect(env).toEqual({ AGENT_CODE_USER_MCP_BEEPER_STDIO_ACCESS_TOKEN: TOKEN }) + }) + + it('drops a server whose secret is missing instead of emitting an empty credential', () => { + const { entries, dropped } = claudeUserMcpEntries([{ ...beeperHttp, secrets: {} }]) + expect(entries).toEqual({}) + expect(dropped).toEqual([{ name: 'beeper', reason: 'A secret is not set' }]) + }) + + it('writes one private file containing both built-in and user servers, and no secret', async () => { + const { entries } = claudeUserMcpEntries([beeperHttp]) + const config = await createPrivateClaudeMcpConfig( + [{ name: 'agent_code', url: 'http://127.0.0.1:1/mcp', headers: {}, bearerToken: 'builtin' }], + entries, + ) + try { + const text = await readFile(config!.path, 'utf8') + expect(Object.keys(JSON.parse(text).mcpServers)).toEqual(['beeper', 'agent_code']) + expect(text).not.toContain(TOKEN) + } finally { + await config?.dispose() + } + }) + + it('still creates the file when only user servers are attached', async () => { + const config = await createPrivateClaudeMcpConfig([], claudeUserMcpEntries([beeperHttp]).entries) + expect(config).not.toBeNull() + await config?.dispose() + }) +}) + +describe('addCodexUserMcpLaunchConfig', () => { + it('passes every HTTP header through env_http_headers so no token reaches argv', () => { + const args: string[] = [] + const env: Record = {} + expect(addCodexUserMcpLaunchConfig([beeperHttp], args, env)).toEqual([]) + expect(args).toEqual([ + '--config', 'mcp_servers.beeper.url="http://localhost:23373/v0/mcp"', + '--config', 'mcp_servers.beeper.env_http_headers.Authorization="AGENT_CODE_USER_MCP_BEEPER_AUTHORIZATION"', + ]) + expect(env).toEqual({ AGENT_CODE_USER_MCP_BEEPER_AUTHORIZATION: `Bearer ${TOKEN}` }) + expect(args.join(' ')).not.toContain(TOKEN) + }) + + it('passes stdio secrets by name through env_vars and literals per server', () => { + const args: string[] = [] + const env: Record = {} + expect(addCodexUserMcpLaunchConfig([beeperStdio], args, env)).toEqual([]) + expect(args).toEqual([ + '--config', 'mcp_servers.beeper-stdio.command="npx"', + '--config', 'mcp_servers.beeper-stdio.args=["-y","@beeper/mcp-remote","--header","Authorization: Bearer ${ACCESS_TOKEN}"]', + '--config', 'mcp_servers.beeper-stdio.env.LOG_LEVEL="info"', + '--config', 'mcp_servers.beeper-stdio.env_vars=["ACCESS_TOKEN"]', + ]) + expect(env).toEqual({ ACCESS_TOKEN: TOKEN }) + expect(args.join(' ')).not.toContain(TOKEN) + }) + + it('drops SSE servers and secrets that would overwrite Codex\'s own environment', () => { + const args: string[] = [] + const dropped = addCodexUserMcpLaunchConfig([ + { id: 's', name: 'linear', entry: { type: 'sse', url: 'https://mcp.linear.app/sse' }, secrets: {} }, + { id: 'o', name: 'openai', entry: { command: 'x', env: { OPENAI_API_KEY: '${input:k}' } }, secrets: { k: 'v' } }, + ], args, {}) + expect(dropped.map(server => server.name)).toEqual(['linear', 'openai']) + expect(args).toEqual([]) + }) + + it('refuses a second server that needs the same env name with a different value', () => { + const env: Record = {} + const mk = (name: string, value: string): ResolvedUserMcpServer => ({ + id: name, name, entry: { command: name, env: { GITHUB_TOKEN: '${input:t}' } }, secrets: { t: value }, + }) + const dropped = addCodexUserMcpLaunchConfig([mk('one', 'a'), mk('two', 'b'), mk('three', 'a')], [], env) + expect(dropped).toEqual([{ name: 'two', reason: 'Secret GITHUB_TOKEN is already used with a different value by one' }]) + expect(env.GITHUB_TOKEN).toBe('a') + }) + + it('leaves no partial table behind for a server dropped half-way', () => { + const args: string[] = [] + addCodexUserMcpLaunchConfig([{ ...beeperStdio, secrets: {} }], args, {}) + expect(args).toEqual([]) + }) +}) + +describe('userMcpSecretVariable', () => { + it('is stable for a server regardless of which other servers are attached', () => { + expect(userMcpSecretVariable('beeper', 'Authorization')).toBe('AGENT_CODE_USER_MCP_BEEPER_AUTHORIZATION') + expect(userMcpSecretVariable('my-server', 'X-Api-Key')).toBe('AGENT_CODE_USER_MCP_MY_SERVER_X_API_KEY') + }) +}) diff --git a/src/providers/shared/runtime/userMcpLaunch.ts b/src/providers/shared/runtime/userMcpLaunch.ts new file mode 100644 index 000000000..ae2e3e743 --- /dev/null +++ b/src/providers/shared/runtime/userMcpLaunch.ts @@ -0,0 +1,209 @@ +import { hasInputReference, substituteInputs } from '@shared/userMcp/inputs.js' +import type { UserMcpDroppedServer, UserMcpServerEntry } from '@shared/userMcp/types.js' +import { isStringArray, isStringRecord, transportOf } from '@shared/userMcp/validate.js' + +/** + * Launch material for one user MCP server (#1143), produced by main's + * UserMcpService right before spawn. `secrets` holds decrypted values and + * exists only for the duration of one launch; it is never persisted, logged, + * or sent to the renderer. + */ +export type ResolvedUserMcpServer = { + id: string + name: string + entry: UserMcpServerEntry + secrets: Record +} + +/** + * Environment variable that carries one secret-bearing env/header value. + * + * WHY deterministic from (server name, key) instead of an index like the + * built-in launcher's `AGENT_CODE_MCP_i_j`: Claude keys a server's stored OAuth + * tokens on `name|sha256({type,url,headers})` (vendor services/mcp/auth.ts), + * and our headers contain these variable NAMES. An index would shift whenever + * another server was toggled on or off, change the hash, and silently discard + * the user's OAuth login for an unrelated server. + */ +export function userMcpSecretVariable(serverName: string, key: string): string { + const part = (value: string) => value.toUpperCase().replace(/[^A-Z0-9]+/g, '_').replace(/^_+|_+$/g, '') + return `AGENT_CODE_USER_MCP_${part(serverName) || 'SERVER'}_${part(key) || 'VALUE'}` +} + +/** + * Claude `mcpServers` entries for the private `--mcp-config` file. + * + * Secret-bearing values become `${AGENT_CODE_USER_MCP_…}` references and the + * substituted value goes into Claude's process environment; Claude expands + * `${VAR}` in command/args/env/url/headers when it loads the file (vendor + * services/mcp/envExpansion.ts). The file itself (mode 0600, deleted on stop) + * therefore never contains a secret either. Literal values and unknown keys + * pass through untouched: Claude's zod schemas strip keys they do not know + * rather than rejecting them, and a README's `oauth` block is a key Claude + * DOES know. + */ +export function claudeUserMcpEntries(servers: readonly ResolvedUserMcpServer[]): { + entries: Record> + env: Record + dropped: UserMcpDroppedServer[] +} { + const entries: Record> = {} + const env: Record = {} + const dropped: UserMcpDroppedServer[] = [] + for (const server of servers) { + const transport = transportOf(server.entry) + if (!transport) { + dropped.push({ name: server.name, reason: 'Invalid server config' }) + continue + } + const entry: Record = { ...server.entry } + let missing = false + for (const field of ['env', 'headers'] as const) { + const record = entry[field] + if (!isStringRecord(record)) continue + const next: Record = {} + for (const [key, value] of Object.entries(record)) { + if (!hasInputReference(value)) { + next[key] = value + continue + } + const resolved = substituteInputs(value, server.secrets) + if (resolved === null) { + missing = true + break + } + const variable = userMcpSecretVariable(server.name, key) + env[variable] = resolved + next[key] = `\${${variable}}` + } + entry[field] = next + } + if (missing) { + dropped.push({ name: server.name, reason: 'A secret is not set' }) + continue + } + // Explicit type keeps Claude's union parse unambiguous for remote entries; + // stdio is valid with or without it. + if (transport !== 'stdio') entry.type = transport + entries[server.name] = entry + } + return { entries, env, dropped } +} + +/** + * Environment names Codex itself depends on. A user stdio server's secret has + * to be placed in Codex's OWN environment (Codex forwards env to MCP children + * only through an allowlist plus `env_vars` pass-through names, and `env_vars` + * cannot rename), so a server asking for one of these would silently change how + * Codex authenticates, finds binaries, or talks to our own MCP host. + */ +const CODEX_PROTECTED_ENV = /^(PATH|HOME|SHELL|USER|LOGNAME|TMPDIR|LANG|TERM|CODEX_.*|OPENAI_.*|AGENT_CODE_.*)$/ + +/** + * Add Codex `-c mcp_servers..*` overrides for user servers. + * + * Why each field travels the way it does (argv is readable by any local + * process and by our incident collectors; see builtInMcpLaunch.ts): + * - command/args/cwd/url: literal on argv. Validation already guarantees they + * contain no `${input:…}` secret. + * - stdio env, literal values: `env.=` on argv. They are not secrets by + * the user's own choice, and passing them per-server avoids changing + * Codex's own environment. + * - stdio env, secret values: set `KEY` in Codex's environment and list it in + * `env_vars`. Codex does no `${VAR}` expansion, so pass-through by the + * server's own variable name is the only secret-safe channel. + * - http headers (all of them): `env_http_headers.=""`, + * exactly like the built-in servers. Codex rejects a literal + * `bearer_token`, and this covers `Authorization` without special casing. + * + * Returns servers that could not be attached; a dropped server never fails + * the launch. + */ +export function addCodexUserMcpLaunchConfig( + servers: readonly ResolvedUserMcpServer[], + args: string[], + env: Record, +): UserMcpDroppedServer[] { + const dropped: UserMcpDroppedServer[] = [] + // Secret env names already claimed by an earlier server in this launch. + const claimed = new Map() + for (const server of servers) { + const transport = transportOf(server.entry) + if (!transport) { + dropped.push({ name: server.name, reason: 'Invalid server config' }) + continue + } + if (transport === 'sse') { + dropped.push({ name: server.name, reason: 'Codex does not support SSE servers' }) + continue + } + const serverArgs: string[] = [] + const serverEnv: Record = {} + const prefix = `mcp_servers.${server.name}` + const set = (key: string, value: unknown) => serverArgs.push('--config', `${prefix}.${key}=${JSON.stringify(value)}`) + let failure: string | null = null + + if (transport === 'stdio') { + const entry = server.entry as Record + set('command', entry.command) + if (isStringArray(entry.args) && entry.args.length > 0) set('args', entry.args) + if (typeof entry.cwd === 'string' && entry.cwd) set('cwd', entry.cwd) + const passThrough: string[] = [] + if (isStringRecord(entry.env)) { + for (const [key, value] of Object.entries(entry.env)) { + if (!hasInputReference(value)) { + set(`env.${key}`, value) + continue + } + const resolved = substituteInputs(value, server.secrets) + if (resolved === null) { + failure = 'A secret is not set' + break + } + if (CODEX_PROTECTED_ENV.test(key)) { + failure = `Codex cannot pass a secret named ${key} without changing its own environment` + break + } + const owner = claimed.get(key) + if (owner !== undefined && env[key] !== resolved) { + failure = `Secret ${key} is already used with a different value by ${owner}` + break + } + serverEnv[key] = resolved + passThrough.push(key) + } + } + if (passThrough.length > 0) set('env_vars', passThrough) + } else { + const entry = server.entry as Record + set('url', entry.url) + if (isStringRecord(entry.headers)) { + for (const [header, value] of Object.entries(entry.headers)) { + const resolved = hasInputReference(value) ? substituteInputs(value, server.secrets) : value + if (resolved === null) { + failure = 'A secret is not set' + break + } + const variable = userMcpSecretVariable(server.name, header) + serverEnv[variable] = resolved + set(`env_http_headers.${header}`, variable) + } + } + } + + if (failure) { + dropped.push({ name: server.name, reason: failure }) + continue + } + // Commit only after the whole server succeeded, so a server dropped + // half-way leaves neither a partial `mcp_servers` table (which Codex would + // reject as an invalid transport, failing the entire launch) nor stray + // variables behind. + args.push(...serverArgs) + Object.assign(env, serverEnv) + for (const key of Object.keys(serverEnv)) { + if (!key.startsWith('AGENT_CODE_USER_MCP_')) claimed.set(key, server.name) + } + } + return dropped +} diff --git a/src/shared/userMcp/importConfig.ts b/src/shared/userMcp/importConfig.ts new file mode 100644 index 000000000..65f9dbaec --- /dev/null +++ b/src/shared/userMcp/importConfig.ts @@ -0,0 +1,157 @@ +import { hasInputReference } from './inputs.js' +import type { + UserMcpImportCandidate, + UserMcpImportResult, + UserMcpInput, + UserMcpServerEntry, +} from './types.js' +import { + coerceInputs, + isPlainObject, + isStringRecord, + normalizeEntry, + transportOf, + validateServer, +} from './validate.js' + +/** + * Turn a pasted config snippet into candidate servers. + * + * Accepted shapes, in detection order: + * 1. `{"mcpServers": {...}}` — Claude Code / Claude Desktop / Cursor, and what + * almost every server README publishes. + * 2. `{"servers": {...}, "inputs": [...]}` — VS Code's mcp.json. + * 3. `{"": {command|url…}, …}` — the inner map on its own, which is what + * people copy when they grab "just the server part". + * 4. `{command|url…}` — one bare entry; the caller supplies the name. + * + * WHY every literal env/header value becomes a secret input: we cannot tell a + * token from a log level by looking at the key (`KEY`, `AUTH`, `X-Api-Key`, + * `DATABASE_URL`…), and guessing wrong in the permissive direction writes a + * credential into a plaintext JSON file. The built-in launcher made the same + * call for the same reason (builtInMcpLaunch.ts: "avoids having to guess which + * future header names are sensitive"). A user can edit a non-sensitive value + * back to a literal afterwards; the reverse mistake cannot be undone. + */ +export function importUserMcpConfig(text: string, fallbackName = 'server'): UserMcpImportResult { + let parsed: unknown + try { + parsed = JSON.parse(text) + } catch (error) { + return { + ok: false, + error: `Not valid JSON: ${error instanceof Error ? error.message : String(error)}`, + } + } + if (!isPlainObject(parsed)) return { ok: false, error: 'Paste a JSON object.' } + + let format: 'mcpServers' | 'vscode' | 'map' | 'entry' + let rawServers: Record + let vscodeInputs: UserMcpInput[] = [] + if (isPlainObject(parsed.mcpServers)) { + format = 'mcpServers' + rawServers = parsed.mcpServers + } else if (isPlainObject(parsed.servers)) { + format = 'vscode' + rawServers = parsed.servers + vscodeInputs = vscodeInputDefinitions(parsed.inputs) + } else if (transportOf(parsed) !== null || 'command' in parsed || 'url' in parsed) { + format = 'entry' + rawServers = { [fallbackName]: parsed } + } else if (Object.values(parsed).length > 0 && Object.values(parsed).every(isPlainObject)) { + format = 'map' + rawServers = parsed + } else { + return { + ok: false, + error: 'No MCP server found. Paste an "mcpServers" block, a VS Code "servers" block, or one server entry.', + } + } + + const names = Object.keys(rawServers) + if (names.length === 0) return { ok: false, error: 'The config contains no servers.' } + + const candidates: UserMcpImportCandidate[] = [] + for (const name of names) { + const raw = rawServers[name] + if (!isPlainObject(raw)) continue + candidates.push(candidateFor(name, raw, vscodeInputs, candidates)) + } + return { ok: true, candidates, format } +} + +function candidateFor( + name: string, + raw: Record, + vscodeInputs: readonly UserMcpInput[], + earlier: readonly UserMcpImportCandidate[], +): UserMcpImportCandidate { + const inputs: UserMcpInput[] = [] + const pendingSecrets: Record = {} + const takenIds = new Set(vscodeInputs.map(input => input.id)) + const entry: Record = { ...raw } + + const liftRecord = (field: 'env' | 'headers') => { + const record = entry[field] + if (!isStringRecord(record)) return + const next: Record = {} + for (const [key, value] of Object.entries(record)) { + if (value === '' || hasInputReference(value)) { + next[key] = value + continue + } + const id = uniqueInputId(`${name}-${key}`, takenIds) + // `Authorization: Bearer ` is the dominant header shape, and the + // scheme word is not secret. Keeping it in the entry means the stored + // secret is exactly what the server's settings page hands the user. + const scheme = /^(Bearer|Token|Basic)\s+(.+)$/i.exec(value) + next[key] = scheme ? `${scheme[1]} \${input:${id}}` : `\${input:${id}}` + const secret = scheme ? scheme[2]! : value + inputs.push({ id, description: `${field === 'env' ? 'Environment variable' : 'Header'} ${key}` }) + // README placeholders ("YOUR_TOKEN_HERE", "") are not secrets. + // Storing them would make the server look configured and then fail at + // tool-call time; leaving the input empty shows "secret not set" instead. + if (!isPlaceholder(secret)) pendingSecrets[id] = secret + } + entry[field] = next + } + liftRecord('env') + liftRecord('headers') + + // VS Code inputs referenced by this entry come along as secret inputs. + const referencedVscode = vscodeInputs.filter(input => + JSON.stringify(entry).includes(`\${input:${input.id}}`)) + inputs.unshift(...referencedVscode) + + const normalized = normalizeEntry(entry as UserMcpServerEntry) + const problems = validateServer( + { name, entry: normalized, inputs }, + earlier.map(candidate => ({ id: candidate.name, name: candidate.name })), + ) + return { name, entry: normalized, inputs, pendingSecrets, problems } +} + +function vscodeInputDefinitions(value: unknown): UserMcpInput[] { + if (!Array.isArray(value)) return [] + // Every VS Code input becomes a secret regardless of `password`: the value + // is supplied by the user at setup time either way, and treating it as a + // secret only costs a masked field. + return coerceInputs(value.map(input => + isPlainObject(input) + ? { id: input.id, description: typeof input.description === 'string' ? input.description : '' } + : input)) +} + +function uniqueInputId(seed: string, taken: Set): string { + const base = seed.toLowerCase().replace(/[^a-z0-9_-]+/g, '-').replace(/^-+|-+$/g, '').slice(0, 60) || 'secret' + let id = base + for (let n = 2; taken.has(id); n++) id = `${base}-${n}` + taken.add(id) + return id +} + +const PLACEHOLDER = /^(<[^>]*>|\[[^\]]*\]|\.\.\.|x{3,}|your[\s_-].*|.*[\s_-]here|changeme|replace[\s_-]?me|token|api[\s_-]?key)$/i + +export function isPlaceholder(value: string): boolean { + return PLACEHOLDER.test(value.trim()) +} diff --git a/src/shared/userMcp/inputs.ts b/src/shared/userMcp/inputs.ts new file mode 100644 index 000000000..706edd77b --- /dev/null +++ b/src/shared/userMcp/inputs.ts @@ -0,0 +1,40 @@ +// `${input:}` references — VS Code's MCP secret syntax (`inputs` with +// `password: true`), adopted verbatim so a VS Code config pastes unchanged and +// so users see a syntax they may already know instead of one we invented. + +const INPUT_REFERENCE = /\$\{input:([A-Za-z0-9_-]{1,64})\}/g + +export const USER_MCP_INPUT_ID_PATTERN = /^[A-Za-z0-9_-]{1,64}$/ + +export function inputReferences(value: string): string[] { + return [...value.matchAll(INPUT_REFERENCE)].map(match => match[1]!) +} + +export function hasInputReference(value: string): boolean { + return inputReferences(value).length > 0 +} + +/** + * Replace every reference with its secret value. + * + * Returns null when any referenced value is missing. WHY not substitute an + * empty string: an `Authorization: Bearer ` header or an empty API key makes the + * server fail in a confusing, server-specific way at tool-call time. Refusing + * here lets main drop the server with an explicit "secret not set" reason + * before the agent launches. + */ +export function substituteInputs( + value: string, + secrets: Readonly>, +): string | null { + let missing = false + const result = value.replace(INPUT_REFERENCE, (_whole, id: string) => { + const secret = secrets[id] + if (secret === undefined || secret === '') { + missing = true + return '' + } + return secret + }) + return missing ? null : result +} diff --git a/src/shared/userMcp/types.ts b/src/shared/userMcp/types.ts new file mode 100644 index 000000000..ea8c2518e --- /dev/null +++ b/src/shared/userMcp/types.ts @@ -0,0 +1,221 @@ +// User-managed MCP servers (#1143). Design: docs/superpowers/specs/ +// 2026-09-22-user-mcp-servers-design.md — read its Evidence section before +// changing any rule here; each one is pinned to a CLI behavior. + +/** + * Providers that can receive a user MCP server today. + * + * WHY only Claude and Codex (not every AgentProviderKind): these are the two + * launchers whose MCP injection semantics were verified against their source + * and installed binaries. OpenCode and Grok also carry built-in MCP, but a user + * server there needs its own translator and evidence. Keeping this list closed + * means a new provider shows an honest "not supported yet" instead of silently + * receiving a config shape nobody checked. + */ +export const USER_MCP_PROVIDERS = ['claude', 'codex'] as const +export type UserMcpProvider = (typeof USER_MCP_PROVIDERS)[number] + +export function isUserMcpProvider(value: unknown): value is UserMcpProvider { + return typeof value === 'string' && (USER_MCP_PROVIDERS as readonly string[]).includes(value) +} + +/** + * Server names Agent Code already owns on every launch. `agent_code` is the + * built-in host's entry (BuiltInMcpHttpHost.serverConfig) and + * `agent-code-control` is denied/disabled for every provider by + * externalControlExclusion. A user entry with either name would replace or be + * replaced by ours: Claude's `--mcp-config` swaps whole entries and Codex + * deep-merges keys, so neither outcome is a usable server. + */ +export const RESERVED_USER_MCP_NAMES = ['agent_code', 'agent-code-control'] as const + +/** + * The de facto `mcpServers` entry, as server READMEs publish it. + * + * WHY we store the published shape instead of a normalized internal one: a + * user adds a server by pasting its README snippet, and later wants to copy it + * back out. A lossless round trip is only possible if our storage IS that + * shape. Unknown keys (Claude's `oauth`, `headersHelper`, a client's `timeout`) + * are therefore kept on the object even though no type names them — the index + * signature is deliberate, not laziness. + */ +export type UserMcpStdioEntry = { + type?: 'stdio' + command: string + args?: string[] + env?: Record + cwd?: string + [extra: string]: unknown +} + +export type UserMcpRemoteEntry = { + type: 'http' | 'sse' + url: string + headers?: Record + [extra: string]: unknown +} + +export type UserMcpServerEntry = UserMcpStdioEntry | UserMcpRemoteEntry + +export type UserMcpTransport = 'stdio' | 'http' | 'sse' + +/** A secret the entry references as `${input:}`. Only the definition is + * stored in the document; the value lives in main's safeStorage blobs. */ +export type UserMcpInput = { + id: string + description: string +} + +export type UserMcpServer = { + /** Stable identity. Per-agent overrides and secrets key on this, never on + * the name, so renaming a server keeps every agent's choice attached. */ + id: string + /** Provider-visible server name (tool names become `mcp____`). */ + name: string + /** Master switch. Off means off for every agent, whatever it overrides. */ + enabled: boolean + /** Whether NEW agents of each provider get it by default. */ + providers: Record + entry: UserMcpServerEntry + inputs: UserMcpInput[] +} + +export type UserMcpDocument = { + version: 1 + servers: UserMcpServer[] +} + +export type UserMcpProblem = + | { kind: 'invalid-name'; message: string } + | { kind: 'reserved-name'; message: string } + | { kind: 'duplicate-name'; message: string } + | { kind: 'invalid-entry'; message: string } + | { kind: 'secret-in-forbidden-field'; message: string } + | { kind: 'unknown-input'; message: string } + | { kind: 'secret-missing'; message: string } + +export type UserMcpSupport = { ok: true } | { ok: false; reason: string } + +export type UserMcpSecretState = { set: boolean; hint?: string } + +/** What crosses IPC to the renderer. Never contains a secret value. */ +export type UserMcpServerView = UserMcpServer & { + transport: UserMcpTransport | null + /** One-line summary for list rows (`localhost:23373/v0/mcp`, `npx -y pkg`). */ + summary: string + secrets: Record + problems: UserMcpProblem[] + support: Record +} + +/** A server the CLI loads from its own config, shown read-only. */ +export type NativeMcpServer = { + provider: UserMcpProvider + name: string + /** Display path of the file it came from, `~`-abbreviated. */ + source: string + transport: UserMcpTransport | null + summary: string + /** The raw entry, translated to the `mcpServers` shape for Copy in. Secret + * values are NOT included — Copy in creates empty secret inputs instead, + * so this snapshot never carries a token across IPC. */ + entry: UserMcpServerEntry | null + /** Env/header keys whose values were withheld from `entry`. */ + withheldSecretKeys: string[] +} + +export type UserMcpSnapshot = { + servers: UserMcpServerView[] + native: NativeMcpServer[] + /** Set when the document on disk could not be read. The file is preserved + * beside the new one, so this is a notice, not data loss. */ + storeProblem?: string + /** Claude's enterprise managed-mcp.json is present: Claude refuses any + * non-SDK `--mcp-config` server, so user servers are held back for Claude. */ + claudeManagedPolicy: boolean +} + +export type UserMcpSaveInput = { + id?: string + name: string + enabled: boolean + providers: Record + entry: UserMcpServerEntry + inputs: UserMcpInput[] + /** Secret values to set with this save (inputId → value). Values for inputs + * not listed stay as they are; an empty string clears. */ + secrets?: Record +} + +export type UserMcpMutationResult = + | { ok: true; snapshot: UserMcpSnapshot; id?: string } + | { ok: false; error: string; problems?: UserMcpProblem[] } + +export type UserMcpImportCandidate = { + name: string + entry: UserMcpServerEntry + inputs: UserMcpInput[] + /** Values lifted out of the pasted text into inputs; saved as secrets. */ + pendingSecrets: Record + problems: UserMcpProblem[] +} + +export type UserMcpImportResult = + | { ok: true; candidates: UserMcpImportCandidate[]; format: 'mcpServers' | 'vscode' | 'map' | 'entry' } + | { ok: false; error: string } + +/** Why a server was not attached to one launch. */ +export type UserMcpDroppedServer = { name: string; reason: string } + +export type UserMcpUnavailableEvent = { servers: UserMcpDroppedServer[] } + +/** + * Per-agent overrides for user servers ride in the pane's existing + * `builtInMcpOverrides` map under this prefix (spec Revision 2 §3). + * + * WHY a prefix in the existing map instead of a second map: that map is already + * threaded through spawn, replace, reload, recovery, undo-close, provider + * switch, duplicate and the control API. A parallel map would have to be + * re-threaded through every one of those paths, and missing one silently drops + * a user's choice. Built-in domain names never contain ':', so the namespaces + * cannot collide. + */ +export const USER_MCP_OVERRIDE_PREFIX = 'user:' +export type UserMcpOverrideKey = `user:${string}` + +export function userMcpOverrideKey(serverId: string): UserMcpOverrideKey { + return `${USER_MCP_OVERRIDE_PREFIX}${serverId}` as UserMcpOverrideKey +} + +const SERVER_ID_PATTERN = /^[A-Za-z0-9_-]{1,64}$/ + +export function isUserMcpServerId(value: unknown): value is string { + return typeof value === 'string' && SERVER_ID_PATTERN.test(value) +} + +export function isUserMcpOverrideKey(key: string): key is UserMcpOverrideKey { + return key.startsWith(USER_MCP_OVERRIDE_PREFIX) + && isUserMcpServerId(key.slice(USER_MCP_OVERRIDE_PREFIX.length)) +} + +/** The `user:` subset of a pane override map, keyed by bare server id — the + * shape that crosses IPC to main with spawn/recover. */ +export function userMcpOverridesFrom(value: unknown): Record { + if (!value || typeof value !== 'object' || Array.isArray(value)) return {} + const result: Record = {} + for (const [key, choice] of Object.entries(value as Record)) { + if (typeof choice !== 'boolean' || !isUserMcpOverrideKey(key)) continue + result[key.slice(USER_MCP_OVERRIDE_PREFIX.length)] = choice + } + return result +} + +/** Validates the IPC form (bare ids) coming from an untrusted renderer. */ +export function normalizeUserMcpOverrides(value: unknown): Record { + if (!value || typeof value !== 'object' || Array.isArray(value)) return {} + const result: Record = {} + for (const [id, choice] of Object.entries(value as Record)) { + if (typeof choice === 'boolean' && isUserMcpServerId(id)) result[id] = choice + } + return result +} diff --git a/src/shared/userMcp/userMcp.test.ts b/src/shared/userMcp/userMcp.test.ts new file mode 100644 index 000000000..ff25e4984 --- /dev/null +++ b/src/shared/userMcp/userMcp.test.ts @@ -0,0 +1,169 @@ +import { describe, expect, it } from 'vitest' + +import { importUserMcpConfig } from './importConfig.js' +import { substituteInputs } from './inputs.js' +import { userMcpOverrideKey, userMcpOverridesFrom } from './types.js' +import { + coerceUserMcpDocument, + providerSupport, + transportOf, + validateServer, +} from './validate.js' + +// Verbatim from https://developers.beeper.com/desktop-api/mcp/ (2026-09-22). +// Real published snippets are the fixture on purpose: the import path exists +// to accept exactly what server READMEs print, not a shape we imagined. +const BEEPER_HTTP_TOKEN = `{ "mcpServers": { "beeper": { "url": "http://localhost:23373/v0/mcp", + "headers": { "Authorization": "Bearer YOUR_TOKEN_HERE" } } } }` +const BEEPER_STDIO_TOKEN = `{ "mcpServers": { "beeper": { "command": "npx", + "args": ["-y", "@beeper/mcp-remote", "--header", "Authorization: Bearer \${ACCESS_TOKEN}"], + "env": { "ACCESS_TOKEN": "YOUR_TOKEN_HERE" } } } }` +const BEEPER_VSCODE = `{ "servers": { "beeper": { "type": "http", "url": "http://localhost:23373/v0/mcp" } } }` + +describe('importUserMcpConfig', () => { + it('imports the Beeper HTTP snippet as an http server whose token is a secret input', () => { + const result = importUserMcpConfig(BEEPER_HTTP_TOKEN) + expect(result.ok).toBe(true) + if (!result.ok) return + expect(result.format).toBe('mcpServers') + const [beeper] = result.candidates + expect(beeper!.name).toBe('beeper') + expect(beeper!.entry).toEqual({ + type: 'http', + url: 'http://localhost:23373/v0/mcp', + headers: { Authorization: 'Bearer ${input:beeper-authorization}' }, + }) + expect(beeper!.inputs.map(input => input.id)).toEqual(['beeper-authorization']) + // The README placeholder is not a secret: storing it would make the server + // look configured and fail at tool-call time. + expect(beeper!.pendingSecrets).toEqual({}) + expect(beeper!.problems).toEqual([]) + }) + + it('lifts a real token out of the stored entry entirely', () => { + const result = importUserMcpConfig(BEEPER_HTTP_TOKEN.replace('YOUR_TOKEN_HERE', 'bpr_live_9f3a')) + if (!result.ok) throw new Error(result.error) + const [beeper] = result.candidates + expect(JSON.stringify(beeper!.entry)).not.toContain('bpr_live_9f3a') + expect(beeper!.pendingSecrets).toEqual({ 'beeper-authorization': 'bpr_live_9f3a' }) + }) + + it('imports the mcp-remote stdio snippet, keeping the server-expanded ${VAR} in args', () => { + const result = importUserMcpConfig(BEEPER_STDIO_TOKEN.replace('YOUR_TOKEN_HERE', 'tok')) + if (!result.ok) throw new Error(result.error) + const [beeper] = result.candidates + expect(transportOf(beeper!.entry)).toBe('stdio') + expect((beeper!.entry as { args: string[] }).args[3]).toBe('Authorization: Bearer ${ACCESS_TOKEN}') + expect((beeper!.entry as { env: Record }).env).toEqual({ + ACCESS_TOKEN: '${input:beeper-access_token}', + }) + expect(beeper!.pendingSecrets).toEqual({ 'beeper-access_token': 'tok' }) + expect(beeper!.problems).toEqual([]) + }) + + it('accepts the VS Code servers form and carries its password inputs', () => { + expect(importUserMcpConfig(BEEPER_VSCODE)).toMatchObject({ ok: true, format: 'vscode' }) + const withInputs = importUserMcpConfig(JSON.stringify({ + inputs: [{ type: 'promptString', id: 'gh-token', description: 'GitHub PAT', password: true }], + servers: { github: { type: 'http', url: 'https://api.githubcopilot.com/mcp/', headers: { Authorization: 'Bearer ${input:gh-token}' } } }, + })) + if (!withInputs.ok) throw new Error(withInputs.error) + expect(withInputs.candidates[0]!.inputs).toEqual([{ id: 'gh-token', description: 'GitHub PAT' }]) + expect(withInputs.candidates[0]!.problems).toEqual([]) + }) + + it('accepts a bare map and a single bare entry', () => { + const map = importUserMcpConfig('{"playwright":{"command":"npx","args":["@playwright/mcp@latest"]}}') + expect(map).toMatchObject({ ok: true, format: 'map', candidates: [{ name: 'playwright' }] }) + const entry = importUserMcpConfig('{"url":"https://mcp.linear.app/sse","type":"sse"}', 'linear') + expect(entry).toMatchObject({ ok: true, format: 'entry', candidates: [{ name: 'linear' }] }) + }) + + it('reports malformed JSON as a result, not a throw', () => { + expect(importUserMcpConfig('{ "mcpServers": ')).toMatchObject({ ok: false }) + expect(importUserMcpConfig('[1,2]')).toMatchObject({ ok: false }) + }) +}) + +describe('validateServer', () => { + const http = { type: 'http' as const, url: 'http://localhost:23373/v0/mcp' } + + it('rejects names that would break Codex -c paths or collide with Agent Code', () => { + expect(validateServer({ name: 'my.server', entry: http, inputs: [] })[0]?.kind).toBe('invalid-name') + expect(validateServer({ name: 'AGENT_CODE', entry: http, inputs: [] })[0]?.kind).toBe('reserved-name') + expect(validateServer({ name: 'agent-code-control', entry: http, inputs: [] })[0]?.kind).toBe('reserved-name') + expect( + validateServer({ name: 'Beeper', entry: http, inputs: [] }, [{ id: 'x', name: 'beeper' }])[0]?.kind, + ).toBe('duplicate-name') + }) + + it('allows secrets only where they can travel through the environment', () => { + const inputs = [{ id: 't', description: '' }] + expect(validateServer({ name: 'a', entry: { command: 'srv', env: { T: '${input:t}' } }, inputs }, [])).toEqual([]) + expect(validateServer({ name: 'a', entry: { ...http, headers: { A: 'Bearer ${input:t}' } }, inputs }, [])).toEqual([]) + const inArgs = validateServer({ name: 'a', entry: { command: 'srv', args: ['--key', '${input:t}'] }, inputs }, []) + expect(inArgs.map(problem => problem.kind)).toEqual(['secret-in-forbidden-field']) + const inUrl = validateServer({ name: 'a', entry: { type: 'http', url: 'https://x.dev/?k=${input:t}' }, inputs }, []) + expect(inUrl.map(problem => problem.kind)).toContain('secret-in-forbidden-field') + }) + + it('flags references to secrets that were never defined', () => { + const problems = validateServer({ name: 'a', entry: { ...http, headers: { A: '${input:nope}' } }, inputs: [] }) + expect(problems.map(problem => problem.kind)).toEqual(['unknown-input']) + }) + + it('rejects mixed and empty transports', () => { + expect(validateServer({ name: 'a', entry: { command: 'x', url: 'http://y' } as never, inputs: [] })[0]?.kind) + .toBe('invalid-entry') + expect(validateServer({ name: 'a', entry: {} as never, inputs: [] })[0]?.kind).toBe('invalid-entry') + }) +}) + +describe('providerSupport', () => { + it('keeps SSE servers Claude-only because Codex has no SSE transport', () => { + expect(providerSupport('sse').codex).toEqual({ ok: false, reason: 'Codex does not support SSE servers' }) + expect(providerSupport('sse').claude).toEqual({ ok: true }) + expect(providerSupport('http').codex).toEqual({ ok: true }) + }) +}) + +describe('coerceUserMcpDocument', () => { + it('keeps unknown entry keys and malformed servers instead of dropping user work', () => { + const doc = coerceUserMcpDocument({ + version: 1, + servers: [ + { id: 'a1', name: 'beeper', enabled: true, providers: { claude: true }, entry: { url: 'http://x', oauth: { clientId: 'c' } }, inputs: [] }, + { id: 'b2', name: 'bad name!', entry: 'nonsense' }, + { name: 'no id at all' }, + ], + }) + expect(doc.servers.map(server => server.id)).toEqual(['a1', 'b2']) + expect(doc.servers[0]!.entry).toMatchObject({ oauth: { clientId: 'c' } }) + expect(doc.servers[0]!.providers).toEqual({ claude: true, codex: false }) + expect(doc.servers[1]!.entry).toEqual({}) + }) + + it('treats anything that is not a document as empty', () => { + expect(coerceUserMcpDocument(null)).toEqual({ version: 1, servers: [] }) + expect(coerceUserMcpDocument({ servers: 'x' })).toEqual({ version: 1, servers: [] }) + }) +}) + +describe('substituteInputs', () => { + it('refuses a partially resolved value rather than emitting an empty credential', () => { + expect(substituteInputs('Bearer ${input:t}', { t: 'abc' })).toBe('Bearer abc') + expect(substituteInputs('Bearer ${input:t}', {})).toBeNull() + expect(substituteInputs('Bearer ${input:t}', { t: '' })).toBeNull() + }) +}) + +describe('user override keys', () => { + it('extracts only well-formed user: keys from a pane override map', () => { + expect(userMcpOverridesFrom({ + tldr: true, + [userMcpOverrideKey('abc-123')]: false, + 'user:bad id': true, + 'user:x': 'yes', + })).toEqual({ 'abc-123': false }) + }) +}) diff --git a/src/shared/userMcp/validate.ts b/src/shared/userMcp/validate.ts new file mode 100644 index 000000000..c648eeb5f --- /dev/null +++ b/src/shared/userMcp/validate.ts @@ -0,0 +1,277 @@ +import { hasInputReference, inputReferences, USER_MCP_INPUT_ID_PATTERN } from './inputs.js' +import { + RESERVED_USER_MCP_NAMES, + USER_MCP_PROVIDERS, + isUserMcpServerId, + type UserMcpDocument, + type UserMcpInput, + type UserMcpProblem, + type UserMcpProvider, + type UserMcpServer, + type UserMcpServerEntry, + type UserMcpSupport, + type UserMcpTransport, +} from './types.js' + +/** + * `[A-Za-z0-9_-]`, 1–64 chars. + * + * WHY this exact charset: it is the intersection of what both CLIs accept AND + * what survives Codex's `-c` override parser. Claude's `mcp add` enforces + * `[A-Za-z0-9_-]`. Codex accepts more (`:@/.`) in config.toml, but `-c + * mcp_servers..url=…` splits the key path on '.' naively and never + * unquotes a quoted segment (codex-rs config/src/overrides.rs), so any wider + * name would silently target the wrong table. + * + * WHY no automatic `ac-` prefix to avoid collisions: tool names become + * `mcp____`, which users copy into permission rules from server + * docs. A prefix would break every documented rule. Collisions are instead + * detected (reserved names here, native Codex names at launch). + */ +export const USER_MCP_NAME_PATTERN = /^[A-Za-z0-9_-]{1,64}$/ + +export function transportOf(entry: unknown): UserMcpTransport | null { + if (!isPlainObject(entry)) return null + const hasCommand = typeof entry.command === 'string' + const hasUrl = typeof entry.url === 'string' + if (entry.type === 'stdio') return hasCommand && !hasUrl ? 'stdio' : null + if (entry.type === 'http' || entry.type === 'sse') return hasUrl && !hasCommand ? entry.type : null + // Entries without `type` are the common README shape. A url-only entry means + // Streamable HTTP — Claude Desktop/Cursor behavior and the current transport; + // SSE servers publish `"type": "sse"` because SSE is the deprecated one. + if (entry.type === undefined) { + if (hasCommand && !hasUrl) return 'stdio' + if (hasUrl && !hasCommand) return 'http' + } + return null +} + +/** + * Canonical stored form: explicit `type` for remote entries so the Codex and + * Claude translators never re-derive it, but stdio keeps whatever the user + * pasted (`type` omitted or `"stdio"`) — both are valid everywhere. + */ +export function normalizeEntry(entry: UserMcpServerEntry): UserMcpServerEntry { + const transport = transportOf(entry) + if ((transport === 'http' || transport === 'sse') && entry.type === undefined) { + return { ...entry, type: transport } as UserMcpServerEntry + } + return entry +} + +export function validateEntry(entry: unknown): UserMcpProblem[] { + if (!isPlainObject(entry)) { + return [{ kind: 'invalid-entry', message: 'The server config must be a JSON object.' }] + } + const transport = transportOf(entry) + if (!transport) { + if (typeof entry.command === 'string' && typeof entry.url === 'string') { + return [{ kind: 'invalid-entry', message: 'Use either "command" (stdio) or "url" (http/sse), not both.' }] + } + if (entry.type !== undefined && !['stdio', 'http', 'sse'].includes(String(entry.type))) { + return [{ kind: 'invalid-entry', message: `Unsupported transport type "${String(entry.type)}". Use stdio, http or sse.` }] + } + return [{ kind: 'invalid-entry', message: 'Add "command" for a local (stdio) server or "url" for a remote one.' }] + } + const problems: UserMcpProblem[] = [] + if (transport === 'stdio') { + if (!(entry.command as string).trim()) { + problems.push({ kind: 'invalid-entry', message: '"command" cannot be empty.' }) + } + if (entry.args !== undefined && !isStringArray(entry.args)) { + problems.push({ kind: 'invalid-entry', message: '"args" must be an array of strings.' }) + } + if (entry.env !== undefined && !isStringRecord(entry.env)) { + problems.push({ kind: 'invalid-entry', message: '"env" must map names to string values.' }) + } + if (entry.cwd !== undefined && typeof entry.cwd !== 'string') { + problems.push({ kind: 'invalid-entry', message: '"cwd" must be a string.' }) + } + } else { + if (!isHttpUrl(entry.url as string)) { + problems.push({ kind: 'invalid-entry', message: '"url" must be an absolute http(s) URL.' }) + } + if (entry.headers !== undefined && !isStringRecord(entry.headers)) { + problems.push({ kind: 'invalid-entry', message: '"headers" must map names to string values.' }) + } + } + problems.push(...forbiddenSecretProblems(entry)) + return problems +} + +/** + * `${input:…}` is only allowed in env and header VALUES. + * + * WHY: Codex does not expand variables in `command`, `args` or `url`, so a + * secret there has to be written literally into a `-c` override — i.e. into + * the Codex process's argv, readable by any local process via `ps` and by our + * own incident collectors. Env and header values can always travel through the + * environment instead (`env_vars`, `env_http_headers`, Claude's `${VAR}`). + * Servers that want a token on their own command line (mcp-remote's + * `--header "Authorization: Bearer ${TOKEN}"`) expand their own environment, + * so the user writes `${TOKEN}` in args and keeps the secret in `env.TOKEN`. + */ +function forbiddenSecretProblems(entry: Record): UserMcpProblem[] { + const problems: UserMcpProblem[] = [] + const check = (field: string, value: unknown) => { + if (typeof value === 'string' && hasInputReference(value)) { + problems.push({ + kind: 'secret-in-forbidden-field', + message: `Secrets can only be used in env or headers values, not in "${field}" (it would be visible to other processes).`, + }) + } + } + check('command', entry.command) + check('url', entry.url) + check('cwd', entry.cwd) + if (Array.isArray(entry.args)) entry.args.forEach(arg => check('args', arg)) + return problems +} + +export function referencedInputIds(entry: UserMcpServerEntry): string[] { + const ids = new Set() + const scan = (record: unknown) => { + if (!isStringRecord(record)) return + for (const value of Object.values(record)) inputReferences(value).forEach(id => ids.add(id)) + } + scan((entry as Record).env) + scan((entry as Record).headers) + return [...ids] +} + +export function validateServer( + server: Pick, + others: readonly Pick[] = [], +): UserMcpProblem[] { + const problems: UserMcpProblem[] = [] + if (!USER_MCP_NAME_PATTERN.test(server.name)) { + problems.push({ + kind: 'invalid-name', + message: 'Use 1–64 letters, digits, "-" or "_" (other characters break Codex config overrides).', + }) + } + if (RESERVED_USER_MCP_NAMES.some(reserved => foldName(reserved) === foldName(server.name))) { + problems.push({ kind: 'reserved-name', message: `"${server.name}" is reserved for Agent Code's own MCP server.` }) + } + // Case- and separator-insensitive on purpose. "Beeper" vs "beeper" is a + // trap for permission rules even where the CLIs would tell them apart, and + // "my-server" vs "my_server" would map to the same generated secret variable + // (userMcpSecretVariable upper-cases and folds separators), so the second + // server's token would silently overwrite the first one's in the launch env. + const folded = foldName(server.name) + if (others.some(other => foldName(other.name) === folded)) { + problems.push({ kind: 'duplicate-name', message: `Another server is already named "${server.name}".` }) + } + problems.push(...validateEntry(server.entry)) + const defined = new Set(server.inputs.map(input => input.id)) + if (isPlainObject(server.entry)) { + for (const id of referencedInputIds(server.entry)) { + if (!defined.has(id)) { + problems.push({ kind: 'unknown-input', message: `"\${input:${id}}" is used but no secret named "${id}" exists.` }) + } + } + } + return problems +} + +function foldName(name: string): string { + return name.toLowerCase().replace(/-/g, '_') +} + +export function providerSupport( + transport: UserMcpTransport | null, +): Record { + return { + claude: { ok: true }, + // Codex speaks stdio and Streamable HTTP only (codex-rs mcp_types.rs has + // no SSE transport). Claude still accepts SSE, so an SSE server is + // Claude-only rather than invalid. + codex: transport === 'sse' + ? { ok: false, reason: 'Codex does not support SSE servers' } + : { ok: true }, + } +} + +export function summarizeEntry(entry: unknown): string { + if (!isPlainObject(entry)) return '' + const transport = transportOf(entry) + if (transport === 'stdio') { + const args = isStringArray(entry.args) ? entry.args : [] + return [entry.command as string, ...args].join(' ') + } + if (transport) { + try { + const url = new URL(entry.url as string) + return `${url.host}${url.pathname === '/' ? '' : url.pathname}` + } catch { + return String(entry.url) + } + } + return '' +} + +/** + * Coerce the persisted document without ever dropping a server the user made. + * + * WHY keep malformed servers instead of filtering them: the document is edited + * by hand, by older builds and by future ones. Deleting an entry because this + * build could not read one field would lose the user's work silently; keeping + * it lets validation flag it in Settings and lets main refuse to launch it. + * Only entries with no usable identity at all (not an object, no id) go. + */ +export function coerceUserMcpDocument(value: unknown): UserMcpDocument { + if (!isPlainObject(value) || !Array.isArray(value.servers)) return { version: 1, servers: [] } + const servers: UserMcpServer[] = [] + const seen = new Set() + for (const raw of value.servers) { + if (!isPlainObject(raw) || !isUserMcpServerId(raw.id) || seen.has(raw.id)) continue + seen.add(raw.id) + const providers = isPlainObject(raw.providers) ? raw.providers : {} + servers.push({ + id: raw.id, + name: typeof raw.name === 'string' ? raw.name : '', + enabled: raw.enabled !== false, + providers: Object.fromEntries( + USER_MCP_PROVIDERS.map(provider => [provider, providers[provider] === true]), + ) as Record, + // Kept verbatim (including unknown keys); validation reports problems. + entry: (isPlainObject(raw.entry) ? raw.entry : {}) as UserMcpServerEntry, + inputs: coerceInputs(raw.inputs), + }) + } + return { version: 1, servers } +} + +export function coerceInputs(value: unknown): UserMcpInput[] { + if (!Array.isArray(value)) return [] + const seen = new Set() + const inputs: UserMcpInput[] = [] + for (const raw of value) { + if (!isPlainObject(raw) || typeof raw.id !== 'string' || !USER_MCP_INPUT_ID_PATTERN.test(raw.id)) continue + if (seen.has(raw.id)) continue + seen.add(raw.id) + inputs.push({ id: raw.id, description: typeof raw.description === 'string' ? raw.description : '' }) + } + return inputs +} + +export function isPlainObject(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value) +} + +export function isStringArray(value: unknown): value is string[] { + return Array.isArray(value) && value.every(item => typeof item === 'string') +} + +export function isStringRecord(value: unknown): value is Record { + return isPlainObject(value) && Object.values(value).every(item => typeof item === 'string') +} + +function isHttpUrl(value: string): boolean { + try { + const url = new URL(value) + return url.protocol === 'http:' || url.protocol === 'https:' + } catch { + return false + } +} From 4ef9cb765477426e7dbbd0120b40dfb4f1b44d9f Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Tue, 22 Sep 2026 19:32:18 -0700 Subject: [PATCH 04/11] feat(mcp): attach user MCP servers to Claude and Codex launches Main owns the server document, the encrypted secrets and the decision of what attaches to each launch: the renderer only contributes the pane's explicit per-agent choices. A requested server that cannot attach is reported with a reason and never fails the launch, and the servers each backend actually launched with are reported on its snapshot the same way built-in MCP domains are. Also lists the servers each CLI loads from its own user-scope config, read-only and with every value withheld. Refs #1143 Co-Authored-By: Claude Opus 5.5 (1M context) --- src/main/index.ts | 8 + src/main/ipc/index.ts | 4 + src/main/ipc/userMcp.ts | 85 +++++ src/main/sessionManager.ts | 91 +++++ src/main/sessionManager.userMcp.test.ts | 100 +++++ src/main/sessions/forwarder.ts | 10 + src/main/userMcp/nativeServers.ts | 197 ++++++++++ src/main/userMcp/secrets.ts | 115 ++++++ src/main/userMcp/service.test.ts | 214 +++++++++++ src/main/userMcp/service.ts | 361 ++++++++++++++++++ src/main/userMcp/store.ts | 52 +++ src/preload/api/index.ts | 2 + src/preload/api/types.ts | 9 + src/preload/api/userMcp.ts | 36 ++ src/providers/claude/runtime/claudeSession.ts | 13 +- src/providers/codex/runtime/codexSession.ts | 10 + src/providers/shared/runtime/userMcpLaunch.ts | 17 +- src/shared/types/session.ts | 14 + src/shared/userMcp/types.ts | 25 +- src/shared/userMcp/validate.ts | 2 +- 20 files changed, 1344 insertions(+), 21 deletions(-) create mode 100644 src/main/ipc/userMcp.ts create mode 100644 src/main/sessionManager.userMcp.test.ts create mode 100644 src/main/userMcp/nativeServers.ts create mode 100644 src/main/userMcp/secrets.ts create mode 100644 src/main/userMcp/service.test.ts create mode 100644 src/main/userMcp/service.ts create mode 100644 src/main/userMcp/store.ts create mode 100644 src/preload/api/userMcp.ts diff --git a/src/main/index.ts b/src/main/index.ts index 4d69ab6f7..5851f60cc 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -138,6 +138,7 @@ import { RemoteController } from '@main/remote/RemoteController.js' import { CaffeinateController } from '@main/caffeinate/CaffeinateController.js' import { createFileVaultStore } from '@main/keyVault/vaultStore.js' import { createSafeStorageCodec } from '@main/keyVault/safeStorageCodec.js' +import { UserMcpService } from '@main/userMcp/service.js' import { VaultService } from '@main/keyVault/VaultService.js' import { buildAppMenu } from '@main/menu/appMenu.js' import { UpdateService } from '@main/updates/UpdateService.js' @@ -1026,6 +1027,11 @@ async function startApp(): Promise { const agentCodeConventionsService = new AgentCodeManagedSkillsService() await agentCodeConventionsService.initialize() assertStartupOpen() + // User MCP servers (#1143). Loaded before the manager so the first restored + // agent already launches with them; initialize() never throws (a corrupt + // document is moved aside and reported in Settings instead). + const userMcpService = new UserMcpService({ stateDir: STATE_DIR, codec: createSafeStorageCodec() }) + await userMcpService.initialize() manager = new SessionManager( tmuxAvailable ? tmuxRegistry : null, builtInMcpHost, @@ -1042,6 +1048,7 @@ async function startApp(): Promise { ) }, ) + manager.setUserMcpResolver(params => userMcpService.resolveForLaunch(params)) // Adapters seal streams a sleep severed (#963); the manager fans each // suspension out to the live agent runtimes. systemSuspension.on('suspension', (suspension: import('@shared/types/systemSuspension.js').SystemSuspension) => { @@ -1415,6 +1422,7 @@ async function startApp(): Promise { const conversationService = createConversationService({ ledger: conversationLedger, listWorktrees: listWorktreesForCwd }) registerAllIpc({ manager, + userMcpService, remoteController, lspManager, ghostJournals, diff --git a/src/main/ipc/index.ts b/src/main/ipc/index.ts index b8f96047e..3c35df518 100644 --- a/src/main/ipc/index.ts +++ b/src/main/ipc/index.ts @@ -47,6 +47,8 @@ import type { AppRunJournal } from '@main/incident/AppRunJournal.js' import { registerIncidentIpc } from '@main/ipc/incident.js' import { registerLifecycleIpc } from '@main/ipc/lifecycle.js' import { registerProviderEnablementIpc } from '@main/ipc/providerEnablement.js' +import { registerUserMcpIpc } from '@main/ipc/userMcp.js' +import type { UserMcpService } from '@main/userMcp/service.js' import { registerUsageIpc } from '@main/ipc/usage.js' import { registerCliUpdatesIpc } from '@main/ipc/cliUpdates.js' import type { CliUpdateOrchestrator } from '@main/setup/cliUpdateOrchestrator.js' @@ -73,6 +75,7 @@ import type { SystemSuspensionTracker } from '@main/systemSuspension/SystemSuspe export type IpcDeps = { manager: SessionManager + userMcpService: UserMcpService lspManager: LspManager ghostJournals: GhostJournalRegistry dictationDebugJournals: DictationDebugJournalRegistry @@ -140,6 +143,7 @@ export function registerAllIpc(deps: IpcDeps): void { ) registerDebugIpc(deps.appRunJournal, lifecycleDiagnostics) registerProviderEnablementIpc() + registerUserMcpIpc(deps.userMcpService) registerUsageIpc() registerCliUpdatesIpc(deps.cliUpdateOrchestrator) registerWorkflowIpc(deps.workflowBridge) diff --git a/src/main/ipc/userMcp.ts b/src/main/ipc/userMcp.ts new file mode 100644 index 000000000..a8f8479f0 --- /dev/null +++ b/src/main/ipc/userMcp.ts @@ -0,0 +1,85 @@ +import { ipcMain } from 'electron' + +import type { UserMcpService } from '@main/userMcp/service.js' +import { broadcastToWindows } from '@main/window/windowRegistry.js' +import { isUserMcpProvider, isUserMcpServerId, type UserMcpSaveInput } from '@shared/userMcp/types.js' +import { isPlainObject } from '@shared/userMcp/validate.js' + +export const USER_MCP_CHANGED_CHANNEL = 'user-mcp:changed' +export const USER_MCP_UNAVAILABLE_CHANNEL = 'user-mcp:unavailable' + +// Pasted configs are small; anything larger is not an MCP snippet and would +// only cost a JSON parse in main. +const MAX_IMPORT_CHARS = 256 * 1024 +const MAX_SECRET_CHARS = 64 * 1024 + +/** + * User MCP servers (#1143). Same contract as provider enablement: every + * mutation returns the fresh snapshot AND broadcasts it, so every window's + * Settings grid and per-agent picker update even though only one was touched. + * + * Arguments are untrusted renderer input. The shapes are checked here; the + * semantic rules (names, transports, secret placement) are enforced again in + * UserMcpService.save, which is the single place that decides what is valid. + */ +export function registerUserMcpIpc(service: UserMcpService): void { + ipcMain.handle('user-mcp:get', () => service.snapshot()) + + ipcMain.handle('user-mcp:save', (_evt, input: unknown) => { + if (!isSaveInput(input)) throw new Error('user-mcp:save: invalid arguments') + return service.save(input) + }) + + ipcMain.handle('user-mcp:delete', (_evt, id: unknown) => { + if (!isUserMcpServerId(id)) throw new Error('user-mcp:delete: invalid id') + return service.delete(id) + }) + + ipcMain.handle('user-mcp:set-enabled', (_evt, id: unknown, enabled: unknown) => { + if (!isUserMcpServerId(id) || typeof enabled !== 'boolean') throw new Error('user-mcp:set-enabled: invalid arguments') + return service.setEnabled(id, enabled) + }) + + ipcMain.handle('user-mcp:set-provider', (_evt, id: unknown, provider: unknown, enabled: unknown) => { + if (!isUserMcpServerId(id) || !isUserMcpProvider(provider) || typeof enabled !== 'boolean') { + throw new Error('user-mcp:set-provider: invalid arguments') + } + return service.setProvider(id, provider, enabled) + }) + + ipcMain.handle('user-mcp:set-secret', (_evt, id: unknown, inputId: unknown, value: unknown) => { + if ( + !isUserMcpServerId(id) || typeof inputId !== 'string' || + typeof value !== 'string' || value.length > MAX_SECRET_CHARS + ) { + throw new Error('user-mcp:set-secret: invalid arguments') + } + return service.setSecret(id, inputId, value) + }) + + ipcMain.handle('user-mcp:import', (_evt, text: unknown, fallbackName: unknown) => { + if (typeof text !== 'string' || text.length > MAX_IMPORT_CHARS) throw new Error('user-mcp:import: invalid text') + return service.importConfig(text, typeof fallbackName === 'string' ? fallbackName : undefined) + }) + + ipcMain.handle('user-mcp:copy-native', (_evt, provider: unknown, name: unknown) => { + if (!isUserMcpProvider(provider) || typeof name !== 'string') throw new Error('user-mcp:copy-native: invalid arguments') + return service.copyNative(provider, name) + }) + + service.onChange(snapshot => broadcastToWindows(USER_MCP_CHANGED_CHANNEL, snapshot)) +} + +function isSaveInput(value: unknown): value is UserMcpSaveInput { + if (!isPlainObject(value)) return false + if (value.id !== undefined && !isUserMcpServerId(value.id)) return false + if (typeof value.name !== 'string' || typeof value.enabled !== 'boolean') return false + if (!isPlainObject(value.providers) || !isPlainObject(value.entry) || !Array.isArray(value.inputs)) return false + if (value.secrets !== undefined) { + if (!isPlainObject(value.secrets)) return false + for (const secret of Object.values(value.secrets)) { + if (typeof secret !== 'string' || secret.length > MAX_SECRET_CHARS) return false + } + } + return true +} diff --git a/src/main/sessionManager.ts b/src/main/sessionManager.ts index 7f4841c34..a22402db6 100644 --- a/src/main/sessionManager.ts +++ b/src/main/sessionManager.ts @@ -69,6 +69,19 @@ import type { } from '@shared/types/providerKind.js' import type { BuiltInMcpDomain, BuiltInMcpServerConfig } from '@mcp/shared/types.js' import type { BuiltInMcpHttpHost } from '@mcp/runtime/BuiltInMcpHttpHost.js' +import { + normalizeUserMcpOverrides, + type ResolvedUserMcpServer, + type UserMcpDroppedServer, +} from '@shared/userMcp/types.js' + +/** Structural so SessionManager does not depend on the service class; see + * UserMcpService.resolveForLaunch for the rules it applies. */ +export type UserMcpResolver = (params: { + provider: string + overrides: Readonly> + cwd: string +}) => Promise<{ servers: ResolvedUserMcpServer[]; attachedIds: string[]; dropped: UserMcpDroppedServer[] }> import type { AppRunJournal } from '@main/incident/AppRunJournal.js' import { SessionLifecycleJournal } from '@main/lifecycle/SessionLifecycleJournal.js' import type { PromptGateState } from '@shared/types/session.js' @@ -218,6 +231,10 @@ type ManagerEvents = { * reconcile could not prepare, and it is being started without them (#1133). * Carries domain names only, never the error: see the pre-spawn gate. */ 'managed-skills-unavailable': [{ sessionId: string; skills: ReportingDomain[] }] + // #1143. A user MCP server the agent asked for could not be attached to this + // launch (missing secret, unsupported transport, name collision…). The + // launch itself proceeded without it. + 'user-mcp-unavailable': [{ sessionId: string; servers: UserMcpDroppedServer[] }] exit: [{ sessionId: string; exitCode: number; signal?: number }] } @@ -719,6 +736,25 @@ export class SessionManager extends EventEmitter { this.lifecycle = new SessionLifecycleJournal(journal) } + /** + * User MCP servers (#1143). A setter rather than another positional + * constructor argument because the service is optional (tests and the + * headless harnesses construct managers without one) and the constructor's + * positional list is already long enough that one more slot invites + * argument-order bugs at every call site. + */ + setUserMcpResolver(resolver: UserMcpResolver | null): void { + this.userMcpResolver = resolver + } + + private userMcpResolver: UserMcpResolver | null = null + // What each live agent was actually launched with, and the per-agent + // choices it was launched from. The first feeds the backend snapshot the + // same way builtInMcpHost.sessionDomains does; the second lets a same-rollout + // Codex restore relaunch with the predecessor's exact choices. + private readonly userMcpAttached = new Map() + private readonly userMcpOverridesBySession = new Map>() + private readonly lifecycle: SessionLifecycleJournal // Terminal attach/replay state. @@ -1035,6 +1071,12 @@ export class SessionManager extends EventEmitter { this.agentPtyAttachCounts.delete(sessionId) this.agentPtyRestoreSizes.delete(sessionId) if (revokeAgentMcp) this.builtInMcpHost?.revokeSession(sessionId) + // Same lifetime as the built-in scope: once the backend is gone its + // launch facts must not outlive it and be reported for a successor. + if (revokeAgentMcp) { + this.userMcpAttached.delete(sessionId) + this.userMcpOverridesBySession.delete(sessionId) + } } forgetFeedDebugSession(sessionId) return true @@ -2350,6 +2392,7 @@ export class SessionManager extends EventEmitter { dangerousMode: predecessorInfo.dangerousMode, useProxy: predecessorInfo.useProxy, builtInMcpDomains: effectiveDomains, + userMcpOverrides: this.userMcpOverridesBySession.get(predecessorSessionId) ?? {}, tldrIdentity: this.builtInMcpHost?.sessionTldrIdentity?.(predecessorSessionId), } reservation.restoreOptions = restoreOptions @@ -2721,6 +2764,50 @@ export class SessionManager extends EventEmitter { * The toast would also have claimed "agents started" off a launch that did * not happen. */ + /** + * Decide which user MCP servers this launch gets (#1143). + * + * Never throws: a broken MCP document, an unreadable keyring or a bad server + * must cost the user that server, not the agent. Every server the agent + * asked for but did not get is reported through `user-mcp-unavailable`, so + * the absence is visible instead of looking like an attached server whose + * tools never appear. + */ + private async resolveUserMcpServers( + sessionId: string, + kind: SessionKind, + options: SessionSpawnOptions, + ): Promise { + if (!this.userMcpResolver || !isAgentProviderKind(kind)) return [] + const overrides = normalizeUserMcpOverrides(options.userMcpOverrides) + this.userMcpOverridesBySession.set(sessionId, overrides) + try { + const resolution = await this.userMcpResolver({ provider: kind, overrides, cwd: options.cwd }) + this.userMcpAttached.set(sessionId, resolution.attachedIds) + if (resolution.dropped.length > 0) { + this.journal?.record({ + area: 'mcp.user', + name: 'user_mcp.unavailable', + severity: 'warn', + ids: { sessionId }, + // Names and reasons only; reasons are fixed strings built by the + // service and never contain a secret value. + data: { servers: resolution.dropped.map(server => `${server.name}: ${server.reason}`) }, + }) + this.emit('user-mcp-unavailable', { sessionId, servers: resolution.dropped }) + } + return resolution.servers + } catch (error) { + this.userMcpAttached.set(sessionId, []) + this.journal?.recordError('user_mcp.resolve_failed', error, undefined, { sessionId }) + this.emit('user-mcp-unavailable', { + sessionId, + servers: [{ name: 'MCP servers', reason: 'Your MCP server settings could not be read' }], + }) + return [] + } + } + private reportSkillsUnavailable(sessionId: string, unavailable: readonly ReportingDomain[]): void { if (unavailable.length === 0) return // Pairs with the `.error` rows (same ids.sessionId): "this session is @@ -2886,6 +2973,7 @@ export class SessionManager extends EventEmitter { }) mcpRegistered = true } + const userMcpServers = await this.resolveUserMcpServers(sessionId, kind, options) this.throwIfSpawnCancelled(recoveryClaim, codexReplacementHandoff) if (this.beforeAgentSessionStart) { const unavailableSkills = await this.runPreSpawnSkillReconcile(sessionId, options) @@ -2943,6 +3031,7 @@ export class SessionManager extends EventEmitter { // `openai_base_url`. useProxy: options.useProxy, builtInMcpServers, + userMcpServers, ...(kind === 'codex' && codexReplacementHandoff ? { // WHY the provider receives execution timing, not policy: Codex @@ -3282,6 +3371,7 @@ export class SessionManager extends EventEmitter { return { sessionId, ...(providerSessionId ? { providerSessionId } : {}), + userMcpServerIds: this.userMcpAttached.get(sessionId) ?? [], } } @@ -5459,6 +5549,7 @@ export class SessionManager extends EventEmitter { ? { builtInMcpDomains: this.builtInMcpHost?.sessionDomains?.(sessionId) ?? [], + userMcpServerIds: this.userMcpAttached.get(sessionId) ?? [], tldrIdentity: this.builtInMcpHost?.sessionTldrIdentity?.(sessionId), } : {}), diff --git a/src/main/sessionManager.userMcp.test.ts b/src/main/sessionManager.userMcp.test.ts new file mode 100644 index 000000000..1af9cdb2a --- /dev/null +++ b/src/main/sessionManager.userMcp.test.ts @@ -0,0 +1,100 @@ +import { EventEmitter } from 'node:events' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +import type { ResolvedUserMcpServer, UserMcpDroppedServer } from '@shared/userMcp/types.js' + +// Harness mirrors sessionManager.recover.test.ts: the provider registry is +// mocked so these tests observe exactly what SessionManager hands a provider, +// which is the contract under test (#1143). +const { createSession } = vi.hoisted(() => ({ createSession: vi.fn() })) + +vi.mock('@main/workspaceDirectory.js', () => ({ + MissingWorkspaceDirectoryError: class extends Error {}, + assertWorkspaceDirectoryExists: vi.fn(async () => {}), +})) +vi.mock('@providers/registry.main.js', () => ({ + getMainProvider: () => ({ createSession, deliverPrompt: vi.fn() }), +})) +vi.mock('@main/setup/toolchain.js', () => ({ getToolPath: () => '/usr/bin/true' })) +vi.mock('@main/performance/PerformanceService.js', () => ({ + performanceService: { mark: vi.fn(), record: vi.fn(), error: vi.fn() }, +})) +vi.mock('@main/storage/feedDebugLog.js', () => ({ forgetFeedDebugSession: vi.fn() })) + +class FakeAgentSession extends EventEmitter { + readonly start = vi.fn(async (): Promise => { + this.emit('started', { projectDir: '/tmp/project' }) + }) + readonly stop = vi.fn(async (): Promise => {}) + readonly write = vi.fn() + readonly resize = vi.fn() +} + +const beeper: ResolvedUserMcpServer = { + id: 'srv-beeper', + name: 'beeper', + entry: { type: 'http', url: 'http://localhost:23373/v0/mcp' }, + secrets: {}, +} + +describe('SessionManager user MCP servers', () => { + beforeEach(() => { + createSession.mockReset() + createSession.mockImplementation(() => new FakeAgentSession()) + }) + + it('hands the provider exactly the servers main resolved and reports them on the snapshot', async () => { + const { SessionManager } = await import('./sessionManager') + const manager = new SessionManager() + const resolver = vi.fn(async () => ({ servers: [beeper], attachedIds: [beeper.id], dropped: [] })) + manager.setUserMcpResolver(resolver) + + const result = await manager.spawn({ kind: 'claude', cwd: '/tmp/project', userMcpOverrides: { 'srv-beeper': true, 'bad id!': true } }) + + // The renderer's override map is untrusted: malformed ids never reach the resolver. + expect(resolver).toHaveBeenCalledWith({ provider: 'claude', overrides: { 'srv-beeper': true }, cwd: '/tmp/project' }) + expect(createSession.mock.calls[0]![0].userMcpServers).toEqual([beeper]) + expect(result.userMcpServerIds).toEqual(['srv-beeper']) + expect(manager.getBackendSnapshot(result.sessionId)?.userMcpServerIds).toEqual(['srv-beeper']) + }) + + it('still launches the agent when a server is dropped, and says why', async () => { + const { SessionManager } = await import('./sessionManager') + const manager = new SessionManager() + const dropped: UserMcpDroppedServer[] = [{ name: 'beeper', reason: 'Secret "beeper-authorization" is not set' }] + manager.setUserMcpResolver(async () => ({ servers: [], attachedIds: [], dropped })) + const events: unknown[] = [] + manager.on('user-mcp-unavailable', event => events.push(event)) + + const result = await manager.spawn({ kind: 'codex', cwd: '/tmp/project' }) + + expect(createSession).toHaveBeenCalledTimes(1) + expect(createSession.mock.calls[0]![0].userMcpServers).toEqual([]) + expect(events).toEqual([{ sessionId: result.sessionId, servers: dropped }]) + }) + + it('treats a resolver failure as "no user servers", never as a failed launch', async () => { + const { SessionManager } = await import('./sessionManager') + const manager = new SessionManager() + manager.setUserMcpResolver(async () => { throw new Error('keyring locked') }) + const events: unknown[] = [] + manager.on('user-mcp-unavailable', event => events.push(event)) + + const result = await manager.spawn({ kind: 'claude', cwd: '/tmp/project' }) + + expect(manager.getBackendSnapshot(result.sessionId)?.lifecycle).toBe('live') + expect(result.userMcpServerIds).toEqual([]) + expect(events).toHaveLength(1) + }) + + it('never consults the resolver for terminal sessions', async () => { + const { SessionManager } = await import('./sessionManager') + const manager = new SessionManager() + const resolver = vi.fn() + manager.setUserMcpResolver(resolver) + // Terminal spawn goes through TerminalSession, which this harness does not + // fake; only the absence of a resolver call matters here. + await manager.spawn({ kind: 'terminal', cwd: '/tmp/project' }).catch(() => {}) + expect(resolver).not.toHaveBeenCalled() + }) +}) diff --git a/src/main/sessions/forwarder.ts b/src/main/sessions/forwarder.ts index 1d09578e7..9782b5f1f 100644 --- a/src/main/sessions/forwarder.ts +++ b/src/main/sessions/forwarder.ts @@ -2,6 +2,8 @@ import type { SessionManager } from '@main/sessionManager.js' import { aliasScreenSnapshotForWire } from '@shared/types/session.js' import type { AgentScreenSnapshot } from '@shared/types/session.js' import type { LspManager } from '@main/lspManager.js' +import { USER_MCP_UNAVAILABLE_CHANNEL } from '@main/ipc/userMcp.js' +import type { UserMcpUnavailableEvent } from '@shared/userMcp/types.js' import { MANAGED_SKILLS_UNAVAILABLE_CHANNEL, type ManagedSkillsUnavailableEvent, @@ -192,6 +194,14 @@ export function wireSessionForwarder( manager.on('managed-skills-unavailable', ({ skills }) => { const event: ManagedSkillsUnavailableEvent = { skills } broadcastToWindows(MANAGED_SKILLS_UNAVAILABLE_CHANNEL, event) + }) + // #1143. Broadcast for the same routing reason as managed skills above: the + // launch can be main-initiated (orchestration child, restore) before any + // window has claimed the id. Only server names and fixed reason strings + // cross; nothing here can carry a secret value. + manager.on('user-mcp-unavailable', ({ servers }) => { + const event: UserMcpUnavailableEvent = { servers } + broadcastToWindows(USER_MCP_UNAVAILABLE_CHANNEL, event) }) // Diagnostics are keyed by file, not by session: two windows can have the // same file open in their editors and both need them. diff --git a/src/main/userMcp/nativeServers.ts b/src/main/userMcp/nativeServers.ts new file mode 100644 index 000000000..796110784 --- /dev/null +++ b/src/main/userMcp/nativeServers.ts @@ -0,0 +1,197 @@ +import { access, readFile } from 'node:fs/promises' +import { homedir } from 'node:os' +import { join } from 'node:path' + +import TOML from '@iarna/toml' + +import type { NativeMcpServer, UserMcpInput, UserMcpServerEntry } from '@shared/userMcp/types.js' +import { isPlainObject, isStringArray, isStringRecord, summarizeEntry, transportOf } from '@shared/userMcp/validate.js' + +/** + * Read-only view of the MCP servers each CLI loads from its OWN config + * (spec Revision 2 §7). Agent Code never writes these files; it only reads + * them to (a) show the user servers they would otherwise not know are loaded, + * (b) offer Copy in, and (c) detect Codex name collisions before launch. + * + * Only user-scope files are listed. Project-scope files (`.mcp.json`, + * `.codex/config.toml`) depend on an agent's cwd, which Settings does not have. + */ + +export type NativeMcpPaths = { + home: string + claudeConfigDir?: string + codexHome?: string + platform: NodeJS.Platform +} + +export function defaultNativeMcpPaths(): NativeMcpPaths { + return { + home: homedir(), + claudeConfigDir: process.env.CLAUDE_CONFIG_DIR || undefined, + codexHome: process.env.CODEX_HOME || undefined, + platform: process.platform, + } +} + +// Mirrors Claude's getGlobalClaudeFile (vendor utils/env.ts): the user-scope +// file lives in CLAUDE_CONFIG_DIR when set, otherwise the home directory. +function claudeUserConfigFile(paths: NativeMcpPaths): string { + return join(paths.claudeConfigDir || paths.home, '.claude.json') +} + +function codexHome(paths: NativeMcpPaths): string { + return paths.codexHome || join(paths.home, '.codex') +} + +// Mirrors Claude's getManagedFilePath (vendor utils/settings/managedPath.ts). +function claudeManagedMcpFile(paths: NativeMcpPaths): string { + const dir = paths.platform === 'darwin' + ? '/Library/Application Support/ClaudeCode' + : paths.platform === 'win32' + ? 'C:\\Program Files\\ClaudeCode' + : '/etc/claude-code' + return join(dir, 'managed-mcp.json') +} + +/** + * When an enterprise managed-mcp.json exists, Claude takes exclusive control + * of MCP and rejects any non-SDK `--mcp-config` server by exiting (vendor + * main.tsx "enterprise MCP config"). User servers must then be held back for + * Claude, or every Claude agent would fail to start. + */ +export async function claudeManagedMcpPolicyPresent(paths = defaultNativeMcpPaths()): Promise { + try { + await access(claudeManagedMcpFile(paths)) + return true + } catch { + return false + } +} + +export async function readNativeMcpServers(paths = defaultNativeMcpPaths()): Promise { + const [claude, codex] = await Promise.all([readClaudeNative(paths), readCodexNative(paths)]) + return [...claude, ...codex] +} + +/** + * Names Codex will already have in `mcp_servers` for an agent in `cwd`. + * + * WHY launch refuses these names for Codex: `-c mcp_servers..*` deep-merges + * key by key into an existing table (codex-rs config/src/merge.rs). Our `url` + * merged into a user's stdio table yields a mixed-transport entry that fails + * config load and takes the WHOLE Codex launch down with it. + */ +export async function codexNativeServerNames(cwd: string, paths = defaultNativeMcpPaths()): Promise> { + const names = new Set() + for (const file of [join(codexHome(paths), 'config.toml'), join(cwd, '.codex', 'config.toml')]) { + const table = await readCodexMcpTable(file) + for (const name of Object.keys(table)) names.add(name) + } + return names +} + +async function readClaudeNative(paths: NativeMcpPaths): Promise { + const file = claudeUserConfigFile(paths) + let parsed: unknown + try { + parsed = JSON.parse(await readFile(file, 'utf8')) + } catch { + return [] + } + if (!isPlainObject(parsed) || !isPlainObject(parsed.mcpServers)) return [] + return Object.entries(parsed.mcpServers).map(([name, raw]) => + nativeServer('claude', name, displayPath(file, paths.home), isPlainObject(raw) ? raw : null)) +} + +async function readCodexNative(paths: NativeMcpPaths): Promise { + const file = join(codexHome(paths), 'config.toml') + const table = await readCodexMcpTable(file) + return Object.entries(table).map(([name, raw]) => + nativeServer('codex', name, displayPath(file, paths.home), isPlainObject(raw) ? codexToMcpServersShape(raw) : null)) +} + +async function readCodexMcpTable(file: string): Promise> { + let text: string + try { + text = await readFile(file, 'utf8') + } catch { + return {} + } + try { + const parsed = TOML.parse(text) as Record + return isPlainObject(parsed.mcp_servers) ? parsed.mcp_servers : {} + } catch { + // An unparseable config.toml also fails Codex itself; we have nothing + // useful to add, so it contributes no names rather than blocking launch. + return {} + } +} + +/** Codex table → `mcpServers` entry, so Copy in can reuse the import path. */ +function codexToMcpServersShape(raw: Record): Record { + if (typeof raw.command === 'string') { + const env: Record = isStringRecord(raw.env) ? { ...raw.env } : {} + // `env_vars` names pass through from Codex's own environment; in an + // mcpServers entry that becomes an env value the user must supply. + if (isStringArray(raw.env_vars)) for (const name of raw.env_vars) env[name] ??= '' + return { + command: raw.command, + ...(isStringArray(raw.args) ? { args: raw.args } : {}), + ...(Object.keys(env).length > 0 ? { env } : {}), + ...(typeof raw.cwd === 'string' ? { cwd: raw.cwd } : {}), + } + } + if (typeof raw.url === 'string') { + const headers: Record = isStringRecord(raw.http_headers) ? { ...raw.http_headers } : {} + if (isStringRecord(raw.env_http_headers)) for (const header of Object.keys(raw.env_http_headers)) headers[header] ??= '' + if (typeof raw.bearer_token_env_var === 'string') headers.Authorization ??= 'Bearer ' + return { type: 'http', url: raw.url, ...(Object.keys(headers).length > 0 ? { headers } : {}) } + } + return raw +} + +function nativeServer( + provider: 'claude' | 'codex', + name: string, + source: string, + raw: Record | null, +): NativeMcpServer { + const transport = raw ? transportOf(raw) : null + if (!raw || !transport) { + return { provider, name, source, transport: null, summary: '', entry: null, inputs: [] } + } + const inputs: UserMcpInput[] = [] + const entry: Record = { ...raw } + // Withhold every value: native configs often store tokens in plaintext and + // this object is sent to the renderer. + for (const field of ['env', 'headers'] as const) { + if (!isStringRecord(entry[field])) { + delete entry[field] + continue + } + const next: Record = {} + for (const [key, value] of Object.entries(entry[field] as Record)) { + const id = `${name}-${key}`.toLowerCase().replace(/[^a-z0-9_-]+/g, '-').slice(0, 64) + // The auth scheme word is not secret and keeping it means the user pastes + // just the token. Codex's bearer_token_env_var arrives as "Bearer ". + const scheme = /^(Bearer|Token|Basic)\s/i.exec(value) + next[key] = scheme ? `${scheme[1]} \${input:${id}}` : `\${input:${id}}` + inputs.push({ id, description: `${field === 'env' ? 'Environment variable' : 'Header'} ${key}` }) + } + entry[field] = next + } + if (transport !== 'stdio') entry.type = transport + return { + provider, + name, + source, + transport, + summary: summarizeEntry(raw), + entry: entry as UserMcpServerEntry, + inputs, + } +} + +function displayPath(file: string, home: string): string { + return file.startsWith(home) ? `~${file.slice(home.length)}` : file +} diff --git a/src/main/userMcp/secrets.ts b/src/main/userMcp/secrets.ts new file mode 100644 index 000000000..16de0ca24 --- /dev/null +++ b/src/main/userMcp/secrets.ts @@ -0,0 +1,115 @@ +import { mkdir, readFile, readdir, rename, rm, writeFile } from 'node:fs/promises' +import { join } from 'node:path' + +import type { SecretCodec } from '@main/keyVault/vaultStore.js' +import type { UserMcpSecretState } from '@shared/userMcp/types.js' + +/* + * Encryption uses the API Key Vault's safeStorage codec (the same OS-derived + * key, injectable so tests do not need Electron), but NOT the vault itself. + * + * WHY not store MCP secrets in the vault: the vault gates every read behind + * Touch ID / the login password once per app run. MCP secrets are read at + * agent LAUNCH, which includes automatic restore of a whole workspace at + * startup — an OS prompt before any window is usable, or a restore that + * silently drops every server until the user unlocks something, are both + * worse than the dictation-key precedent (src/main/dictation/apiKeyStore.ts): + * encrypted at rest with an OS-derived key, readable without a prompt. + */ + +/** + * One encrypted blob per secret at `//.bin`. + * + * WHY one file per secret instead of one encrypted document: a blob that stops + * decrypting (Keychain reset, copied profile) then costs exactly one secret — + * shown as "not set" — instead of every server's credentials at once. Server + * ids and input ids are both validated to `[A-Za-z0-9_-]`, so they are safe + * path segments by construction. + */ +export class UserMcpSecretStore { + constructor( + private readonly dir: string, + private readonly codec: SecretCodec, + ) {} + + available(): boolean { + try { + return this.codec.isEncryptionAvailable() + } catch { + return false + } + } + + async get(serverId: string, inputId: string): Promise { + if (!this.available()) return null + let ciphertext: Buffer + try { + ciphertext = await readFile(this.path(serverId, inputId)) + } catch { + return null + } + try { + const value = this.codec.decrypt(ciphertext) + return value === '' ? null : value + } catch { + // Left in place on purpose: if the keyring comes back, so does the value. + return null + } + } + + async set(serverId: string, inputId: string, value: string): Promise { + if (value === '') { + await this.clear(serverId, inputId) + return + } + if (!this.available()) { + throw new Error('Secure storage is not available on this system, so the secret cannot be saved.') + } + const directory = join(this.dir, serverId) + await mkdir(directory, { recursive: true, mode: 0o700 }) + const target = this.path(serverId, inputId) + const temporary = `${target}.${process.pid}.${Date.now()}.tmp` + await writeFile(temporary, this.codec.encrypt(value), { mode: 0o600 }) + await rename(temporary, target) + } + + async clear(serverId: string, inputId: string): Promise { + await rm(this.path(serverId, inputId), { force: true }) + } + + async clearServer(serverId: string): Promise { + await rm(join(this.dir, serverId), { recursive: true, force: true }) + } + + /** Drop blobs for inputs the server no longer defines, so a renamed or + * removed secret does not linger as an orphaned credential on disk. */ + async prune(serverId: string, keepInputIds: readonly string[]): Promise { + let files: string[] + try { + files = await readdir(join(this.dir, serverId)) + } catch { + return + } + const keep = new Set(keepInputIds.map(id => `${id}.bin`)) + await Promise.all(files.filter(file => !keep.has(file)).map(file => + rm(join(this.dir, serverId, file), { force: true }))) + } + + /** Presence and a last-4 hint only. The renderer never receives a value. */ + async state(serverId: string, inputIds: readonly string[]): Promise> { + const entries = await Promise.all(inputIds.map(async id => { + const value = await this.get(serverId, id) + // No hint for short values: the last four characters of a six-character + // PIN are most of the secret. + const state: UserMcpSecretState = value === null + ? { set: false } + : { set: true, ...(value.length >= 12 ? { hint: value.slice(-4) } : {}) } + return [id, state] as const + })) + return Object.fromEntries(entries) + } + + private path(serverId: string, inputId: string): string { + return join(this.dir, serverId, `${inputId}.bin`) + } +} diff --git a/src/main/userMcp/service.test.ts b/src/main/userMcp/service.test.ts new file mode 100644 index 000000000..304c84bd8 --- /dev/null +++ b/src/main/userMcp/service.test.ts @@ -0,0 +1,214 @@ +import { mkdtemp, readFile, readdir, rm, stat, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { afterEach, beforeEach, describe, expect, it } from 'vitest' + +import type { SecretCodec } from '@main/keyVault/vaultStore.js' +import type { NativeMcpServer, UserMcpSaveInput } from '@shared/userMcp/types.js' + +import { readNativeMcpServers, codexNativeServerNames } from './nativeServers.js' +import { UserMcpService } from './service.js' + +// Reversible stand-in for safeStorage: the tests are about WHERE values go, +// not about the OS cipher. The prefix makes an accidental plaintext write +// (a value that skipped encrypt) visible as a mismatch. +const codec: SecretCodec = { + isEncryptionAvailable: () => true, + encrypt: plain => Buffer.from(`enc:${plain}`, 'utf8'), + decrypt: cipher => { + const text = cipher.toString('utf8') + if (!text.startsWith('enc:')) throw new Error('not ours') + return text.slice(4) + }, +} + +const TOKEN = 'bpr_live_9f3a1c7d' + +let dir: string +let native: NativeMcpServer[] +let codexNames: Set +let managed: boolean + +function service(): UserMcpService { + return new UserMcpService({ + stateDir: dir, + codec, + native: { + list: async () => native, + codexNames: async () => codexNames, + claudeManagedPolicy: async () => managed, + }, + }) +} + +const beeper = (overrides: Partial = {}): UserMcpSaveInput => ({ + name: 'beeper', + enabled: true, + providers: { claude: true, codex: true }, + entry: { type: 'http', url: 'http://localhost:23373/v0/mcp', headers: { Authorization: 'Bearer ${input:beeper-authorization}' } }, + inputs: [{ id: 'beeper-authorization', description: 'Header Authorization' }], + secrets: { 'beeper-authorization': TOKEN }, + ...overrides, +}) + +beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), 'user-mcp-')) + native = [] + codexNames = new Set() + managed = false +}) +afterEach(async () => { + await rm(dir, { recursive: true, force: true }) +}) + +describe('UserMcpService storage', () => { + it('persists the server without its secret, and the snapshot shows only a hint', async () => { + const result = await service().save(beeper()) + expect(result.ok).toBe(true) + const onDisk = await readFile(join(dir, 'mcp-servers.json'), 'utf8') + expect(onDisk).toContain('beeper') + expect(onDisk).not.toContain(TOKEN) + expect((await stat(join(dir, 'mcp-servers.json'))).mode & 0o777).toBe(0o600) + if (!result.ok) return + expect(JSON.stringify(result.snapshot)).not.toContain(TOKEN) + expect(result.snapshot.servers[0]!.secrets['beeper-authorization']).toEqual({ set: true, hint: '1c7d' }) + }) + + it('refuses structurally invalid servers but accepts a server whose secret is not set yet', async () => { + const svc = service() + expect(await svc.save(beeper({ name: 'agent_code' }))).toMatchObject({ ok: false }) + const pending = await svc.save(beeper({ secrets: {} })) + expect(pending.ok).toBe(true) + if (!pending.ok) return + expect(pending.snapshot.servers[0]!.problems.map(problem => problem.kind)).toEqual(['secret-missing']) + }) + + it('moves an unreadable document aside instead of erasing it', async () => { + await writeFile(join(dir, 'mcp-servers.json'), '{ not json') + const snapshot = await service().snapshot() + expect(snapshot.servers).toEqual([]) + expect(snapshot.storeProblem).toMatch(/moved to/) + expect((await readdir(dir)).some(file => file.startsWith('mcp-servers.json.corrupt-'))).toBe(true) + }) + + it('deletes a server together with its secrets', async () => { + const svc = service() + const saved = await svc.save(beeper()) + if (!saved.ok) throw new Error(saved.error) + await svc.delete(saved.id!) + expect(await readdir(join(dir, 'mcp-secrets'))).toEqual([]) + }) + + it('copies a CLI-native server in, attached only to the other provider, with secrets unset', async () => { + native = [{ + provider: 'codex', name: 'sentry', source: '~/.codex/config.toml', transport: 'http', summary: 'mcp.sentry.dev/mcp', + entry: { type: 'http', url: 'https://mcp.sentry.dev/mcp', headers: { Authorization: 'Bearer ${input:sentry-authorization}' } }, + inputs: [{ id: 'sentry-authorization', description: 'Header Authorization' }], + }] + const result = await service().copyNative('codex', 'sentry') + if (!result.ok) throw new Error(result.error) + const copy = result.snapshot.servers[0]! + expect(copy.providers).toEqual({ claude: true, codex: false }) + expect(copy.secrets['sentry-authorization']).toEqual({ set: false }) + }) +}) + +describe('UserMcpService.resolveForLaunch', () => { + async function saved(input: UserMcpSaveInput = beeper()) { + const svc = service() + const result = await svc.save(input) + if (!result.ok) throw new Error(result.error) + return { svc, id: result.id! } + } + + it('attaches a default-on server with its secret resolved', async () => { + const { svc, id } = await saved() + const resolution = await svc.resolveForLaunch({ provider: 'codex', overrides: {}, cwd: dir }) + expect(resolution.attachedIds).toEqual([id]) + expect(resolution.servers[0]!.secrets).toEqual({ 'beeper-authorization': TOKEN }) + expect(resolution.dropped).toEqual([]) + }) + + it('lets a per-agent override add or remove a server', async () => { + const { svc, id } = await saved(beeper({ providers: { claude: false, codex: false } })) + expect((await svc.resolveForLaunch({ provider: 'claude', overrides: {}, cwd: dir })).attachedIds).toEqual([]) + expect((await svc.resolveForLaunch({ provider: 'claude', overrides: { [id]: true }, cwd: dir })).attachedIds).toEqual([id]) + const { svc: svc2, id: id2 } = await saved(beeper({ name: 'beeper2' })) + expect((await svc2.resolveForLaunch({ provider: 'claude', overrides: { [id2]: false }, cwd: dir })).attachedIds) + .not.toContain(id2) + }) + + it('never attaches a server whose master switch is off, even with a per-agent on', async () => { + const { svc, id } = await saved(beeper({ enabled: false })) + const resolution = await svc.resolveForLaunch({ provider: 'claude', overrides: { [id]: true }, cwd: dir }) + expect(resolution.attachedIds).toEqual([]) + // Silent: the user turned it off everywhere, so there is nothing to warn about. + expect(resolution.dropped).toEqual([]) + }) + + it('drops a requested server with a reason when its secret is missing', async () => { + const { svc } = await saved(beeper({ secrets: {} })) + const resolution = await svc.resolveForLaunch({ provider: 'claude', overrides: {}, cwd: dir }) + expect(resolution.attachedIds).toEqual([]) + expect(resolution.dropped).toEqual([{ name: 'beeper', reason: 'Secret "beeper-authorization" is not set' }]) + }) + + it('refuses a Codex name that is already in the user\'s Codex config, but not for Claude', async () => { + const { svc, id } = await saved() + codexNames = new Set(['beeper']) + expect((await svc.resolveForLaunch({ provider: 'codex', overrides: {}, cwd: dir })).dropped[0]?.reason) + .toMatch(/already in your Codex config/) + expect((await svc.resolveForLaunch({ provider: 'claude', overrides: {}, cwd: dir })).attachedIds).toEqual([id]) + }) + + it('holds user servers back for Claude under an enterprise MCP policy', async () => { + const { svc } = await saved() + managed = true + const resolution = await svc.resolveForLaunch({ provider: 'claude', overrides: {}, cwd: dir }) + expect(resolution.attachedIds).toEqual([]) + expect(resolution.dropped[0]?.reason).toMatch(/policy/) + }) + + it('keeps SSE servers off Codex with a reason', async () => { + const { svc } = await saved(beeper({ + name: 'linear', entry: { type: 'sse', url: 'https://mcp.linear.app/sse' }, inputs: [], secrets: {}, + })) + const resolution = await svc.resolveForLaunch({ provider: 'codex', overrides: {}, cwd: dir }) + expect(resolution.dropped).toEqual([{ name: 'linear', reason: 'Codex does not support SSE servers' }]) + }) + + it('gives providers without user MCP support nothing', async () => { + const { svc } = await saved() + expect(await svc.resolveForLaunch({ provider: 'opencode', overrides: {}, cwd: dir })) + .toEqual({ servers: [], attachedIds: [], dropped: [] }) + }) +}) + +describe('native server discovery', () => { + it('reads user-scope servers from both CLIs and never forwards their values', async () => { + await writeFile(join(dir, '.claude.json'), JSON.stringify({ + mcpServers: { context7: { type: 'http', url: 'https://mcp.context7.com/mcp', headers: { CONTEXT7_API_KEY: 'ctx7_secret' } } }, + })) + const codexHome = join(dir, 'codex') + await import('node:fs/promises').then(fs => fs.mkdir(codexHome)) + await writeFile(join(codexHome, 'config.toml'), [ + '[mcp_servers.sentry]', + 'url = "https://mcp.sentry.dev/mcp"', + 'bearer_token_env_var = "SENTRY_TOKEN"', + '', + '[mcp_servers.fs]', + 'command = "npx"', + 'args = ["-y", "@modelcontextprotocol/server-filesystem"]', + 'env = { ROOT_TOKEN = "fs_secret" }', + ].join('\n')) + const servers = await readNativeMcpServers({ home: dir, codexHome, platform: 'darwin' }) + expect(servers.map(server => `${server.provider}:${server.name}`)).toEqual(['claude:context7', 'codex:sentry', 'codex:fs']) + expect(JSON.stringify(servers)).not.toMatch(/ctx7_secret|fs_secret/) + expect(servers.find(server => server.name === 'sentry')!.entry).toEqual({ + type: 'http', url: 'https://mcp.sentry.dev/mcp', headers: { Authorization: 'Bearer ${input:sentry-authorization}' }, + }) + expect(await codexNativeServerNames(join(dir, 'project'), { home: dir, codexHome, platform: 'darwin' })) + .toEqual(new Set(['sentry', 'fs'])) + }) +}) diff --git a/src/main/userMcp/service.ts b/src/main/userMcp/service.ts new file mode 100644 index 000000000..eb2c2bc7a --- /dev/null +++ b/src/main/userMcp/service.ts @@ -0,0 +1,361 @@ +import { randomUUID } from 'node:crypto' +import { join } from 'node:path' + +import type { SecretCodec } from '@main/keyVault/vaultStore.js' +import { + addCodexUserMcpLaunchConfig, + claudeUserMcpEntries, + type ResolvedUserMcpServer, +} from '@providers/shared/runtime/userMcpLaunch.js' +import { importUserMcpConfig } from '@shared/userMcp/importConfig.js' +import { + isUserMcpProvider, + type NativeMcpServer, + type UserMcpDocument, + type UserMcpDroppedServer, + type UserMcpImportResult, + type UserMcpMutationResult, + type UserMcpProblem, + type UserMcpProvider, + type UserMcpSaveInput, + type UserMcpServer, + type UserMcpServerView, + type UserMcpSnapshot, +} from '@shared/userMcp/types.js' +import { + coerceInputs, + normalizeEntry, + providerSupport, + referencedInputIds, + summarizeEntry, + transportOf, + validateServer, +} from '@shared/userMcp/validate.js' + +import { + claudeManagedMcpPolicyPresent, + codexNativeServerNames, + readNativeMcpServers, +} from './nativeServers.js' +import { UserMcpSecretStore } from './secrets.js' +import { loadUserMcpDocument, saveUserMcpDocument } from './store.js' + +export type UserMcpLaunchResolution = { + servers: ResolvedUserMcpServer[] + attachedIds: string[] + dropped: UserMcpDroppedServer[] +} + +export type UserMcpServiceDeps = { + stateDir: string + codec: SecretCodec + /** Injectable for tests; production reads the real CLI config files. */ + native?: { + list(): Promise + codexNames(cwd: string): Promise> + claudeManagedPolicy(): Promise + } +} + +/** + * Single owner of user MCP servers (#1143): the document, the secrets, and the + * launch-time decision of what attaches to which agent. + * + * WHY main decides what attaches, not the renderer (spec Revision 2 §4): main + * owns the document and the only copy of the secrets, and a renderer holding a + * stale snapshot must not be able to attach a server the user has since + * deleted or switched off. The renderer contributes only the pane's explicit + * per-agent choices; everything else is read here at launch. + */ +export class UserMcpService { + private document: UserMcpDocument = { version: 1, servers: [] } + private storeProblem: string | undefined + private readonly file: string + private readonly secrets: UserMcpSecretStore + private readonly native: NonNullable + private readonly listeners = new Set<(snapshot: UserMcpSnapshot) => void>() + // Every mutation runs after the previous one settles. Two windows toggling + // at once would otherwise both read-modify-write the same document and the + // later write would silently discard the earlier change. + private tail: Promise = Promise.resolve() + private initialized: Promise | null = null + + constructor(deps: UserMcpServiceDeps) { + this.file = join(deps.stateDir, 'mcp-servers.json') + this.secrets = new UserMcpSecretStore(join(deps.stateDir, 'mcp-secrets'), deps.codec) + this.native = deps.native ?? { + list: () => readNativeMcpServers(), + codexNames: cwd => codexNativeServerNames(cwd), + claudeManagedPolicy: () => claudeManagedMcpPolicyPresent(), + } + } + + initialize(): Promise { + this.initialized ??= (async () => { + const loaded = await loadUserMcpDocument(this.file) + this.document = loaded.document + this.storeProblem = loaded.problem + })() + return this.initialized + } + + onChange(listener: (snapshot: UserMcpSnapshot) => void): () => void { + this.listeners.add(listener) + return () => this.listeners.delete(listener) + } + + async snapshot(): Promise { + await this.initialize() + const [native, claudeManagedPolicy] = await Promise.all([ + this.native.list().catch(() => [] as NativeMcpServer[]), + this.native.claudeManagedPolicy().catch(() => false), + ]) + const servers = await Promise.all(this.document.servers.map(server => this.view(server, claudeManagedPolicy))) + return { + servers, + native, + claudeManagedPolicy, + ...(this.storeProblem ? { storeProblem: this.storeProblem } : {}), + } + } + + importConfig(text: string, fallbackName?: string): UserMcpImportResult { + return importUserMcpConfig(text, fallbackName) + } + + save(input: UserMcpSaveInput): Promise { + return this.mutate(async () => { + const existing = input.id ? this.document.servers.find(server => server.id === input.id) : undefined + if (input.id && !existing) return { ok: false, error: 'That server no longer exists.' } + const server: UserMcpServer = { + id: existing?.id ?? randomUUID(), + name: input.name.trim(), + enabled: input.enabled, + providers: { claude: input.providers.claude === true, codex: input.providers.codex === true }, + entry: normalizeEntry(input.entry), + inputs: coerceInputs(input.inputs), + } + const others = this.document.servers.filter(other => other.id !== server.id) + // Structural problems block the save. A missing secret does not: it is a + // normal intermediate state (paste config now, fetch the token later), + // and launch already refuses to attach the server until it is set. + const problems = validateServer(server, others) + if (problems.length > 0) return { ok: false, error: problems[0]!.message, problems } + for (const [inputId, value] of Object.entries(input.secrets ?? {})) { + if (server.inputs.some(candidate => candidate.id === inputId)) { + await this.secrets.set(server.id, inputId, value) + } + } + await this.secrets.prune(server.id, server.inputs.map(candidate => candidate.id)) + this.document = { + version: 1, + servers: existing + ? this.document.servers.map(candidate => candidate.id === server.id ? server : candidate) + : [...this.document.servers, server], + } + await this.persist() + return { ok: true, id: server.id } + }) + } + + delete(id: string): Promise { + return this.mutate(async () => { + if (!this.document.servers.some(server => server.id === id)) return { ok: false, error: 'That server no longer exists.' } + this.document = { version: 1, servers: this.document.servers.filter(server => server.id !== id) } + await this.persist() + await this.secrets.clearServer(id) + return { ok: true } + }) + } + + setEnabled(id: string, enabled: boolean): Promise { + return this.update(id, server => ({ ...server, enabled })) + } + + setProvider(id: string, provider: UserMcpProvider, enabled: boolean): Promise { + return this.update(id, server => ({ ...server, providers: { ...server.providers, [provider]: enabled } })) + } + + setSecret(id: string, inputId: string, value: string): Promise { + return this.mutate(async () => { + const server = this.document.servers.find(candidate => candidate.id === id) + if (!server) return { ok: false, error: 'That server no longer exists.' } + if (!server.inputs.some(input => input.id === inputId)) return { ok: false, error: `No secret named "${inputId}".` } + await this.secrets.set(id, inputId, value) + return { ok: true } + }) + } + + /** + * Copy a CLI-native server into Agent Code. + * + * The copy starts attached only to the OTHER provider. WHY: the source CLI + * keeps loading its own entry, so attaching the copy there too duplicates it + * — and for Codex, a same-name launch entry is refused at launch (see + * codexNativeServerNames). Sharing a server the user set up in one CLI with + * the other is the main reason to copy it in. + */ + copyNative(provider: UserMcpProvider, name: string): Promise { + return this.mutate(async () => { + const native = (await this.native.list()).find(server => server.provider === provider && server.name === name) + if (!native?.entry) return { ok: false, error: 'That server can no longer be read from its config file.' } + const others = this.document.servers + const server: UserMcpServer = { + id: randomUUID(), + name: native.name, + enabled: true, + providers: { claude: provider !== 'claude', codex: provider !== 'codex' }, + entry: normalizeEntry(native.entry), + inputs: native.inputs, + } + const problems = validateServer(server, others) + if (problems.length > 0) return { ok: false, error: problems[0]!.message, problems } + this.document = { version: 1, servers: [...others, server] } + await this.persist() + return { ok: true, id: server.id } + }) + } + + /** + * Decide and materialize the user servers for one agent launch. + * + * `overrides` are the pane's explicit per-agent choices (bare server ids). + * Order of rules, and why: + * 1. master switch off → skip silently. The user turned it off everywhere, + * so there is nothing to warn about, even for a per-agent "on". + * 2. not requested (no override and provider default off) → skip silently. + * 3. requested but unusable (invalid, unsupported transport, enterprise + * policy, native name collision, missing secret, translator refusal) → + * drop WITH a reason. The user asked for it, so silence would read as + * "it's attached" while the agent has no such tools. + * A dropped server never fails the launch. + */ + async resolveForLaunch(params: { + provider: string + overrides: Readonly> + cwd: string + }): Promise { + await this.initialize() + const empty: UserMcpLaunchResolution = { servers: [], attachedIds: [], dropped: [] } + if (!isUserMcpProvider(params.provider)) return empty + const provider = params.provider + const requested = this.document.servers.filter(server => + server.enabled && (params.overrides[server.id] ?? server.providers[provider])) + if (requested.length === 0) return empty + + const dropped: UserMcpDroppedServer[] = [] + const candidates: ResolvedUserMcpServer[] = [] + const claudeManaged = provider === 'claude' && await this.native.claudeManagedPolicy().catch(() => false) + const codexNames = provider === 'codex' + ? await this.native.codexNames(params.cwd).catch(() => new Set()) + : new Set() + for (const server of requested) { + const others = this.document.servers.filter(other => other.id !== server.id) + const problem = validateServer(server, others)[0] + if (problem) { + dropped.push({ name: server.name, reason: problem.message }) + continue + } + const support = providerSupport(transportOf(server.entry))[provider] + if (!support.ok) { + dropped.push({ name: server.name, reason: support.reason }) + continue + } + if (claudeManaged) { + dropped.push({ name: server.name, reason: "Your organization's Claude MCP policy only allows its own servers" }) + continue + } + if (codexNames.has(server.name)) { + dropped.push({ name: server.name, reason: 'A server with this name is already in your Codex config.toml' }) + continue + } + const secrets: Record = {} + let missing: string | null = null + for (const inputId of referencedInputIds(server.entry)) { + const value = await this.secrets.get(server.id, inputId) + if (value === null) { + missing = inputId + break + } + secrets[inputId] = value + } + if (missing) { + dropped.push({ name: server.name, reason: `Secret "${missing}" is not set` }) + continue + } + candidates.push({ id: server.id, name: server.name, entry: server.entry, secrets }) + } + + // Dry-run the provider translator here, where drops can be reported, so + // the provider session never has to silently omit a server it was handed. + // Both translators are pure and deterministic over the same input. + const translatorDrops = provider === 'claude' + ? claudeUserMcpEntries(candidates).dropped + : addCodexUserMcpLaunchConfig(candidates, [], {}) + const refused = new Set(translatorDrops.map(server => server.name)) + dropped.push(...translatorDrops) + const servers = candidates.filter(server => !refused.has(server.name)) + return { servers, attachedIds: servers.map(server => server.id), dropped } + } + + private update(id: string, change: (server: UserMcpServer) => UserMcpServer): Promise { + return this.mutate(async () => { + const server = this.document.servers.find(candidate => candidate.id === id) + if (!server) return { ok: false, error: 'That server no longer exists.' } + this.document = { + version: 1, + servers: this.document.servers.map(candidate => candidate.id === id ? change(candidate) : candidate), + } + await this.persist() + return { ok: true } + }) + } + + private mutate( + operation: () => Promise<{ ok: true; id?: string } | { ok: false; error: string; problems?: UserMcpProblem[] }>, + ): Promise { + const run = this.tail.then(async (): Promise => { + await this.initialize() + try { + const outcome = await operation() + if (!outcome.ok) return outcome + const snapshot = await this.snapshot() + for (const listener of this.listeners) listener(snapshot) + return { ok: true, snapshot, ...(outcome.id ? { id: outcome.id } : {}) } + } catch (error) { + return { ok: false, error: error instanceof Error ? error.message : String(error) } + } + }) + this.tail = run.catch(() => {}) + return run + } + + private async persist(): Promise { + await saveUserMcpDocument(this.file, this.document) + // A successful write supersedes whatever made the old file unreadable. + this.storeProblem = undefined + } + + private async view(server: UserMcpServer, claudeManagedPolicy: boolean): Promise { + const transport = transportOf(server.entry) + const others = this.document.servers.filter(other => other.id !== server.id) + const secrets = await this.secrets.state(server.id, server.inputs.map(input => input.id)) + const problems = validateServer(server, others) + for (const inputId of referencedInputIds(server.entry)) { + if (secrets[inputId] && !secrets[inputId]!.set) { + problems.push({ kind: 'secret-missing', message: `Secret "${inputId}" is not set` }) + } + } + const support = providerSupport(transport) + return { + ...server, + transport, + summary: summarizeEntry(server.entry), + secrets, + problems, + support: claudeManagedPolicy + ? { ...support, claude: { ok: false, reason: "Your organization's Claude MCP policy only allows its own servers" } } + : support, + } + } +} diff --git a/src/main/userMcp/store.ts b/src/main/userMcp/store.ts new file mode 100644 index 000000000..f16c0448e --- /dev/null +++ b/src/main/userMcp/store.ts @@ -0,0 +1,52 @@ +import { mkdir, readFile, rename, writeFile } from 'node:fs/promises' +import { dirname } from 'node:path' + +import type { UserMcpDocument } from '@shared/userMcp/types.js' +import { coerceUserMcpDocument } from '@shared/userMcp/validate.js' + +export type LoadedUserMcpDocument = { + document: UserMcpDocument + /** Human-readable notice when the file existed but could not be used. */ + problem?: string +} + +/** + * Read `mcp-servers.json`. + * + * WHY a corrupt file is moved aside instead of overwritten or left in place: + * leaving it would make every later save either fail or clobber it, and + * silently resetting would erase every server the user configured. Renaming + * it keeps the bytes recoverable (`mcp-servers.json.corrupt-