-
Notifications
You must be signed in to change notification settings - Fork 60
feat(install-mcp): register a local-scope mcp server so the vscode extension connects #523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -78,6 +78,26 @@ class InstallResult: | |
| # from ``skipped`` so "already installed" and "install failed" never look | ||
| # the same to the caller / CLI. | ||
| failed: list[str] = field(default_factory=list) | ||
| # user-scope MCP servers registered outside the target tree (in the host's | ||
| # user config, e.g. ~/.claude.json). Reported separately because the write | ||
| # lands outside `target` and, unlike the tier files, is a config the host | ||
| # engine reads on launch rather than a file the user edits. | ||
| registered: list[str] = field(default_factory=list) | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class _UserMcp: | ||
| """A local-scope MCP registration declared by an adapter's manifest. | ||
|
|
||
| Written to ``<home>/<config>`` (e.g. ``~/.claude.json``). ``scope: project`` | ||
| keys the server under ``projects[<abs target>].mcpServers``; ``scope: user`` | ||
| puts it in the top-level ``mcpServers``. Idempotent: an existing server of | ||
| the same ``name`` is never overwritten. | ||
| """ | ||
| config: str # path relative to the user's home dir | ||
| scope: str # "project" | "user" | ||
| name: str # server key | ||
| spec: dict[str, Any] # the server definition (command/args/env/…) | ||
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
|
|
@@ -96,6 +116,7 @@ class _Manifest: | |
| tiers: dict[str, list[_FileEntry]] | ||
| fence_begin: str = _DEFAULT_FENCE_BEGIN | ||
| fence_end: str = _DEFAULT_FENCE_END | ||
| user_mcp: _UserMcp | None = None | ||
| # false for hosts whose install target is a staging dir for user-global | ||
| # config (claude-desktop) rather than project wiring — a KB bootstrapped | ||
| # at the target would land wherever the user happened to be standing. | ||
|
|
@@ -205,10 +226,40 @@ def _flag(name: str, raw: Any = raw, tier_name: str = tier_name) -> bool: | |
| tiers=parsed, | ||
| fence_begin=fence_begin, | ||
| fence_end=fence_end, | ||
| user_mcp=_parse_user_mcp(host, data.get("user_mcp")), | ||
| kb_bootstrap=kb_bootstrap, | ||
| ) | ||
|
|
||
|
|
||
| def _parse_user_mcp(host: str, raw: Any) -> _UserMcp | None: | ||
| """Parse an optional ``user_mcp:`` manifest block, or return None.""" | ||
| if raw is None: | ||
| return None | ||
| if not isinstance(raw, dict): | ||
| raise AdapterError(f"{host}: install.yaml `user_mcp:` must be a mapping") | ||
| config = raw.get("config") | ||
| if not isinstance(config, str) or not config.strip(): | ||
| raise AdapterError(f"{host}: install.yaml user_mcp needs a non-empty `config`") | ||
| # `config` names a file in the user's home dir; a `..`/absolute value would | ||
| # let a manifest scribble anywhere on disk. Keep it a plain relative name. | ||
| if config != Path(config).name or config in {"", ".", ".."}: | ||
| raise AdapterError( | ||
| f"{host}: install.yaml user_mcp `config` must be a bare filename, got {config!r}" | ||
| ) | ||
| scope = raw.get("scope", "project") | ||
| if scope not in {"project", "user"}: | ||
| raise AdapterError( | ||
| f"{host}: install.yaml user_mcp `scope` must be 'project' or 'user', got {scope!r}" | ||
| ) | ||
| name = raw.get("name") | ||
| if not isinstance(name, str) or not name.strip(): | ||
| raise AdapterError(f"{host}: install.yaml user_mcp needs a non-empty `name`") | ||
| spec = raw.get("spec") | ||
| if not isinstance(spec, dict): | ||
| raise AdapterError(f"{host}: install.yaml user_mcp `spec` must be a mapping") | ||
| return _UserMcp(config=config, scope=scope, name=name, spec=spec) | ||
|
|
||
|
|
||
| def wants_kb_bootstrap(host: str) -> bool: | ||
| """Whether ``install-mcp`` may bootstrap a KB at this host's target. | ||
|
|
||
|
|
@@ -232,12 +283,26 @@ def available_adapters() -> list[str]: | |
| return sorted(out) | ||
|
|
||
|
|
||
| def install(adapter: str, *, target: Path, tier: str = "T4") -> InstallResult: | ||
| def install( | ||
| adapter: str, | ||
| *, | ||
| target: Path, | ||
| tier: str = "T4", | ||
| approve: bool = True, | ||
| home: Path | None = None, | ||
| ) -> InstallResult: | ||
| """Install ``adapter``'s templates under ``target`` up to ``tier``. | ||
|
|
||
| The call is idempotent: rerunning against a previously-installed tree | ||
| produces an :class:`InstallResult` with everything in ``skipped`` and | ||
| nothing in ``written`` / ``appended``. | ||
|
|
||
| When the adapter manifest declares a ``user_mcp:`` block and ``approve`` is | ||
| true, a local-scope MCP server is registered in the host's user config | ||
| (``home``/``<config>``, default ``~/.claude.json``). That is what makes a | ||
| fresh install connect in the Claude Code *extension*, whose engine ignores | ||
| an unapproved project ``.mcp.json``. Pass ``approve=False`` to skip it. | ||
| ``home`` overrides the user-config directory (tests point it at a tmp dir). | ||
| """ | ||
| if tier not in _TIER_ORDER: | ||
| raise AdapterError( | ||
|
|
@@ -302,9 +367,71 @@ def install(adapter: str, *, target: Path, tier: str = "T4") -> InstallResult: | |
| shutil.copy2(src, dst) | ||
| result.written.append(entry.dst) | ||
|
|
||
| if approve and manifest.user_mcp is not None: | ||
| _register_user_mcp(manifest.user_mcp, target=target, home=home, result=result) | ||
|
|
||
| return result | ||
|
|
||
|
|
||
| def _register_user_mcp( | ||
| reg: _UserMcp, *, target: Path, home: Path | None, result: InstallResult | ||
| ) -> None: | ||
| """Merge a local-scope MCP server into the host user config (idempotent). | ||
|
|
||
| Mirrors ``claude mcp add``: writes ``<home>/<reg.config>`` with the server | ||
| under ``projects[<abs target>].mcpServers`` (scope=project) or the top-level | ||
| ``mcpServers`` (scope=user). Every other key in the file is preserved and | ||
| the write is atomic. Failures are recorded (``result.failed``), never | ||
| raised — a config we can't safely rewrite must not abort a working install. | ||
| """ | ||
| cfg_path = (home or Path.home()) / reg.config | ||
| label = f"{reg.config}:{reg.name} (mcp)" | ||
| try: | ||
| if cfg_path.exists(): | ||
| raw = cfg_path.read_text(encoding="utf-8") | ||
| data = json.loads(raw) if raw.strip() else {} | ||
| if not isinstance(data, dict): | ||
| result.failed.append(label) | ||
| return | ||
| else: | ||
| data = {} | ||
|
|
||
| if reg.scope == "project": | ||
| projects = data.setdefault("projects", {}) | ||
| if not isinstance(projects, dict): | ||
| result.failed.append(label) | ||
| return | ||
| container = projects.setdefault(str(target), {}) | ||
| else: | ||
| container = data | ||
| if not isinstance(container, dict): | ||
| result.failed.append(label) | ||
| return | ||
| servers = container.setdefault("mcpServers", {}) | ||
| if not isinstance(servers, dict): | ||
| result.failed.append(label) | ||
| return | ||
|
|
||
| if reg.name in servers: | ||
| # already registered — never clobber the user's own edits, and stay | ||
| # out of `result.skipped` (a file-path channel; this is a config | ||
| # entry, not a file). The CLI reminds the user to reload regardless. | ||
| return | ||
| servers[reg.name] = dict(reg.spec) | ||
|
|
||
| cfg_path.parent.mkdir(parents=True, exist_ok=True) | ||
| tmp = cfg_path.with_name(cfg_path.name + ".vouch-tmp") | ||
| tmp.write_text(json.dumps(data, indent=2) + "\n", encoding="utf-8") | ||
| if cfg_path.exists(): | ||
| shutil.copymode(cfg_path, tmp) | ||
| tmp.replace(cfg_path) | ||
|
Comment on lines
+422
to
+427
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
rg -n -C3 'NamedTemporaryFile|mkstemp|copymode|\.vouch-tmp' src testsRepository: vouchdev/vouch Length of output: 1437 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '390,460p' src/vouch/install_adapter.py
printf '\n---\n'
sed -n '1,80p' src/vouch/migrations/rewriter.pyRepository: vouchdev/vouch Length of output: 5474 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -C4 'json\.dumps\(data|servers\[reg\.name\]|cfg_path|install-mcp|registered' src/vouch/install_adapter.pyRepository: vouchdev/vouch Length of output: 4211 🏁 Script executed: #!/bin/bash
set -euo pipefail
sed -n '420,455p' src/vouch/install_adapter.pyRepository: vouchdev/vouch Length of output: 1720 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n 'tmp\.unlink|unlink\(|replace\(cfg_path\)|except \(OSError, ValueError\)' src/vouch/install_adapter.pyRepository: vouchdev/vouch Length of output: 224 use an exclusive tempfile here. 🧰 Tools🪛 ast-grep (0.44.1)[info] 423-423: use jsonify instead of json.dumps for JSON output (use-jsonify) 🤖 Prompt for AI Agents |
||
| result.registered.append(label) | ||
| except (OSError, ValueError): | ||
| # malformed json, unreadable/unwritable file — surface as failed, but | ||
| # the tier files are already installed, so don't unwind the whole run. | ||
| result.failed.append(label) | ||
|
|
||
|
|
||
| def _install_fenced( | ||
| src: Path, | ||
| dst: Path, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
distinguish an identical registration from a name conflict.
the installer currently emits the same empty result for both states, causing the cli to claim success when a user-owned server blocks vouch.
src/vouch/install_adapter.py#L415-L419: compare the existing specification; preserve exact matches, but report differing entries as conflicts or failures.src/vouch/cli.py#L4050-L4055: show the registered reminder only for an exact existing match, and surface conflicts explicitly.📍 Affects 2 files
src/vouch/install_adapter.py#L415-L419(this comment)src/vouch/cli.py#L4050-L4055🤖 Prompt for AI Agents