diff --git a/.agents/skills/testing-mcp-with-cli-agents/SKILL.md b/.agents/skills/testing-mcp-with-cli-agents/SKILL.md index 5a59556c..ab0ca476 100644 --- a/.agents/skills/testing-mcp-with-cli-agents/SKILL.md +++ b/.agents/skills/testing-mcp-with-cli-agents/SKILL.md @@ -2,7 +2,8 @@ name: testing-mcp-with-cli-agents description: >- Test an MCP server by driving real CLI agents (Claude, Codex, Cursor, Gemini, - Grok, agy) against it, using isolated tmux sockets and send-keys instead of + Grok, agy, opencode) against it, using isolated tmux sockets and send-keys + instead of trusting unit tests alone. Use this whenever verifying MCP-server behavior end-to-end, checking that a local branch or checkout works across installed agent CLIs, comparing trunk-vs-branch MCP behavior, driving an interactive @@ -195,7 +196,17 @@ transcripts), and the **ground-truth socket state** after the run. ## Wiring a checkout into the CLIs: mcp_swap `scripts/mcp_swap.py` rewrites each CLI's config to `uv --directory run -` and preserves existing env on replacement: +` and preserves existing env on replacement. It covers eight CLIs; the +two newest are not yet driven through this harness, so +`references/cli-matrix.md` has no verified row for them: + +- **opencode** — `$XDG_CONFIG_HOME/opencode/opencode.jsonc`. JSONC, so + comments survive a swap; the entry packs argv into one `command` array + under a top-level `mcp` key, and its env table is spelled `environment`. + A scalar `command` there is a decode error that stops opencode starting. +- **pi** — `~/.pi/agent/mcp.json`. pi ships no MCP client of its own; that + file is read by the third-party `pi-mcp-adapter` extension, so a swap + does nothing until it is installed. `detect` reports this. ```console $ uv run scripts/mcp_swap.py detect # which CLIs are present diff --git a/.agents/skills/testing-mcp-with-cli-agents/references/cli-matrix.md b/.agents/skills/testing-mcp-with-cli-agents/references/cli-matrix.md index 2c29da9d..b4763dfd 100644 --- a/.agents/skills/testing-mcp-with-cli-agents/references/cli-matrix.md +++ b/.agents/skills/testing-mcp-with-cli-agents/references/cli-matrix.md @@ -75,9 +75,28 @@ below. | gemini | `gemini -p` | project `.gemini/settings.json` from cwd | `gemini mcp list` | `--approval-mode yolo` (`--skip-trust`) | no — `IneligibleTierError`, CLI unsupported for individuals | | grok | `grok -p` / `--single` | `GROK_HOME` **or** `mcp add --scope project` | `grok mcp doctor tmux --json` (real handshake) | `--permission-mode bypassPermissions` | yes | | agy | `agy -p` | hidden `--gemini_dir ` (**credentials do not follow it — copy the token in**) | none short of a model call | `--dangerously-skip-permissions` | yes | +| opencode | not yet verified | not yet verified | not yet verified | not yet verified | not yet driven through this harness | +| pi | n/a — no MCP client (see below) | n/a | n/a | n/a | n/a | ## Per-CLI detail +### opencode and pi — registered in mcp_swap, not yet driven here + +`mcp_swap` writes both, but neither has been taken through the tmux harness, so +the row above is blank rather than guessed. What is known from the source: + +- **opencode** stores MCP servers under a top-level `mcp` key in + `$XDG_CONFIG_HOME/opencode/opencode.jsonc`, as + `{"type": "local", "command": [argv...], "environment": {...}}`. `command` is + one array, not a command/args pair, and the env table is `environment` — an + `env` key is dropped in silence, while a scalar `command` fails the whole + config's decode and stops opencode starting. `opencode mcp add -- ` + is non-interactive once both a name and a `--` command are given. +- **pi** has no MCP client at all: its README says "No MCP", and the released + build contains no MCP code. `~/.pi/agent/mcp.json` is a convention of the + third-party `pi-mcp-adapter` extension. Until that package is installed, + nothing reads what a swap writes, and there is no agent behavior to drive. + ### codex — two isolation styles, both verified - **Config-less (leanest):** a home dir containing only a **copy** of the real `auth.json`, no `config.toml`, plus `-c` overrides: diff --git a/CHANGES b/CHANGES index 388e8d70..b40708cb 100644 --- a/CHANGES +++ b/CHANGES @@ -6,6 +6,59 @@ _Notes on upcoming releases will be added here_ +### Documentation + +#### opencode joins the install picker + +The install widget gains an opencode panel. `opencode mcp add tmux -- ` +is non-interactive once a name and a `--` command are both given, so it is a CLI +panel rather than a paste-this-JSON one — which also sidesteps opencode's +unusual entry shape. + +### Development + +**`mcp_swap.py` covers opencode and pi** + +`use-local`, `status`, `revert`, `doctor` and `detect` now reach two more agent +CLIs. + +opencode is the first config the script edits that is not plain JSON or TOML. +Its `$XDG_CONFIG_HOME/opencode/opencode.jsonc` is JSONC, its server map hangs +off a top-level `mcp` key rather than `mcpServers`, and one entry packs argv +into a single `command` array with its environment table spelled +`environment`. Getting the shape wrong is not a soft failure there: a scalar +`command` is a decode error that stops opencode starting, and an `env` key is +dropped without a word. Comments survive a swap, including one written directly +above the `command` it explains. + +pi ships no MCP client — its README says so outright, and the released build +contains no MCP code. `~/.pi/agent/mcp.json` is read by the third-party +`pi-mcp-adapter` extension, so a swap written there takes effect only once that +package is installed. `detect` says so rather than reporting a swap that cannot +do anything. + +**JSONC is edited rather than reserialized** + +The JSON writer rebuilds the whole document, which for a commented file would +mean deleting every comment in it. JSONC values now come from stdlib `json` +after comments and trailing commas are blanked in place, and writes are applied +as text splices, so every byte outside a replaced value is untouched. The +obvious dependency was measured and rejected: `json-five` round-trips comments, +but raises on the valid JSON string `"C:\\x"` and silently decodes a literal +`\u0041` to `"A"`. + +**Per-CLI behavior is declared, not branched** + +`CLIInfo` gained `container` (the key path to the server map) and `dialect` (the +entry shape) alongside `fmt`. The four `cli in (...)` membership tuples that +`get_server`, `set_server`, `delete_server` and `_all_server_specs` each carried +are gone. Two of them ended in a bare `else` that fell through to the TOML key, +so a CLI registered but forgotten in one tuple reported "no entry" instead of +failing; the other two raised `AssertionError`, which the caller did not catch. +A non-mapping at a container key now raises a `RuntimeError` naming the path for +every CLI, not just Claude. `scripts/README.md`'s extension guide described +three branch sites when there were four; it now describes the fields instead. + ## libtmux-mcp 0.1.0a20 (2026-08-09) libtmux-mcp 0.1.0a20 changes no tool behavior. `scripts/mcp_swap.py` gains `use-local --pr N`, which points installed agent CLIs at a pull request without a checkout and verifies the server before it rewrites any configuration, and its edits now preserve unrelated config text, file permissions, and symlink targets. ruff's curated default rule set is enabled behind a `ruff>=0.16.0` floor, taking the project from 351 enabled rules to 565 and fixing what that surfaced, and the CI workflow actions move to their current majors. In the documentation, the dataclass identifying an MCP caller describes each of its fields instead of reaching the API reference as "Alias for field number 0". diff --git a/docs/_ext/widgets/mcp_install.py b/docs/_ext/widgets/mcp_install.py index 39e6e411..88b8a867 100644 --- a/docs/_ext/widgets/mcp_install.py +++ b/docs/_ext/widgets/mcp_install.py @@ -170,6 +170,18 @@ class Panel: ), ) +#: User scope only: `opencode mcp add` writes the global config whether or +#: not a project one exists, so a Project panel would advertise a file the +#: command it prints never touches. +_OPENCODE_SCOPES: tuple[Scope, ...] = ( + Scope( + id="user", + label="User", + config_file="~/.config/opencode/opencode.jsonc", + note=None, + ), +) + _GROK_SCOPES: tuple[Scope, ...] = ( Scope( id="user", @@ -238,6 +250,12 @@ class Panel: kind="json", scopes=_ANTIGRAVITY_SCOPES, ), + Client( + id="opencode", + label="opencode", + kind="cli", + scopes=_OPENCODE_SCOPES, + ), ) @@ -377,6 +395,14 @@ def _cli_body(client: Client, scope: Scope, method: Method, cooldown: Cooldown) # ``--`` is handed to the server process verbatim. if client.id == "grok": return f"grok mcp add --scope {scope.id} tmux -- {tool_cmd}" + # opencode: ``opencode mcp add -- `` is non-interactive as + # soon as a name and a ``--`` command are both given, and it writes + # whichever config file is in scope for the current directory. The + # stored entry packs argv into a single ``command`` array rather than + # the command/args pair the JSON-kind clients use, which is why this + # is a CLI panel and not a paste-this-JSON one. + if client.id == "opencode": + return f"opencode mcp add tmux -- {tool_cmd}" # codex: CLI doesn't write project scope; the project-scope panel # uses the TOML body path (see ``_body_for``). return f"codex mcp add tmux -- {tool_cmd}" diff --git a/justfile b/justfile index 396845b4..a0cec91d 100644 --- a/justfile +++ b/justfile @@ -119,7 +119,7 @@ watch-mypy: format-markdown: prettier --parser=markdown -w *.md docs/*.md docs/**/*.md CHANGES -# Detect which CLI agents (claude/codex/cursor/gemini) exist on this machine +# Detect which agent CLIs exist on this machine [group: 'mcp'] mcp-detect: uv run scripts/mcp_swap.py detect diff --git a/scripts/README.md b/scripts/README.md index 1588f3c0..21f07969 100644 --- a/scripts/README.md +++ b/scripts/README.md @@ -131,14 +131,18 @@ the user-level fallback; the project entry stays. `revert` without ### Scope -Covers four CLIs and their canonical **global** config paths: - -| CLI | Config | Format | -|--------|-------------------------------|--------| -| Claude | `~/.claude.json` | JSON (per-project keying) | -| Codex | `~/.codex/config.toml` | TOML (format-preserving via `tomlkit`) | -| Cursor | `~/.cursor/mcp.json` | JSON | -| Gemini | `~/.gemini/settings.json` | JSON | +Covers eight CLIs and their canonical **global** config paths: + +| CLI | Config | Format | +|-----|--------|--------| +| Claude | `~/.claude.json` | JSON (per-project keying) | +| Codex | `~/.codex/config.toml` | TOML (format-preserving via `tomlkit`) | +| Cursor | `~/.cursor/mcp.json` | JSON | +| Gemini | `~/.gemini/settings.json` | JSON | +| Grok | `~/.grok/config.toml` | TOML (same shape as Codex) | +| agy | `~/.gemini/config/mcp_config.json` | JSON | +| opencode | `$XDG_CONFIG_HOME/opencode/opencode.jsonc` | JSONC (comments preserved) | +| pi | `~/.pi/agent/mcp.json` | JSONC (read by `pi-mcp-adapter`, not by pi) | Claude's config is keyed per-project under the repo's absolute path — the script writes only under the current repo's key, leaving other projects' @@ -146,11 +150,22 @@ entries untouched. #### Out of scope (use the CLI's native command) -- **Workspace / project-local configs** for Cursor and Gemini - (`$PWD/.cursor/mcp.json`, `$PWD/.gemini/settings.json`). When - workspace precedence matters, use `cursor mcp add` / `gemini mcp add` - directly — workspace files take precedence over the global ones this - script writes. +- **Workspace / project-local configs** for Cursor, Gemini and opencode + (`$PWD/.cursor/mcp.json`, `$PWD/.gemini/settings.json`, + `$PWD/opencode.json`). When workspace precedence matters, use + `cursor mcp add` / `gemini mcp add` directly — workspace files take + precedence over the global ones this script writes. opencode has no + non-interactive project-scope add (`opencode mcp add` writes the global + file), so edit `$PWD/opencode.json` by hand. +- **opencode's sibling global files.** opencode merges `config.json`, + `opencode.json` and `opencode.jsonc` from the same directory, with + `.jsonc` winning. This script writes `.jsonc`, so its entry is the one + that takes effect, but a stale `mcp.` in a sibling + `opencode.json` merges underneath rather than being shadowed. +- **pi without `pi-mcp-adapter`.** pi ships no MCP client. The file this + script writes is read by that third-party extension, so until it is + installed the swap has no effect — `detect` reports this rather than + claiming otherwise. - **Custom binary install locations.** Detection is `shutil.which` plus the file existing at the configured global path. Homebrew, npm prefixes (`~/.npm-global/bin`), and the canonical local-install @@ -159,7 +174,27 @@ entries untouched. ### Extending to a new CLI -Add an entry to the `CLIS` table in `mcp_swap.py` and extend the three -per-CLI branches in `get_server` / `set_server` / `delete_server`. Tests -in `tests/test_mcp_swap.py` use a `fake_home` fixture that monkeypatches -`CLIS`, so the extension pattern is already established. +Add an entry to the `CLIS` table in `mcp_swap.py`. Each `CLIInfo` carries +the three things that vary per CLI, so no `get_server` / `set_server` / +`delete_server` / `_all_server_specs` branch needs touching: + +- `fmt` — `json`, `jsonc` or `toml`, selecting the reader and writer +- `container` — the key path to the server map, e.g. `("mcpServers",)` + or `("mcp",)` +- `dialect` — the shape of one entry: `standard` (scalar `command`, + sibling `args`, optional `env`), `claude` (adds `type` and always + writes `env`), or `opencode` (one `command` array, env under + `environment`) + +Add the name to `CLIName` and `ALL_CLIS` too — a CLI in `CLIS` but not +`ALL_CLIS` has its state entries dropped on load, so `revert` forgets the +swap and leaves the config rewritten. + +A dialect no existing CLI speaks needs a branch in +`McpServerSpec.to_entry_dict` and its mirror in `_spec_from_entry`; that +mirror is what keeps `is_local_uv_directory`, `local_repo_path` and +`pr_ref` working, and those drive the "already local" short-circuit. + +Tests in `tests/test_mcp_swap.py` use a `fake_home` fixture that +monkeypatches `CLIS` wholesale, so every new CLI must be added there as +well — `test_fake_home_covers_every_registered_cli` enforces it. diff --git a/scripts/mcp_swap.py b/scripts/mcp_swap.py index 6139b10b..78428a08 100755 --- a/scripts/mcp_swap.py +++ b/scripts/mcp_swap.py @@ -3,7 +3,7 @@ # requires-python = ">=3.10" # dependencies = ["tomlkit>=0.13"] # /// -"""Swap MCP server configs across Claude / Codex / Cursor / Gemini / Grok / agy. +"""Swap MCP server configs across every installed agent CLI. Use when you want every installed agent CLI to run a local checkout of an MCP server (editable) instead of a pinned release. ``use-local`` rewrites @@ -37,16 +37,35 @@ - **Global configs only.** Writes to ``~/.cursor/mcp.json``, ``~/.claude.json``, ``~/.codex/config.toml``, ``~/.gemini/settings.json``, ``~/.grok/config.toml`` (TOML - ``mcp_servers``, same shape as Codex), and + ``mcp_servers``, same shape as Codex), ``~/.gemini/config/mcp_config.json`` (agy / Antigravity CLI, JSON ``mcpServers`` — the shared-config file the CLI reads, sibling to the - ``config.json`` it loads at startup). Workspace / project-local configs - (``$PWD/.cursor/mcp.json``, ``$PWD/.gemini/settings.json``, - per-project ``projects..mcpServers`` entries inside - ``~/.claude.json`` *are* recognised for Claude only) are NOT - walked — workspace files for Cursor/Gemini are silently ignored. + ``config.json`` it loads at startup), + ``$XDG_CONFIG_HOME/opencode/opencode.jsonc`` (JSONC ``mcp``, comments + preserved) and ``~/.pi/agent/mcp.json`` (JSONC too -- the adapter that + reads it strips comments). Workspace / project-local + configs (``$PWD/.cursor/mcp.json``, ``$PWD/.gemini/settings.json``, + ``$PWD/opencode.json``, per-project ``projects..mcpServers`` + entries inside ``~/.claude.json`` *are* recognised for Claude only) + are NOT walked — workspace files for the others are silently ignored. When workspace precedence matters, run the CLI's own - ``cursor mcp add ...`` / ``gemini mcp add ...`` directly. + ``cursor mcp add ...`` / ``gemini mcp add ...`` directly. opencode has + no non-interactive project-scope add -- ``opencode mcp add`` writes the + global file -- so edit ``$PWD/opencode.json`` by hand for that. + +- **opencode reads three global files.** ``config.json``, + ``opencode.json`` and ``opencode.jsonc`` in the same directory are all + loaded and merged, with ``.jsonc`` winning. This script owns + ``.jsonc`` — the file opencode itself writes to — so its entry is the + one that takes effect. A stale ``mcp.`` left in a sibling + ``opencode.json`` still merges underneath rather than being shadowed + outright; remove it by hand if that matters. + +- **pi has no MCP client of its own.** Its README says so, and the + released build ships no MCP code. ``~/.pi/agent/mcp.json`` is read by + the third-party ``pi-mcp-adapter`` extension, so a swap written there + takes effect only once that package is installed. ``detect`` says as + much rather than reporting a swap that cannot do anything. - **Claude scope.** ``use-local`` and ``revert`` accept ``--scope {user,project}``. The default ``project`` writes the @@ -55,8 +74,8 @@ pre-flag behaviour. ``--scope user`` writes Claude's top-level ``mcpServers`` fallback so every project that has no per-project override picks up the swap; useful when QA-ing a branch across - many directories. Codex, Cursor, Gemini, Grok, and agy have no per-project - layer in their config files; the flag is silently coerced to + many directories. Every other CLI here has no per-project layer in + the config file this script writes; the flag is silently coerced to ``user`` for them. Both Claude scopes can coexist with independent backups; full ``revert`` unwinds in LIFO order. - **Simple binary detection.** Probing is ``shutil.which()`` @@ -92,8 +111,23 @@ import tomlkit import tomlkit.items -CLIName = t.Literal["claude", "codex", "cursor", "gemini", "grok", "agy"] -ALL_CLIS: tuple[CLIName, ...] = ("claude", "codex", "cursor", "gemini", "grok", "agy") +CLIName = t.Literal[ + "claude", "codex", "cursor", "gemini", "grok", "agy", "opencode", "pi" +] +ALL_CLIS: tuple[CLIName, ...] = ( + "claude", + "codex", + "cursor", + "gemini", + "grok", + "agy", + "opencode", + "pi", +) + +#: Width of the CLI-name column in ``detect`` output, derived rather +#: than hardcoded so adding a longer name cannot silently misalign it. +_CLI_COLUMN = max(len(name) for name in ALL_CLIS) + 1 #: Claude config scope: ``"user"`` targets the user/system-level top-level #: ``mcpServers`` fallback that applies to every project without its own @@ -192,6 +226,18 @@ def _xdg_state_home() -> pathlib.Path: # --------------------------------------------------------------------------- +#: Per-entry shape a CLI expects under its server map. ``standard`` is +#: the Claude-Desktop lineage every CLI here started from — scalar +#: ``command``, sibling ``args`` list, optional ``env`` table. +#: ``claude`` is that shape plus an explicit ``type``/``env`` that +#: Claude writes even when empty. ``opencode`` packs argv into a single +#: ``command`` array and spells the environment table ``environment``. +#: Dialects exist because the shape is not implied by the file format: +#: two CLIs sharing ``fmt="json"`` can still disagree about how one +#: entry is spelled. +Dialect = t.Literal["standard", "claude", "opencode"] + + @dataclasses.dataclass(frozen=True) class CLIInfo: """Static descriptor for a CLI's config file and discovery heuristics.""" @@ -199,7 +245,28 @@ class CLIInfo: name: CLIName binary: str config_path: pathlib.Path - fmt: t.Literal["json", "toml"] + fmt: t.Literal["json", "jsonc", "toml"] + #: Key path from the document root down to the mapping of server + #: name -> entry. A path rather than a single key so a CLI that + #: nests deeper needs no new branch in the four functions that + #: read, write, delete and enumerate entries. + container: tuple[str, ...] + #: Entry shape written and read back for this CLI. + dialect: Dialect + + +def _xdg_config_home() -> pathlib.Path: + """``$XDG_CONFIG_HOME`` when absolute, else ``~/.config``. + + The spec requires these variables to be absolute and says to ignore + them otherwise. A relative value would resolve against the working + directory, so the swap would record a backup path that revert could + no longer find from anywhere else. + """ + raw = os.environ.get("XDG_CONFIG_HOME") + if raw and pathlib.Path(raw).is_absolute(): + return pathlib.Path(raw) + return pathlib.Path.home() / ".config" CLIS: dict[CLIName, CLIInfo] = { @@ -208,39 +275,92 @@ class CLIInfo: binary="claude", config_path=pathlib.Path.home() / ".claude.json", fmt="json", + container=("mcpServers",), + dialect="claude", ), "codex": CLIInfo( name="codex", binary="codex", config_path=pathlib.Path.home() / ".codex" / "config.toml", fmt="toml", + container=("mcp_servers",), + dialect="standard", ), "cursor": CLIInfo( name="cursor", binary="cursor-agent", config_path=pathlib.Path.home() / ".cursor" / "mcp.json", fmt="json", + container=("mcpServers",), + dialect="standard", ), "gemini": CLIInfo( name="gemini", binary="gemini", config_path=pathlib.Path.home() / ".gemini" / "settings.json", fmt="json", + container=("mcpServers",), + dialect="standard", ), "grok": CLIInfo( name="grok", binary="grok", config_path=pathlib.Path.home() / ".grok" / "config.toml", fmt="toml", + container=("mcp_servers",), + dialect="standard", ), "agy": CLIInfo( name="agy", binary="agy", config_path=(pathlib.Path.home() / ".gemini" / "config" / "mcp_config.json"), fmt="json", + container=("mcpServers",), + dialect="standard", + ), + "opencode": CLIInfo( + name="opencode", + binary="opencode", + # opencode reads config.json, opencode.json and opencode.jsonc from + # this directory and merges all three, with .jsonc winning. It writes + # to the first that exists, defaulting to .jsonc — so that is the one + # file a swap can own without being shadowed. + config_path=_xdg_config_home() / "opencode" / "opencode.jsonc", + fmt="jsonc", + container=("mcp",), + dialect="opencode", + ), + "pi": CLIInfo( + name="pi", + binary="pi", + # Read by the pi-mcp-adapter extension, not by pi itself; see + # PI_ADAPTER_DIR. Claude-Desktop schema, so the standard dialect. + # The adapter parses through strip-json-comments with trailing + # commas allowed, so the file is JSONC despite the .json suffix. + config_path=pathlib.Path.home() / ".pi" / "agent" / "mcp.json", + fmt="jsonc", + container=("mcpServers",), + dialect="standard", ), } +#: Written into an opencode config this script creates from nothing. +#: opencode injects the same line itself on first load; seeding it here +#: keeps the swap from being followed by a surprise rewrite. +OPENCODE_SCHEMA_URL = "https://opencode.ai/config.json" + +#: pi ships no MCP client — its README says "No MCP" outright, and the +#: released build contains no MCP code at all. MCP reaches pi only +#: through the third-party ``pi-mcp-adapter`` extension, which is what +#: reads ``~/.pi/agent/mcp.json``. The swap writes that file because it +#: is the one pi-family location with a settled schema, but until the +#: adapter is installed pi does not read it, so ``detect`` says so +#: instead of reporting a swap that cannot take effect. +PI_ADAPTER_DIR = ( + pathlib.Path.home() / ".pi" / "agent" / "npm" / "node_modules" / "pi-mcp-adapter" +) +PI_ADAPTER_HINT = "needs the pi-mcp-adapter package; pi has no built-in MCP client" + #: A ``--from`` argument pointing at a pull request's head commit. #: GitHub publishes ``refs/pull//head`` on the *base* repository, so @@ -256,17 +376,28 @@ class McpServerSpec: args: list[str] = dataclasses.field(default_factory=list) env: dict[str, str] = dataclasses.field(default_factory=dict) - def to_json_dict(self, *, include_stdio_type: bool = False) -> dict[str, t.Any]: - """Serialize to the JSON shape (Claude-extended when ``include_stdio_type``).""" - # Claude's format always includes ``type`` and ``env`` (even when empty); - # Cursor/Gemini omit both. include_stdio_type selects Claude shape. - if include_stdio_type: + def to_entry_dict(self, dialect: Dialect = "standard") -> dict[str, t.Any]: + """Serialize to the entry shape ``dialect`` expects.""" + # Claude's format always includes ``type`` and ``env`` (even when + # empty); the standard shape omits both when there is nothing to say. + if dialect == "claude": return { "type": "stdio", "command": self.command, "args": list(self.args), "env": dict(self.env), } + if dialect == "opencode": + # One array for argv, and the table is "environment" -- an + # "env" key here is dropped in silence, and a scalar command + # is a decode error that takes the whole config down with it. + local: dict[str, t.Any] = { + "type": "local", + "command": [self.command, *self.args], + } + if self.env: + local["environment"] = dict(self.env) + return local out: dict[str, t.Any] = {"command": self.command, "args": list(self.args)} if self.env: out["env"] = dict(self.env) @@ -331,18 +462,359 @@ class SwapStateError(RuntimeError): """Swap state is unsafe to use for a mutating operation.""" +# --------------------------------------------------------------------------- +# JSONC — comments and trailing commas, edited without reserializing +# --------------------------------------------------------------------------- +# +# tomlkit gives TOML a format-preserving round trip; JSONC has no +# equivalent on PyPI that is safe to depend on here. ``json-five`` was +# measured first and rejected: it raises on ``"C:\\x"`` and silently +# decodes the literal six characters ``\u0041`` to ``"A"`` — both valid +# JSON that stdlib reads correctly, and the second is exactly the silent +# rewrite this script exists to avoid. +# +# So values come from stdlib ``json`` (correct escape semantics) and +# edits are applied as text splices located by an offset-preserving +# scanner. Every byte outside a replaced value survives untouched, which +# is the same technique opencode's own config writer uses via +# ``jsonc-parser``'s ``modify()``. + +_JSON_WS = " \t\n\r" + +#: Longest inline rendering of a scalar list before it is broken across +#: lines. A swapped ``command`` array is the common case and reads +#: better on one line, which is how these configs are written by hand. +_INLINE_WIDTH = 88 + + +def _jsonc_blank_comments(text: str) -> str: + """Replace comment bytes with spaces, preserving every offset. + + Scanning rather than matching a regex is the whole point: ``//`` + inside a URL and ``/*`` inside a Windows path are string content, not + comments, and only a scanner that tracks string state can tell them + apart. Offsets are preserved so a span found in the blanked text + addresses the same bytes in the original. + """ + out = list(text) + i, n = 0, len(text) + in_string = False + while i < n: + char = text[i] + if in_string: + if char == "\\": + i += 2 + continue + if char == '"': + in_string = False + i += 1 + elif char == '"': + in_string = True + i += 1 + elif char == "/" and i + 1 < n and text[i + 1] == "/": + while i < n and text[i] != "\n": + out[i] = " " + i += 1 + elif char == "/" and i + 1 < n and text[i + 1] == "*": + end = text.find("*/", i + 2) + end = n if end == -1 else end + 2 + for j in range(i, end): + if out[j] != "\n": + out[j] = " " + i = end + else: + i += 1 + return "".join(out) + + +def _jsonc_blank_trailing_commas(blanked: str) -> str: + """Blank trailing commas so stdlib :func:`json.loads` accepts the text.""" + out = list(blanked) + i, n = 0, len(blanked) + in_string = False + last_comma = -1 + while i < n: + char = blanked[i] + if in_string: + if char == "\\": + i += 2 + continue + if char == '"': + in_string = False + i += 1 + continue + if char == '"': + in_string = True + last_comma = -1 + elif char == ",": + last_comma = i + elif char in "}]": + if last_comma != -1: + out[last_comma] = " " + last_comma = -1 + elif char not in _JSON_WS: + last_comma = -1 + i += 1 + return "".join(out) + + +def _jsonc_loads(text: str) -> t.Any: + """Parse JSONC text into plain Python objects.""" + if not text.strip(): + return {} + return json.loads(_jsonc_blank_trailing_commas(_jsonc_blank_comments(text))) + + +class _JsoncScanner: + """Locate value spans inside comment-blanked JSON text.""" + + def __init__(self, text: str) -> None: + self.text = text + self.pos = 0 + + def skip_ws(self) -> None: + """Advance past insignificant whitespace.""" + while self.pos < len(self.text) and self.text[self.pos] in _JSON_WS: + self.pos += 1 + + def read_string(self) -> str: + """Consume one string token and return its raw text, quotes included.""" + start = self.pos + self.pos += 1 + while self.pos < len(self.text): + char = self.text[self.pos] + if char == "\\": + self.pos += 2 + continue + self.pos += 1 + if char == '"': + break + return self.text[start : self.pos] + + def read_value(self) -> tuple[int, int]: + """Consume one value and return its ``(start, end)`` span.""" + self.skip_ws() + start = self.pos + char = self.text[self.pos] + if char == '"': + self.read_string() + elif char in "{[": + self._read_container() + else: + while ( + self.pos < len(self.text) + and self.text[self.pos] not in ",}]" + and self.text[self.pos] not in _JSON_WS + ): + self.pos += 1 + return start, self.pos + + def _read_container(self) -> None: + self.pos += 1 + depth = 1 + while self.pos < len(self.text) and depth: + char = self.text[self.pos] + if char == '"': + self.read_string() + continue + if char in "{[": + depth += 1 + elif char in "}]": + depth -= 1 + self.pos += 1 + + def read_members(self, obj_start: int) -> list[_JsoncMember]: + """Enumerate an object's members. ``obj_start`` indexes its ``{``.""" + self.pos = obj_start + 1 + found: list[_JsoncMember] = [] + while True: + self.skip_ws() + if self.pos >= len(self.text) or self.text[self.pos] == "}": + return found + if self.text[self.pos] == ",": + self.pos += 1 + continue + member_start = self.pos + raw_key = self.read_string() + self.skip_ws() + self.pos += 1 # the ':' + value_start, value_end = self.read_value() + found.append( + _JsoncMember( + key=json.loads(raw_key), + start=member_start, + end=value_end, + value_start=value_start, + value_end=value_end, + ) + ) + + +class _JsoncMember(t.NamedTuple): + """One ``"key": value`` pair located inside a JSONC document. + + Attributes + ---------- + key : str + The decoded member name. + start : int + Offset of the opening quote of the key. + end : int + Offset just past the value — the end of the whole member. + value_start : int + Offset of the first byte of the value. + value_end : int + Offset just past the last byte of the value. + """ + + key: str + start: int + end: int + value_start: int + value_end: int + + +def _jsonc_render(value: t.Any, depth: int, *, ensure_ascii: bool) -> str: + """Render ``value`` as JSON text indented for nesting ``depth``.""" + pad = " " * depth + if isinstance(value, list) and all( + isinstance(item, (str, int, float, bool)) or item is None for item in value + ): + inline = json.dumps(value, ensure_ascii=ensure_ascii) + if len(inline) + len(pad) <= _INLINE_WIDTH: + return inline + return json.dumps(value, indent=2, ensure_ascii=ensure_ascii).replace( + "\n", "\n" + pad + ) + + +def _jsonc_object_span(blanked: str, path: tuple[str, ...]) -> tuple[int, int] | None: + """Return the span of the object reached by ``path``, or ``None``.""" + scanner = _JsoncScanner(blanked) + scanner.skip_ws() + if scanner.pos >= len(blanked) or blanked[scanner.pos] != "{": + return None + cursor = scanner.pos + for key in path: + match = next( + (m for m in _JsoncScanner(blanked).read_members(cursor) if m.key == key), + None, + ) + if match is None or blanked[match.value_start] != "{": + return None + cursor = match.value_start + tail = _JsoncScanner(blanked) + tail.pos = cursor + return tail.read_value() + + +def _jsonc_next_edit( + text: str, + data: t.Mapping[str, t.Any], + path: tuple[str, ...], + *, + ensure_ascii: bool, +) -> tuple[int, int, str] | None: + """Find the one next splice that brings ``path`` closer to ``data``.""" + blanked = _jsonc_blank_comments(text) + span = _jsonc_object_span(blanked, path) + if span is None: + return None + obj_start, obj_end = span + members = _JsoncScanner(blanked).read_members(obj_start) + by_key = {member.key: member for member in members} + depth = len(path) + 1 + pad = " " * depth + + for key, value in data.items(): + member = by_key.get(key) + if member is None: + body = _jsonc_render(value, depth, ensure_ascii=ensure_ascii) + # Escape the key like any other value: written raw, a backslash + # or quote in a server name emits text that cannot be parsed + # back, so the member is never found and the merge re-inserts + # it until the pass ceiling, holding the swap lock throughout. + name = json.dumps(key, ensure_ascii=ensure_ascii) + if members: + tail = members[-1].end + return tail, tail, f",\n{pad}{name}: {body}" + if blanked[obj_start + 1 : obj_end - 1].strip(): + return None + # Blanking hid any comment the object holds, so measure the + # interior in the original text and splice after it, not over it. + interior = text[obj_start + 1 : obj_end - 1] + anchor = obj_start + 1 + len(interior.rstrip()) + closing = " " * (depth - 1) + return anchor, obj_end - 1, f"\n{pad}{name}: {body}\n{closing}" + current = json.loads( + _jsonc_blank_trailing_commas(blanked[member.value_start : member.value_end]) + ) + if isinstance(value, dict) and isinstance(current, dict): + nested = _jsonc_next_edit( + text, value, (*path, key), ensure_ascii=ensure_ascii + ) + if nested is not None: + return nested + elif current != value: + return ( + member.value_start, + member.value_end, + _jsonc_render(value, depth, ensure_ascii=ensure_ascii), + ) + + for index, member in enumerate(members): + if member.key in data: + continue + # Exactly one delimiter leaves with the member: the comma before + # it, or, for the first member which has none, the comma after. + if index: + return members[index - 1].end, member.end, "" + # Read that comma out of the blanked text -- one inside a comment + # is not a delimiter, and a real one behind a comment still is. + trailing = blanked[member.end : obj_end] + drop_to = member.end + if trailing.lstrip(_JSON_WS).startswith(","): + drop_to += trailing.index(",") + 1 + return obj_start + 1, drop_to, "" + return None + + +def _jsonc_merge(text: str, data: t.Mapping[str, t.Any], *, ensure_ascii: bool) -> str: + """Reconcile ``data`` into ``text``, rewriting only members that differ. + + Applies one splice at a time and rescans, so offsets are always + computed against current text rather than patched up after the fact. + Config files are small enough that the extra passes do not matter and + the invariant is worth far more than the cycles. + """ + if not text.strip(): + return json.dumps(dict(data), indent=2, ensure_ascii=ensure_ascii) + "\n" + # One splice per member, plus slack; a config that needs more than + # this has a pathology worth surfacing rather than looping on. + for _ in range(10_000): + edit = _jsonc_next_edit(text, data, (), ensure_ascii=ensure_ascii) + if edit is None: + return text + start, end, replacement = edit + text = text[:start] + replacement + text[end:] + msg = "JSONC merge did not converge" + raise RuntimeError(msg) + + # --------------------------------------------------------------------------- # Config IO — per format # --------------------------------------------------------------------------- def load_config(info: CLIInfo) -> t.Any: - """Parse a CLI's config file (JSON or TOML) into an editable structure. + """Parse a CLI's config file (JSON, JSONC or TOML) into an editable structure. Empty JSON files are treated as empty objects so first-run MCP configs can be seeded with their initial server entry. """ raw = info.config_path.read_bytes() + if info.fmt == "jsonc": + return _jsonc_loads(raw.decode()) if info.fmt == "json": text = raw.decode().strip() return json.loads(text) if text else {} @@ -373,8 +845,20 @@ def dump_config_bytes(info: CLIInfo, config: t.Any, *, original: bytes) -> bytes which is the defect this parameter exists to prevent. tomlkit preserves those conventions itself; only the JSON writer needs it. """ - if info.fmt != "json": + # Dispatched on the exact format rather than "not json": a third + # format reaching the TOML writer by fall-through would silently + # write TOML bytes into a JSON file. + if info.fmt == "toml": return tomlkit.dumps(config).encode() + if info.fmt == "jsonc": + # The merge derives its output from the original text, so the + # file's own trailing-newline convention carries over untouched + # and needs no _json_trailer fixup. + source = original.decode() + try: + return _jsonc_merge(source, config, ensure_ascii=False).encode() + except UnicodeEncodeError: + return _jsonc_merge(source, config, ensure_ascii=True).encode() trailer = _json_trailer(original) # ensure_ascii would re-escape every non-ASCII character in the file, # including config text the swap never read. @@ -560,6 +1044,72 @@ def _claude_user_servers( return existing +@t.overload +def _server_map( + info: CLIInfo, config: t.Any, *, create: t.Literal[True] +) -> dict[str, t.Any]: ... + + +@t.overload +def _server_map( + info: CLIInfo, config: t.Any, *, create: t.Literal[False] +) -> dict[str, t.Any] | None: ... + + +def _server_map( + info: CLIInfo, config: t.Any, *, create: bool +) -> dict[str, t.Any] | None: + """Walk ``info.container`` to the mapping holding this CLI's entries. + + Returns ``None`` when the path is absent and ``create`` is false. + Intermediate levels are created on demand so a nested container needs + no special case; TOML gets tomlkit tables so the written document + keeps its formatting. + + Raises + ------ + RuntimeError + A key along the path holds something other than a mapping. + Reported rather than overwritten — a swap must never discard + config it cannot interpret. + """ + node: dict[str, t.Any] = config + for depth, key in enumerate(info.container): + child = node.get(key) + if child is None: + if not create: + return None + child = tomlkit.table() if info.fmt == "toml" else {} + node[key] = child + elif not isinstance(child, dict): + path = ".".join(info.container[: depth + 1]) + msg = ( + f"{info.config_path}: {path} is a {type(child).__name__}, " + f"expected a table of server entries" + ) + raise RuntimeError(msg) + node = child + return node + + +def _as_toml_table(entry: dict[str, t.Any]) -> tomlkit.items.Table: + """Render one entry dict as a tomlkit table. + + Nested mappings (``env``) become sub-tables so the written document + keeps TOML's own structure instead of an inline dict literal. + """ + table = tomlkit.table() + for key, value in entry.items(): + if isinstance(value, dict): + sub = tomlkit.table() + for sub_key, sub_value in value.items(): + sub[sub_key] = sub_value + table[key] = sub + else: + table[key] = value + return table + + def get_server( cli: CLIName, config: t.Any, @@ -583,13 +1133,12 @@ def get_server( if not node: return None entry = node.get("mcpServers", {}).get(name) - elif cli in ("cursor", "gemini", "agy"): - entry = config.get("mcpServers", {}).get(name) - else: # cli in ("codex", "grok") - entry = config.get("mcp_servers", {}).get(name) + else: + servers = _server_map(CLIS[cli], config, create=False) + entry = servers.get(name) if servers else None if entry is None: return None - return _spec_from_entry(entry, fmt=CLIS[cli].fmt) + return _spec_from_entry(entry, info=CLIS[cli]) def set_server( @@ -613,37 +1162,23 @@ def set_server( if scope == "user": servers = _claude_user_servers(config, create=True) had = name in servers - servers[name] = spec.to_json_dict(include_stdio_type=True) + servers[name] = spec.to_entry_dict("claude") return "replaced" if had else "added" node = _claude_project_node(config, repo, create=True) servers = node.setdefault("mcpServers", {}) had = name in servers - servers[name] = spec.to_json_dict(include_stdio_type=True) - return "replaced" if had else "added" - if cli in ("cursor", "gemini", "agy"): - servers = config.setdefault("mcpServers", {}) - had = name in servers - servers[name] = spec.to_json_dict() - return "replaced" if had else "added" - if cli in ("codex", "grok"): - # tomlkit: top-level tables are accessed via dict protocol too. - mcp_servers = config.get("mcp_servers") - if mcp_servers is None: - mcp_servers = tomlkit.table() - config["mcp_servers"] = mcp_servers - had = name in mcp_servers - table = tomlkit.table() - table["command"] = spec.command - table["args"] = list(spec.args) - if spec.env: - env_tbl = tomlkit.table() - for k, v in spec.env.items(): - env_tbl[k] = v - table["env"] = env_tbl - mcp_servers[name] = table + servers[name] = spec.to_entry_dict("claude") return "replaced" if had else "added" - msg = f"unreachable: unknown CLI {cli!r}" - raise AssertionError(msg) + info = CLIS[cli] + if info.dialect == "opencode" and not config: + # Seeding from nothing: opencode rewrites the file on load to add + # this line, so writing it now avoids an immediate second edit. + config["$schema"] = OPENCODE_SCHEMA_URL + servers = _server_map(info, config, create=True) + had = name in servers + entry = spec.to_entry_dict(info.dialect) + servers[name] = _as_toml_table(entry) if info.fmt == "toml" else entry + return "replaced" if had else "added" def delete_server( @@ -671,33 +1206,45 @@ def delete_server( return False servers = node.get("mcpServers", {}) return servers.pop(name, None) is not None - if cli in ("cursor", "gemini", "agy"): - return config.get("mcpServers", {}).pop(name, None) is not None - if cli in ("codex", "grok"): - mcp_servers = config.get("mcp_servers") - if mcp_servers is None: - return False - if name in mcp_servers: - del mcp_servers[name] - return True + servers = _server_map(CLIS[cli], config, create=False) + if servers is None or name not in servers: return False - msg = f"unreachable: unknown CLI {cli!r}" - raise AssertionError(msg) + del servers[name] + return True -def _spec_from_entry(entry: t.Any, *, fmt: t.Literal["json", "toml"]) -> McpServerSpec: - """Convert a raw config entry (dict or tomlkit Table) into an McpServerSpec.""" +def _spec_from_entry(entry: t.Any, *, info: CLIInfo) -> McpServerSpec: + """Convert a raw config entry (dict or tomlkit Table) into an McpServerSpec. + + Every dialect is normalised down to the portable scalar-command + shape, so the helpers that reason about a spec — + :meth:`McpServerSpec.is_local_uv_directory`, :meth:`McpServerSpec.pr_ref`, + ``_points_at`` — stay dialect-agnostic. Skipping this is not a + cosmetic loss: an unsplit array command makes the "already local, no + change" check miss, and every run rewrites a config it did not need + to touch. + """ # tomlkit items quack like dicts/lists; coerce to plain Python for our spec. - if fmt == "toml": + if info.fmt == "toml": entry = ( tomlkit.items.Table.unwrap(entry) if isinstance(entry, tomlkit.items.Table) else dict(entry) ) - command = str(entry.get("command", "")) - raw_args = entry.get("args", []) - args = [str(a) for a in raw_args] if raw_args else [] - raw_env = entry.get("env") or {} + if info.dialect == "opencode": + raw_command = entry.get("command", []) + argv = ( + [str(part) for part in raw_command] + if isinstance(raw_command, (list, tuple)) + else [str(raw_command)] + ) + command, args = (argv[0], argv[1:]) if argv else ("", []) + raw_env = entry.get("environment") or {} + else: + command = str(entry.get("command", "")) + raw_args = entry.get("args", []) + args = [str(a) for a in raw_args] if raw_args else [] + raw_env = entry.get("env") or {} env = {str(k): str(v) for k, v in dict(raw_env).items()} return McpServerSpec(command=command, args=args, env=env) @@ -1042,8 +1589,10 @@ def cmd_detect(args: argparse.Namespace) -> int: extra.append("binary missing") if not p.config_found: extra.append(f"config missing: {CLIS[p.cli].config_path}") + if p.cli == "pi" and not PI_ADAPTER_DIR.is_dir(): + extra.append(PI_ADAPTER_HINT) suffix = f" ({', '.join(extra)})" if extra else "" - print(f" [{flag}] {p.cli:<7}{suffix}") + print(f" [{flag}] {p.cli:<{_CLI_COLUMN}}{suffix}") return 0 @@ -1555,17 +2104,15 @@ def _add(raw: t.Any) -> None: for name, entry in raw.items(): if not isinstance(entry, dict): continue - out[str(name)] = _spec_from_entry(entry, fmt=CLIS[cli].fmt) + out[str(name)] = _spec_from_entry(entry, info=CLIS[cli]) if cli == "claude": _add(_claude_user_servers(config, create=False)) node = _claude_project_node(config, repo, create=False) if node: _add(node.get("mcpServers")) - elif cli in ("cursor", "gemini", "agy"): - _add(config.get("mcpServers")) - else: # codex, grok - _add(config.get("mcp_servers")) + else: + _add(_server_map(CLIS[cli], config, create=False)) return out diff --git a/tests/test_mcp_swap.py b/tests/test_mcp_swap.py index 5291f636..bfac89d0 100644 --- a/tests/test_mcp_swap.py +++ b/tests/test_mcp_swap.py @@ -48,36 +48,64 @@ def fake_home(tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch) -> pathli binary="claude", config_path=tmp_path / ".claude.json", fmt="json", + container=("mcpServers",), + dialect="claude", ), "codex": mcp_swap.CLIInfo( name="codex", binary="codex", config_path=tmp_path / ".codex" / "config.toml", fmt="toml", + container=("mcp_servers",), + dialect="standard", ), "cursor": mcp_swap.CLIInfo( name="cursor", binary="cursor-agent", config_path=tmp_path / ".cursor" / "mcp.json", fmt="json", + container=("mcpServers",), + dialect="standard", ), "gemini": mcp_swap.CLIInfo( name="gemini", binary="gemini", config_path=tmp_path / ".gemini" / "settings.json", fmt="json", + container=("mcpServers",), + dialect="standard", ), "grok": mcp_swap.CLIInfo( name="grok", binary="grok", config_path=tmp_path / ".grok" / "config.toml", fmt="toml", + container=("mcp_servers",), + dialect="standard", ), "agy": mcp_swap.CLIInfo( name="agy", binary="agy", config_path=tmp_path / ".gemini" / "config" / "mcp_config.json", fmt="json", + container=("mcpServers",), + dialect="standard", + ), + "opencode": mcp_swap.CLIInfo( + name="opencode", + binary="opencode", + config_path=tmp_path / ".config" / "opencode" / "opencode.jsonc", + fmt="jsonc", + container=("mcp",), + dialect="opencode", + ), + "pi": mcp_swap.CLIInfo( + name="pi", + binary="pi", + config_path=tmp_path / ".pi" / "agent" / "mcp.json", + fmt="jsonc", + container=("mcpServers",), + dialect="standard", ), }, ) @@ -213,7 +241,14 @@ def test_load_config_tolerates_empty_json(tmp_path: pathlib.Path) -> None: """An empty JSON config can be seeded with the first MCP server entry.""" cfg = tmp_path / "mcp_config.json" cfg.write_text("") - info = mcp_swap.CLIInfo(name="agy", binary="agy", config_path=cfg, fmt="json") + info = mcp_swap.CLIInfo( + name="agy", + binary="agy", + config_path=cfg, + fmt="json", + container=("mcpServers",), + dialect="standard", + ) assert mcp_swap.load_config(info) == {} @@ -2296,7 +2331,12 @@ def _json_config(tmp_path: pathlib.Path, body: str) -> tuple[t.Any, bytes]: raw = body.encode() path.write_bytes(raw) info = mcp_swap.CLIInfo( - name="cursor", binary="cursor-agent", config_path=path, fmt="json" + name="cursor", + binary="cursor-agent", + config_path=path, + fmt="json", + container=("mcpServers",), + dialect="standard", ) return info, raw @@ -2944,3 +2984,602 @@ def test_revert_uses_the_original_target_when_a_config_link_is_replaced( assert mcp_swap.cmd_revert(parser.parse_args(["revert", "--cli", "cursor"])) == 0 assert original_target.read_bytes() == original assert replacement_path.read_bytes() == replacement + + +# --------------------------------------------------------------------------- +# opencode and pi +# +# These two exercise axes the first six never did. opencode is the first +# JSONC config, the first container key that is not ``mcpServers`` or +# ``mcp_servers``, and the first entry dialect that packs argv into one +# array; pi is the first CLI whose config is read by an extension rather +# than by the agent itself. The comment-fidelity cases are the point of +# the JSONC codec, so they are asserted on bytes, not on parsed values. +# --------------------------------------------------------------------------- + + +def test_fake_home_covers_every_registered_cli(fake_home: pathlib.Path) -> None: + """``fake_home`` replaces ``CLIS`` wholesale, so it must list every CLI. + + Regression guard rather than a behavior test. ``_config_present_clis`` + iterates ``ALL_CLIS`` while indexing ``CLIS``, so a CLI added to the + registry but not to this fixture raises ``KeyError`` from half a dozen + unrelated doctor and naming-hint tests. Naming the invariant here turns + that into one obvious failure. + """ + assert set(mcp_swap.CLIS) == set(mcp_swap.ALL_CLIS) + + +@pytest.mark.parametrize("raw", ["relcfg", "", " ", "./cfg"]) +def test_relative_xdg_config_home_is_ignored( + raw: str, monkeypatch: pytest.MonkeyPatch +) -> None: + """Regression: a relative XDG_CONFIG_HOME resolved against the cwd. + + The spec requires these to be absolute and to be ignored otherwise. + Honouring a relative one made opencode's config -- and the backup path + recorded for it -- depend on where the swap was run from, so revert + from any other directory reported the backup missing for good. + """ + monkeypatch.setenv("XDG_CONFIG_HOME", raw) + assert mcp_swap._xdg_config_home() == pathlib.Path.home() / ".config" + + +def test_absolute_xdg_config_home_is_honoured( + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """Opencode resolves XDG the way its own loader does.""" + monkeypatch.setenv("XDG_CONFIG_HOME", str(tmp_path)) + assert mcp_swap._xdg_config_home() == tmp_path + + +def test_opencode_and_pi_registered() -> None: + """Both new CLIs are first-class ``--cli`` choices with their own shapes.""" + assert "opencode" in mcp_swap.ALL_CLIS + assert "pi" in mcp_swap.ALL_CLIS + opencode = mcp_swap.CLIS["opencode"] + assert opencode.fmt == "jsonc" + assert opencode.config_path.name == "opencode.jsonc" + assert opencode.container == ("mcp",) + assert opencode.dialect == "opencode" + pi = mcp_swap.CLIS["pi"] + assert pi.fmt == "jsonc" + assert pi.config_path.name == "mcp.json" + assert pi.container == ("mcpServers",) + assert pi.dialect == "standard" + parser = mcp_swap.build_parser() + assert parser.parse_args(["status", "--cli", "opencode"]).cli == ["opencode"] + assert parser.parse_args(["status", "--cli", "pi"]).cli == ["pi"] + + +@pytest.mark.parametrize("cli", ["opencode", "pi"]) +def test_new_cli_set_get_delete_roundtrip(cli: str, fake_repo: pathlib.Path) -> None: + """Each new CLI's four container branches agree with one another. + + Proves the name was threaded through ``get_server``, ``set_server``, + ``delete_server`` and ``_all_server_specs`` rather than falling through + to another CLI's container key. + """ + config: dict[str, t.Any] = {} + spec = mcp_swap.McpServerSpec( + command="uv", args=["--directory", str(fake_repo), "run", "libtmux-mcp"] + ) + assert mcp_swap.set_server(cli, config, "libtmux", spec, fake_repo) == "added" + assert mcp_swap.CLIS[cli].container[0] in config + got = mcp_swap.get_server(cli, config, "libtmux", fake_repo) + assert got is not None + assert got.is_local_uv_directory() + assert got.local_repo_path() == fake_repo + assert mcp_swap.set_server(cli, config, "libtmux", spec, fake_repo) == "replaced" + assert mcp_swap._all_server_specs(cli, config, fake_repo).keys() == {"libtmux"} + assert mcp_swap.delete_server(cli, config, "libtmux", fake_repo) + assert mcp_swap.get_server(cli, config, "libtmux", fake_repo) is None + + +def test_opencode_entry_packs_argv_into_one_command_array( + fake_repo: pathlib.Path, +) -> None: + """The opencode dialect uses one argv array and the key ``environment``. + + A scalar ``command`` is a decode error that stops opencode starting at + all, and an ``env`` key is dropped without a warning, so both spellings + are pinned here rather than left to the round-trip tests. + """ + spec = mcp_swap.McpServerSpec( + command="uv", args=["--directory", "/repo", "run"], env={"A": "b"} + ) + entry = spec.to_entry_dict("opencode") + assert entry["type"] == "local" + assert entry["command"] == ["uv", "--directory", "/repo", "run"] + assert entry["environment"] == {"A": "b"} + assert "args" not in entry + assert "env" not in entry + + +def test_opencode_array_entry_reads_back_as_command_plus_args() -> None: + """An array ``command`` normalizes to the portable scalar-plus-args spec. + + Regression: without the split, ``command`` becomes the ``str()`` of a + Python list, ``is_local_uv_directory`` is False for a correct entry, and + the "already local — no change" short-circuit never fires, so every run + rewrites a config that needed no change. + """ + info = mcp_swap.CLIS["opencode"] + spec = mcp_swap._spec_from_entry( + { + "type": "local", + "command": ["uv", "--directory", "/repo", "run", "libtmux-mcp"], + "environment": {"A": "b"}, + }, + info=info, + ) + assert spec.command == "uv" + assert spec.args == ["--directory", "/repo", "run", "libtmux-mcp"] + assert spec.env == {"A": "b"} + assert spec.is_local_uv_directory() + assert spec.local_repo_path() == pathlib.Path("/repo") + + +def test_opencode_array_entry_round_trips_a_pr_spec() -> None: + """``pr_ref`` still recognises a pull-request spec in the array shape.""" + info = mcp_swap.CLIS["opencode"] + spec = mcp_swap.build_pr_spec( + "https://github.com/tmux-python/libtmux-mcp", 115, "libtmux-mcp" + ) + decoded = mcp_swap._spec_from_entry(spec.to_entry_dict("opencode"), info=info) + assert decoded.pr_ref() == ("https://github.com/tmux-python/libtmux-mcp", 115) + + +def _opencode_config(fake_home: pathlib.Path, body: str) -> t.Any: + """Write ``body`` to the fake opencode config and return its ``CLIInfo``.""" + info = mcp_swap.CLIS["opencode"] + info.config_path.parent.mkdir(parents=True, exist_ok=True) + info.config_path.write_text(body) + return info + + +def _swap_opencode(fake_repo: pathlib.Path) -> int: + """Run ``use-local`` against opencode only.""" + args = mcp_swap.build_parser().parse_args( + ["use-local", "--repo", str(fake_repo), "--cli", "opencode"] + ) + return int(mcp_swap.cmd_use_local(args)) + + +def test_opencode_swap_preserves_jsonc_comments( + fake_home: pathlib.Path, fake_repo: pathlib.Path +) -> None: + """Line comments, block comments and sibling servers survive a swap.""" + info = _opencode_config( + fake_home, + "{\n" + " // header comment\n" + ' "$schema": "https://opencode.ai/config.json",\n' + " /* a block comment\n" + " spanning lines */\n" + ' "model": "openrouter/x",\n' + ' "mcp": {\n' + ' "other": { "type": "local", "command": ["echo", "keep"] }\n' + " }\n" + "}\n", + ) + assert _swap_opencode(fake_repo) == 0 + text = info.config_path.read_text() + assert "// header comment" in text + assert "/* a block comment" in text + assert "spanning lines */" in text + doc = mcp_swap._jsonc_loads(text) + assert doc["model"] == "openrouter/x" + assert doc["mcp"]["other"]["command"] == ["echo", "keep"] + assert doc["mcp"]["libtmux"]["command"][0] == "uv" + + +def test_opencode_comment_inside_the_replaced_entry_survives( + fake_home: pathlib.Path, fake_repo: pathlib.Path +) -> None: + """A comment attached to the entry being rewritten is not collateral. + + The case a whole-entry rewrite loses and a field-level splice keeps. + Real opencode configs carry the rationale for a pinned ``command`` + directly above it, which is exactly the text a swap would destroy. + """ + info = _opencode_config( + fake_home, + "{\n" + ' "mcp": {\n' + ' "libtmux": {\n' + ' "type": "local",\n' + " // Pinned deliberately; this rationale must outlive the swap.\n" + ' "command": ["uvx", "libtmux-mcp==0.1.0a2"],\n' + ' "environment": { "KEEP": "me" }\n' + " }\n" + " }\n" + "}\n", + ) + assert _swap_opencode(fake_repo) == 0 + text = info.config_path.read_text() + assert "// Pinned deliberately; this rationale must outlive the swap." in text + entry = mcp_swap._jsonc_loads(text)["mcp"]["libtmux"] + assert entry["command"][0] == "uv" + assert entry["environment"] == {"KEEP": "me"} + + +def test_opencode_swap_and_revert_round_trip_is_byte_identical( + fake_home: pathlib.Path, fake_repo: pathlib.Path +) -> None: + """Revert restores a commented JSONC config byte for byte.""" + body = ( + "{\n" + " // keep me\n" + ' "model": "m",\n' + ' "mcp": {\n' + ' "libtmux": {\n' + ' "type": "local",\n' + ' "command": ["uvx", "libtmux-mcp==0.1.0a2"]\n' + " }\n" + " }\n" + "}\n" + ) + info = _opencode_config(fake_home, body) + original = info.config_path.read_bytes() + assert _swap_opencode(fake_repo) == 0 + assert info.config_path.read_bytes() != original + revert = mcp_swap.build_parser().parse_args(["revert", "--cli", "opencode"]) + assert mcp_swap.cmd_revert(revert) == 0 + assert info.config_path.read_bytes() == original + + +def test_opencode_second_swap_reports_no_change( + fake_home: pathlib.Path, fake_repo: pathlib.Path +) -> None: + """The idempotence check fires for the array command shape. + + Depends on ``_spec_from_entry`` splitting the array; without it the + config is rewritten on every invocation. + """ + info = _opencode_config(fake_home, '{\n "mcp": {}\n}\n') + assert _swap_opencode(fake_repo) == 0 + after_first = info.config_path.read_bytes() + assert _swap_opencode(fake_repo) == 0 + assert info.config_path.read_bytes() == after_first + + +def test_opencode_seeds_schema_into_an_empty_config( + fake_home: pathlib.Path, fake_repo: pathlib.Path +) -> None: + """Seeding an empty file writes ``$schema`` alongside the server entry.""" + info = _opencode_config(fake_home, "") + assert _swap_opencode(fake_repo) == 0 + doc = mcp_swap._jsonc_loads(info.config_path.read_text()) + assert doc["$schema"] == mcp_swap.OPENCODE_SCHEMA_URL + assert doc["mcp"]["libtmux"]["type"] == "local" + + +def test_opencode_symlinked_config_swap_updates_target_not_link( + fake_home: pathlib.Path, fake_repo: pathlib.Path +) -> None: + """A JSONC config symlinked into a dotfiles tree keeps its link.""" + info = mcp_swap.CLIS["opencode"] + target = fake_home / "dotfiles" / "opencode.jsonc" + target.parent.mkdir(parents=True) + target.write_text('{\n // linked\n "mcp": {}\n}\n') + info.config_path.parent.mkdir(parents=True) + info.config_path.symlink_to(target) + + assert _swap_opencode(fake_repo) == 0 + assert info.config_path.is_symlink() + assert info.config_path.readlink() == target + text = target.read_text() + assert "// linked" in text + assert mcp_swap._jsonc_loads(text)["mcp"]["libtmux"]["command"][0] == "uv" + + +def test_pi_config_with_comments_is_readable( + fake_home: pathlib.Path, fake_repo: pathlib.Path +) -> None: + """Regression: pi's adapter accepts JSONC, so strict JSON rejected it. + + ``pi-mcp-adapter`` reads the file through ``strip-json-comments`` with + trailing commas allowed. Parsing it as strict JSON made ``status`` and + ``use-local`` report a config the adapter reads fine as unreadable. + """ + info = mcp_swap.CLIS["pi"] + info.config_path.parent.mkdir(parents=True) + info.config_path.write_text( + '{\n // the adapter allows comments\n "mcpServers": {\n' + ' "keep": { "command": "echo", "args": ["hi"] },\n }\n}\n' + ) + args = mcp_swap.build_parser().parse_args( + ["use-local", "--repo", str(fake_repo), "--cli", "pi"] + ) + assert mcp_swap.cmd_use_local(args) == 0 + text = info.config_path.read_text() + assert "// the adapter allows comments" in text + servers = mcp_swap._jsonc_loads(text)["mcpServers"] + assert servers["keep"]["command"] == "echo" + assert servers["libtmux"]["command"] == "uv" + + +def test_detect_reports_the_pi_adapter_prerequisite( + fake_home: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """``detect`` says why a pi swap will not take effect on its own. + + pi ships no MCP client, so the file this script writes is read only by + the ``pi-mcp-adapter`` extension. Reporting pi as swappable without + that caveat would be the one thing this script must never do: claim an + agent will run something it will not. + """ + monkeypatch.setattr(mcp_swap, "PI_ADAPTER_DIR", fake_home / "absent") + monkeypatch.setattr(mcp_swap.shutil, "which", lambda _binary: "/usr/bin/stub") + info = mcp_swap.CLIS["pi"] + info.config_path.parent.mkdir(parents=True) + info.config_path.write_text('{"mcpServers": {}}\n') + + assert mcp_swap.cmd_detect(mcp_swap.build_parser().parse_args(["detect"])) == 0 + out = capsys.readouterr().out + assert mcp_swap.PI_ADAPTER_HINT in out + + monkeypatch.setattr(mcp_swap, "PI_ADAPTER_DIR", fake_home) + assert mcp_swap.cmd_detect(mcp_swap.build_parser().parse_args(["detect"])) == 0 + assert mcp_swap.PI_ADAPTER_HINT not in capsys.readouterr().out + + +# --------------------------------------------------------------------------- +# JSONC writer fidelity +# +# The JSON writer reserializes the whole document, so it can only promise +# to preserve values. The JSONC writer splices text and therefore promises +# bytes: anything it did not deliberately change must come back identical, +# including the comments, the trailing comma, the indent width and the +# absence of a final newline. The string cases exist because a +# comment-stripper that is not string-aware corrupts a URL or a Windows +# path silently, which is the worst failure this codec could have. +# --------------------------------------------------------------------------- + + +PRESERVED_JSONC: list[JSONFidelityCase] = [ + JSONFidelityCase("line_comment", '{\n // note\n "mcp": {}\n}\n'), + JSONFidelityCase("block_comment", '{\n /* note\n more */\n "mcp": {}\n}\n'), + JSONFidelityCase("comment_after_last_member", '{\n "mcp": {}\n // tail\n}\n'), + JSONFidelityCase("trailing_comma", '{\n "mcp": {},\n}\n'), + JSONFidelityCase("no_trailing_newline", '{\n "mcp": {}\n}'), + JSONFidelityCase("four_space_indent", '{\n "mcp": {}\n}\n'), + JSONFidelityCase("url_containing_double_slash", '{\n "a": "https://x/y//z"\n}\n'), + JSONFidelityCase("block_marker_inside_string", '{\n "a": "/* not one */"\n}\n'), + JSONFidelityCase("windows_path", '{\n "a": "C:\\\\tmp\\\\x"\n}\n'), + JSONFidelityCase("literal_backslash_u", '{\n "a": "C:\\\\u0041"\n}\n'), + JSONFidelityCase("emoji_and_cjk", '{\n "a": "🙂 日本語 café"\n}\n'), + JSONFidelityCase("empty_object", "{}\n"), + JSONFidelityCase("comment_only_object", '{\n "mcp": {\n // none yet\n }\n}\n'), + JSONFidelityCase( + "comment_before_the_delimiter", '{\n "a": 1 /* x */,\n "b": 2\n}\n' + ), +] + + +def _jsonc_config(tmp_path: pathlib.Path, body: str) -> tuple[t.Any, bytes]: + """Write ``body`` verbatim and return its ``CLIInfo`` and exact bytes.""" + path = tmp_path / "opencode.jsonc" + path.write_text(body) + info = mcp_swap.CLIInfo( + name="opencode", + binary="opencode", + config_path=path, + fmt="jsonc", + container=("mcp",), + dialect="opencode", + ) + return info, path.read_bytes() + + +@pytest.mark.parametrize( + JSONFidelityCase._fields, + PRESERVED_JSONC, + ids=[c.test_id for c in PRESERVED_JSONC], +) +def test_untouched_jsonc_config_round_trips_byte_identical( + test_id: str, body: str, tmp_path: pathlib.Path +) -> None: + """Loading and rewriting an unmodified JSONC config changes no byte.""" + assert test_id + info, raw = _jsonc_config(tmp_path, body) + config = mcp_swap.load_config(info) + assert mcp_swap.dump_config_bytes(info, config, original=raw) == raw + + +@pytest.mark.parametrize( + JSONFidelityCase._fields, + PRESERVED_JSONC, + ids=[c.test_id for c in PRESERVED_JSONC], +) +def test_jsonc_values_match_stdlib_json( + test_id: str, body: str, tmp_path: pathlib.Path +) -> None: + r"""JSONC parsing agrees with stdlib json wherever stdlib can parse. + + Escape handling is the standard library's, not a reimplementation's. + The rejected ``json-five`` dependency failed exactly here: it raised on + ``"C:\\x"`` and decoded a literal ``\\u0041`` to ``"A"``. + """ + assert test_id + try: + expected = json.loads(body) + except json.JSONDecodeError: + pytest.skip("comment or trailing comma — stdlib cannot parse it") + assert mcp_swap._jsonc_loads(body) == expected + + +def test_jsonc_config_is_not_written_through_the_toml_writer( + tmp_path: pathlib.Path, +) -> None: + """A jsonc config comes back as JSON text, not TOML. + + Regression: ``dump_config_bytes`` branched on ``fmt != "json"``, so any + third format reached ``tomlkit.dumps`` and put TOML bytes in a JSON + file. The dispatch is on the exact format now. + """ + info, raw = _jsonc_config(tmp_path, '{\n "mcp": {}\n}\n') + out = mcp_swap.dump_config_bytes( + info, {"mcp": {"x": {"type": "local"}}}, original=raw + ) + text = out.decode() + assert text.lstrip().startswith("{") + assert mcp_swap._jsonc_loads(text)["mcp"]["x"]["type"] == "local" + + +class JsoncDeletionCase(t.NamedTuple): + """A member removal whose exact resulting text is pinned. + + Attributes + ---------- + test_id : str + Identifier shown in the parametrized test name. + body : str + The config text before the merge. + data : dict[str, t.Any] + The reconciled data the merge is driven with. + expected : str + The exact text the merge must produce. + """ + + test_id: str + body: str + data: dict[str, t.Any] + expected: str + + +JSONC_DELETIONS: list[JsoncDeletionCase] = [ + JsoncDeletionCase( + "first_member", + '{\n "a": 1,\n "b": 2\n}\n', + {"b": 2}, + '{\n "b": 2\n}\n', + ), + JsoncDeletionCase( + "middle_member", + '{\n "a": 1,\n "b": 2,\n "c": 3\n}\n', + {"a": 1, "c": 3}, + '{\n "a": 1,\n "c": 3\n}\n', + ), + JsoncDeletionCase( + "last_member", + '{\n "a": 1,\n "b": 2\n}\n', + {"a": 1}, + '{\n "a": 1\n}\n', + ), + JsoncDeletionCase( + "comma_hidden_behind_a_comment", + '{\n "a": 1 /* x, y */,\n "b": 2\n}\n', + {"b": 2}, + '{\n "b": 2\n}\n', + ), +] + + +@pytest.mark.parametrize( + JsoncDeletionCase._fields, + JSONC_DELETIONS, + ids=[c.test_id for c in JSONC_DELETIONS], +) +def test_jsonc_merge_removing_a_member_takes_exactly_one_comma( + test_id: str, body: str, data: dict[str, t.Any], expected: str +) -> None: + """Regression: a removal took the comma on both sides of the member. + + Deleting a member between two others left its neighbours undelimited, + so the next merge pass raised ``JSONDecodeError`` and the swap reported + the config unreadable. ``comma_hidden_behind_a_comment`` covers the + partner defect: the delimiter scan read the raw text, where a comma + inside a comment passes for the separator. + """ + assert test_id + assert mcp_swap._jsonc_merge(body, data, ensure_ascii=False) == expected + + +@pytest.mark.parametrize( + "name", ["back\\slash", 'quo"te', "new\nline", "tab\tbed", "unicode\u00e9"] +) +def test_jsonc_merge_escapes_an_inserted_key(name: str) -> None: + """Regression: an inserted key was written raw, so a swap could not converge. + + ``--server`` takes an arbitrary string. Written unescaped, a backslash or + quote in it emitted text that would not parse back, so the member was + never found again and the merge re-inserted it until the pass ceiling -- + spinning while holding the swap lock and then failing. + """ + src = '{\n "mcp": {}\n}\n' + data = mcp_swap._jsonc_loads(src) + data["mcp"][name] = {"type": "local"} + out = mcp_swap._jsonc_merge(src, data, ensure_ascii=False) + assert mcp_swap._jsonc_loads(out)["mcp"][name] == {"type": "local"} + + +def test_jsonc_merge_removing_a_middle_member_stays_parseable() -> None: + """The shape that surfaced it: an opencode entry losing optional fields.""" + src = ( + '{\n "mcp": {\n "tmux": {\n "type": "local",\n' + ' "enabled": true,\n "timeout": 5000,\n' + ' "command": ["uvx", "old"]\n }\n }\n}\n' + ) + data = mcp_swap._jsonc_loads(src) + data["mcp"]["tmux"] = {"type": "local", "command": ["uv", "run", "x"]} + out = mcp_swap._jsonc_merge(src, data, ensure_ascii=False) + assert mcp_swap._jsonc_loads(out) == data + + +def test_jsonc_merge_inserting_into_a_comment_only_object_keeps_the_comment() -> None: + """Regression: blanking made a documented object look empty. + + The emptiness guard reads the comment-blanked text, where a comment is + indistinguishable from whitespace, so insertion used to splice over the + whole interior and take the comment with it. + """ + src = '{\n "mcp": {\n // why there are no servers yet\n }\n}\n' + data = mcp_swap._jsonc_loads(src) + data["mcp"]["tmux"] = {"type": "local", "command": ["uv"]} + out = mcp_swap._jsonc_merge(src, data, ensure_ascii=False) + assert out == ( + '{\n "mcp": {\n // why there are no servers yet\n' + ' "tmux": {\n "type": "local",\n "command": [\n' + ' "uv"\n ]\n }\n }\n}\n' + ) + + +def test_jsonc_merge_inserting_into_a_comment_only_document_keeps_the_comment() -> None: + """The same splice at the root, where there is no enclosing member.""" + src = "{\n // root rationale\n}\n" + data = mcp_swap._jsonc_loads(src) + data["mcp"] = {} + out = mcp_swap._jsonc_merge(src, data, ensure_ascii=False) + assert out == '{\n // root rationale\n "mcp": {}\n}\n' + + +@pytest.mark.parametrize( + "body", + ["{}\n", "{ }\n", '{\n "mcp": {}\n}\n', '{\n "mcp": {\n }\n}\n'], +) +def test_jsonc_merge_inserting_into_an_empty_object_is_unchanged(body: str) -> None: + """A genuinely empty interior still collapses to the old splice point.""" + data = mcp_swap._jsonc_loads(body) + data.setdefault("mcp", {})["tmux"] = {"type": "local"} + out = mcp_swap._jsonc_merge(body, data, ensure_ascii=False) + assert mcp_swap._jsonc_loads(out)["mcp"]["tmux"] == {"type": "local"} + assert out.rstrip().endswith("}") + + +def test_jsonc_comment_blanking_preserves_offsets() -> None: + """Blanking a comment must not move the bytes around it. + + Offsets are what let a span found in the blanked text address the same + bytes in the original; if blanking changed the length, every splice + would land in the wrong place. + """ + src = '{\n // note\n "a": 1, /* x */\n "b": "//not a comment"\n}\n' + blanked = mcp_swap._jsonc_blank_comments(src) + assert len(blanked) == len(src) + assert "//not a comment" in blanked + assert "note" not in blanked + assert blanked.count("\n") == src.count("\n")