mirror of
https://github.com/headroomlabs-ai/headroom.git
synced 2026-08-27 14:17:10 -04:00
## Description `headroom wrap cursor` unconditionally injected an `rtk`-usage instructions block into `.cursorrules`. rtk itself supports a native hook for Cursor (`rtk init --agent cursor`) — the same registration mechanism headroom already uses for Claude Code — which rewrites shell commands transparently with zero custom-instructions text needed. Headroom never tried that path for Cursor, so users got a redundant `.cursorrules` file duplicating guidance the native hook already provides silently. A follow-up commit hardens the switch: `register_agent_hooks` returns `True` on rtk exit 0, but some rtk builds exit 0 without writing `~/.cursor/hooks.json`. headroom now trusts the on-disk hook file, not the exit code, before skipping the `.cursorrules` fallback. Closes #756 ## 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 - `headroom/rtk/installer.py`: generalized `register_claude_hooks` into `register_agent_hooks(rtk_path, *, agent="claude")`, which passes `--agent <agent>` to `rtk init` for non-Claude agents. `register_claude_hooks` kept as a thin wrapper for backward compatibility. Added `RTK_NATIVE_HOOK_AGENTS` documenting which agents rtk supports a native hook for. - `headroom/cli/wrap.py`: `wrap cursor` now calls `register_agent_hooks(rtk_path, agent="cursor")` first, and only skips the `.cursorrules` fallback when `~/.cursor/hooks.json` is actually on disk; otherwise it falls back to `_inject_rtk_instructions(...)`. - Tests: `tests/test_rtk_installer.py` and `tests/test_cli/test_wrap_bridge.py` cover the native-hook path, the on-disk verification, and the `.cursorrules` fallback. - `CHANGELOG.md`: added an entry under `## Unreleased` / `### Fixed`. ## Testing - [x] Unit tests pass (`pytest`) - [x] Linting passes (`ruff check .`) - [x] Type checking passes (`mypy headroom`) - [x] New tests added for new functionality - [x] Manual testing performed ### Test Output ```text $ python -m ruff format --check headroom/ tests/ e2e/ 953 files already formatted $ python -m ruff check headroom/cli/wrap.py headroom/rtk/installer.py tests/test_cli/test_wrap_bridge.py tests/test_rtk_installer.py All checks passed! $ python -m pytest tests/test_cli/test_wrap_bridge.py -k cursor -q 3 passed ``` ## Real Behavior Proof - Environment: Windows 11, Python 3.13, local checkout; `python -m pytest` / `ruff` run directly. - Exact command / steps: `python -m pytest tests/test_cli/test_wrap_bridge.py -k cursor -q` — the first test mocks `register_agent_hooks` to write `~/.cursor/hooks.json` and asserts `.cursorrules` is NOT created; the second mocks it to write nothing and asserts `.cursorrules` IS created with the `headroom:rtk-instructions` marker; the third exercises the explicit registration-failure fallback. - Observed result: `3 passed`. Native-hook path skips `.cursorrules` only when the hook file exists on disk; every other outcome falls back to `.cursorrules`, so Cursor always gets RTK guidance. - Not tested: real `rtk` binary writing `~/.cursor/hooks.json` end-to-end — that path is covered by the `docker-wrap-e2e` CI job, not locally. ## 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 ## Screenshots (if applicable) N/A — CLI-only change. ## Additional Notes Scope: rtk's native-hook-capable agents include `claude`, `cursor`, `windsurf`, `cline`, `kilocode`, `antigravity`, `pi`, `hermes`, but only `cursor` and `claude` have a corresponding `headroom wrap` subcommand today, so this fix only changes `wrap cursor` behavior. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Parideboy
GitHub
parent
cfcd40f8ac
commit
1573f1fd07
6 changed files with 203 additions and 11 deletions
73
CHANGELOG.md
73
CHANGELOG.md
|
|
@ -8,6 +8,79 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
|||
|
||||
## Unreleased
|
||||
|
||||
### Fixed
|
||||
- `headroom wrap cursor` no longer injects the `rtk` custom-instructions
|
||||
block into `.cursorrules` when rtk's own native Cursor hook registers
|
||||
successfully. rtk supports a real hook for Cursor via
|
||||
`rtk init --agent cursor` (the same mechanism headroom already uses for
|
||||
Claude Code), which rewrites shell commands transparently — the injected
|
||||
`.cursorrules` text duplicated that guidance for no benefit. `wrap cursor`
|
||||
now tries the native hook first and only falls back to injecting
|
||||
`.cursorrules` if hook registration fails (#756).
|
||||
- `headroom wrap claude` no longer leaves a dead `ANTHROPIC_BASE_URL` in a
|
||||
project's `.claude/settings.local.json` after an unclean exit (`SIGKILL`,
|
||||
OOM, reboot, or terminal/tmux close via `SIGHUP`, which was not caught).
|
||||
`_write_claude_wrap_base_url`/`_restore_claude_wrap_base_url` only removed
|
||||
or restored the entry from the wrap process's own `finally` block, so a
|
||||
crash skipped it and every later bare `claude` invocation in that project
|
||||
inherited the stale proxy URL and hung indefinitely retrying a dead port.
|
||||
A wrap session now stamps a sidecar marker (pid, port, prior value); the
|
||||
next `wrap`, `unwrap`, or `headroom doctor` run detects a marker whose pid
|
||||
is dead or reused and restores the recorded prior value automatically.
|
||||
`claude()` also now catches `SIGHUP` alongside the existing `SIGTERM`
|
||||
handler ([#1768](https://github.com/headroomlabs-ai/headroom/issues/1768)).
|
||||
- Non-finite values (`NaN`, `Infinity`) in `proxy_savings.json` or in upstream
|
||||
cost/token metadata no longer crash the proxy or corrupt the savings
|
||||
dashboard. `SavingsTracker`'s numeric coercion caught only `TypeError` and
|
||||
`ValueError`, so `int(float('inf'))` raised an uncaught `OverflowError` while
|
||||
loading persisted state (`SavingsTracker.__init__` failed and the proxy would
|
||||
not start), and `float('nan')`/`float('inf')` passed straight through, then
|
||||
serialized to `NaN`/`Infinity` literals that the dashboard's `JSON.parse`
|
||||
rejects. `json.loads` accepts those literals, so one bad write poisoned every
|
||||
later start. Both coercion helpers now also catch `OverflowError` and reject
|
||||
non-finite floats, failing open to safe defaults.
|
||||
- `headroom learn` now honors `CLAUDE_CONFIG_DIR`. It resolved the Claude
|
||||
config directory as `~/.claude` and wrote global memory to
|
||||
`~/.claude/CLAUDE.md`, so users who relocate their Claude config via that
|
||||
env var had `learn` scan the wrong directory and detect no projects. The
|
||||
scanner and memory writer now read/write the configured directory
|
||||
([#1630](https://github.com/headroomlabs-ai/headroom/issues/1630)).
|
||||
- `--backend bedrock` now fails fast with an actionable error when temporary
|
||||
AWS credentials (`AWS_SESSION_TOKEN`) are used but botocore is not installed
|
||||
(e.g. the slim default Docker image). litellm's session-token auth path
|
||||
imports botocore, so the missing dependency previously surfaced only at
|
||||
request time as a misleading `authentication_error: No module named
|
||||
'botocore'`. The proxy now tells the user to install the `bedrock` extra up
|
||||
front ([#1551](https://github.com/headroomlabs-ai/headroom/issues/1551)).
|
||||
- Content detection no longer crashes the proxy on text containing an
|
||||
orphaned `+++ ` target line with no preceding `--- ` source line (common in
|
||||
`set -x` xtrace output and partial diffs). The bundled `unidiff` 0.4.0 parser
|
||||
panics on that input instead of returning an error; the Rust diff detector now
|
||||
contains the panic and treats the fragment as plain text, so the request is
|
||||
compressed and forwarded normally instead of returning HTTP 500
|
||||
([#1547](https://github.com/headroomlabs-ai/headroom/issues/1547)).
|
||||
- Proactive expansion blocks injected into user turns are now wrapped in
|
||||
`<headroom_proactive_expansion>` XML tags, giving downstream consumers
|
||||
(LLMs, loggers, attribution parsers) a machine-readable provenance
|
||||
boundary and preventing misattribution in multi-agent threads.
|
||||
- **cli:** the startup banner no longer advertises
|
||||
`HEADROOM_COMPRESSION_STABLE_AFTER_TURN` and
|
||||
`HEADROOM_STALE_READ_COMPRESS_AFTER_TURNS` as tuning knobs. Both were read
|
||||
only to render the `Performance Tuning` banner section and were never wired
|
||||
into the compression path, so setting them changed the banner but had no
|
||||
effect on behavior. The banner now surfaces only the embedding sidecar,
|
||||
which is a real, consumed setting.
|
||||
- **memory/embedder:** cap CPU thread oversubscription in the local
|
||||
torch/sentence-transformers embedder. Concurrent encodes previously each
|
||||
fanned out to ~`os.cpu_count()` BLAS/OpenMP threads, so under load the memory
|
||||
path starved the asyncio event loop and spiked `/livez` latency to several
|
||||
seconds. CPU encodes now run on a dedicated, size-limited executor whose
|
||||
workers each pin their thread pool, bounding total embedding threads to
|
||||
`HEADROOM_EMBED_CONCURRENCY` × `HEADROOM_EMBED_NUM_THREADS` (defaults
|
||||
`min(4, cpu)` × 1). The ONNX embedder already capped its threads; this brings
|
||||
the torch path to parity
|
||||
([#198](https://github.com/headroomlabs-ai/headroom/issues/198)).
|
||||
|
||||
### Changed
|
||||
|
||||
* **telemetry:** anonymous usage telemetry is now **opt-in** (off by default) instead of opt-out. Nothing is collected or sent unless you set `HEADROOM_TELEMETRY=on` or pass `--telemetry` to `headroom proxy` / `headroom install apply`. `is_telemetry_enabled()` is fail-closed — only explicit on-values (`on`/`true`/`1`/`yes`/`enable`/`enabled`) enable it; unset, empty, or unrecognized values stay disabled. The existing `--no-telemetry` flag and `HEADROOM_TELEMETRY=off` remain accepted for back-compat, and install manifests now write the `HEADROOM_TELEMETRY` value explicitly so generated deployments are unambiguous.
|
||||
|
|
|
|||
|
|
@ -715,12 +715,24 @@ def verify_cursor_wrap(base_env: dict[str, str], project_dir: Path) -> None:
|
|||
"Cursor wrap should print the Anthropic base URL override",
|
||||
)
|
||||
wait_for_http(f"http://127.0.0.1:{port}/health", timeout=15)
|
||||
# rtk registers a native Cursor hook (rtk init --agent cursor) when it
|
||||
# can (~/.cursor exists); headroom only falls back to injecting
|
||||
# .cursorrules text if that registration fails (GH #756). Accept
|
||||
# either outcome rather than assuming the fallback path.
|
||||
cursorrules = project_dir / ".cursorrules"
|
||||
assert_true(cursorrules.exists(), "Cursor wrap should create .cursorrules")
|
||||
assert_true(
|
||||
RTK_MARKER in cursorrules.read_text(encoding="utf-8"),
|
||||
"Cursor wrap should inject RTK instructions",
|
||||
cursor_hooks_json = Path(base_env["HOME"]) / ".cursor" / "hooks.json"
|
||||
native_hook_registered = (
|
||||
cursor_hooks_json.exists() and "rtk" in cursor_hooks_json.read_text(encoding="utf-8")
|
||||
)
|
||||
if not native_hook_registered:
|
||||
assert_true(
|
||||
cursorrules.exists(),
|
||||
"Cursor wrap should create .cursorrules when the native rtk hook is unavailable",
|
||||
)
|
||||
assert_true(
|
||||
RTK_MARKER in cursorrules.read_text(encoding="utf-8"),
|
||||
"Cursor wrap should inject RTK instructions",
|
||||
)
|
||||
finally:
|
||||
stop_process(proc)
|
||||
|
||||
|
|
|
|||
|
|
@ -4970,14 +4970,32 @@ def cursor(
|
|||
headroom wrap cursor --port 9999 # Custom proxy port
|
||||
"""
|
||||
cursorrules: Path | None = Path.cwd() / ".cursorrules" if not no_rtk else None
|
||||
cursor_hook_registered = False
|
||||
if not no_rtk:
|
||||
|
||||
def _register_cursor_hook(rtk_path: Path) -> None:
|
||||
# rtk registers a native hook for Cursor (`rtk init --agent cursor`),
|
||||
# same mechanism as Claude Code. Prefer that over injecting the
|
||||
# RTK_INSTRUCTIONS_BLOCK text into .cursorrules — a silent hook makes
|
||||
# the custom-rules text redundant guidance (GH #756).
|
||||
nonlocal cursor_hook_registered
|
||||
from headroom.rtk.installer import register_agent_hooks
|
||||
|
||||
# rtk may exit 0 without writing hooks.json (e.g. an rtk build that
|
||||
# doesn't support --agent cursor), so trust the file, not the exit
|
||||
# code: only skip the .cursorrules fallback if the native hook is
|
||||
# actually on disk (GH #756).
|
||||
cursor_hooks_json = Path.home() / ".cursor" / "hooks.json"
|
||||
if register_agent_hooks(rtk_path, agent="cursor") and cursor_hooks_json.is_file():
|
||||
cursor_hook_registered = True
|
||||
else:
|
||||
_inject_rtk_instructions(cast(Path, cursorrules), verbose=verbose)
|
||||
|
||||
_setup_context_tool_for_agent(
|
||||
agent="cursor",
|
||||
agent_display="Cursor",
|
||||
marker_path=cursorrules,
|
||||
on_rtk_ready=lambda _rtk: _inject_rtk_instructions(
|
||||
cast(Path, cursorrules), verbose=verbose
|
||||
),
|
||||
on_rtk_ready=_register_cursor_hook,
|
||||
verbose=verbose,
|
||||
)
|
||||
|
||||
|
|
@ -4991,6 +5009,8 @@ def cursor(
|
|||
click.echo()
|
||||
if _selected_context_tool() == _CONTEXT_TOOL_LEAN_CTX:
|
||||
click.echo(" lean-ctx configured for Cursor")
|
||||
elif cursor_hook_registered:
|
||||
click.echo(" rtk hook registered for Cursor")
|
||||
else:
|
||||
click.echo(" rtk instructions injected into .cursorrules")
|
||||
click.echo(" Cursor will use token-optimized commands automatically.")
|
||||
|
|
|
|||
|
|
@ -160,15 +160,39 @@ def download_rtk(version: str | None = None) -> Path:
|
|||
return target_path
|
||||
|
||||
|
||||
# Agents rtk registers a *native* hook for via `rtk init --agent <name>`.
|
||||
# For these, headroom must not also inject the RTK_INSTRUCTIONS_BLOCK text
|
||||
# into a rules/instructions file — that duplicates guidance rtk's own hook
|
||||
# already provides silently (GH #756).
|
||||
RTK_NATIVE_HOOK_AGENTS = frozenset(
|
||||
{"claude", "cursor", "windsurf", "cline", "kilocode", "antigravity", "pi", "hermes"}
|
||||
)
|
||||
|
||||
|
||||
def register_claude_hooks(rtk_path: Path | None = None) -> bool:
|
||||
"""Register rtk hooks in Claude Code settings.
|
||||
|
||||
Runs `rtk init --global` which adds a PreToolUse hook to
|
||||
~/.claude/settings.json that rewrites Bash commands through rtk.
|
||||
|
||||
Returns True if hooks were registered successfully.
|
||||
"""
|
||||
return register_agent_hooks(rtk_path, agent="claude")
|
||||
|
||||
|
||||
def register_agent_hooks(rtk_path: Path | None = None, *, agent: str = "claude") -> bool:
|
||||
"""Register rtk's native hook for ``agent`` via ``rtk init --agent``.
|
||||
|
||||
Only agents in ``RTK_NATIVE_HOOK_AGENTS`` support this; callers must not
|
||||
invoke this for agents rtk has no native hook for (rtk itself will just
|
||||
reject the ``--agent`` value).
|
||||
|
||||
Returns True if hooks were registered successfully.
|
||||
"""
|
||||
rtk_path = rtk_path or RTK_BIN_PATH
|
||||
args = [str(rtk_path), "init", "--global", "--auto-patch"]
|
||||
if agent != "claude":
|
||||
args += ["--agent", agent]
|
||||
|
||||
# Capture output to a temp file rather than pipes: `rtk init` may fork a
|
||||
# background process that inherits our stdout/stderr, and a piped
|
||||
|
|
@ -181,7 +205,7 @@ def register_claude_hooks(rtk_path: Path | None = None) -> bool:
|
|||
with tempfile.TemporaryFile(mode="w+", encoding="utf-8", errors="replace") as out:
|
||||
try:
|
||||
result = subprocess.run(
|
||||
[str(rtk_path), "init", "--global", "--auto-patch"],
|
||||
args,
|
||||
stdin=subprocess.DEVNULL,
|
||||
stdout=out,
|
||||
stderr=out,
|
||||
|
|
@ -195,7 +219,7 @@ def register_claude_hooks(rtk_path: Path | None = None) -> bool:
|
|||
logger.warning("rtk init timed out: %s", out.read().strip())
|
||||
return False
|
||||
if result.returncode == 0:
|
||||
logger.info("rtk hooks registered in Claude Code")
|
||||
logger.info("rtk hooks registered for %s", agent)
|
||||
return True
|
||||
out.seek(0)
|
||||
logger.warning("rtk init failed: %s", out.read().strip())
|
||||
|
|
|
|||
|
|
@ -155,12 +155,43 @@ def test_wrap_aider_prepare_only_injects_conventions(monkeypatch, tmp_path: Path
|
|||
assert "headroom:rtk-instructions" in conventions.read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def test_wrap_cursor_prepare_only_injects_cursorrules(monkeypatch, tmp_path: Path) -> None:
|
||||
def test_wrap_cursor_prepare_only_registers_native_hook(monkeypatch, tmp_path: Path) -> None:
|
||||
# GH #756: when rtk's own `--agent cursor` hook registers successfully,
|
||||
# headroom must not also inject RTK_INSTRUCTIONS_BLOCK into .cursorrules.
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
runner = CliRunner()
|
||||
|
||||
# headroom trusts the on-disk hook, not rtk's exit code, so simulate rtk
|
||||
# actually writing ~/.cursor/hooks.json when registration succeeds.
|
||||
def _register(_rtk_path, *, agent):
|
||||
hooks = tmp_path / ".cursor" / "hooks.json"
|
||||
hooks.parent.mkdir(parents=True, exist_ok=True)
|
||||
hooks.write_text('{"hooks": {"preToolUse": [{"command": "rtk hook cursor"}]}}')
|
||||
return True
|
||||
|
||||
with runner.isolated_filesystem(temp_dir=str(tmp_path)):
|
||||
with (
|
||||
patch("headroom.cli.wrap._ensure_rtk_binary", return_value=Path("rtk")),
|
||||
patch("headroom.rtk.installer.register_agent_hooks", side_effect=_register) as register,
|
||||
):
|
||||
result = runner.invoke(main, ["wrap", "cursor", "--prepare-only"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
register.assert_called_once_with(Path("rtk"), agent="cursor")
|
||||
assert not Path(".cursorrules").exists()
|
||||
|
||||
|
||||
def test_wrap_cursor_prepare_only_falls_back_to_cursorrules_when_hook_fails(
|
||||
monkeypatch, tmp_path: Path
|
||||
) -> None:
|
||||
_set_test_home(monkeypatch, tmp_path)
|
||||
runner = CliRunner()
|
||||
|
||||
with runner.isolated_filesystem(temp_dir=str(tmp_path)):
|
||||
with patch("headroom.cli.wrap._ensure_rtk_binary", return_value=Path("rtk")):
|
||||
with (
|
||||
patch("headroom.cli.wrap._ensure_rtk_binary", return_value=Path("rtk")),
|
||||
patch("headroom.rtk.installer.register_agent_hooks", return_value=False),
|
||||
):
|
||||
result = runner.invoke(main, ["wrap", "cursor", "--prepare-only"])
|
||||
|
||||
assert result.exit_code == 0, result.output
|
||||
|
|
|
|||
|
|
@ -72,3 +72,35 @@ def test_register_claude_hooks_survives_forked_daemon(tmp_path: Path) -> None:
|
|||
fake_rtk.chmod(fake_rtk.stat().st_mode | stat.S_IEXEC)
|
||||
|
||||
assert installer.register_claude_hooks(fake_rtk) is True
|
||||
|
||||
|
||||
def test_register_agent_hooks_passes_agent_flag_for_non_claude(tmp_path: Path, monkeypatch) -> None:
|
||||
calls: list[list[str]] = []
|
||||
|
||||
class FakeResult:
|
||||
returncode = 0
|
||||
|
||||
def fake_run(args, **kwargs):
|
||||
calls.append(args)
|
||||
return FakeResult()
|
||||
|
||||
monkeypatch.setattr(installer.subprocess, "run", fake_run)
|
||||
|
||||
assert installer.register_agent_hooks(Path("rtk"), agent="cursor") is True
|
||||
assert calls == [["rtk", "init", "--global", "--auto-patch", "--agent", "cursor"]]
|
||||
|
||||
|
||||
def test_register_agent_hooks_omits_agent_flag_for_claude(tmp_path: Path, monkeypatch) -> None:
|
||||
calls: list[list[str]] = []
|
||||
|
||||
class FakeResult:
|
||||
returncode = 0
|
||||
|
||||
def fake_run(args, **kwargs):
|
||||
calls.append(args)
|
||||
return FakeResult()
|
||||
|
||||
monkeypatch.setattr(installer.subprocess, "run", fake_run)
|
||||
|
||||
assert installer.register_agent_hooks(Path("rtk"), agent="claude") is True
|
||||
assert calls == [["rtk", "init", "--global", "--auto-patch"]]
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue