mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(wrap): keep Claude context-tool setup explicit (#1999)
## Description `headroom wrap claude` currently installs RTK's global Claude hook and instruction imports on a flag-free launch, even though the wrapped session already routes through Headroom's proxy. The wrapper now requires an explicit Claude context-tool opt-in before it runs the existing RTK or lean-ctx setup path. Existing negative flags remain accepted, and other wrapped agents keep their current behavior. Closes #1915 ## 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 - Made Claude context-tool installation explicit instead of running it on every default wrap. - Preserved the existing RTK and lean-ctx installers behind the positive opt-in. - Kept `--no-context-tool` and `--no-rtk` compatible and left other agent wrappers unchanged. - Added focused command-parser coverage for default, opt-in, selector, and negative-space behavior. - Documented the changed default and opt-in command in `CHANGELOG.md`. ## Testing - [x] Unit tests pass (`uv run --no-project pytest tests/test_cli/test_wrap_helpers.py -q`) - [x] Linting passes (`uv run --no-project ruff check headroom/cli/wrap.py tests/test_cli/test_wrap_helpers.py`) - [ ] Type checking passes (`uv run mypy headroom`) - [x] New tests added for new functionality when applicable - [ ] Manual testing performed ### Test Output ```text uv run --no-project pytest tests/test_cli/test_wrap_helpers.py -q 65 passed uv run --no-project ruff check headroom/cli/wrap.py tests/test_cli/test_wrap_helpers.py All checks passed uv run --no-project ruff format --check headroom/cli/wrap.py tests/test_cli/test_wrap_helpers.py 2 files already formatted ``` ## Real Behavior Proof - Environment: isolated HOME on Linux or macOS, Python 3.12+, Claude CLI available. - Exact command / steps: run `headroom wrap claude --prepare-only` without a context-tool flag, inspect the isolated Claude config, then repeat with the explicit context-tool opt-in. - Observed result: the focused Click harness now proves the default run creates no RTK setup calls, the explicit opt-in performs the existing RTK setup, `--no-context-tool` still wins if both flags are present, and Copilot still keeps its default context-tool behavior. - Not tested: a live `headroom wrap claude` run against a real Claude installation and a real RTK or lean-ctx hook write on this host. - Scope: Claude context-tool activation and global configuration artifacts. ## 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 - [ ] I have commented my code, particularly in hard-to-understand areas - [x] 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 ## Screenshots (if applicable) N/A ## Additional Notes The exact project-bound `uv sync --extra dev` flow was blocked on this host by a `rustup.exe` access error, so the focused checks used `uv run --no-project` against the existing environment. This PR does not change RTK installation internals, proxy compression, or context-tool defaults for other agents.
This commit is contained in:
parent
e92c253977
commit
f536aa0801
4 changed files with 82 additions and 4 deletions
|
|
@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
* **cache/semantic:** key entries by the full-context hash, not the trailing query text. `SemanticCache.put` stored each response under `sha256(query)[:16]` where `query` is only the last user message, and the exact-match branch of `get` returned the slot without checking the stored entry's `messages_hash`. Two requests that share a trailing message ("continue", "yes", "run the tests") but differ in earlier context therefore collided on one slot — the second overwrote the first, and the first's hash then resolved to the second's cached response (wrong data served). Entries are now keyed by `messages_hash` when present, and `get` verifies `entry.messages_hash` before returning.
|
||||
* **proxy/openai:** stop PRE_SEND from reintroducing `tools: []` after the direct #728 fix. The OpenAI request handler now mirrors the existing `tools or _original_tools is not None` body-write guard during PRE_SEND write-back, so providers that reject empty tool arrays no longer see a tools field when the client omitted it, while explicit client `tools: []` remains preserved ([#1983](https://github.com/headroomlabs-ai/headroom/issues/1983)).
|
||||
* **proxy/openai:** keep the exact Responses function name `terminal` resident during OpenAI tool-search deferral so cache-mode optimization stops forwarding `terminal.terminal` and triggering the reserved-namespace 400 on Codex Responses ([#1946](https://github.com/headroomlabs-ai/headroom/issues/1946)).
|
||||
* **wrap:** `headroom wrap claude` no longer installs RTK or lean-ctx by default. Claude context-tool setup is now explicit via `--context-tool`, `--no-context-tool` remains accepted, and other wrap commands keep their current defaults ([#1915](https://github.com/headroomlabs-ai/headroom/issues/1915)).
|
||||
* **proxy/openai:** thread the savings-profile kwargs into the live `/v1/chat/completions` compression path. The chat handler called `openai_pipeline.apply()` without `proxy_pipeline_kwargs(config)`, so `HEADROOM_SAVINGS_PROFILE=agent-90` (and the individual `compress_user_messages`/`target_ratio`/`min_tokens_to_compress`/... knobs) were silently dropped — OpenAI-compatible clients like OpenCode kept protecting user messages and missed the configured profile. Both the token-mode and non-token chat branches now pass the profile kwargs, matching `handlers/anthropic.py` and the dedicated OpenAI compress endpoint ([#1534](https://github.com/headroomlabs-ai/headroom/issues/1534)).
|
||||
* **proxy:** forward Codex Desktop `/v1/responses` posts byte-faithfully so they stop returning upstream `400 {"detail":"Bad Request"}`. `handle_openai_responses` decoded the inbound body to inspect it but always re-serialized a canonical body on the way out, and it never stripped the inbound `content-encoding` header — so a `content-encoding: zstd` Codex Desktop request was forwarded as already-decoded JSON still advertising `zstd`, and the upstream ChatGPT Codex endpoint rejected it. The handler now keeps the original decoded bytes and forwards them verbatim whenever nothing (compression or memory injection) mutated the request, and drops the stale `content-encoding` header, mirroring the byte-faithful passthrough the chat and Anthropic paths already use ([#1542](https://github.com/headroomlabs-ai/headroom/issues/1542)).
|
||||
* **wrap/codex:** `headroom unwrap codex` now removes the Headroom rtk instruction block from the Codex global `AGENTS.md`. `wrap codex` injects it there, but unwrap only restored `config.toml` and MCP state, so a plain `codex` launch kept following the "prefix shell commands with `rtk`" guidance and failed once the managed rtk binary was off PATH. Unwrap now strips the marker-fenced block (preserving the rest of the file), mirroring `unwrap copilot` ([#1421](https://github.com/headroomlabs-ai/headroom/issues/1421)).
|
||||
|
|
|
|||
|
|
@ -3755,6 +3755,12 @@ def unwrap() -> None:
|
|||
is_flag=True,
|
||||
help="Skip CLI context-tool setup",
|
||||
)
|
||||
@click.option(
|
||||
"--context-tool",
|
||||
"context_tool",
|
||||
is_flag=True,
|
||||
help="Enable CLI context-tool setup",
|
||||
)
|
||||
@click.option(
|
||||
"--no-mcp",
|
||||
is_flag=True,
|
||||
|
|
@ -3824,6 +3830,7 @@ def unwrap() -> None:
|
|||
def claude(
|
||||
port: int,
|
||||
no_rtk: bool,
|
||||
context_tool: bool,
|
||||
no_mcp: bool,
|
||||
no_tokensave: bool,
|
||||
serena: bool,
|
||||
|
|
@ -3855,13 +3862,15 @@ def claude(
|
|||
headroom wrap claude # tokensave code graph (primary)
|
||||
headroom wrap claude --no-tokensave # Skip tokensave; fall back to Serena
|
||||
headroom wrap claude --serena # Also register the Serena backup
|
||||
headroom wrap claude --context-tool # Enable CLI context-tool setup
|
||||
headroom wrap claude --no-context-tool # Skip CLI context-tool setup
|
||||
headroom wrap claude --no-mcp # Skip MCP retrieve tool registration
|
||||
headroom wrap claude --no-serena # Never register the Serena backup
|
||||
headroom wrap claude --1m # Preserve the 1M context window
|
||||
"""
|
||||
setup_context_tool = context_tool and not no_rtk
|
||||
if prepare_only:
|
||||
if not no_rtk:
|
||||
if setup_context_tool:
|
||||
if _selected_context_tool() == _CONTEXT_TOOL_LEAN_CTX:
|
||||
_setup_lean_ctx_agent("claude", verbose=verbose)
|
||||
else:
|
||||
|
|
@ -3984,7 +3993,7 @@ def claude(
|
|||
port_holder[0] = actual_port
|
||||
_push_runtime_env(actual_port, no_proxy)
|
||||
|
||||
if not no_rtk:
|
||||
if setup_context_tool:
|
||||
if _selected_context_tool() == _CONTEXT_TOOL_LEAN_CTX:
|
||||
click.echo(" Setting up lean-ctx...")
|
||||
_setup_lean_ctx_agent("claude", verbose=verbose)
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ def test_wrap_claude_prepare_only_skips_host_binary_lookup() -> None:
|
|||
|
||||
with patch("headroom.cli.wrap._prepare_wrap_rtk") as prepare_rtk:
|
||||
with patch("headroom.cli.wrap.shutil.which") as which_mock:
|
||||
result = runner.invoke(main, ["wrap", "claude", "--prepare-only"])
|
||||
result = runner.invoke(main, ["wrap", "claude", "--prepare-only", "--context-tool"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
prepare_rtk.assert_called_once()
|
||||
|
|
@ -48,7 +48,7 @@ def test_wrap_claude_prepare_only_uses_lean_ctx_when_configured(monkeypatch) ->
|
|||
"headroom.cli.wrap._setup_lean_ctx_agent",
|
||||
return_value=Path("lean-ctx"),
|
||||
) as setup:
|
||||
result = runner.invoke(main, ["wrap", "claude", "--prepare-only"])
|
||||
result = runner.invoke(main, ["wrap", "claude", "--prepare-only", "--context-tool"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
prepare_rtk.assert_not_called()
|
||||
|
|
|
|||
|
|
@ -19,6 +19,7 @@ import subprocess
|
|||
import sys
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
from unittest.mock import patch
|
||||
|
||||
import click
|
||||
import pytest
|
||||
|
|
@ -26,6 +27,7 @@ from click.testing import CliRunner
|
|||
|
||||
from headroom import paths as paths_mod
|
||||
from headroom.cli import wrap as wrap_mod
|
||||
from headroom.cli.main import main
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _print_wrap_banner — centering math + box drawing.
|
||||
|
|
@ -104,6 +106,72 @@ def test_print_wrap_banner_title_is_centered_or_near_centered() -> None:
|
|||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def test_claude_context_tool_is_opt_in_for_prepare_only(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Claude skips context-tool setup unless the positive flag is passed."""
|
||||
monkeypatch.delenv("HEADROOM_CONTEXT_TOOL", raising=False)
|
||||
runner = CliRunner()
|
||||
|
||||
with patch.object(wrap_mod, "_prepare_wrap_rtk") as prepare_rtk:
|
||||
default = runner.invoke(main, ["wrap", "claude", "--prepare-only"])
|
||||
opt_in = runner.invoke(main, ["wrap", "claude", "--prepare-only", "--context-tool"])
|
||||
|
||||
assert default.exit_code == 0, default.output
|
||||
assert opt_in.exit_code == 0, opt_in.output
|
||||
assert prepare_rtk.call_count == 1
|
||||
|
||||
|
||||
def test_claude_context_tool_opt_in_preserves_lean_ctx_selection(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The positive flag enables the configured lean-ctx installer."""
|
||||
monkeypatch.setenv("HEADROOM_CONTEXT_TOOL", "lean-ctx")
|
||||
runner = CliRunner()
|
||||
|
||||
with patch.object(wrap_mod, "_setup_lean_ctx_agent") as setup_lean_ctx:
|
||||
result = runner.invoke(main, ["wrap", "claude", "--prepare-only", "--context-tool"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
setup_lean_ctx.assert_called_once_with("claude", verbose=False)
|
||||
|
||||
|
||||
def test_claude_no_context_tool_wins_over_context_tool(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""The legacy opt-out remains authoritative when both flags are supplied."""
|
||||
runner = CliRunner()
|
||||
|
||||
with patch.object(wrap_mod, "_prepare_wrap_rtk") as prepare_rtk:
|
||||
result = runner.invoke(
|
||||
main,
|
||||
["wrap", "claude", "--prepare-only", "--context-tool", "--no-context-tool"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
prepare_rtk.assert_not_called()
|
||||
|
||||
|
||||
def test_non_claude_context_tool_setup_remains_default(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""Copilot still sets up RTK without a new positive opt-in flag."""
|
||||
monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-test-dummy")
|
||||
|
||||
with (
|
||||
patch.object(wrap_mod.shutil, "which", return_value="copilot"),
|
||||
patch.object(wrap_mod, "_ensure_rtk_binary", return_value=Path("/tmp/rtk")) as ensure_rtk,
|
||||
patch.object(wrap_mod, "_launch_tool"),
|
||||
):
|
||||
result = CliRunner().invoke(
|
||||
main,
|
||||
["wrap", "copilot", "--no-proxy", "--", "--model", "claude-sonnet-4-20250514"],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
ensure_rtk.assert_called_once_with(verbose=False)
|
||||
|
||||
|
||||
def test_setup_context_tool_lean_ctx_calls_lean_ctx_setup(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
"""When HEADROOM_CONTEXT_TOOL=lean-ctx, helper calls _setup_lean_ctx_agent."""
|
||||
monkeypatch.setenv("HEADROOM_CONTEXT_TOOL", "lean-ctx")
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue