Matching the RTK command alone settled the round-1 defect, where a matcher or
timeout the user had edited made uninstall report "nothing to remove" and a
reinstall append a duplicate. It also went one step too far in both directions,
and this restores the two distinctions that were lost with it.
Presence now only counts a registration that would run for the tool RTK serves:
a group with no matcher, or one naming RunCommand among its `|`-separated
tools. Anything unrecognised counts as not covering RunCommand, so an install
adds an entry that fires rather than skipping one that never would. Removal
stays matcher-blind, which is what round 1 asked for, but skips an entry whose
explicit type is not `command`: a `prompt` entry is by construction the user's,
and deleting it also took the group and its `matcher` and `description` keys.
The "Already updated" list was also per-run, so the rerun the documentation
recommends after a write failure reported "none" while a target was fully
installed. It is now seeded from the preflight, and the uninstall path — which
has the same two-target failure mode — reports it too.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
std::process::Command wraps an argument in quotes only when it contains a
space or a tab, so an argument like a"b reaches the raw command line bare.
MSVCRT and UCRT parse that back correctly, Cygwin and MSYS do not, which is
why `rtk grep -c '"type"'` silently matched nothing (#3727). Re-encode with
libuv's quote_cmd_arg rules, and only when the argument contains a quote, so
every argument that works today keeps its exact bytes. Batch programs stay
on the old path: raw_arg would bypass std's CVE-2024-24576 mitigation, and
gradlew.bat and mvnw.cmd both reach run_passthrough.
tree forwards the user's own arguments the same way (#4101). MSYS2 ships an
MSYS-linked tree, so it belongs with ls, wc and find rather than with the
native-only spawn sites, which are deliberately left alone.
`rtk gain` printed its own missing-hook line from a bare `status()` match,
outside `maybe_warn`, so neither `hooks.suppress_hook_warning` nor
`RTK_SUPPRESS_HOOK_WARNING` reached it — on the report the no-hook audience
reads most. Gate that arm on the same helper; the outdated-hook prompt stays
visible, as it does everywhere else.
The new unit tests exercise the parser alone, so folding env and config
together with `||` — which drops the falsy force-off — left the whole suite
green. Drive the composition through the binary instead, and give the env
table the `=0` row that overrides the config flag.
The suite pins `XDG_CONFIG_HOME`/`XDG_DATA_HOME` to the temporary home and
seeds the config in both the XDG and the macOS location, so the loader cannot
resolve past it: `dirs::config_dir` is `~/.config` on Linux but
`~/Library/Application Support` on macOS, and a runner that exports either
`XDG_*` reached the developer's own config before.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
std::fs::File::set_modified has covered this since Rust 1.75, well under
the crate's 1.91 rust-version, so the filetime dev-dependency buys nothing.
Dropping it also drops the Cargo.toml hunk, which added a second
[dev-dependencies] table and made the manifest a duplicate key once develop
grew its own.
Extend the same treatment to the remaining sleep-ordered mtime test,
merge_test_summary_from_trx_ignores_stale_fallback_file, whose 5 ms sleep
left the stale/fresh distinction to filesystem mtime granularity.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Parse RTK_SUPPRESS_HOOK_WARNING as a truthy/falsy override that falls
back to config when unset or unrecognised, keep the HookStatus::Ok fast
path free of config loading, and suppress only the missing-hook warning
so the outdated-hook upgrade prompt stays visible.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CNmJZVV9u6pBhcwVhwkPp6
Add hooks.suppress_hook_warning config option and RTK_SUPPRESS_HOOK_WARNING
env var to disable "No hook installed" and "Hook outdated" warnings.
Users running rtk via CLAUDE.md instructions instead of hooks, or with
tools like OpenCode, get these warnings on every command. The warnings
waste tokens and confuse AI agents since rtk is working correctly.
RTK_SUPPRESS_HOOK_WARNING=1 suppresses; any other value forces the warning
on; unset falls back to the config file. Both default to false so existing
behavior is unchanged. The env parse follows the repo's
as_deref() == Ok("1") convention.
The suppression check runs before status() so the suppressed path does not
pay for the hook probe.
Documented in configuration.md (the hooks block and the env var table) and
README.md.
Fixes#682
Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
test_parse_trx_files_in_dir_since_ignores_older_files flaked when
filesystem mtime granularity quantized new.trx's mtime at or below
the 'since' threshold, leaving zero summaries and panicking on
.expect("merged summary") (fixes#4037).
Set explicit mtimes (now-10s / now+10s) via the filetime dev crate
instead of deriving them from write order, and apply the same fix to
test_find_recent_trx_in_dir_picks_newest_trx, which relied on a 5ms
sleep plus max_by_key over unspecified read_dir order.
The strip ran in classify_command but the rewrite matches rewrite_prefixes
against the original flag-first text. For the tool rules reachable via
`pnpm exec`/`pnpm run` (`pnpm -r exec vitest`, `pnpm -r lint`, ...) the
stripped form matched a tool rule, so classify returned Supported while the
rewrite never fired — discover/session then reported savings the hook cannot
deliver.
Only adopt the stripped form when it routes to the `rtk pnpm` rule
(matches_pnpm_rule); every form the PR targets (install/list/ls/i/outdated/run)
is that rule, so nothing it claims is lost, and the tool rules classify exactly
as on develop.
Also fold the two optional items from the review:
- assert `pnpm -x install` -> None, making the fixed-set guard load-bearing.
- tolerate extra spaces before the flag (`pnpm -r install`) via trim_start,
while keeping the single-ASCII-space boundary that strip_word_prefix requires
so classify and rewrite never diverge on a tab/other whitespace separator.
The style-edition reformat touched 69 files without changing behaviour,
which buries the real authorship of those lines. GitHub's web blame applies
.git-blame-ignore-revs automatically; locally it needs blame.ignoreRevsFile
set, so CONTRIBUTING now says so where contributors first clone.
Only the reformat is listed. The let-chain commit also touches many files
but changes control flow and drops a crate-wide lint allow, so its
authorship is real and should stay visible in blame.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
show_claude_config read the Cursor hook file only to compute _is_thin, which
nothing reads. The `?` on that read meant an unreadable hook file aborted
`rtk init --status` before it printed the rest of the report -- a status
command failing on the thing it is meant to report about. The executable-bit
check above it is what the branch actually uses.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Let-chains are edition 2024 syntax, so clippy::collapsible_if now fires on
every nested `if let` / `if` pair, and with warnings = "deny" they are build
errors rather than suggestions. Applied with cargo clippy --fix, which
handles all of them in one pass, then cargo fmt to re-indent the collapsed
bodies.
The deepest cases flatten four levels of nesting into one condition:
if let Some(cutoff_time) = cutoff
&& let Ok(meta) = fs::metadata(file_path)
&& let Ok(mtime) = meta.modified()
&& mtime < cutoff_time
{
continue;
}
Drops the crate-wide clippy::collapsible_if allow added in the previous
commit.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bump edition 2021 -> 2024. rust-version stays at 1.91, already well above
the 1.85 floor the edition needs; docs/guide/resources/troubleshooting.md
still claimed 1.70+, which is where a failed `cargo install --git` lands.
Four things the edition forces:
- std::env::{set_var,remove_var} are unsafe in 2024 with no safe std
replacement. Rather than wrap the test call sites in unsafe -- which the
crate denies and .semgrep.yml flags -- route them through temp-env, a
dev-only dependency whose closure API is safe and which restores the
previous value even when the body panics. The hand-rolled CLAUDE_DIR_LOCK
and PI_DIR_LOCK guards existed only to serialise those mutations and are
now redundant; CWD_LOCK and TEST_ENV_LOCK stay, they order more than the
env var itself.
- unsafe_op_in_unsafe_fn is on by default, so the libc calls in the proxy
signal handler and in stream.rs's relay handler need explicit unsafe
blocks, scoped to the libc calls themselves.
- `gen` is a reserved keyword, so the closure by that name in diff_cmd.rs
becomes make_lines.
- Tightened tail-expression temporary scopes let clippy prove the binding in
setup_test_env is inlinable, so let_and_return now fires there.
if_let_rescope changes when the scrutinee temporary drops in an if let/else.
The two sites in show_claude_config take cargo fix --edition's match rewrite,
which keeps the 2021 drop timing.
rustfmt.toml is kept rather than dropped: cargo fmt passes --edition from
Cargo.toml, but a bare rustfmt invocation has no crate context and falls
back to edition 2015, which cannot parse the let-chains the next commit
introduces. Pinning it there keeps format-on-save and pre-commit hooks in
agreement with CI.
clippy::collapsible_if is allowed crate-wide for now; the follow-up commit
adopts let-chains and removes the allow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every ecosystem mod.rs under src/cmds/ generated its module list with
automod::dir!(). rustfmt only follows literal `mod` items and cannot expand
macros, so `cargo fmt` never reached any of those files -- and neither did
CI's `cargo fmt --all -- --check` gate, which has been silently passing
over half the crate.
Demonstrated by appending `fn __probe( )->u8{let x=1;x}` to a file
under src/cmds/: with automod, `cargo fmt --all -- --check` reports no
diff; with an explicit `pub mod`, it reports the diff and `cargo fmt --all`
fixes it. The preceding commit had to invoke rustfmt directly for the same
reason; from here `cargo fmt` covers the crate on its own.
Listing modules by hand trades one failure mode for another, so build.rs
now guards the new one: automod compiled any stray .rs file, whereas an
explicit list silently drops a file whose `pub mod` line is forgotten --
never compiled, never linted, tests never run, and check-test-presence.sh
still reporting PASS because it only greps the file for #[cfg(test)]. The
build now fails with the missing declaration named.
Declaring the modules explicitly also exposed clippy::module_inception on
cmds::git::git, which clippy skips inside macro-generated code. git.rs is
renamed to git_cmd.rs, which resolves the lint and matches the convention
its siblings already follow -- diff_cmd, gh_cmd, glab_cmd, gt_cmd. Docs
that point at the module are updated; sample command output that merely
shows a "git.rs" path is left alone.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pin style_edition = "2024" in rustfmt.toml and reformat, ahead of the
edition 2024 bump. Setting style_edition explicitly decouples the reformat
from the crate edition, so this lands on edition 2021 and CI's
`cargo fmt --all -- --check` still passes.
Run through rustfmt directly rather than `cargo fmt`. Every module under
src/cmds/ is currently declared by automod::dir!, and rustfmt only follows
literal `mod` items, so `cargo fmt` cannot see those files -- the next
commit makes them reachable. Roughly two thirds of the diff is therefore in
src/cmds/, mostly the version-sorted imports style edition 2024 introduces.
No logic changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pins both directions: the commands that report hook state themselves stay
quiet, and the filtered commands still warn. Also pins the reason the
suppression is worth having — `maybe_warn` touches the once-a-day marker, so
warning on a meta command would leave the next filtered command silent.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`rewritten_head_spellings_match_native_on_non_utf8_files` isolates
`CLAUDE_CONFIG_DIR` but reads the ambient `XDG_CONFIG_HOME`, so a config
carrying `exclude_commands = ["head", "tail"]` suppresses the rewrite and
the exit-code-3 assertion fails. Clean runners have no such config, so the
test passes in CI and fails only for the developers most likely to have
opted out of these very rewrites.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The pipeline diagram, the discover walkthrough and the `rtk read` option
table still described the `--max-lines` mapping this branch replaced, and
the repo's own suggest hook still proposed `--max-lines N` for `head -N`
— the mapping this branch exists to correct.
Drop the `head -` disjunct in the rewrite guard: every string with that
prefix also has the `head ` prefix that sits next to it.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`get_by_command` moved to the volume-weighted rate while this branch was
open (#891), summing every call and guarding the division at group level.
The two telemetry queries now use the same shape: the row-level
`WHERE input_tokens > 0` is gone, so a call with no input that still
printed something counts against its command exactly as it does in the
By Command table, and `HAVING SUM(input_tokens) > 0` keeps commands that
never had any input out of both, as before.
`low_savings_commands` lists net-regressing commands: `sav > 0.0` became
`sav <> 0.0`, so only exact 0% stays with `passthrough_top`. A command
that emits more than it saves is the filter to fix first, and with the
weighted rate the old bound also dropped commands the unweighted query
used to list.
The two regression tests opened the on-disk tracker, so every `cargo test`
wrote ten synthetic rows (one at 100k input tokens) into whichever history
database `Tracker::new()` resolves to, on a developer machine the real one.
They now use `Tracker::new_in_memory()` like the rest of the module, and
`test_avg_savings_per_command_inner_weighted` only asserted a `0..=100`
range that the unweighted query also satisfied. Both tests now assert the
exact weighted figures, check each listed rate against `get_summary()`,
and fail when either query goes back to `AVG`, to the row filter, or to
the positive bound.
Tests are numbered 18 and 19 after the ones develop already carries as
14-17. The `low_savings_commands` doc no longer points at a PR, and both
telemetry field tables describe the new semantics.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Move the weighted-rate regression test off the real tracking database, which
it wrote two rows into on every run, and onto an in-memory tracker. Add the
long-tailed pair that pins the weighted rate against the mean of percentages,
the same assertion through get_summary(), and a zero-input group: without the
CASE guard that last one fails on a NULL rate, and no other test covers it.
Name the 4th field of GainSummary::by_command for what it now holds, and state
the rate formula on the CommandStats warning in percentage order, with the zero
guard, so that following it verbatim cannot reintroduce the NULL.
Head the column Total% rather than Rate, keeping the shape of the Avg% it
replaces and naming the total the figure is now taken over.
Drop the hand-written CHANGELOG entry: release-please generates it from the
commit subject, and CONTRIBUTING.md asks for that file to be left alone.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Filtering a failing `gh pr checks` makes the summary the only thing the user
sees, so a bucket it does not count disappears. `parse_pr_check_line` dropped
every row that was not pass/fail/pending, which turned a cancelled run -- the
usual outcome when a new push supersedes CI -- into "Passed: 0, Failed: 0",
and left 5 of PR 3982's 15 checks out of the totals.
Count the remaining buckets and report them, so the summary always adds up to
the checks gh listed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`gh pr checks` spells a pending bucket `pending`, never `*`, and prints five
tab-separated fields with the description last. The transition test used a
shape gh does not emit; the new case carries the trailing description column
and a pending row that resolves across polls.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects in `filter_ast_grep`, both verified against ast-grep 0.45.3.
`ast-grep scan` diagnostics parse only on their locator line, so filtering
line by line kept ` ┌─ a.rs:2:13` and dropped the rule id, severity,
message and source line — at exit code 0, so it read as success. The
whole-output fallback never fired because `order` was not empty.
`unparsed_signal()` now passes any shape through untouched when a single
non-blank line fails to parse, which is what `search.rs` already does for
grep/rg. This also covers `--heading` mode and Windows drive-letter paths,
where `[^:]+` cannot match `C:\src\a.rs`.
The per-file overflow hint was computed as `entries.len() - max_per_file`,
which ignores `max_total` cutting a file short: a file under its own cap
lost its remainder with no hint at all, and a file over it under-reported
the drop. Both now count against what was actually printed.
The hint said "matches" while counting lines. ast-grep prints one line per
matched source line and a structural match spans several, so a repo search
reported "19 more matches" where five matches remained.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`RtkRule::pipeline_final_safe` was replaced by the `PipelineSafety` enum
on develop (#3171). The ast-grep rule still set the old field, so the
branch merged cleanly but failed to compile (E0560).
`ProducerOnly` keeps the rule's original intent: safe as a pipeline's
first stage, never as its final stage, since run() execs with stdin null.
Adding a producer-safe rule also requires listing it in
`test_pipeline_producer_safe_rule_set`, which pins the exact set.
`savings_pct` was left at the 60.0 default; the repo-wide measurement is
84.6%, which is what README and what-rtk-covers.md already claim.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codex and Vibe both return empty rule vectors; keeping them as separate
arms multiplies branches that have to stay in sync. Group them, sorted,
with one comment covering both.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Nicolas Le Cam