Repository navigation
Conversation
Users asked whether RTK breaks Claude's prompt cache. Short answer: no. RTK filters output once, the result is cached normally. Smaller outputs also improve cache economics (cheaper writes and reads). - Add FAQ blockquote in README after 'How It Works' section - Add TROUBLESHOOTING.md with Problem/Short Answer/Details pattern Signed-off-by: Florian BRUNIAUX <florian@bruniaux.com>
Replace the RTK reference (meta commands, rtk proxy bypass, install verification, branding, hook framing) with a minimal note stating that condensed output is complete/authoritative and commands should be run normally. Removes the runnable command blocks and bypass hint that led agents to run extra commands and re-verify. Claude only.
Bash's default $IFS is space/tab/newline, never a bare CR. The lexer's newline arm already conflated a lone \r with \n/\r\n as a command separator, and its generic whitespace arm independently treated \r as a word boundary too — two places that had to independently get this right and didn't. Fixes both: - tokenize_inner's newline arm now only splits on \n or the \r of a real CRLF pair; a lone \r falls through and stays glued to its word. - tokenize_inner's whitespace arm no longer treats \r as a boundary, via a new shared is_word_boundary_whitespace() predicate. - permissions.rs::command_matches_pattern now normalizes through that same shared predicate instead of str::split_whitespace() (which is \r-inclusive), so an allow rule like "git status" can no longer auto-approve "git status\rrm -rf ~" by collapsing the embedded CR into a space before matching. - registry.rs's raw_breaks parity check updated to match the corrected newline model (counts \n and CRLF's \r, never a lone \r). Regression tests added in lexer.rs and permissions.rs reproducing the exact divergence.
tokenize_inner, QuoteScan (registry.rs), and shell_split each independently tracked "am I inside a quote" - QuoteScan's own comment already admitted it was reimplementing "the same model the lexer applies" from scratch. Extract the one rule all three actually encode (an unescaped quote char opens a span if none is open, only the same character closes it) into a single advance_quote_state() primitive, and drive all three from it: - tokenize_inner's Option<char> quote tracking now calls it instead of duplicating the open/close branching inline. - QuoteScan switches its internal (in_single, in_double) bool pair to the same Option<char> model, built on the same primitive - its external (offset, byte, in_single_before, in_double_before) API and all 5 downstream consumers (comment_start, bracket/test-bracket balance, ansi_c_quote_defeats_lexer, quotes_balanced) are unchanged. - shell_split switches from its own bool-pair toggle to the same primitive, as a byproduct simplifying its match arms to one merged case. Behavior-preserving: all 2708 existing tests pass unchanged, including the ~750 lines of lexer.rs quote/escape coverage - a future quote-state correction now only needs to happen once.
split_token_spans was a from-scratch, quote-blind whitespace splitter with a single caller (golangci-lint's global-flag pre-processing). Replace it with the lexer's tokenize(), reusing its already-tracked byte offsets - genuine dedup, and a real fix: a quoted global-flag value containing a space (`--config "a path/x.yml"`) used to get mis-split at the space inside the quotes, which made parse_golangci_run_parts miss the `run` subcommand entirely and fall back to leaving the command unclassified. New regression test covers the quoted-value case; all other existing golangci-lint classification tests pass unchanged.
split_for_permissions (permission gate), split_on_operators / split_command_chain (analytics/discovery classification), and rewrite_compound's inline token walk (actual rewrite) all segment the same kind of compound-command string, but deliberately differently: pipe-stop behavior, whether background `&` or `(`/`)` grouping counts as a boundary, and whether a trailing redirect gets truncated all diverge across the three. That's intentional - the permission gate must stay the most conservative - but it was undocumented and untested as a set, so a future edit to any one of them could silently drift further from the other two without anything catching it. - Cross-referencing doc comments on all three functions, including a comparison table on split_for_permissions. - A new segmenter_consistency test module in registry.rs pinning today's actual, verified output for all three across four representative inputs (background &, subshell grouping, pipe+&&, redirect-in-segment) - including a real quirk this surfaced: in `(git status; cargo build)`, the rewritten output only prefixes "cargo build", not "git status", because the leading `(` glued to the first segment defeats rewrite_segment's own command matching while the trailing `)` on the second segment doesn't. Pinned as documented existing behavior, not changed here. No behavior change; pure documentation and test coverage.
Two follow-ups from /code-review high on this branch:
1. parse_golangci_run_parts's switch to the full shell tokenize() (Phase
3) fixed the intended quoted-value bug but introduced a real
regression: tokenize() splits unquoted shell metacharacters (*, ?,
`, (, ), {, }, !) into their own tokens even outside quotes, so an
unquoted glob value like `--config *.yml` desynced the flag-skip
loop and got the whole command misclassified as Unsupported.
split_token_spans's actual job - "was there a space here", not full
shell syntax - is genuinely different from tokenize()'s, so it
should not have been replaced by it. Restored split_token_spans as
its own function, now built on the shared advance_quote_state /
is_word_boundary_whitespace primitives instead of being quote-blind,
which is what actually fixes the original quoted-value bug without
the metacharacter regression. New regression test for the unquoted
case.
2. Extracted is_crlf_at() and used it in both tokenize_inner's
newline-operator guard and registry.rs's raw_breaks parity check -
this exact duplication (two independent "is this \r part of a CRLF
pair" checks) was flagged as a follow-up during the PR #3600 review
that prompted this whole consolidation, and had been reintroduced
here without actually being fixed.
All 2714 tests pass, clippy clean.
…s on it
Even after the previous consolidation, split_token_spans and shell_split
each still ran their own scanning loop over the raw command string,
just now sharing the same quote/whitespace *rules* with tokenize_inner
rather than the same *scan*. The gap between "one lexer token" and
"one bash word" is exactly the tokens tokenize() splits apart (e.g.
Shellism("*") + Arg(".yml") for an unquoted "*.yml") that sit with no
gap between them in the original string - bash sees one word there.
Add coalesce_words(cmd, tokens): merges directly-adjacent tokens from
tokenize()'s output into single words. Rebuild split_token_spans as a
one-line wrapper around tokenize() + coalesce_words, removing its own
quote-aware scanning loop entirely - golangci-lint's flag parser now
runs zero bespoke scanning code.
Existing quoted-value and unquoted-glob golangci-lint regression tests
(added when this exact call site regressed once already) pass
unchanged. All 2716 tests green, clippy clean.
shell_split's per-char scanning loop duplicated the same quote/escape handling tokenize_inner already does, differing only in two genuinely distinct concerns: it splits on whitespace only (not shell operators), and it resolves quotes/escapes into argv-ready text rather than preserving them. Split that into two composable pieces instead of one bespoke loop: - resolve_word_text(): given one coalesced word's raw text, strips quote characters and resolves backslash escapes - the inverse of what tokenize_inner preserves, built on the same advance_quote_state so it can't drift from the tokenizer's own quote model. - shell_split() becomes tokenize() + coalesce_words() (from the previous commit) + resolve_word_text() per word - no scanning of its own. All 15 existing shell_split tests pass unchanged (verified byte-for- byte before writing this commit). One real, intentional behavior refinement: word boundaries now come from the shared is_word_boundary_whitespace (bash's actual default $IFS - space, tab, newline) instead of shell_split's previous bespoke ' '|'\t'-only check, so an embedded unquoted newline is now correctly treated as a word boundary. Covered by a new explicit test, since this wouldn't have been caught by the existing suite otherwise. Also added a test combining an unquoted glob directly adjacent to a quoted segment (the same token-coalescing case that mattered for split_token_spans, exercised here through shell_split's quote-stripped output). This affects 3 real call sites: hooks/mod.rs::is_claude_hook_command, registry.rs::search_uses_pattern_file, and main.rs's `rtk proxy '...'` argv construction (the highest-risk one - it directly controls what gets exec'd). All covered by the existing suite, all green. 2718 tests pass, clippy clean.
Add `--agent omp` to `rtk init` (with `-g`, `--uninstall`, `--show`) for the Oh My Pi coding agent (https://github.com/can1357/oh-my-pi). OMP loads the same `hooks/pi/rtk.ts` extension via its built-in legacy-pi-compat layer, which remaps the Pi package imports to OMP's bundled equivalent — so no separate OMP implementation is needed and the rewrite behavior stays byte-identical (mutualization). - Local scope: <project>/.omp/extensions/rtk.ts - Global scope: ~/.omp/agent/extensions/rtk.ts - `--uninstall` is three-way safe: missing → no-op, stock content → removed, modified RTK content → bail with manual-removal guidance - `--show` reports both scopes (installed / stock / modified / absent) Co-authored-by: makoMakoGo <makoMakoGo@users.noreply.github.com>
The extension is shared with OMP via its legacy-pi-compat layer. Add
guarded helpers that are strict no-ops on Pi: a persistent
"RTK disabled: <reason>" session status registered on session_start
(OMP wipes one-shot notify toasts on the initial render) and a
setLabel("RTK") UI label set before the version probe, so the
extension is identifiable in the session UI even when rtk is missing
or too old.
Co-authored-by: makoMakoGo <makoMakoGo@users.noreply.github.com>
- Restrict is_word_boundary_whitespace to bash's actual $IFS (space/tab/newline) instead of Rust's char::is_whitespace(), which wrongly counts non-IFS Unicode whitespace like NBSP as a word boundary and broke shell_split on pasted commands containing it. - Restore the permission gate's conservative lone-CR segmentation. tokenize_inner takes a NewlineMode (None/Bash/Conservative) instead of a bool, so split_for_permissions can opt into treating a bare \r as a command separator (matching pre-consolidation behavior) while every other caller keeps the bash-accurate "lone CR stays glued to its word" rule. The mode lives inside the shared char loop, so it stays quote/escape-aware and doesn't split inside quoted text. - Trim doc comments down to their essential rationale: dropped references to review tooling/history, and cut multi-paragraph explanations to one or two lines, letting the code carry the rest.
Per review feedback on #3681 (document new shared systems in their module README): lexer.rs's public functions are consumed well outside discover/ (hooks/permissions.rs, hooks/mod.rs, main.rs's rtk proxy), so document them as shared infrastructure rather than leaving them as an internal implementation detail.
# Conflicts: # hooks/README.md
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
…oc drift, sorted families Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
…d env tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
…e migration notices Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
…ation driven Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
… prefix Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
…riven Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
`-l` was bound to RTK's `--max-len`, colliding with GNU grep where `-l` means `--files-with-matches`. Running `grep -l <pattern>` made clap read the pattern as a usize and error (`invalid value '<pattern>' for '--max-len'`), dropping the command to a raw-grep fallback at 0% savings. Drop the short binding. `-l`/`-L` now flow to `extra_args`, where the existing `has_format_flag` already routes them to the grep passthrough (GNU semantics, minimal output). `--max-len` keeps its long form and its default of 80; nothing internal used the short. - src/main.rs: remove `short = 'l'` from `Grep::max_len` + clap-parse regression test. - tests/grep_faithful_format_test.rs: end-to-end test asserting `rtk grep -l/-L` is byte-identical to `grep -l/-L`, covering `-l` leading and trailing. - scripts/benchmark.sh: the max-len bench used the old `-l 40` alias -> `--max-len 40`. - docs/usage/FEATURES.md: drop the `-l` short from the grep options table.
…_grep Address review: - benchmark.sh: with trailing_var_arg on extra_args, --max-len after the positionals was swallowed and forwarded raw to grep (unknown option -> bench FAIL). Move it before -rn/pattern so rtk parses it; verified 320KB -> 14.8KB (~95%) and that --max-len 40 vs 200 changes output size. - grep_faithful_format_test: the inline cmp closure duplicated the existing assert_eq_grep helper (rtk_grep + grep_plain, same stdout/exit asserts); drop it in.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
… gain --reset Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MNqtCagEUryc1fgw62APkm
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>
feat(recall): SQLite recall system
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>
test(grep): pin -l/-L parity with grep, including numeric patterns
Re-raise SIGINT/SIGTERM once output is printed so callers still see signal death, not a plain exit code.
fix(rewrite): process wrapper prefixes
aeppling
approved these changes
Sep 10, 2026
KuSh
approved these changes
Sep 10, 2026
3 tasks
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.
Feats
Fix
Other