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>
Six cases over a fake tool on PATH, covering both halves of the contract: the
tool's own stderr has to reach the user, and rtk must not invent a stdout
message in its place.
The prettier case is the one that is easy to get wrong. It writes its report to
stderr and nothing to stdout even on a successful run, so a filter's empty-input
placeholder ("Error: prettier produced no output") would be printed over a run
that worked. An empty stdout from a command that printed nothing on stdout is
the correct answer.
Also pinned: stderr is not replayed onto stdout on top of being forwarded, a
genuinely silent command stays silent, forwarding is not golangci-specific, and
exit codes propagate -- including golangci-lint's exit 1, which means issues
were found and is reported without failing the build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`RunOptions::stdout_only()` is documented as "stdout-only to filter, stderr
passthrough", but the passthrough half was never implemented: stderr was captured
and dropped on every path except `skip_filter_on_failure`. A tool that reports on
stderr and leaves stdout empty therefore produced nothing at all.
golangci-lint is the visible case -- a config or build error goes to stderr, the
filter is handed an empty stdout, and the never-worse guard replaces its
"JSON parse failed" fallback with the empty string, so both streams end up
silent. The same shape applies to every other stdout-only filter: ruff, pytest,
rspec, rubocop, gh, glab, prettier, tree, wc.
Savings are also measured against running the command directly, so the stderr
that is forwarded has to be counted as emitted. Comparing stdout to stdout while
printing stderr as well booked a passed-through stream as if it had been
filtered away.
The guard keeps comparing against stdout. stderr is forwarded verbatim on both
sides, so it cancels: never-worse reduces to `filtered <= stdout`, and widening
the baseline to the combined output would let rtk emit more than the command it
replaces.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
develop rewrote `run_show`'s routing underneath this branch (#3265, eight
commits) into `ShowRoute` with a `cat-file` probe for blob detection. That
structure is newer and better than what this branch had, so it is kept whole
and this branch's contribution is folded into it rather than the other way
round.
`commit_or_stat_route` now classifies with the tokenizer instead of scanning
strings. develop matched `a == "--stat"` and `a.starts_with("--pretty")`,
which reads a pathspec named `--stat` past the boundary as the flag and
`--prettyish` as `--pretty`, and covers three stat spellings where the set of
shapes the compaction cannot render is larger. `git show -- --stat` takes the
compact path again.
`consumes_next_token_as_value` is reimplemented on `log_takes_value` rather
than restored as its own table. develop's copy is a subset -- it omits
`--max-count`, `--ignore-matching-lines`, `--min-age`, `--max-age` and
`--stat-graph-width` -- and keeping both would be the two-lists-drift this
branch exists to remove. Its callers are unchanged, so blob-show keeps its
own grammar rules; the attachment distinction (`-M50` attaches, `-M 50` does
not) is what the tokenizer adds.
This branch's `is_blob_show_arg` and free-positional blob pre-filter are
dropped in favour of develop's, which also handles index and merge-stage
blobs (`:file`, `:2:file`) and probes with `cat-file` rather than trusting
the shape of the argument.
Verified: seven blob-show invocations byte-identical to develop, including
the `-wG a:b HEAD:blob` cluster their walker exists for; 3536 unit tests and
every integration suite green; clippy clean.
develop's `show_positionals` carries a `TODO(after #3681)` to replace its
hand-rolled short-cluster walk with the ValueSpec factorization. Left alone
deliberately -- that is follow-up on develop, as its author intended, not
something to change inside this merge.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`tokens_saved_24h` and `total_tokens_saved` both return `i64` from an unclamped
`SUM(saved_tokens)` -- deliberately, unlike the `as usize` readers that take
`.max(0)` for their unsigned API. A window whose filters emitted more than they
saved sums honestly negative, so `assert!(saved_24h >= 0)` panics on any DB
holding a net-negative row.
No ordering invariant replaces the floor: with a positive row inside the 24h
window and a larger negative one outside it, saved_24h (90) exceeds saved_total
(-110). Nothing about either value is assertable, so the test stops binding them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`rtk hook check` called `registry::rewrite_command` directly, with no
permission verdict and none of the gates the hooks apply. It therefore
reported a rewrite for command substitutions, file redirects and heredocs
that both hook paths refuse to touch -- the diagnostic disagreed with the
thing it exists to diagnose, and did so in the direction that matters, by
claiming RTK would rewrite a command it deliberately leaves alone.
Route it through `hooks::decision` so it answers the same question, and
report a deny rule distinctly from "no rewrite" rather than collapsing both
into one message. Both still exit 1.
That makes the answer agent-dependent, so `--agent` stops being discarded.
`AgentPath` records what actually differs between agents, which is whose
permission rules their hook consults: the six that decide in-process via
`rtk hook <agent>` use their own host's rules, the five whose plugin shells
out to `rtk rewrite` get Claude's (that entry point cannot be told who is
asking), and the six that install only a rules file have no hook and so no
rules at all. Every install target resolves -- including `codex` and
`openclaw`, which are install flags rather than `AgentTarget` variants --
and only a genuine typo is rejected.
What does *not* differ is a rewrite that changed nothing: every agent
discards it, the in-process hosts in `hook_cmd` and the others in their own
plugin, since `hooks/opencode/rtk.ts`, `hooks/pi/rtk.ts` (shared with omp)
and hermes' `__init__.py` all gate on `rewritten != command`. `AgentPath`
suppresses it for every variant. Only the bare `rtk rewrite` CLI reports the
no-op, and no agent consumes that answer raw.
Consulting no rules and ignoring `--agent` is what made the diagnostic
contradict every host: under a Claude deny rule for `git status` it reported
`rtk git status` while `rtk hook claude` refused the command outright; it
reported a rewrite for `rtk git status`, which no agent applies; and since
`--agent` selected nothing, the answer described no host in particular.
The expectations pinned in the characterization commit are updated here, in
the same commit, so the behavior change is visible as a diff rather than as a
test that quietly stopped asserting. That harness now also asserts the hook
exits 0 and never panics: a crash produces empty stdout, which would let every
"expect no output" assertion pass vacuously. The agent list is derived from
`AgentTarget::value_variants()` so a new variant fails the test instead of
silently becoming unanswerable.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`rtk hook <agent>` and `rtk rewrite` asked the same question -- may this
command be rewritten, and may the rewrite be auto-allowed? -- through two
independently written copies of the same four steps, over two isomorphic
enums (`HookDecision` and `RewriteOutcome`). A fix to the gate order, or a
new construct to refuse, had to be made twice. #3704 consolidated raw-command
lexing and named this duplication as its follow-up.
Move the decision into `hooks::decision`. What legitimately differs between
the callers stays outside it: `decide_with_params` takes both the permission
verdict and the rewrite parameters, so each host consults its own rules and no
test answers differently on a machine whose config.toml excludes a command
(#3146); `decide` is the wrapper that reads config for production callers.
The identity-rewrite policy is likewise applied by the caller that wants it:
`decide_for_agent` is the composition every hook shares, while `decide`
alone is what the `rtk rewrite` CLI renders.
That policy is the one place the two paths disagree. `get_rewritten`
suppressed a rewrite that changed nothing; `rewrite_cmd` had no such check and
reported it as a normal rewrite. Rather than silently picking a side, the
suppression is now an explicit `suppress_identity` applied at `hook_cmd`'s
single seam, with the difference and its one observable consequence documented
where it lives. Resolving it is a deliberate behavior change and is not part
of this refactor.
`get_rewritten`'s heredoc check is dropped as a duplicate rather than as the
guarantee itself: `rewrite_command` refuses heredocs through its own
`has_heredoc` (registry.rs:601,616), which is what the hook path was asking a
second time. It was also unreachable for the common forms, since a `<<`
operand reads as a file target and `contains_unattestable_construct` returns
first -- the exception being `<< /dev/null`, which that gate lets past and the
registry still refuses (#3980).
`rewrite_cmd::run` is reduced to exit-code rendering and `hook_cmd`'s eight
response builders are unchanged -- `HookDecision` keeps its name, so they
match on the shared type without edits. `rewrite_cmd`'s unattestable-construct
tests are dropped as verbatim duplicates of the shared module's that exercised
nothing in that file.
No behavior change. The characterization tests added in the previous commit
pass unmodified, and the out-of-crate suites are unchanged from their
pre-refactor baselines: hooks/claude/test-rtk-rewrite.sh 58/66 (the 8 are the
pre-existing audit-log gap, `rtk rewrite` never having logged), hermes 18
passed, scripts/test-all.sh 105/13/5. `rtk rewrite "git status"` benchmarks at
6.7ms.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The signed record() change (9da3270) lets a net-regressing filter store a
negative savings_pct, so both overall_savings_pct and avg_savings_per_command
can now be negative. Two telemetry tests still asserted `(0.0..=100.0)` against
the real user DB, so they panicked deterministically on any machine whose DB
held a net-negative row -- which is why CI failed on all three runners after the
approval, on test_enriched_stats_returns_valid_data.
Widen both assertions (test_enriched_stats and its twin test_get_stats_returns_tuple)
to the real invariant -- a saving never exceeds 100% -- dropping the stale 0
floor, and add a hermetic in-memory test pinning that a worsening command yields
a negative aggregate instead of a fake 0.
The v2 tag is the action's only tag and has not moved since 2024-11-27,
while development continued on main. Four fixes sit between them, one of
which matters here: the action runs cargo binstall komac, and cargo is
missing on ubuntu-slim runners without it.
Pin to the most recent functional commit rather than to main, whose two
newer commits are documentation only. Its inputs are a superset of the
tag's, so the step needs no other change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The frontmatter description renders as the page's meta description and
search snippet, so it advertised an install method list that omitted the
one the page now documents.
The winget job sat in the release workflow's critical path: cd.yml gates
update-latest-tag on build-release without `if: always()`, so a failed
manifest submission would fail release.yml and leave the 'latest' tag on
the previous version. Mark the job continue-on-error so a submission
failure cannot hold back the release.
Pin winget-releaser to 4ffc7888bffd451b357355dc214d43bb9f23917e (v2)
rather than the mutable major tag, matching the pinning underway across
the repo. The step receives a classic PAT with write access to public
repositories, so the resolved ref should not be able to move underneath
it.
`rtk rewrite`'s exit-code protocol is a public contract -- the claude and
cursor shell hooks, the opencode and pi TypeScript plugins, the hermes
Python adapter and openclaw all branch on it -- but no Rust test ever
called `run()`. `rewrite_cmd`'s own `exit_code_protocol` module asserts
against a locally re-implemented `expected_exit_code()` table, so the real
mapping could change without a single failure, including the #1155
invariant that a `Default` verdict must exit 3 and never 0.
Add an integration test that spawns the binary in a sandboxed
HOME/XDG_CONFIG_HOME/CLAUDE_CONFIG_DIR with project-level permission rules,
and pins the actual (exit code, stdout) pairs for allow, ask, deny,
compound deny, passthrough, default, compound rewrite, fd-dup redirect,
unattestable constructs and heredocs.
Alongside it, pin the two decision paths against each other on one corpus.
`rtk rewrite` and `rtk hook claude` answer the same question through two
independently written flows; the corpus asserts they agree, and a separate
test pins the one place they don't -- an already-RTK-prefixed command,
which the in-process hook defers on and `rtk rewrite` reports as an
ask-rewrite with the command unchanged. Modelled on registry.rs's
`segmenter_consistency` module.
Also pin `rtk hook check`, which had no test at all. It calls
`rewrite_command` directly with no verdict and none of the hooks' gates, so
it reports a rewrite for command substitutions and file redirects that both
hook paths refuse to touch.
No source change: this characterizes today's behavior so the decision-flow
consolidation can be shown to preserve it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A non-numeric pattern stops at clap's parse error and falls back to raw
grep, so rtk and grep agree byte-for-byte whether or not `-l` is bound to
--max-len: the three existing cases still pass with the bug reintroduced.
Only a numeric pattern reaches the wrong answer -- rtk printed nothing and
exited 1 where grep listed the file.
Drop the clap-level test and the comment sentence with it. Develop's
test_grep_parse_files_with_matches_l already fails under the same mutation,
and -L never had a short binding on --max-len, so src/main.rs is untouched.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The usage guard returns early when no hash and no --list is given, and
--list returns before this match, so the arm could never run. Removing it
keeps the usage string in one place.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The blocker hid for three rounds because the fuzzer only ever paired a colon-carrying
flag cluster (`-wG x:y`, `-pS url:1`, …) with a blob-LESS `HEAD` (the CommitMisroute
trap). It never placed such a cluster BEFORE a real blob, which is exactly where the
walker gap bit.
Adds four crossed kinds:
* ClusterLargeUtf8 — cluster + large UTF-8 blob → MUST window (0 silent loss);
* ClusterLatin1 — cluster + Latin-1 blob → byte-identical to `git show <same args>`,
no U+FFFD mojibake;
* TextconvLargeUtf8 / PathspecLargeUtf8 — `--textconv` or a trailing `-- <path>` →
never windowed, byte-identical passthrough.
Plus a dedicated end-to-end test that `-wG x:y HEAD:large.txt` windows and that its
recovery hint (which names only the blob arg) reconstructs the blob byte-for-byte. The
extended fuzzer FAILS on the pre-fix code (verified: cluster windowing regression +
Latin-1 fidelity break) and passes on the fix. Seed and iteration count unchanged.
The blob-show classifier still had a selection gap for short-flag CLUSTERS. git
re-parses a cluster's tail (`-wG x:y` == `-w -G x:y`), so `git show -wG x:y
HEAD:big` feeds `x:y` to -G and dumps only `HEAD:big`. But `show_positionals`
knew only single-letter value flags, so it kept `x:y` as the first positional; the
cat-file probe rejected it and the real blob fell through to the commit-diff path —
either dropping the windowing savings (large UTF-8 blob) or lossily decoding a Latin-1
blob into U+FFFD mojibake.
Two changes, neither of which mirrors git's flag grammar:
* Cluster-aware walker. `flag_token_consumes_next` extends the existing flag/value
table to a short cluster's tail: the first value-taking short flag takes the rest of
the cluster inline (`-Sfoo`) or, if it is the last char, the next token (`-wG x`).
`is_short_value_flag` derives its char set FROM `consumes_next_token_as_value` so
the table stays the single source of truth. (TODO points at #3681's ValueSpec.)
* Probe every candidate, keep any blob on the byte-safe path. `run_show` now
cat-file-probes EVERY `rev:path` positional, not just the first, and takes the
byte-safe path whenever ANY resolves to a blob — so even a walker miss can't misroute
a blob's raw bytes through the lossy commit-diff decode. Windowing itself stays gated
on a single sole positional, which also keeps `git show <commit> <rev:path>` (two
real objects git concatenates) as byte-identical passthrough.
Also force byte-identical passthrough (never window) when a content-transforming flag
(--textconv/--filters/--ext-diff) or a trailing `-- <pathspec>` is present: the
`git show rev:path | tail` recovery hint omits them, so windowing could not reconstruct
git's bytes exactly. Narrows the fidelity-invariant comment accordingly.
`blob_show_target` (first-positional-only) is replaced by `blob_candidates` (all
candidates); unit tests migrated and extended to cover the cluster shapes.
Nicolas Le Cam