fix(hooks): honor exclude_commands for head/tail line-range fast path#2887
Open
AZERDSQ131 wants to merge 1 commit into
Open
fix(hooks): honor exclude_commands for head/tail line-range fast path#2887AZERDSQ131 wants to merge 1 commit into
AZERDSQ131 wants to merge 1 commit into
Conversation
The head/tail line-range rewrite in rewrite_segment_inner returned
before the exclusion check, which only runs in the
Classification::Supported and Unsupported arms further down. Any
command routed through the head/tail fast path (cmd_part.starts_with
("head -") || cmd_part.starts_with("tail ")) was rewritten to
`rtk read ... --max-lines/--tail-lines` even when explicitly listed
in [hooks] exclude_commands, both via `rtk rewrite` and the Claude
Code hook path.
Moved the same is_excluded check already used by the Supported arm
(ENV_PREFIX-stripped command, matched against the configured
ExcludePattern list) in front of the fast path's early return.
Adds three regression tests: tail and head are no longer rewritten
when excluded, and head still rewrites normally when nothing is
excluded (guards against fixing this by breaking the fast path
entirely).
|
|
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
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.
Closes #2823
[hooks] exclude_commandsis bypassed forhead/tailbecause the line-range fast path inrewrite_segment_innerreturns before the exclusion check:is_excludedis only consulted in theClassification::SupportedandUnsupportedarms further down, both downstream of this early return.Repro before this fix (config:
[hooks] exclude_commands = ["tail", "head", "rg"]):Fix: moved the same
is_excludedcheck theSupportedarm already uses (ENV_PREFIX-stripped command matched against the configuredExcludePatternlist) in front of the fast path's early return, so excludedhead/tailinvocations fall through to the raw-passthrough path like every other excluded command.Adds three regression tests in
src/discover/registry.rs:test_exclude_tail_line_range_fast_path— excludedtailno longer rewritestest_exclude_head_line_range_fast_path— excludedheadno longer rewritestest_head_line_range_fast_path_still_rewrites_when_not_excluded— guards against fixing this by disabling the fast path entirelyRan the mandatory gate locally:
cargo fmt --all— no changes neededcargo clippy --all-targets— cleancargo test --all— 2396 passed (+ 6/11/11/14/5/11 in the other suites), 0 failed