Skip to content

Commit 660a1a2

Browse files
committed
fix(dsh): ensure config home exists before wrap dsh registration
DshRegistrar.detect() requires ~/.dsh to exist, so on a fresh install headroom wrap dsh registered nothing and dsh created its home without Serena. Add DshRegistrar.ensure_home() and call it on the explicit wrap path before registration; detect() stays conservative for global headroom mcp install. Wrap tests redirect DSH_HOME / HEADROOM_WORKSPACE_DIR / HOME to tmp so they cannot mutate a developer's real ~/.dsh, and a new test proves the managed entry exists before launch when no DSH home is present.
1 parent c71e536 commit 660a1a2

4 files changed

Lines changed: 110 additions & 7 deletions

File tree

headroom/cli/wrap.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7519,7 +7519,9 @@ def dsh(
75197519

75207520
from headroom.mcp_registry import DshRegistrar
75217521

7522-
_setup_coding_compressor(DshRegistrar(), serena_context="agent", verbose=verbose)
7522+
registrar = DshRegistrar()
7523+
registrar.ensure_home()
7524+
_setup_coding_compressor(registrar, serena_context="agent", verbose=verbose)
75237525

75247526
_launch_tool(
75257527
binary=argv[0],

headroom/mcp_registry/dsh.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,16 @@ def _config_file(self) -> Path:
175175
def detect(self) -> bool:
176176
return _dsh_home().exists()
177177

178+
def ensure_home(self) -> None:
179+
"""Create the dsh config home if missing (explicit wrap path only).
180+
181+
``headroom wrap dsh`` targets a harness that is about to launch, so it
182+
must register before dsh creates its own home on first boot. Global
183+
``headroom mcp install`` detection stays conservative: ``detect()``
184+
still requires an existing home.
185+
"""
186+
_dsh_home().mkdir(parents=True, exist_ok=True)
187+
178188
def get_server(self, server_name: str) -> ServerSpec | None:
179189
managed = _read_managed_block(self._config_file)
180190
if managed.entries is None:

tests/test_cli/test_wrap_dsh.py

Lines changed: 72 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,28 @@ def fake_launch_tool(**kwargs: object) -> None:
2020
return fake_launch_tool
2121

2222

23+
def _hermetic_home(monkeypatch: pytest.MonkeyPatch, tmp_path) -> None:
24+
"""Redirect every home-adjacent path the wrap flow can write to.
25+
26+
The wrap flow registers Serena/headroom MCP entries into the dsh home and
27+
records installs in the Headroom workspace ledger. Without redirects these
28+
tests would mutate the developer's real `~/.dsh` / `~/.headroom` when
29+
the harness is installed on the machine running the suite.
30+
"""
31+
monkeypatch.setenv("DSH_HOME", str(tmp_path / "dsh"))
32+
monkeypatch.setenv("HEADROOM_WORKSPACE_DIR", str(tmp_path / "headroom"))
33+
monkeypatch.setenv("HOME", str(tmp_path / "home"))
34+
# The repo root carries a `.serena/project.yml`, so a successful Serena
35+
# registration would synchronously run `serena project index` (a real
36+
# subprocess, up to _SERENA_INDEX_TIMEOUT). Registration is what these
37+
# tests assert; the pre-index is orthogonal and must not run here.
38+
monkeypatch.setattr("headroom.cli.wrap._index_serena_project", lambda **_kwargs: None)
39+
40+
2341
def test_wrap_dsh_launches_web_with_proxy_env(
24-
runner: CliRunner, monkeypatch: pytest.MonkeyPatch
42+
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path
2543
) -> None:
44+
_hermetic_home(monkeypatch, tmp_path)
2645
captured: dict[str, object] = {}
2746
monkeypatch.setattr("headroom.cli.wrap._launch_tool", _capture(captured))
2847
monkeypatch.setattr(
@@ -40,7 +59,10 @@ def test_wrap_dsh_launches_web_with_proxy_env(
4059
assert captured["agent_type"] == "dsh"
4160

4261

43-
def test_wrap_dsh_headless_profile(runner: CliRunner, monkeypatch: pytest.MonkeyPatch) -> None:
62+
def test_wrap_dsh_headless_profile(
63+
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path
64+
) -> None:
65+
_hermetic_home(monkeypatch, tmp_path)
4466
captured: dict[str, object] = {}
4567
monkeypatch.setattr("headroom.cli.wrap._launch_tool", _capture(captured))
4668
monkeypatch.setattr(
@@ -54,8 +76,9 @@ def test_wrap_dsh_headless_profile(runner: CliRunner, monkeypatch: pytest.Monkey
5476

5577

5678
def test_wrap_dsh_forwards_deepseek_api_url(
57-
runner: CliRunner, monkeypatch: pytest.MonkeyPatch
79+
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path
5880
) -> None:
81+
_hermetic_home(monkeypatch, tmp_path)
5982
captured: dict[str, object] = {}
6083
monkeypatch.setattr("headroom.cli.wrap._launch_tool", _capture(captured))
6184
monkeypatch.setattr(
@@ -69,8 +92,9 @@ def test_wrap_dsh_forwards_deepseek_api_url(
6992

7093

7194
def test_wrap_dsh_captures_ambient_deepseek_base_url(
72-
runner: CliRunner, monkeypatch: pytest.MonkeyPatch
95+
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path
7396
) -> None:
97+
_hermetic_home(monkeypatch, tmp_path)
7498
captured: dict[str, object] = {}
7599
monkeypatch.setattr("headroom.cli.wrap._launch_tool", _capture(captured))
76100
monkeypatch.setattr("headroom.providers.dsh.runtime.shutil.which", lambda _name: "/usr/bin/dsh")
@@ -80,15 +104,21 @@ def test_wrap_dsh_captures_ambient_deepseek_base_url(
80104
assert captured["deepseek_api_url"] == "https://gateway.internal"
81105

82106

83-
def test_wrap_dsh_missing_binary_fails(runner: CliRunner, monkeypatch: pytest.MonkeyPatch) -> None:
107+
def test_wrap_dsh_missing_binary_fails(
108+
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path
109+
) -> None:
110+
_hermetic_home(monkeypatch, tmp_path)
84111
monkeypatch.setattr("headroom.providers.dsh.runtime.shutil.which", lambda _name: None)
85112

86113
result = runner.invoke(main, ["wrap", "dsh"])
87114
assert result.exit_code == 1
88115
assert "not found in PATH" in result.output
89116

90117

91-
def test_unwrap_dsh_stops_proxy(runner: CliRunner, monkeypatch: pytest.MonkeyPatch) -> None:
118+
def test_unwrap_dsh_stops_proxy(
119+
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path
120+
) -> None:
121+
_hermetic_home(monkeypatch, tmp_path)
92122
monkeypatch.setattr(
93123
"headroom.cli.wrap._stop_local_proxy_for_unwrap",
94124
lambda _port: "stopped",
@@ -100,3 +130,39 @@ def test_unwrap_dsh_stops_proxy(runner: CliRunner, monkeypatch: pytest.MonkeyPat
100130

101131
result = runner.invoke(main, ["unwrap", "dsh"])
102132
assert result.exit_code == 0, result.output
133+
134+
135+
def test_wrap_dsh_establishes_managed_entry_before_launch_without_home(
136+
runner: CliRunner, monkeypatch: pytest.MonkeyPatch, tmp_path
137+
) -> None:
138+
"""Fresh install: no DSH home exists — the managed entry must land before
139+
dsh launches (review #4985851059: detect() alone would skip registration).
140+
"""
141+
_hermetic_home(monkeypatch, tmp_path)
142+
dsh_home = tmp_path / "dsh"
143+
assert not dsh_home.exists()
144+
145+
captured: dict[str, object] = {}
146+
launch_saw_entry: list[bool] = []
147+
148+
def fake_launch_tool(**kwargs: object) -> None:
149+
# Prove the managed entry already exists at launch time.
150+
launch_saw_entry.append((dsh_home / "cordis.patch.yml").exists())
151+
captured.update(kwargs)
152+
153+
monkeypatch.setattr("headroom.cli.wrap._launch_tool", fake_launch_tool)
154+
monkeypatch.setattr(
155+
"headroom.providers.dsh.runtime.shutil.which",
156+
lambda _name: "/usr/bin/dsh",
157+
)
158+
159+
result = runner.invoke(main, ["wrap", "dsh", "--port", "9000"])
160+
assert result.exit_code == 0, result.output
161+
162+
patch = dsh_home / "cordis.patch.yml"
163+
assert patch.exists(), "managed entry must exist before dsh launches"
164+
text = patch.read_text(encoding="utf-8")
165+
assert "@deepseek-ai/dsh-mcp-client" in text
166+
assert "serverName: serena" in text
167+
assert captured["agent_type"] == "dsh"
168+
assert launch_saw_entry == [True]

tests/test_mcp_registry_dsh.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,31 @@ def test_not_detected_without_home(monkeypatch: pytest.MonkeyPatch, tmp_path: Pa
3434
assert DshRegistrar().detect() is False
3535

3636

37+
def test_ensure_home_creates_missing_home(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
38+
home = tmp_path / "dsh"
39+
monkeypatch.setenv("DSH_HOME", str(home))
40+
reg = DshRegistrar()
41+
assert reg.detect() is False
42+
reg.ensure_home()
43+
assert reg.detect() is True
44+
assert home.is_dir()
45+
46+
47+
def test_register_after_ensure_home_without_home(
48+
monkeypatch: pytest.MonkeyPatch, tmp_path: Path
49+
) -> None:
50+
"""Fresh install: no DSH home yet — the explicit wrap path must be able to
51+
register before dsh creates its home on first boot."""
52+
home = tmp_path / "dsh"
53+
monkeypatch.setenv("DSH_HOME", str(home))
54+
reg = DshRegistrar()
55+
reg.ensure_home()
56+
result = reg.register_server(_spec())
57+
assert result.status is RegisterStatus.REGISTERED
58+
text = (home / "cordis.patch.yml").read_text(encoding="utf-8")
59+
assert "@deepseek-ai/dsh-mcp-client" in text
60+
61+
3762
def test_register_writes_patch(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
3863
home = tmp_path / "dsh"
3964
home.mkdir()

0 commit comments

Comments
 (0)