Skip to content

fix(hooks/exclude): match multi-token entries against command prefix#1985

Open
YOMXXX wants to merge 1 commit into
rtk-ai:developfrom
YOMXXX:fix/exclude-commands-subcommands
Open

fix(hooks/exclude): match multi-token entries against command prefix#1985
YOMXXX wants to merge 1 commit into
rtk-ai:developfrom
YOMXXX:fix/exclude-commands-subcommands

Conversation

@YOMXXX
Copy link
Copy Markdown
Contributor

@YOMXXX YOMXXX commented May 20, 2026

Summary

[hooks].exclude_commands entries with more than one whitespace-separated
token (e.g. "git diff", "docker compose up") were matched by an
anchored regex built from the user input. The matching itself happened
to work for the well-formed case, but the construction was fragile and
the user-facing contract was undocumented — issue #1919 reports that a
config like

[hooks]
exclude_commands = ["diff", "git diff"]

does not reliably stop git diff HEAD from being rewritten. This PR
makes the contract explicit and the implementation literal: a
multi-token entry matches a command iff the command equals the entry
exactly
or starts with the entry followed by a whitespace
character
.

Reproduction (before)

# ~/.config/rtk/config.toml
[hooks]
exclude_commands = ["diff", "git diff"]
rtk rewrite "diff a b"       # exit 1  — correctly excluded (single-token)
rtk rewrite "git diff HEAD"  # exit 0  — NOT excluded (multi-token broken)

Root cause

src/discover/registry.rs::compile_exclude_patterns wrapped every entry
in ^<regex::escape>($|\s) and then matched it as a regex. For
multi-token entries this:

  1. relies on regex::escape turning the literal space into \ — a
    detail that is easy to break in future refactors;
  2. degrades silently to a substring starts_with whenever the regex
    fails to compile, which means an entry like git[diff would happily
    exclude git-difftool as well as git diff;
  3. obscures the actual user-facing contract behind two layers of
    indirection.

Fix approach

  • Treat ^-prefixed entries as regex (keeps the existing
    ^curl-style escape hatch for power users).
  • Treat every other entry as a literal command prefix and store it as
    ExcludePattern::Prefix.
  • In is_excluded, a Prefix matches when the (left-trimmed) command
    is either equal to the entry or starts with the entry followed by a
    whitespace character — never on a substring boundary.
  • Canonicalise interior whitespace in entries so "git diff" (two
    spaces, typo) still matches git diff HEAD.
  • Keep the invalid-regex fallback path so a malformed ^… regex
    degrades to a literal prefix on the un-anchored body rather than
    silently doing nothing.

Test plan

  • test_exclude_single_token_still_matches"diff" still excludes diff a b
  • test_exclude_two_token_subcommand_git_diff — headline [hooks] exclude_commands does not cover subcommands (e.g. "git diff") #1919 case
  • test_exclude_two_token_with_many_trailing_argsgit diff --staged path/to/file
  • test_exclude_three_token_subcommanddocker compose up -d
  • test_exclude_exact_match_with_no_trailing_args — bare git diff
  • test_exclude_two_token_does_not_match_sibling_subcommandgit status still rewrites
  • test_exclude_two_token_does_not_match_subcommand_prefixgit diffstat, git-difftool not excluded
  • test_exclude_extra_whitespace_in_entry_is_normalised"git diff" typo tolerated
  • cargo fmt --all clean
  • cargo clippy --all-targets zero warnings
  • cargo test --bin rtk -- --test-threads=8: 1909 passed / 6 ignored / 0 failed
  • Manual smoke test against the binary with a real config.toml confirms rtk rewrite "git diff HEAD" now exits 1.

Fixes #1919

`[hooks].exclude_commands` was implemented as a single anchored regex per
entry (`^<escaped-entry>($|\s)`). That regex is correct for the common
single-token case ("curl", "diff") but its multi-token behaviour was
both subtle (relies on `regex::escape` turning the literal space into
`\ `) and easy to break — a typo in the entry, or an unfortunate
metacharacter, silently produced an invalid regex that the code then
downgraded to a substring `starts_with`, which would happily exclude
`git-difftool` along with `git diff`.

This change splits the two intents apart: entries beginning with `^` are
still compiled as regexes (so power users can keep patterns like
`^docker (compose|exec)`), and everything else is treated as a literal
command prefix. The new `is_excluded` then matches a command iff it
either equals the entry or begins with the entry followed by a
whitespace character — giving the obvious semantics requested in rtk-ai#1919.
Interior whitespace inside an entry is canonicalised so `"git  diff"`
(two spaces) keeps matching `git diff HEAD`.

Single-token entries ("diff"), invalid-regex fallback, env-prefixed
commands ("PGPASSWORD=… psql"), the existing two-token case
("git push"), bare `^` and empty patterns, the no-substring-match
contract ("go" vs "golangci-lint") and `^`-anchored regex entries all
keep their previous behaviour — covered by the existing tests plus
eight new regression cases that drive the matcher and `rewrite_command`
through `git diff`, three-token wrappers, exact-equality, sibling
subcommands and the `git diffstat` boundary.

Fixes rtk-ai#1919
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[hooks] exclude_commands does not cover subcommands (e.g. "git diff")

1 participant