mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
fix(wrap/opencode): unwrap removes the rtk block from AGENTS.md (#2025)
## Description `headroom wrap opencode` injects the marker-fenced rtk guidance block — "prefix shell commands with `rtk`" — into **both** instruction files (`headroom/cli/wrap.py`): ```python # wrap opencode project_agents = Path.cwd() / "AGENTS.md" _inject_rtk_instructions(project_agents, verbose=verbose) global_agents = _opencode_home_dir() / "AGENTS.md" _inject_rtk_instructions(global_agents, verbose=verbose) ``` But `unwrap_opencode` only restores the OpenCode config and cleans up MCP servers — it never removes that rtk block. So after `unwrap opencode`, both `AGENTS.md` files still contain the marker-fenced instruction, and a plain `opencode` launch keeps following "prefix shell commands with `rtk`" and fails once the managed rtk binary is off PATH. `unwrap_codex` (#1421) and `unwrap_copilot` both already do this cleanup via `_remove_rtk_instructions`; opencode was simply never given the equivalent — a wrap/unwrap asymmetry. Closes: no issue filed — found while auditing wrap/unwrap symmetry across agents. ## Fix In `unwrap_opencode`, after the MCP cleanup, strip the rtk block from both files it was injected into, mirroring `unwrap_codex`: ```python for _agents_md in (Path.cwd() / "AGENTS.md", _opencode_home_dir() / "AGENTS.md"): if _remove_rtk_instructions(_agents_md): click.echo(f" Removed Headroom rtk instructions from {_agents_md}.") ``` Best-effort and unconditional, matching the existing MCP cleanup and the codex/copilot unwrap paths. `_remove_rtk_instructions` already no-ops when the file or marker is absent. ## Type of Change - [x] Bug fix (non-breaking change that fixes an issue) ## Changes Made - `headroom/cli/wrap.py`: `unwrap_opencode` removes the rtk block from the project and global `AGENTS.md`. - `tests/test_cli/test_wrap_opencode.py`: add `test_unwrap_opencode_removes_rtk_from_agents_md` (wrap injects into both, unwrap removes from both). ## Testing - [x] New regression test added (`tests/test_cli/test_wrap_opencode.py`) - [x] Linting/formatting clean — run with the CI-pinned `ruff==0.15.17` - [ ] Full `pytest` deferred to CI (local-OOM reason below). ```text $ uvx ruff@0.15.17 check headroom/cli/wrap.py tests/test_cli/test_wrap_opencode.py All checks passed! ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.10, headroom from this branch. Importing `headroom` pulls in the torch/transformers stack and a full `pytest` gets OOM-killed on this box, so I verified the wrap→unwrap round-trip through the new Click-runner test (which drives the real command) and reasoned through the marker logic; the full pytest runs on CI. - Exact command / steps: the added test runs `wrap opencode --no-mcp` (asserts `_RTK_MARKER` present in both the project and global `AGENTS.md`), then `unwrap opencode`, and asserts the marker is gone from both. - Observed result (the assertions the test enforces): before the fix, `unwrap opencode` left `_RTK_MARKER` in both files; after the fix both are clean: ```text after wrap: _RTK_MARKER in project AGENTS.md ✓ _RTK_MARKER in global AGENTS.md ✓ after unwrap: _RTK_MARKER absent (project) ✓ _RTK_MARKER absent (global) ✓ ``` - Not tested: launching a real `opencode` binary (mocked in the test, as the existing wrap tests do). Full local `pytest` deferred to CI (OOM, per above). ## 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 - [ ] New and existing unit tests pass locally with my changes — ran lint + the new Click-runner test path; full pytest deferred to CI (local OOM, disclosed above) - [x] I have updated the CHANGELOG.md if applicable ## Additional Notes - Directly parallels the merged `unwrap codex` rtk cleanup (#1421); no new dependencies. - @JerrettDavis tagging you — same class as the codex rtk fix, just the opencode side that was missed. Thanks! --------- Co-authored-by: JerrettDavis <mxjerrett@gmail.com> Co-authored-by: Tejas Chopra <chopratejas@gmail.com>
This commit is contained in:
parent
6ecbdd6b52
commit
20968a4fa4
3 changed files with 48 additions and 0 deletions
|
|
@ -115,6 +115,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
* **proxy/vertex:** route Vertex `publisher=google` (Gemini) requests to the region matching the request path. `vertex_generate_content`, `vertex_stream_generate_content`, and `vertex_count_tokens` discarded the path's `location` and forwarded to the single fixed host from `_api_target(proxy, "vertex")` (default `us-central1`), instead of the region-aware `_vertex_target_for_location` the sibling Anthropic `rawPredict` route already uses. So a request to `.../locations/europe-west1/publishers/google/...` was sent to a `us-central1` host, which Vertex rejects on the region/host mismatch. The three google routes now derive the host from the request's `location` (operator-pinned upstreams are still honored).
|
||||
* **proxy/anthropic:** give each Anthropic conversation its own session id. `SessionTrackerStore.compute_session_id` derived its fallback id from `model` + system text harvested only from `role:"system"` entries inside `messages` — but Anthropic carries the system prompt as a top-level `body["system"]` field, so genuine Anthropic requests (which never carry `x-headroom-session-id`) collapsed to `md5(model:[])` and every conversation on the same model shared one `PrefixCacheTracker`. That let session-sticky state cross-contaminate: conversation A's sticky `headroom_retrieve`/memory tools and `anthropic-beta` headers were injected into conversation B, and frozen-prefix/compression-cache state mixed across conversations. The Anthropic handler now folds the top-level `system` into the session-id inputs (prepending a synthetic `role:"system"` message used only to derive the id), giving distinct conversations distinct ids.
|
||||
* **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.
|
||||
* **wrap/opencode:** `headroom unwrap opencode` now removes the Headroom rtk instruction block from the project and global `AGENTS.md`. `wrap opencode` injects the marker-fenced "prefix shell commands with `rtk`" guidance into both `./AGENTS.md` and `<opencode-home>/AGENTS.md`, but unwrap only restored the config and MCP state — so a plain `opencode` launch kept following the rtk guidance and failed once the managed rtk binary was off PATH. Unwrap now strips the marker-fenced block from both files, mirroring `unwrap codex` (#1421) and `unwrap copilot`.
|
||||
* **proxy/savings:** stop billing the $3/M fallback rate for genuinely free models. `_estimate_compression_savings_usd` and `_estimate_input_cost_usd` read `input_cost_per_token` from litellm and used `if not input_cost_per_token: raise`, which treats a legitimate `0.0` (a free / local / vendored-at-0 model that litellm does carry) as "price unavailable" and falls back to `DEFAULT_FALLBACK_INPUT_COST_PER_TOKEN` — fabricating dollar savings/cost for a model that costs nothing. Both now use an explicit `is None` check so a present `0.0` flows through as `$0` while a missing key still falls back.
|
||||
* **proxy/cost:** value prefix-cache savings with the most-used model's price, not the first-recorded one. `build_prefix_cache_stats` scanned `cost_tracker._tokens_sent_by_model` and broke on the *first* provider-matching model with a price — despite the "most-used model" comment — so a Claude Code session (Sonnet for the main loop, Haiku for titles/subagents) priced all of a provider's cache-read savings at whichever model happened to be recorded first. If Haiku ($0.80/M) came before Sonnet ($3/M), the dashboard understated cache savings ~3.75x (and vice-versa). It now picks the provider-matching, priced model with the highest token volume.
|
||||
* **proxy/openai:** stop overriding an explicit client `stream_options.include_usage` on the streaming chat path. To count tokens from the trailing usage chunk, the handler set `include_usage: True` unconditionally — including flipping an explicit client `false` to `true`. The upstream then appended a usage-only chunk (`choices: []`) the client never requested, and the common `chunk.choices[0].delta` loop raised `IndexError`. The option is now only filled in when the client left the choice open (no `stream_options`, or a dict without `include_usage`); an explicit `true`/`false` is respected.
|
||||
|
|
|
|||
|
|
@ -6180,6 +6180,17 @@ def unwrap_opencode(port: int, no_stop_proxy: bool) -> None:
|
|||
elif serena_status == "failed":
|
||||
click.echo(" Serena MCP server matched Headroom ledger but could not be removed.")
|
||||
|
||||
# `wrap opencode` injects the marker-fenced rtk guidance into both the project
|
||||
# `AGENTS.md` and the global `_opencode_home_dir() / "AGENTS.md"`; that block is
|
||||
# durable state the config restore above does not touch. Without removing it, a
|
||||
# plain `opencode` launch keeps following Headroom's "prefix shell commands with
|
||||
# rtk" instruction and fails when the managed rtk binary is off PATH. Mirror what
|
||||
# unwrap_codex / unwrap_copilot already do. Best-effort and unconditional, like
|
||||
# the MCP cleanup above.
|
||||
for _agents_md in (Path.cwd() / "AGENTS.md", _opencode_home_dir() / "AGENTS.md"):
|
||||
if _remove_rtk_instructions(_agents_md):
|
||||
click.echo(f" Removed Headroom rtk instructions from {_agents_md}.")
|
||||
|
||||
click.echo()
|
||||
click.echo("✓ OpenCode is no longer routed through the Headroom proxy.")
|
||||
if not no_stop_proxy and status != "noop":
|
||||
|
|
|
|||
|
|
@ -241,6 +241,42 @@ def test_wrap_opencode_injects_rtk_into_agents_md(
|
|||
assert wrap_mod._RTK_MARKER in project_agents.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_unwrap_opencode_removes_rtk_from_agents_md(
|
||||
runner: CliRunner,
|
||||
tmp_path: Path,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
"""unwrap opencode removes the rtk block that wrap opencode injected into both
|
||||
the project and global AGENTS.md — mirroring unwrap_codex / unwrap_copilot."""
|
||||
monkeypatch.chdir(tmp_path)
|
||||
monkeypatch.delenv("HEADROOM_CONTEXT_TOOL", raising=False)
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
|
||||
with patch.object(wrap_mod.shutil, "which", return_value="opencode"):
|
||||
with patch.object(wrap_mod, "_launch_tool", side_effect=SystemExit(0)):
|
||||
with patch.object(wrap_mod, "_ensure_rtk_binary", return_value=Path("/tmp/rtk")):
|
||||
runner.invoke(main, ["wrap", "opencode", "--port", "9000", "--no-mcp"])
|
||||
|
||||
global_agents = tmp_path / ".config" / "opencode" / "AGENTS.md"
|
||||
project_agents = tmp_path / "AGENTS.md"
|
||||
assert wrap_mod._RTK_MARKER in global_agents.read_text(encoding="utf-8")
|
||||
assert wrap_mod._RTK_MARKER in project_agents.read_text(encoding="utf-8")
|
||||
|
||||
with patch.object(wrap_mod, "_stop_local_proxy_for_unwrap", return_value="stopped"):
|
||||
result = runner.invoke(main, ["unwrap", "opencode"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
|
||||
# Both rtk blocks are gone after unwrap (previously left behind). A file that
|
||||
# held only the rtk block is removed entirely by _remove_rtk_instructions, so
|
||||
# treat a missing file as "block gone".
|
||||
def _rtk_absent(path: Path) -> bool:
|
||||
return not path.exists() or wrap_mod._RTK_MARKER not in path.read_text(encoding="utf-8")
|
||||
|
||||
assert _rtk_absent(global_agents)
|
||||
assert _rtk_absent(project_agents)
|
||||
|
||||
|
||||
def test_wrap_opencode_no_project_rtk_only_skips_project_agents_md(
|
||||
runner: CliRunner,
|
||||
tmp_path: Path,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue