mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-10 14:27:00 -04:00
## Description `headroom unwrap claude` currently removes Claude-local wrap state but can still leave Claude effectively routed through Headroom when the same port belongs to a managed persistent deployment. The command already knows how to discover same-port persistent manifests, but its stop path only kills the current pid and never uses that deployment metadata. This patch keeps the existing local settings cleanup, then applies an ownership-aware same-port audit: Claude-owned deployments are stopped through the install lifecycle path, while ambiguous same-port residue is surfaced with exact remediation instead of a false clean-success claim. Refs #2340. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [ ] Documentation update - [ ] Performance improvement - [ ] Code refactoring (no functional changes) ## Changes Made - extend the Claude unwrap stop path in `headroom/cli/wrap.py` to distinguish local pid stops, Claude-owned same-port persistent deployments, and ambiguous same-port residue - reuse the install lifecycle teardown path for Claude-owned persistent deployments instead of re-implementing supervisor cleanup - keep the existing Claude-local settings, hook, and base-url cleanup unchanged - add focused CLI regressions that prove a matching Claude-owned deployment is stopped during unwrap, ambiguous same-port residue is reported truthfully, and different-port manifests stay untouched ## Testing - [x] Unit tests pass (`uv run pytest tests/test_cli/test_unwrap_claude.py tests/test_cli/test_wrap_persistent.py -q`) - [x] Linting passes (`uv run ruff check headroom/cli/wrap.py tests/test_cli/test_unwrap_claude.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality when applicable - [x] Manual testing performed ### Test Output ```text uv run pytest tests/test_cli/test_unwrap_claude.py tests/test_cli/test_wrap_persistent.py -q ============================= 43 passed in 0.51s ============================== uv run pytest tests/test_cli/test_unwrap_claude.py -q ============================= 13 passed in 0.41s ============================== uv run pytest tests/test_cli/test_wrap_persistent.py -q ============================= 30 passed in 0.39s ============================== uv run ruff check headroom/cli/wrap.py tests/test_cli/test_unwrap_claude.py All checks passed! uv run ruff format headroom/cli/wrap.py tests/test_cli/test_unwrap_claude.py --check 2 files already formatted ``` ## Real Behavior Proof - Environment: Windows worktree `D:\Repos\headroom-pr-2340-claude-unwrap-effective-routing` with `uv sync --extra dev` - Exact command / steps: run the focused pytest and Ruff commands above, then run a constructed `CliRunner` replay against both `D:\Repos\headroom` and this branch with the same same-port Claude-owned manifest harness - Observed result: base prints `base: exit=0; local=[8787]; deactivated=[]; stopped=[]` and still routes through the pid-only helper; head prints `head: exit=0; local=[]; deactivated=['unwrap-2340']; stopped=['unwrap-2340']` and reports `Stopped Claude-owned persistent deployment 'unwrap-2340' on port 8787.` - Not tested: a live macOS launchd deployment on this host ## Review Readiness - [x] I have performed a self-review - [x] This PR is ready for human review ## Checklist - [x] My code follows the project's style guidelines - [x] I have performed a self-review of my code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] New and existing unit tests pass locally with my changes - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes `CHANGELOG.md` is not applicable because Headroom derives release notes from conventional commits. Scope stays below the broader uninstall workflow in open PR `#749`: this patch makes Claude unwrap truthful and ownership-aware, but it does not remove install artifacts or introduce a new uninstall command. --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
This commit is contained in:
Rod Boev
GitHub
parent
b75999017f
commit
cf5fa644b6
2 changed files with 254 additions and 3 deletions
|
|
@ -3186,6 +3186,143 @@ def _stop_local_proxy_for_unwrap(port: int) -> str:
|
|||
return "stopped" if _kill_proxy_by_pid(pid, port) else "failed"
|
||||
|
||||
|
||||
def _manifest_targets_claude(manifest: Any) -> bool:
|
||||
targets = getattr(manifest, "targets", None)
|
||||
if isinstance(targets, list) and any(
|
||||
str(target).strip().lower() == "claude" for target in targets
|
||||
):
|
||||
return True
|
||||
tool_envs = getattr(manifest, "tool_envs", None)
|
||||
if isinstance(tool_envs, dict) and any(
|
||||
str(name).strip().lower() == "claude" for name in tool_envs
|
||||
):
|
||||
return True
|
||||
mutations = getattr(manifest, "mutations", None)
|
||||
if isinstance(mutations, list):
|
||||
for mutation in mutations:
|
||||
if str(getattr(mutation, "target", "")).strip().lower() == "claude":
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _can_unwrap_stop_persistent_manifest(manifest: Any) -> bool:
|
||||
if not _manifest_targets_claude(manifest):
|
||||
return False
|
||||
supervisor_kind = str(getattr(manifest, "supervisor_kind", "")).strip().lower()
|
||||
return supervisor_kind in {"", "none", "service"}
|
||||
|
||||
|
||||
def _same_port_claude_env_keys(port: int) -> list[str]:
|
||||
matches: list[str] = []
|
||||
for key in (
|
||||
"ANTHROPIC_BASE_URL",
|
||||
"ANTHROPIC_FOUNDRY_BASE_URL",
|
||||
"ANTHROPIC_VERTEX_BASE_URL",
|
||||
):
|
||||
raw = os.environ.get(key, "").strip()
|
||||
if not raw:
|
||||
continue
|
||||
try:
|
||||
parsed = urllib.parse.urlparse(raw)
|
||||
except Exception:
|
||||
continue
|
||||
try:
|
||||
parsed_port = parsed.port
|
||||
except ValueError:
|
||||
continue
|
||||
if parsed_port != port:
|
||||
continue
|
||||
host = (parsed.hostname or "").strip().lower()
|
||||
if host not in {"127.0.0.1", "localhost", "::1"}:
|
||||
continue
|
||||
matches.append(key)
|
||||
return matches
|
||||
|
||||
|
||||
def _stop_persistent_manifest_for_claude_unwrap(manifest: Any) -> str | None:
|
||||
from headroom.cli.install import _deactivate_deployment_mutations, _stop_deployment
|
||||
|
||||
try:
|
||||
_deactivate_deployment_mutations(manifest)
|
||||
_stop_deployment(manifest)
|
||||
return None
|
||||
except Exception as exc:
|
||||
return str(exc)
|
||||
|
||||
|
||||
def _unwrap_claude_route_cleanup(port: int) -> dict[str, Any]:
|
||||
manifest = _find_persistent_manifest(port)
|
||||
env_keys = _same_port_claude_env_keys(port)
|
||||
if manifest is not None:
|
||||
if _can_unwrap_stop_persistent_manifest(manifest):
|
||||
error = _stop_persistent_manifest_for_claude_unwrap(manifest)
|
||||
if error is None:
|
||||
return {
|
||||
"kind": "persistent_stopped",
|
||||
"manifest": manifest,
|
||||
"env_keys": env_keys,
|
||||
}
|
||||
return {
|
||||
"kind": "persistent_failed",
|
||||
"manifest": manifest,
|
||||
"env_keys": env_keys,
|
||||
"error": error,
|
||||
}
|
||||
return {
|
||||
"kind": "persistent_residue",
|
||||
"manifest": manifest,
|
||||
"env_keys": env_keys,
|
||||
}
|
||||
return {
|
||||
"kind": "local",
|
||||
"status": _stop_local_proxy_for_unwrap(port),
|
||||
"env_keys": env_keys,
|
||||
}
|
||||
|
||||
|
||||
def _echo_claude_unwrap_route_cleanup(result: dict[str, Any], port: int) -> bool:
|
||||
kind = str(result.get("kind") or "")
|
||||
env_keys = [str(key) for key in result.get("env_keys", []) if isinstance(key, str)]
|
||||
clean = True
|
||||
if kind == "local":
|
||||
status = str(result.get("status") or "failed")
|
||||
_echo_unwrap_proxy_stop_status(status, port)
|
||||
clean = status in {"stopped", "not_running"}
|
||||
elif kind == "persistent_stopped":
|
||||
manifest = result["manifest"]
|
||||
click.echo(
|
||||
f" Stopped Claude-owned persistent deployment '{manifest.profile}' on port {port}."
|
||||
)
|
||||
elif kind == "persistent_residue":
|
||||
manifest = result["manifest"]
|
||||
click.echo(
|
||||
" Warning: same-port persistent deployment "
|
||||
f"'{manifest.profile}' still owns port {port}; left it running because it is not "
|
||||
"clearly Claude-targeted."
|
||||
)
|
||||
click.echo(f" To stop it, run `headroom install stop --profile {manifest.profile}`.")
|
||||
click.echo(
|
||||
f" To remove it completely, run `headroom install remove --profile {manifest.profile}`."
|
||||
)
|
||||
clean = False
|
||||
elif kind == "persistent_failed":
|
||||
manifest = result["manifest"]
|
||||
click.echo(
|
||||
" Warning: failed to stop Claude-owned persistent deployment "
|
||||
f"'{manifest.profile}' on port {port}: {result.get('error')}"
|
||||
)
|
||||
click.echo(f" Retry with `headroom install stop --profile {manifest.profile}`.")
|
||||
clean = False
|
||||
if env_keys:
|
||||
click.echo(
|
||||
" Warning: current shell still exports "
|
||||
+ ", ".join(env_keys)
|
||||
+ f" for port {port}; restart Claude and your shell or unset those variables."
|
||||
)
|
||||
clean = False
|
||||
return clean
|
||||
|
||||
|
||||
def _echo_unwrap_proxy_stop_status(status: str, port: int) -> None:
|
||||
"""Print a human-readable proxy stop result for unwrap commands."""
|
||||
|
||||
|
|
@ -4778,9 +4915,18 @@ def unwrap_claude(
|
|||
)
|
||||
|
||||
click.echo()
|
||||
click.echo("✓ Claude is no longer durably wrapped by Headroom.")
|
||||
if not no_stop_proxy:
|
||||
_echo_unwrap_proxy_stop_status(_stop_local_proxy_for_unwrap(port), port)
|
||||
clean_unwrap = True
|
||||
if no_stop_proxy:
|
||||
click.echo(" Kept proxy stop disabled (--no-stop-proxy).")
|
||||
clean_unwrap = False
|
||||
else:
|
||||
clean_unwrap = _echo_claude_unwrap_route_cleanup(_unwrap_claude_route_cleanup(port), port)
|
||||
if clean_unwrap:
|
||||
click.echo("✓ Claude is no longer durably wrapped by Headroom.")
|
||||
else:
|
||||
click.echo(
|
||||
" Claude local wrap settings were removed, but effective routing residue remains."
|
||||
)
|
||||
click.echo()
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -16,6 +16,11 @@ def runner() -> CliRunner:
|
|||
return CliRunner()
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _no_persistent_manifest(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setattr(wrap_cli, "_find_persistent_manifest", lambda _port: None)
|
||||
|
||||
|
||||
def test_remove_claude_rtk_hooks_preserves_unrelated_hooks(tmp_path: Path) -> None:
|
||||
settings = tmp_path / "settings.json"
|
||||
settings.write_text(
|
||||
|
|
@ -246,6 +251,106 @@ def test_unwrap_claude_restores_all_base_url_modes(runner: CliRunner) -> None:
|
|||
]
|
||||
|
||||
|
||||
def test_unwrap_claude_stops_claude_owned_persistent_deployment(
|
||||
runner: CliRunner,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
class Manifest:
|
||||
profile = "unwrap-2340"
|
||||
targets = ["claude"]
|
||||
tool_envs = {"claude": {"ANTHROPIC_BASE_URL": "http://127.0.0.1:8787"}}
|
||||
mutations: list[object] = []
|
||||
supervisor_kind = "service"
|
||||
|
||||
stopped: list[str] = []
|
||||
deactivated: list[str] = []
|
||||
|
||||
monkeypatch.setattr(wrap_cli, "_find_persistent_manifest", lambda port: Manifest())
|
||||
monkeypatch.setattr(
|
||||
"headroom.cli.install._deactivate_deployment_mutations",
|
||||
lambda manifest: deactivated.append(manifest.profile),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"headroom.cli.install._stop_deployment",
|
||||
lambda manifest: stopped.append(manifest.profile),
|
||||
)
|
||||
|
||||
with (
|
||||
patch("headroom.cli.wrap._stop_local_proxy_for_unwrap") as stop_local,
|
||||
):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["unwrap", "claude", "--keep-mcp", "--keep-rtk", "--port", "8787"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
stop_local.assert_not_called()
|
||||
assert deactivated == ["unwrap-2340"]
|
||||
assert stopped == ["unwrap-2340"]
|
||||
assert "Stopped Claude-owned persistent deployment 'unwrap-2340' on port 8787." in result.output
|
||||
assert "Claude is no longer durably wrapped by Headroom." in result.output
|
||||
|
||||
|
||||
def test_unwrap_claude_reports_ambiguous_same_port_persistent_deployment(
|
||||
runner: CliRunner,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
class Manifest:
|
||||
profile = "shared-proxy"
|
||||
targets = ["codex"]
|
||||
tool_envs = {"codex": {"OPENAI_BASE_URL": "http://127.0.0.1:8787"}}
|
||||
mutations: list[object] = []
|
||||
supervisor_kind = "service"
|
||||
|
||||
monkeypatch.setattr(wrap_cli, "_find_persistent_manifest", lambda port: Manifest())
|
||||
|
||||
with patch("headroom.cli.wrap._stop_local_proxy_for_unwrap") as stop_local:
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["unwrap", "claude", "--keep-mcp", "--keep-rtk", "--port", "8787"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
stop_local.assert_not_called()
|
||||
assert "same-port persistent deployment 'shared-proxy' still owns port 8787" in result.output
|
||||
assert "headroom install stop --profile shared-proxy" in result.output
|
||||
assert "Claude is no longer durably wrapped by Headroom." not in result.output
|
||||
|
||||
|
||||
def test_unwrap_claude_warns_about_same_port_inherited_env(
|
||||
runner: CliRunner,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setenv("ANTHROPIC_BASE_URL", "http://127.0.0.1:8787")
|
||||
|
||||
with patch("headroom.cli.wrap._stop_local_proxy_for_unwrap", return_value="stopped"):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["unwrap", "claude", "--keep-mcp", "--keep-rtk", "--port", "8787"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "current shell still exports ANTHROPIC_BASE_URL for port 8787" in result.output
|
||||
assert "Claude is no longer durably wrapped by Headroom." not in result.output
|
||||
|
||||
|
||||
def test_unwrap_claude_ignores_malformed_inherited_env_port(
|
||||
runner: CliRunner,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
monkeypatch.setenv("ANTHROPIC_BASE_URL", "http://127.0.0.1:notaport")
|
||||
|
||||
with patch("headroom.cli.wrap._stop_local_proxy_for_unwrap", return_value="stopped"):
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["unwrap", "claude", "--keep-mcp", "--keep-rtk", "--port", "8787"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
assert "current shell still exports ANTHROPIC_BASE_URL" not in result.output
|
||||
assert "Claude is no longer durably wrapped by Headroom." in result.output
|
||||
|
||||
|
||||
def test_remove_claude_rtk_hooks_removes_init_hooks_and_env(tmp_path: Path) -> None:
|
||||
settings = tmp_path / "settings.json"
|
||||
settings.write_text(
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue