Conversation
|
|
ahundt
force-pushed
the
v3/hook-collision-manifest-fallthrough
branch
from
August 2, 2026 19:28
c36a954 to
18f9049
Compare
ahundt
force-pushed
the
v3/hook-collision-manifest-fallthrough
branch
from
August 3, 2026 16:18
18f9049 to
17d62d0
Compare
Claude Code can discard a hook's updatedInput when multiple PreToolUse groups match Bash (issue rtk-ai#1515). Before this change, rtk init registered its own Bash matcher while leaving plugin Bash matchers in place, so RTK rewrites could be lost and displaced safety handlers could stop running. Patch compatible active plugin-cache entries during global init, retain every displaced command in an exact pre/post manifest, run those commands against the original payload before RTK emits a rewrite, and veto the rewrite on a Claude or Gemini-shaped deny response. Direct command hooks with shell operators remain untouched rather than being reinterpreted. Global uninstall restores only files that still match RTK's patched snapshot, preserving plugin updates. Changed src/hooks/manifest.rs, src/hooks/hook_cmd.rs, src/hooks/init.rs, and src/hooks/mod.rs. Added matcher, quoted-argument, operator rejection, idempotence, plugin upgrade, subprocess deny, opt-out, and install/uninstall round-trip tests. Verification: cargo test -q passed with 2521 tests and 8 ignored; cargo check -q; cargo clippy --all-targets -- -D warnings; cargo fmt --all -- --check; git diff --check. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
Claude plugin hooks rely on CLAUDE_PLUGIN_ROOT and CLAUDE_PLUGIN_DATA, but manifest replay previously executed displaced commands without those values and resolved root placeholders against marketplace source paths. Record the active cache version and sanitized plugin data path, expand root/data placeholders before direct execution, export the documented variables plus compatibility aliases, and create the data directory on first replay. Files: src/hooks/manifest.rs. Functions: run_manifest_handlers, resolve_plugin_placeholders, plugin_data_path, resolved_commands, patch_hook_file, patch_plugin_caches. Verification: cargo test -q manifest (11 passed), cargo check -q, cargo clippy --all-targets -- -D warnings, cargo fmt --all -- --check, and git diff --check. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
A direct-token plugin command such as PLUGIN_MODE=1 /bin/echo ok was treated as an executable name. RTK removed Bash from the plugin matcher, then could not spawn the displaced handler, silently losing the hook. Reject shell environment-assignment prefixes in split_handler_command so unsupported handlers remain in the plugin cache and continue to run under Claude Code. Files: src/hooks/manifest.rs. Functions: split_handler_command and is_shell_assignment. Verification: cargo test -q manifest (12 passed), cargo check -q, cargo clippy --all-targets -- -D warnings, cargo fmt --all -- --check, and git diff --check. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
The manifest fallthrough path previously split every command string and launched its first token directly. That skipped valid shell-form hooks containing pipelines and discarded the exact args supplied by Claude for exec-form hooks. Store shell-form handlers as platform-shell commands and args-bearing handlers as exact argv, while leaving hooks with timeout, async, asyncRewake, custom shell, or non-command types in the plugin cache for Claude to dispatch. Pass the recursion guard only to spawned handlers so concurrent RTK invocations do not share mutable process-wide state. Changed src/hooks/manifest.rs and its manifest/runtime tests. Verification: cargo test -q manifest (12 passed), cargo check -q, cargo clippy --all-targets -- -D warnings, cargo fmt --all -- --check, and git diff --check. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
Displaced plugin commands without an explicit timeout were previously run with wait_with_output(), so a hung Bash hook could block RTK indefinitely after installation moved the handler out of Claude's dispatcher. Poll the child with Claude's 10-second default hook timeout, kill and reap it on expiry, and continue without converting a timeout into a blocking or deny response. Keep explicit timeout, async, asyncRewake, custom-shell, non-command, and malformed handler entries in the plugin cache so Claude remains their dispatcher. Add a Unix regression test that proves a sleeping handler is terminated within the test deadline. Tests: cargo test -q (2524 unit passed, 8 ignored, 64 integration passed); cargo check -q; cargo clippy --all-targets -- -D warnings; cargo fmt --all -- --check; git diff --check. File: src/hooks/manifest.rs. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
wait_for_output now drains stdout and stderr on dedicated reader threads while polling the child timeout, so a valid displaced hook that emits more than the OS pipe buffer can finish instead of being killed after the ten-second deadline. Timed-out children are still killed without joining readers that descendants may hold open. Add a Unix regression test covering a completed handler that emits 100000 lines through a pipe. Verified with cargo test -q manifest, cargo fmt --all -- --check, and git diff --check. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
run_manifest_handlers previously called ChildStdin::write_all before starting timeout polling. A compatible Claude plugin that did not read stdin could block RTK once the bounded hook payload exceeded the pipe capacity. Move the payload write to a background thread and retain write-success gating for exit-code-2 denials; timeout handling can now kill the child without waiting on the writer. Add non_reading_handler_cannot_block_before_timeout with a 1 MiB payload and a 25 ms wait_for_output deadline. Tests: cargo test -q manifest (15 passed); cargo fmt --all; git diff --check. Files: src/hooks/manifest.rs. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
ahundt
force-pushed
the
v3/hook-collision-manifest-fallthrough
branch
from
August 3, 2026 18:15
17d62d0 to
4c47956
Compare
The Claude fallthrough manifest replays commands that already exist in plugin hook caches. Semgrep correctly flags dynamic executables, POSIX shell dispatch, and manifest cleanup as high-risk patterns, but rejecting these lines prevents RTK from preserving the configured hook behavior this branch provides.\n\nAdd line-level Semgrep justifications at the exact replay and cleanup sites. The comments document that executable-plus-argv and shell forms originate from an existing Claude plugin entry, while the deletion markers are limited to RTK's own consumed manifest. Mark the Unix shell subprocess fixtures separately so the security scan continues to enforce these rules everywhere else.\n\nNo runtime behavior changes.\n\nTests: RUSTC_WRAPPER= cargo test -q manifest (15 passed)\nChecks: cargo fmt --all -- --check; git diff --check
The manifest_preserves_exact_arguments_for_exec_form_hooks test hard-coded /bin/printf and /bin/sh, so the Windows job could not start the handler and returned NoBlock. Keep the POSIX fixture on Unix and use cmd.exe /C on Windows, while asserting the exact platform-specific command and argv stored in the manifest.\n\nThe production manifest replay path is unchanged; this only makes its cross-platform regression test exercise a command available on each supported host.\n\nTests: isolated cargo test -q -- --test-threads=1 (2577 passed, 8 ignored)\nChecks: cargo fmt --all -- --check; git diff --check
Why: The pull request must be evaluated against the current develop branch without losing its bounded Claude manifest fallthrough behavior. What: Merge current upstream/develop into v4/hook-collision-manifest-fallthrough. The merge is conflict-free and retains upstream hook adapters alongside the branch manifest implementation. Verification: merge-base scope and merge-tree conflict check completed before merge; full gates follow on the merged head. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
ahundt
added a commit
to ahundt/rtk
that referenced
this pull request
Aug 24, 2026
Why: The combined integration proof must include the latest PR rtk-ai#2556 manifest dispatcher and collision behavior. What: Merge v4/hook-collision-manifest-fallthrough with bounded input, exact argv replay, denial precedence, and conditional Claude hook restoration while retaining the current lexer rewrite path. Verification: - git diff --cached --check - inspected staged hook and manifest scope before commit Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Preserve compatible Claude Bash hooks when RTK installs its hook
Summary
RTK now preserves compatible Claude plugin Bash hooks when installing its own Bash matcher. A plugin deny remains authoritative, while RTK remains the single rewrite producer for non-deny responses instead of silently losing the plugin handler.
This addresses issue #1515, where RTK could stop working alongside other CLI packages such as oh-my-codex and oh-my-claude.
Before and after
BashImplementation
src/hooks/manifest.rsowns the collision lifecycle: it patches only cache entries whose execution semantics RTK can reproduce, records the pre- and post-patch arrays, expands supported placeholders, and leaves unsupported timeout, async, custom-shell, and non-command entries with Claude's dispatcher.src/hooks/init.rsperforms installation and conditional restoration.src/hooks/hook_cmd.rsgives a displaced deny response precedence over an RTK rewrite.Foreign matchers and unsupported metadata remain intact. The manifest is removed only when every recorded cache file is restored; a plugin-modified file remains in place.
Verification
/bin/printfand/bin/shon Unix andcmd.exe /Con Windows, so the same argument-preservation and deny-dispatch coverage runs on every supported CI platform.cargo clippy --all-targets --all-features -- -D warnings,cargo fmt --all -- --check, andgit diff --checkpass.