fix(rtk): stop hook registration timing out on a forked daemon (#1314)

## Description

Every `headroom wrap claude` launch was printing this:

```
Failed to register rtk hooks: Command '[..., 'rtk', 'init', '--global', '--auto-patch']' timed out after 10 seconds
rtk hook registration failed — continuing without it
```

Run that exact `rtk init --global --auto-patch` by hand and it finishes
instantly and registers the hooks fine. The hang only happens through
`register_claude_hooks`, and when it does it always burns the full 10
seconds.

It's the pipes. `rtk init` forks a background process that inherits our
`stdout`/`stderr`, and `subprocess.run(capture_output=True)` drains
those pipes until EOF. EOF never comes while the daemon is holding them
open, so the parent sits there until the timeout even though `rtk init`
itself already exited and already wrote the hooks. So registration was
actually succeeding every time. We just threw the result away on timeout
and printed a failure for something that had worked.

The fix points `rtk init`'s output at a temp file instead of pipes. A
file fd has no reader waiting on EOF, so we only ever wait on the direct
child and return the moment it exits. `stdin` is `DEVNULL` too so a
stray prompt can't block us either.

A few notes:

1. On `TimeoutExpired` I read the temp file before the `with` closes it,
otherwise the outer handler has nothing to log.
2. Nothing else changes: a clean exit still logs and returns `True`, a
non-zero exit still logs the output and returns `False`.
3. This is the hook-registration path only, it's the one place that
shells out to `rtk init`.

Closes # N/A (no tracking issue)

## 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`: in `register_claude_hooks`, send `rtk
init`'s output to a `tempfile.TemporaryFile` and set `stdin=DEVNULL`, so
a forked rtk daemon that inherits the pipes can no longer keep us
blocked until the 10s timeout. The timeout branch reads the temp file
before the `with` closes it so the diagnostic survives.
- `tests/test_rtk_installer.py`: cover the timeout-with-daemon case and
the success/failure return paths.

## 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
$ uv run --extra dev python -m pytest tests/test_rtk_installer.py -q
4 passed, 1 warning in 0.36s

$ uv run --extra dev ruff check headroom/rtk/installer.py tests/test_rtk_installer.py
All checks passed!

$ uv run --extra dev mypy headroom/rtk/installer.py
Success: no issues found in 1 source file
```

## Real Behavior Proof

- Environment: macOS, Python 3.13, this branch.
- Exact command / steps: the test spawns a fake `rtk` that registers,
then forks a child which keeps the inherited stdout/stderr open well
past the 10s window, reproducing the daemon-holds-the-pipe case. Before
the fix that pegs `subprocess.run` to the timeout; after it the call
returns as soon as the direct child exits.
- Observed result: the registration call returns success in a fraction
of a second instead of timing out, and `headroom wrap claude` no longer
prints the "rtk hook registration failed" line on launch.
- Not tested: I did not re-run this on Linux or Windows. The change is
in how we read the child's output, not anything platform-specific, but
the pipe/daemon timing is what it is so a second pair of eyes there is
welcome.

## 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
- [ ] I have updated the CHANGELOG.md if applicable

## Additional Notes

ruff and mypy are clean on the files I touched. I left the docs and
CHANGELOG boxes unchecked because this is an internal reliability fix
with no user-facing API change, happy to add a CHANGELOG line if you'd
prefer one.
This commit is contained in:
Lucas Santos 2026-06-25 17:10:18 +02:00 committed by GitHub
parent 35939c3536
commit 9758817979
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 49 additions and 11 deletions

View file

@ -16,6 +16,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* **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.
### Fixed
* **rtk:** stop `rtk` hook registration from spuriously timing out during `headroom wrap`. Output is captured to a temp file instead of pipes, and `stdin` is closed, so a background process forked by `rtk init` can no longer hold the pipe open and block `subprocess.run` past its 10s timeout after the hooks were already registered.
### Features
* **learn:** weight loops in `headroom learn`. A new loop detector (`headroom/learn/loops.py`) recognizes repeated tool-call patterns — including RTK re-fetch loops, where RTK's output truncation makes the agent re-run larger-limit variants of a *successful* command — collapses output-limit variants to one signature, measures the wasted tokens, surfaces loops as a highest-priority digest section, and weights loop guardrails above one-off rules by their measured waste. Previously loops had no special weight and a no-failure re-fetch loop was skipped entirely. Adds an RTK-loop eval (`benchmarks/rtk_loop_learn_eval.py`) that reproduces a loop, runs it through Learn, and asserts the generated guardrail ranks first and prevents re-triggering.

View file

@ -9,6 +9,7 @@ import platform
import stat
import subprocess
import tarfile
import tempfile
import zipfile
from pathlib import Path
from urllib.request import urlopen
@ -169,18 +170,35 @@ def register_claude_hooks(rtk_path: Path | None = None) -> bool:
"""
rtk_path = rtk_path or RTK_BIN_PATH
# Capture output to a temp file rather than pipes: `rtk init` may fork a
# background process that inherits our stdout/stderr, and a piped
# `subprocess.run` drains those pipes until EOF — which never arrives while
# the daemon holds them open, so it blocks to the timeout even though
# `rtk init` itself exited and already registered the hooks. A file fd has
# no such reader, so we wait only on the direct child. stdin is DEVNULL so a
# stray prompt can never block either.
try:
result = run(
[str(rtk_path), "init", "--global", "--auto-patch"],
capture_output=True,
text=True,
timeout=10,
)
if result.returncode == 0:
logger.info("rtk hooks registered in Claude Code")
return True
else:
logger.warning("rtk init failed: %s", result.stderr)
with tempfile.TemporaryFile(mode="w+", encoding="utf-8", errors="replace") as out:
try:
result = subprocess.run(
[str(rtk_path), "init", "--global", "--auto-patch"],
stdin=subprocess.DEVNULL,
stdout=out,
stderr=out,
timeout=10,
)
except subprocess.TimeoutExpired:
# Read the temp file while it is still open — the outer handler
# runs after the `with` closes it, so any captured diagnostics
# would be gone by then.
out.seek(0)
logger.warning("rtk init timed out: %s", out.read().strip())
return False
if result.returncode == 0:
logger.info("rtk hooks registered in Claude Code")
return True
out.seek(0)
logger.warning("rtk init failed: %s", out.read().strip())
return False
except Exception as e:
logger.warning("Failed to register rtk hooks: %s", e)

View file

@ -3,6 +3,7 @@
from __future__ import annotations
import io
import stat
import tarfile
from pathlib import Path
from unittest.mock import patch
@ -56,3 +57,18 @@ def test_download_rtk_skips_verify_for_non_native_target(monkeypatch, tmp_path:
assert installed_path == tmp_path / "rtk"
assert installed_path.exists()
subprocess_run.assert_not_called()
def test_register_claude_hooks_survives_forked_daemon(tmp_path: Path) -> None:
"""rtk init that exits fast but leaves a child holding stdout must not hang.
Regression: capturing through pipes made subprocess.run drain until EOF,
which a lingering grandchild deferred past the 10s timeout even though the
hooks were already registered. Output now goes to a temp file, so we wait
only on the direct child.
"""
fake_rtk = tmp_path / "rtk"
fake_rtk.write_text("#!/bin/bash\n( sleep 30 ) &\necho done\nexit 0\n")
fake_rtk.chmod(fake_rtk.stat().st_mode | stat.S_IEXEC)
assert installer.register_claude_hooks(fake_rtk) is True