6 hook bugs on master — routing, streaming, extensibility, hook collision (fixed in PR #156)

[ahundt]

TL;DR: PR #156 replaces the current shell script with a fully tested Rust hook engine featuring an integrated lexer that prevents AI timeouts via line-by-line output streaming, increases tokens saved by successfully rewriting compound commands, stops RTK from breaking standard tools like find and vitest, and prevents Claude Code from silently discarding all hook rewrites when any other plugin registers for the same Bash event.

Edit: I made two PRs at @pszymkowiak's request that are to the develop branch:

• PR #536 — Full PR with all tests: feat: binary hook engine with lexer, streaming, and rewrite_compound integration #536
• PR #537 — Code review only (tests stripped): DO NOT MERGE — core code review only (tests stripped from PR #536) #537

These are both the same as PR #156 but they merge into the develop branch instead.

Summary

The current shell hook (hooks/rtk-rewrite.sh, v0.28.2) works well for its original purpose — rewriting single commands via rtk rewrite. However, six gaps have emerged as the hook system needs to handle streaming output, multiple handlers, broader command routing, and competing Bash hook registrations from other installed plugins. The shell hook calls rtk rewrite "$CMD", emits the result as JSON, and exits — which means it can't stream test output incrementally, can't coordinate additional handlers (e.g. for Gemini CLI or safety rules), routes some commands through RTK filters that aren't designed for them (via classify_command(), originally built for rtk discover history analysis), and can't prevent Claude Code from silently discarding all hook results when another plugin also registers for the same Bash event.

PR #156 (feat/rust-hooks-v2) extends the hook system with a Rust-based hook engine: conservative command routing via hook_lookup() whitelist, line-by-line streaming for test runners (BufReader in new src/stream.rs), multi-handler coordination (shell: parallel-merge, binary: manifest-based), and robust stderr/exit-code handling for deny decisions. 1183 tests, developed with TDD.

Master's hook flow: Claude Code calls hooks/rtk-rewrite.sh → shell runs rtk rewrite "$CMD" → Rust calls registry::rewrite_command() in src/rewrite_cmd.rs → routing via classify_command() using RULES from src/discover/rules.rs → returns rewritten command or exits non-zero if unrecognized.

Hook engine scope (PR #156, not counting upstream merge): 6,533 lines across 14 files. 62% tests (~4,050 lines), 38% production code (~2,483 lines). The two alternative approaches (#141 JS/Bun hook by Fernando Basilis, master's shell hook) have no hook-specific tests.

---

Bug 1 🔴 CRITICAL — No streaming: .output() buffers all command output

What happens on master

RTK's command modules currently use Rust's .output() method, which collects all stdout/stderr in memory before processing — this works fine for quick commands, but becomes a problem for long-running test suites. Since the shell hook rewrites commands like cargo test → rtk cargo test, the buffering behavior applies to every hook-rewritten test command.

Code paths on master (v0.28.2):

• src/runner.rs:17-19: .stdout(Stdio::piped()).stderr(Stdio::piped()).output() — blocks until child exits
• src/cargo_cmd.rs:86: String::from_utf8_lossy(&output.stdout) — entire test suite collected before filtering
• src/go_cmd.rs:63: same collect-then-process for go test -json NDJSON output
• src/pytest_cmd.rs, src/vitest_cmd.rs: same .output() pattern

Concrete impact

When an AI coding assistant runs cargo test on a project with a large test suite:

1. Hook rewrites to rtk cargo test
2. RTK spawns cargo test and calls .output() — blocks for the entire test run duration
3. AI sees zero output until all tests finish (could be minutes)
4. Claude Code may timeout the Bash tool waiting for output
5. If parent is killed (timeout or user interrupt), child process may orphan
6. No incremental feedback → AI assumes command hung → kills and retries → wasted tokens

Affected commands: cargo test, go test, pytest, vitest run — every test runner the hook rewrites.

How to reproduce

# In a project with a slow test suite (>10 seconds):
rtk cargo test    # No output appears until ALL tests complete
cargo test        # Compare: output streams incrementally

Fix in PR #156

New src/stream.rs module with BufReader wrapping child stdout for line-by-line streaming (line 22). Output flows incrementally to the AI as tests run. Broken pipes handled cleanly via map_while(Result::ok) (line 276) — stops iteration on first I/O error instead of spinning infinitely.

PR comparison

Approach	Streaming
master (v0.28.2)	❌ .output() blocks until child exits — zero incremental output
#156 feat/rust-hooks-v2	✅ BufReader line-by-line streaming with clean pipe break handling
#141 JS hook (closed)	❌ No streaming support

---

Bug 2 🔴 CRITICAL — Shell hook cannot coordinate multiple handlers (no extensibility)

What happens on master

The shell hook hooks/rtk-rewrite.sh (61 lines) rewrites one command and exits. There is no extension point. Its core logic:

# hooks/rtk-rewrite.sh on master (v0.28.2)
REWRITTEN=$(rtk rewrite "$CMD" 2>/dev/null) || exit 0
if [ "$CMD" = "$REWRITTEN" ]; then exit 0; fi
# emit JSON with permissionDecision: "allow" and the rewritten command

It calls rtk rewrite, emits the result, and exits. The current design handles one handler (RTK's rewrite) well, but doesn't have a way to discover or run additional handlers. If a Claude Code extension registers a PreToolUse hook handler (e.g., Gemini CLI support via PR #158, future TOML-based safety rules, or any third-party extension), the shell hook won't discover or execute it.

Master's src/init.rs only patches ~/.claude/settings.json to register this shell hook (via insert_hook_entry() at line 630). It has no concept of extension handler discovery, handler registration manifests, or cache file patching. Running rtk init registers the shell hook — nothing more.

Concrete impact

If a user installs a Claude Code extension that registers a PreToolUse hook handler (e.g., Gemini CLI support from PR #158), the shell hook doesn't know about it. The extension appears installed but its handler never runs — with no error or warning to indicate why.

Fix in PR #156

Three complementary pieces:

1. Shell hook (hooks/rtk-rewrite.sh): parallel-merge coordinator. All registered handlers launch in background (BEGIN/END_RTK_BASH_HANDLERS section), COLLECT phase waits for all to finish, MERGE phase applies deny-wins logic. RTK rewrite only applied if no handler denied.

2. Binary hook (src/cmd/hook/claude.rs): run_manifest_handlers() called on both NoOpinion (line 203) and Allow (line 221) code paths. Handlers discovered via manifest file. Deny from any handler wins over RTK's allow/rewrite decision.

3. Init handler registration (src/init.rs): patch_plugin_caches() and patch_single_cache_file() scan ~/.claude/plugins/cache/*/hooks/ for plugin handlers with Bash matchers, write a handler manifest, and include a reconstruction path so rtk init is idempotent (running it twice doesn't break registration).

All file modifications use atomic_write() (temp-file-then-rename) with pre-patch .rtk-backup files. rtk init --uninstall restores original plugin state from the manifest. See Bug 6 below for full safety details.

PR comparison

Approach	Handler coordination	Init registers handlers
master (v0.28.2)	❌ rtk rewrite "$CMD" → exit 0 — no coordination	❌ Only patches settings.json
#156 feat/rust-hooks-v2	✅ Parallel-merge (shell) + run_manifest_handlers() on both paths (binary)	✅ patch_plugin_caches() + manifest + idempotent reconstruction
#141 JS hook (closed)	❌ No handler coordination	❌ No handler registration

---

Bug 3 🟠 HIGH — Claude Code bug #4669: stderr at exit 0 disables hook (fail-open)

What happens on master

Claude Code treats ANY stderr output at exit 0 as a hook error and runs the tool unmodified (fail-open). The shell hook uses 2>/dev/null on the rtk rewrite call, suppressing RTK's own stderr. But if any other subprocess writes to stderr during the hook's execution, Claude Code interprets it as a hook error and runs the command unmodified.

All error paths in rtk-rewrite.sh exit 0 (version guard, missing jq, missing rtk, empty command), and there's no exit-code-based deny mechanism yet. This means that even if deny logic were added to the shell hook, a stray stderr line from any subprocess could cause Claude Code to bypass it (due to upstream bug #4669).

Concrete impact

A deny decision emitted as {"hookSpecificOutput":{"permissionDecision":"deny"}} at exit 0 is silently ignored if any stderr was written during hook execution. The command executes unmodified. No user-visible error.

Fix in PR #156

src/cmd/hook/claude.rs: deny path uses exit 2 + stderr message (not exit 0 + JSON) at line 210 and line 235. Commands are reliably blocked regardless of Claude Code bug #4669. Exit 0 paths (NoOpinion, Allow) never write to stderr — they are stderr-clean by construction.

PR comparison

Approach	Deny reliability
master (v0.28.2)	❌ exit 0 + any stderr = fail-open (Claude Code bug #4669)
#156 feat/rust-hooks-v2	✅ exit 2 + stderr for deny; exit 0 paths are stderr-clean
#141 JS hook (closed)	❌ Not addressed

---

Bug 4 🟡 MEDIUM — Wrong command routing: docker run, find, etc. incorrectly go through RTK

What happens on master

The shell hook calls rtk rewrite → registry::rewrite_command() → rewrite_segment() → classify_command() (in src/discover/registry.rs). The problem: classify_command() uses the RULES table from src/discover/rules.rs, which was designed for history analysis (rtk discover). It matches any command RTK recognizes, not just commands where RTK has well-tested filters. This is too broad for hook use — the hook should only rewrite commands where RTK's output is known to be correct.

Examples of wrong routing on master:

• find . -name '*.rs' → rtk find . -name '*.rs' — breaks: RTK's find expects rtk find <PATTERN> [PATH] syntax, standard find flags like -name fail with "unexpected argument" (exit 2)
• docker run --rm ubuntu bash → rtk docker run --rm ubuntu bash — works via DockerCommands::Other passthrough (src/main.rs:1421), but adds unnecessary RTK overhead for a command RTK doesn't filter

Concrete impact

find is the clearest breakage: rtk find . -name '*.rs' fails with exit 2 because RTK's find command uses rtk find <PATTERN> [PATH] syntax — standard find flags like -name, -type, -maxdepth are not recognized. Other commands like docker run work via passthrough but add unnecessary RTK overhead.

How to reproduce

# Rewrite routes these through RTK even though RTK's filters aren't designed for them:
rtk rewrite "find . -name '*.rs'"           # Returns "rtk find . -name '*.rs'"
rtk find . -name '*.rs'                     # ERROR: "unexpected argument '-n'" (RTK find has different syntax)
rtk rewrite "docker run --rm ubuntu bash"   # Returns "rtk docker run ..." (works via passthrough, but unnecessary)

Fix in PR #156

hook_lookup() in src/cmd/hook/mod.rs: conservative whitelist matching only subcommands RTK has tested filters for. Examples:

• docker: only ps, images, logs (NOT run, exec, build)
• git: only status, log, diff, show, add, commit, push, pull, fetch, stash
• cargo: only test, build, clippy, check, install, fmt (NOT publish, run)

Unknown commands pass through unchanged instead of being wrapped in rtk run -c.

PR comparison

Approach	Command routing
master (v0.28.2)	❌ classify_command() routes docker run/exec/build, find, tree, wget through RTK
#156 feat/rust-hooks-v2	✅ hook_lookup() whitelist — only routes subcommands with tested filters
#141 JS hook (closed)	❌ No routing whitelist

---

Bug 5 🟡 MEDIUM — vitest without run subcommand produces broken invocation

What happens on master

The rewrite prefix for vitest in src/discover/rules.rs is "vitest", so bare vitest → rtk vitest (without the required run subcommand). Upstream tests only cover vitest run, never bare vitest.

Concrete impact

rtk vitest (no run) prints Clap help text and exits with code 2 — the test suite never runs. The VitestCommands enum in main.rs:824 only has a Run variant, so Clap requires the subcommand.

How to reproduce

rtk rewrite "vitest"       # Returns "rtk vitest" (missing "run")
rtk vitest                 # Prints help and exits 2 — Clap requires a subcommand
rtk rewrite "vitest run"   # Returns "rtk vitest run" (correct — works)

Fix in PR #156

hook_lookup() and route_pnpm() (line 282) / route_npx() (line 324) inject run when absent: bare vitest → rtk vitest run. Test: test_routing_vitest_no_double_run and table test.

PR comparison

Approach	vitest handling
master (v0.28.2)	❌ vitest → rtk vitest (no run injection)
#156 feat/rust-hooks-v2	✅ vitest → rtk vitest run (automatic injection)
#141 JS hook (closed)	❌ No run injection

---

Bug 6 🔴 CRITICAL — Claude Code silently drops ALL hook rewrites when any other plugin registers for the same Bash PreToolUse event

What happens on master

When RTK is installed alongside any Claude Code plugin or MCP server that also registers a Bash PreToolUse hook handler, Claude Code runs all competing handlers but silently discards the updatedInput from every one of them. RTK's command rewrites have zero effect. The original command executes unmodified with no error or warning.

This is not specific to any single plugin — it affects every combination where two or more hooks register for the same (event, matcher) pair. Any plugin that adds a PreToolUse handler with "matchers": ["Bash"] will collide with RTK. Known examples include:

• autorun (Claude Code automation plugin)
• Any MCP server that registers Bash PreToolUse hooks
• Custom user hooks in settings.json that match Bash
• Future Claude Code extensions with Bash-level interception

Master's rtk init only patches ~/.claude/settings.json — it has no awareness of plugin cache files (~/.claude/plugins/cache/*/hooks/*.json) and no concept of handler collision detection or priority.

How the collision occurs:

1. rtk init -g registers RTK's hook in ~/.claude/settings.json for Bash PreToolUse events
2. Another plugin registers its own handler via ~/.claude/plugins/cache/<plugin>/*/hooks/PreToolUse.json with "matchers": ["Bash"]
3. On Claude Code startup, both handlers are active for the same (PreToolUse, Bash) event
4. Claude Code runs any Bash command (e.g. git status): both handlers execute, both return updatedInput, Claude Code discards all results — undefined behavior when multiple handlers compete for the same event

Concrete impact

# After installing RTK + any plugin with a Bash PreToolUse hook:

# In Claude Code session:
# Claude: "I'll run git status"  →  Bash tool: "git status"
# Expected: hook rewrites to "rtk git status"
# Actual:   "git status" runs unchanged — ALL competing hook outputs silently discarded

# Symptom:
rtk gain   # shows 0 rewrites saved — looks like RTK isn't running
           # but no error, no warning — indistinguishable from correct operation

This is a platform-level issue in Claude Code's hook runner that affects any tool (not just RTK) relying on updatedInput when another hook shares the same (event, matcher) pair. RTK must work around it because Claude Code users commonly have multiple plugins installed.

How to reproduce

# 1. Install any Claude Code plugin that registers a Bash PreToolUse handler
#    (e.g. autorun, or add a custom hook to settings.json matching "Bash")
# 2. Run:
rtk init -g   # registers rtk hook claude in ~/.claude/settings.json

# 3. Start new Claude Code session, run any command through the Bash tool
# 4. Observe: zero rewrites occur — rtk gain shows 0 savings
# 5. Verify the collision:
cat ~/.claude/settings.json                          # RTK hook present — looks correct
ls ~/.claude/plugins/cache/*/hooks/PreToolUse.json   # Other plugin also has Bash matcher
# Both registered for same (PreToolUse, Bash) → Claude Code drops all updatedInput

Fix in PR #156

src/init.rs: patch_plugin_caches() scans all plugin cache files at ~/.claude/plugins/cache/*/hooks/PreToolUse.json, removes "Bash" from their matchers, and registers displaced handler commands in ~/.claude/hooks/rtk-bash-manifest.json. RTK becomes the sole Bash PreToolUse handler, eliminating the collision. Displaced plugins still run — they're invoked as subprocesses inside RTK's hook via run_manifest_handlers() in src/cmd/hook/claude.rs, preserving their behavior (including deny/block decisions) while ensuring updatedInput is never silently discarded.

Safety & reversibility:

• Pre-patch backups: Every modified cache file is copied to <file>.rtk-backup before any change. Backups are never overwritten on subsequent runs — they preserve the original pre-RTK state.
• Atomic writes: All file modifications use temp-file-then-rename (atomic_write() via Rust's tempfile crate). Files are never partially written, even on crash or disk-full.
• Manifest-based recovery: rtk-bash-manifest.json stores each displaced plugin's original matcher, patched matcher, file path, and fallthrough command. Full state reconstruction is possible without needing the backup files.
• Idempotent: Re-running rtk init detects already-patched files via manifest and skips them. Running it 2-3 times produces identical results.
• Empty-matcher guard: If removing Bash would leave an empty matcher (plugin ONLY handled Bash), the entry is skipped with a warning — never written with a broken empty matcher.
• Clean uninstall: rtk init --uninstall reads the manifest and restores each plugin's original Bash matcher via atomic_write(). Manual restore also works: copy *.rtk-backup files over the patched originals.
• Displaced plugins still run: run_manifest_handlers() invokes each displaced handler as a subprocess during hook execution. Their deny/block decisions are preserved — deny from any handler wins over RTK's rewrite.
• No elevated permissions: Only modifies files in the user's ~/.claude/ directory.

# Example output from rtk init:
Plugin caches: 2 patched (autorun, custom-safety → registered in manifest)
Backups: ~/.claude/plugins/cache/autorun/.../hooks/PreToolUse.json.rtk-backup
         ~/.claude/plugins/cache/custom-safety/.../hooks/PreToolUse.json.rtk-backup

Why cache patching is necessary (no alternative exists):

Claude Code runs competing hooks in parallel with no priority, ordering, or updatedInput merging mechanism. When two handlers both return updatedInput for the same (event, matcher) pair, all results are silently discarded. There is no way to:

• Declare hook priority or execution order
• Pass environment variables between hooks (Claude Code issue #9567)
• Use a different hook event (only PreToolUse supports command rewriting via updatedInput)
• Detect at runtime that rewrites were dropped (no error signal from Claude Code)

Making RTK the sole Bash handler — while preserving displaced plugins as manifest subprocesses — is the only approach that works within Claude Code's current hook architecture. Plugin cache files do not regenerate on restart (confirmed by Claude Code issues #28492, #29074), so the patch is durable across sessions.

PR comparison

Approach	Competing hook collision
master (v0.28.2)	❌ No collision detection; any plugin sharing (PreToolUse, Bash) causes Claude Code to silently discard all updatedInput
#156 feat/rust-hooks-v2	✅ patch_plugin_caches() makes RTK sole Bash handler; all displaced plugins preserved via manifest subprocess
#141 JS hook (closed)	❌ No collision detection or prevention

---

Additional capabilities in PR #156

Dual-format deny detection (🟡 medium)
is_json_deny() in claude.rs detects both Claude Code format (hookSpecificOutput.permissionDecision == "deny") and Gemini format (decision == "deny"). Master has no deny detection (no binary hook exists).

---

Dependent PRs (optional features built on PR #156's hook engine)

PR	Feature	Dependency	Status
#158	Gemini CLI support	Requires #156 (imports from hook.rs, exec.rs)	Ready after #156 merge
#157	Data safety rules	Optional — upstream may use TOML-based safety instead	Independent, can be skipped

PRs #157 and #158 will not compile against current master — both import from modules added by #156. They have zero file overlap and can merge in any order after #156.

---

Merge sequence

1. Merge PR #156 (hook engine) → master    — fixes all 6 bugs, 1183 tests
2. Merge PR #158 (Gemini support) → master — depends on #156
3. Optional: PR #157 (data safety) → master — independent, can be skipped

Here are some of the differences vs the version on the develop branch:

while rewrite_compound() (~130 lines) partially handles &&, ||, ;, | with quote awareness. He's right for the basic cases. But PR #156's tests reveal edge cases develop does not handle:

Edge case	Develop's rewrite_compound()	PR #156's lexer+analysis
git commit -m "Fix && Bug"	✅ Handles (quote tracking)	✅ Handles
cargo test 2>&1	✅ Handles (redirect detection)	✅ Handles + routes to rtk cargo with suffix preserved
cargo test | tee /tmp/log	❌ Rewrites only left of pipe, pipe target is raw	✅ Suffix-aware: rtk cargo test | tee /tmp/log
git log | head -20	❌ rtk git log | head -20 but via pipe-first-only rule	✅ Same result but via suffix analysis
ls *.rs	❌ Rewrites to rtk ls *.rs — glob breaks	✅ Detects glob → rtk run -c 'ls *.rs' (shell passthrough)
echo $(date)	❌ Rewrites echo segment (though echo is ignored)	✅ Detects $() → shell passthrough
echo `date`	❌ No backtick detection	✅ Detects backtick → shell passthrough
noglob gh release create v1	❌ Treats noglob as a command	✅ Strips shell prefix, routes inner command
command git status	❌ Treats command as a command	✅ Strips builtin prefix, routes git status
exec git status	❌ Treats exec as a command	✅ Strips exec prefix, routes git status
git log $BRANCH	❌ No variable classification	✅ $SIMPLE_VAR = Arg (native route), $(...) = Shellism (passthrough)
/opt/homebrew/bin/git status	❌ No basename extraction	✅ Extracts git from full path
NODE_ENV=test CI=1 npx vitest run	Partial (single env prefix via regex)	✅ Multi-var env prefix stripping
Unknown commands (e.g. htop)	Returns None (caller decides)	✅ Explicit passthrough (no rtk run -c wrapping)