Skip to content

Commit 62a200a

Browse files
committed
scripts(fix[mcp_swap]): preserve existing env on use-local replacement
cmd_use_local previously constructed the replacement spec via build_local_spec (which leaves env={}) and wrote it directly. Any env entries the user had set on the prior pinned-PyPI block — LIBTMUX_SAFETY, LIBTMUX_SOCKET, custom dev knobs — were silently dropped on swap. The timestamped backup at .bak.mcp-swap-<ts> still captured the original config so revert restored everything, but a day-to-day swap shouldn't require manual env restoration. Merge current.env into the new spec via dataclasses.replace before set_server. The merge is symmetric with _spec_from_entry, which already round-trips env on the read side. Only fires when current is not None, so the Codex "added" path keeps writing empty env. Two regression tests: one seeds env on a Cursor entry and asserts preservation through swap; the paired test pins the no-prior-entry Codex path to lock the merge logic against accidental env synthesis.
1 parent 199404a commit 62a200a

3 files changed

Lines changed: 90 additions & 1 deletion

File tree

CHANGES

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,19 @@ _Notes on upcoming releases will be added here_
4040

4141
### Scripts
4242

43+
- ``scripts/mcp_swap.py``: ``use-local`` now preserves an existing
44+
entry's ``env`` dict on replacement. Previously a swap silently
45+
dropped client-side environment settings (``LIBTMUX_SAFETY``,
46+
``LIBTMUX_SOCKET``, custom dev knobs) because ``build_local_spec``
47+
returned a spec with empty env and ``set_server`` wrote it unchanged.
48+
The timestamped backup at ``.bak.mcp-swap-<ts>`` still captures the
49+
full original config for full recovery via ``revert``, but day-to-day
50+
swaps no longer require manual env restoration. The merge mirrors
51+
``_spec_from_entry`` (which round-trips env on the read side).
52+
Regression test:
53+
``tests/test_mcp_swap.py:test_use_local_preserves_existing_env_when_replacing``
54+
plus a paired ``test_use_local_with_no_prior_entry_writes_empty_env``
55+
to lock the Codex "added" path against accidental env synthesis.
4356
- ``scripts/mcp_swap.py``: ``_claude_project_node`` validates that
4457
Claude's undocumented ``projects.<abs>.mcpServers`` layout is still
4558
mapping-shaped before mutating it. If a future Claude release

scripts/mcp_swap.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,13 @@ def cmd_use_local(args: argparse.Namespace) -> int:
556556
):
557557
print(f"[{cli}] already local (this repo) — no change")
558558
continue
559-
action = set_server(cli, config, server, spec, repo)
559+
# Preserve the existing entry's env on replacement. ``build_local_spec``
560+
# writes an empty env, so without this merge a swap would silently drop
561+
# client-side settings (LIBTMUX_SAFETY, LIBTMUX_SOCKET, custom dev
562+
# knobs). Symmetric with ``_spec_from_entry`` which round-trips env on
563+
# the read side.
564+
cli_spec = dataclasses.replace(spec, env={**current.env}) if current else spec
565+
action = set_server(cli, config, server, cli_spec, repo)
560566
new_bytes = dump_config_bytes(info, config)
561567

562568
if args.dry_run:

tests/test_mcp_swap.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,76 @@ def test_json_swap_and_revert_round_trip(
158158
assert info.config_path.read_bytes() == original
159159

160160

161+
def test_use_local_preserves_existing_env_when_replacing(
162+
fake_home: pathlib.Path, fake_repo: pathlib.Path
163+
) -> None:
164+
"""Existing ``env`` on a replaced entry survives ``use-local``.
165+
166+
Regression: ``cmd_use_local`` previously constructed the replacement
167+
spec via ``build_local_spec`` (env={}) and wrote it directly,
168+
silently dropping client-side settings like ``LIBTMUX_SAFETY`` or
169+
``LIBTMUX_SOCKET`` that the user had set on the prior pinned-PyPI
170+
entry. The fix merges ``current.env`` into the new spec; this test
171+
locks the behaviour by seeding env on a Cursor entry, running
172+
``use-local``, and asserting both the new local-uv command shape and
173+
the original env survived.
174+
"""
175+
info = mcp_swap.CLIS["cursor"]
176+
_write_json(
177+
info.config_path,
178+
{
179+
"mcpServers": {
180+
"libtmux": {
181+
"command": "uvx",
182+
"args": ["libtmux-mcp==0.1.0a2"],
183+
"env": {"LIBTMUX_SAFETY": "readonly", "FOO": "bar"},
184+
}
185+
}
186+
},
187+
)
188+
189+
args = mcp_swap.build_parser().parse_args(
190+
["use-local", "--repo", str(fake_repo), "--cli", "cursor"]
191+
)
192+
assert mcp_swap.cmd_use_local(args) == 0
193+
194+
entry = json.loads(info.config_path.read_text())["mcpServers"]["libtmux"]
195+
assert entry["command"] == "uv"
196+
assert entry["args"] == [
197+
"--directory",
198+
str(fake_repo.resolve()),
199+
"run",
200+
"libtmux-mcp",
201+
]
202+
assert entry["env"] == {"LIBTMUX_SAFETY": "readonly", "FOO": "bar"}
203+
204+
205+
def test_use_local_with_no_prior_entry_writes_empty_env(
206+
fake_home: pathlib.Path, fake_repo: pathlib.Path
207+
) -> None:
208+
"""When no prior entry exists, the new spec lands with empty env.
209+
210+
The env-merge branch only fires for replacements; the "added" path
211+
(e.g. Codex with no prior libtmux block) should match
212+
``build_local_spec``'s default empty env. This pins the Codex add
213+
case so the merge logic doesn't accidentally synthesise env from
214+
nothing.
215+
"""
216+
info = mcp_swap.CLIS["codex"]
217+
info.config_path.parent.mkdir(parents=True, exist_ok=True)
218+
info.config_path.write_text("# empty config\n")
219+
220+
args = mcp_swap.build_parser().parse_args(
221+
["use-local", "--repo", str(fake_repo), "--cli", "codex"]
222+
)
223+
assert mcp_swap.cmd_use_local(args) == 0
224+
225+
config = tomlkit.parse(info.config_path.read_text())
226+
table = config["mcp_servers"]["libtmux"] # type: ignore[index]
227+
assert isinstance(table, tomlkit.items.Table)
228+
assert "env" not in table
229+
230+
161231
def test_json_swap_preserves_unrelated_servers(
162232
fake_home: pathlib.Path, fake_repo: pathlib.Path
163233
) -> None:

0 commit comments

Comments
 (0)