Skip to content

Commit 88ea2a7

Browse files
fix: preserve user config when init cannot parse existing JSON (#94)
Fixes #93. Add a shared _load_existing_config helper so existing config files that fail to parse raise a clear ClickException instead of being treated as empty objects. Migrate save_root_mcp_config, _merge_mcp_entry, and install_claude_code_hooks to the shared loader, preserving missing-file initialization while preventing silent overwrites of malformed user settings. Co-authored-by: Raghav Chamadiya <65403859+RaghavChamadiya@users.noreply.github.com>
1 parent 5e24864 commit 88ea2a7

2 files changed

Lines changed: 182 additions & 15 deletions

File tree

packages/cli/src/repowise/cli/mcp_config.py

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
import sys
77
from pathlib import Path
88

9+
import click
10+
911

1012
def _claude_desktop_config_path() -> Path | None:
1113
"""Return the Claude Desktop config path for this OS, or None if unsupported."""
@@ -66,10 +68,7 @@ def save_root_mcp_config(repo_path: Path) -> Path:
6668
new_entry = generate_mcp_config(repo_path)["mcpServers"]
6769

6870
if config_path.exists():
69-
try:
70-
existing = json.loads(config_path.read_text(encoding="utf-8"))
71-
except (json.JSONDecodeError, OSError):
72-
existing = {}
71+
existing = _load_existing_config(config_path)
7372
servers = dict(existing.get("mcpServers", {}))
7473
servers.update(new_entry)
7574
existing["mcpServers"] = servers
@@ -88,10 +87,7 @@ def _merge_mcp_entry(config_path: Path, new_entry: dict) -> bool:
8887
"""
8988
try:
9089
if config_path.exists():
91-
try:
92-
existing = json.loads(config_path.read_text(encoding="utf-8"))
93-
except (json.JSONDecodeError, OSError):
94-
existing = {}
90+
existing = _load_existing_config(config_path)
9591
else:
9692
config_path.parent.mkdir(parents=True, exist_ok=True)
9793
existing = {}
@@ -105,6 +101,28 @@ def _merge_mcp_entry(config_path: Path, new_entry: dict) -> bool:
105101
return False
106102

107103

104+
def _load_existing_config(config_path: Path) -> dict:
105+
"""Load an existing JSON config without silently replacing bad content."""
106+
try:
107+
existing = json.loads(config_path.read_text(encoding="utf-8"))
108+
except json.JSONDecodeError as exc:
109+
raise click.ClickException(
110+
f"Cannot update {config_path}: existing file is not valid JSON. "
111+
"Fix or remove it and retry; no changes were written."
112+
) from exc
113+
except OSError as exc:
114+
raise click.ClickException(
115+
f"Cannot update {config_path}: existing file could not be read. "
116+
"Fix the file permissions and retry; no changes were written."
117+
) from exc
118+
if not isinstance(existing, dict):
119+
raise click.ClickException(
120+
f"Cannot update {config_path}: existing file must contain a JSON object. "
121+
"Fix or remove it and retry; no changes were written."
122+
)
123+
return existing
124+
125+
108126
def register_with_claude_desktop(repo_path: Path) -> Path | None:
109127
"""Add repowise MCP server to Claude Desktop's config.
110128
@@ -171,10 +189,7 @@ def install_claude_code_hooks() -> Path | None:
171189

172190
try:
173191
if settings_path.exists():
174-
try:
175-
existing = json.loads(settings_path.read_text(encoding="utf-8"))
176-
except (json.JSONDecodeError, OSError):
177-
existing = {}
192+
existing = _load_existing_config(settings_path)
178193
else:
179194
settings_path.parent.mkdir(parents=True, exist_ok=True)
180195
existing = {}
@@ -191,9 +206,7 @@ def install_claude_code_hooks() -> Path | None:
191206
if not _has_repowise_hook(post_hooks):
192207
post_hooks.append(post_hook_entry)
193208

194-
settings_path.write_text(
195-
json.dumps(existing, indent=2) + "\n", encoding="utf-8"
196-
)
209+
settings_path.write_text(json.dumps(existing, indent=2) + "\n", encoding="utf-8")
197210
return settings_path
198211
except OSError:
199212
return None

tests/unit/cli/test_mcp_config.py

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
import json
2+
import re
3+
from pathlib import Path
4+
5+
import click
6+
import pytest
7+
8+
from repowise.cli import mcp_config
9+
10+
11+
def _repowise_entry(repo_path: Path) -> dict:
12+
return mcp_config.generate_mcp_config(repo_path)["mcpServers"]
13+
14+
15+
def test_save_root_mcp_config_creates_missing_file(tmp_path: Path) -> None:
16+
config_path = mcp_config.save_root_mcp_config(tmp_path)
17+
18+
saved = json.loads(config_path.read_text(encoding="utf-8"))
19+
assert "repowise" in saved["mcpServers"]
20+
21+
22+
def test_save_root_mcp_config_merges_valid_existing_file(tmp_path: Path) -> None:
23+
config_path = tmp_path / ".mcp.json"
24+
config_path.write_text(
25+
json.dumps(
26+
{
27+
"mcpServers": {"other": {"command": "other"}},
28+
"custom": {"preserved": True},
29+
}
30+
),
31+
encoding="utf-8",
32+
)
33+
34+
mcp_config.save_root_mcp_config(tmp_path)
35+
36+
saved = json.loads(config_path.read_text(encoding="utf-8"))
37+
assert saved["mcpServers"]["other"] == {"command": "other"}
38+
assert "repowise" in saved["mcpServers"]
39+
assert saved["custom"] == {"preserved": True}
40+
41+
42+
def test_save_root_mcp_config_rejects_invalid_existing_file(tmp_path: Path) -> None:
43+
config_path = tmp_path / ".mcp.json"
44+
original = '{\n "mcpServers": {},\n}\n'
45+
config_path.write_text(original, encoding="utf-8")
46+
47+
with pytest.raises(click.ClickException, match=re.escape(str(config_path))):
48+
mcp_config.save_root_mcp_config(tmp_path)
49+
50+
assert config_path.read_text(encoding="utf-8") == original
51+
52+
53+
def test_merge_mcp_entry_creates_missing_file(tmp_path: Path) -> None:
54+
config_path = tmp_path / "settings.json"
55+
56+
assert mcp_config._merge_mcp_entry(config_path, _repowise_entry(tmp_path))
57+
58+
saved = json.loads(config_path.read_text(encoding="utf-8"))
59+
assert "repowise" in saved["mcpServers"]
60+
61+
62+
def test_merge_mcp_entry_merges_valid_existing_file(tmp_path: Path) -> None:
63+
config_path = tmp_path / "settings.json"
64+
config_path.write_text(
65+
json.dumps(
66+
{
67+
"mcpServers": {"existing": {"command": "existing"}},
68+
"permissions": {"allow": ["Bash(git status:*)"]},
69+
}
70+
),
71+
encoding="utf-8",
72+
)
73+
74+
assert mcp_config._merge_mcp_entry(config_path, _repowise_entry(tmp_path))
75+
76+
saved = json.loads(config_path.read_text(encoding="utf-8"))
77+
assert saved["mcpServers"]["existing"] == {"command": "existing"}
78+
assert "repowise" in saved["mcpServers"]
79+
assert saved["permissions"] == {"allow": ["Bash(git status:*)"]}
80+
81+
82+
def test_merge_mcp_entry_rejects_invalid_existing_file(tmp_path: Path) -> None:
83+
config_path = tmp_path / "settings.json"
84+
original = '{\n "permissions": {},\n}\n'
85+
config_path.write_text(original, encoding="utf-8")
86+
87+
with pytest.raises(click.ClickException, match=re.escape(str(config_path))):
88+
mcp_config._merge_mcp_entry(config_path, _repowise_entry(tmp_path))
89+
90+
assert config_path.read_text(encoding="utf-8") == original
91+
92+
93+
def test_install_claude_code_hooks_creates_missing_file(
94+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
95+
) -> None:
96+
monkeypatch.setattr(Path, "home", lambda: tmp_path)
97+
98+
settings_path = mcp_config.install_claude_code_hooks()
99+
100+
assert settings_path == tmp_path / ".claude" / "settings.json"
101+
saved = json.loads(settings_path.read_text(encoding="utf-8"))
102+
assert "PreToolUse" in saved["hooks"]
103+
assert "PostToolUse" in saved["hooks"]
104+
105+
106+
def test_install_claude_code_hooks_merges_valid_existing_file(
107+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
108+
) -> None:
109+
monkeypatch.setattr(Path, "home", lambda: tmp_path)
110+
settings_path = tmp_path / ".claude" / "settings.json"
111+
settings_path.parent.mkdir(parents=True)
112+
settings_path.write_text(
113+
json.dumps(
114+
{
115+
"permissions": {"allow": ["Bash(git status:*)"]},
116+
"hooks": {
117+
"PreToolUse": [
118+
{
119+
"matcher": "Read",
120+
"hooks": [{"type": "command", "command": "echo read"}],
121+
}
122+
]
123+
},
124+
}
125+
),
126+
encoding="utf-8",
127+
)
128+
129+
assert mcp_config.install_claude_code_hooks() == settings_path
130+
131+
saved = json.loads(settings_path.read_text(encoding="utf-8"))
132+
assert saved["permissions"] == {"allow": ["Bash(git status:*)"]}
133+
assert saved["hooks"]["PreToolUse"][0]["matcher"] == "Read"
134+
assert any(
135+
hook["command"] == "repowise augment"
136+
for entry in saved["hooks"]["PreToolUse"]
137+
for hook in entry["hooks"]
138+
)
139+
assert "PostToolUse" in saved["hooks"]
140+
141+
142+
def test_install_claude_code_hooks_rejects_invalid_existing_file(
143+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
144+
) -> None:
145+
monkeypatch.setattr(Path, "home", lambda: tmp_path)
146+
settings_path = tmp_path / ".claude" / "settings.json"
147+
settings_path.parent.mkdir(parents=True)
148+
original = '{\n "permissions": {},\n}\n'
149+
settings_path.write_text(original, encoding="utf-8")
150+
151+
with pytest.raises(click.ClickException, match=re.escape(str(settings_path))):
152+
mcp_config.install_claude_code_hooks()
153+
154+
assert settings_path.read_text(encoding="utf-8") == original

0 commit comments

Comments
 (0)