From ae384862a4950cec057103e9daf75e74107640df Mon Sep 17 00:00:00 2001 From: Abhay Singh Date: Wed, 12 Aug 2026 06:45:37 +0530 Subject: [PATCH] fix(wrap/opencode): verify the opencode binary before mutating config Verify the OpenCode executable before changing configuration. --- headroom/cli/wrap.py | 22 ++++++++++++++++----- tests/test_cli/test_wrap_opencode.py | 29 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/headroom/cli/wrap.py b/headroom/cli/wrap.py index 094454206..212b52261 100644 --- a/headroom/cli/wrap.py +++ b/headroom/cli/wrap.py @@ -6947,6 +6947,20 @@ def opencode( ) subscription_resolution = _require_copilot_subscription_resolution() + # Verify the opencode binary exists BEFORE mutating any config. Otherwise a + # missing binary leaves headroom MCP/Serena/memory entries in the user's + # opencode config and an injected AGENTS.md, then errors with no cleanup -- + # the config-before-verify anti-pattern (#1614). Siblings (claude, codex, + # goose, omp) already check first. `--prepare-only` intentionally writes + # config without launching, so it is exempt. + opencode_bin: str | None = None + if not prepare_only: + opencode_bin = shutil.which("opencode") + if not opencode_bin: + click.echo("Error: 'opencode' not found in PATH.") + click.echo("Install OpenCode: https://opencode.ai") + raise SystemExit(1) + # Snapshot OpenCode config.json BEFORE any wrap-time mutation so # `headroom unwrap opencode` can restore the user's pre-wrap state. _opencode_config_file, _opencode_backup_file = opencode_config_paths() @@ -6987,11 +7001,9 @@ def opencode( inject_opencode_provider_config(port) return - opencode_bin = shutil.which("opencode") - if not opencode_bin: - click.echo("Error: 'opencode' not found in PATH.") - click.echo("Install OpenCode: https://opencode.ai") - raise SystemExit(1) + # Past the prepare-only return the launch path always ran the binary check + # above, so opencode_bin is resolved. + assert opencode_bin is not None # Register our proxy client marker BEFORE _ensure_proxy so that another # wrapper's cleanup sees us as an active client and doesn't terminate a diff --git a/tests/test_cli/test_wrap_opencode.py b/tests/test_cli/test_wrap_opencode.py index f7cdaf1e5..55914fcef 100644 --- a/tests/test_cli/test_wrap_opencode.py +++ b/tests/test_cli/test_wrap_opencode.py @@ -364,6 +364,35 @@ def test_wrap_opencode_missing_binary_errors_clearly( assert "'opencode' not found in PATH" in result.output +def test_wrap_opencode_missing_binary_does_not_mutate_config( + runner: CliRunner, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A missing opencode binary must not leave memory side-effects behind (#1614 class). + + The MCP/Serena registrations are already gated on ``registrar.detect()``, but + the ``--memory`` injections (AGENTS.md, the .headroom dir, the memory MCP + config) are not -- they ran unconditionally before the binary check. Verify + the binary first, like claude/codex/goose/omp, so an absent tool cannot write + those and then error with nothing launched. + """ + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("HEADROOM_CONTEXT_TOOL", raising=False) + _set_test_home(monkeypatch, tmp_path) + + agents_md = tmp_path / "AGENTS.md" + headroom_dir = tmp_path / ".headroom" + + with patch.object(wrap_mod.shutil, "which", return_value=None): + result = runner.invoke(main, ["wrap", "opencode", "--memory"]) + + assert result.exit_code == 1 + assert "'opencode' not found in PATH" in result.output + assert not agents_md.exists(), "AGENTS.md was created before the missing-binary check" + assert not headroom_dir.exists(), ".headroom dir was created before the missing-binary check" + + def test_wrap_opencode_prepare_only_injects_config( runner: CliRunner, tmp_path: Path,