feat: TOML Part 1 — filter DSL engine + 14 built-in filters#349
Merged
pszymkowiak merged 9 commits intomasterfrom Mar 10, 2026
Merged
feat: TOML Part 1 — filter DSL engine + 14 built-in filters#349pszymkowiak merged 9 commits intomasterfrom
pszymkowiak merged 9 commits intomasterfrom
Conversation
This was referenced Mar 5, 2026
Closed
2093f33 to
9ad4540
Compare
navidemad
added a commit
to navidemad/rtk
that referenced
this pull request
Mar 6, 2026
Remove bundle_cmd.rs and rails_cmd.rs from this branch per upstream feedback (PR rtk-ai#349). These modules will be re-added in a separate feat/ruby-bundle-rails branch for future TOML filter DSL. Removed: Bundle/Rails enum variants, RailsCommands sub-enum, discover rules/registry tests, smoke test assertions. Renamed test-rails.sh to test-ruby.sh (rspec+rubocop only). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
5 tasks
Collaborator
Code Review — TOML Filter DSLThe TOML engine itself is well-designed: Note: This branch needs a rebase onto current master before merge (missing Graphite support, diff limit fixes, Discord notification, 0.27.2 changelog). The issues below are specific to this PR's code, not rebase artifacts. P1 — ImportantP1-1: Missing trailing newline in TOML-filtered output P1-2: Dead TOML filters — P1-3: P1-4: Tee hint silently discarded for TOML-filtered commands P1-5: P2 — Minor
P3 — Nits
Action items
|
aeppling
approved these changes
Mar 6, 2026
FlorianBruniaux
added a commit
that referenced
this pull request
Mar 7, 2026
P1-1: println! — add trailing newline to TOML-filtered output
print!("{}", filtered) → println!("{}", filtered). Fixes broken
pipe compatibility (rtk make all | wc -l returned 0 for 1 line).
P1-2: remove dead filters — git-checkout, git-remote, git-merge, cargo-run
Clap routes these commands to git.rs/cargo_cmd.rs before run_fallback
is reached, so these TOML filters never activate. Tests passed but
gave false confidence. Removed filter definitions and test sections.
P1-3: map_or instead of is_none_or (MSRV compatibility)
is_none_or requires Rust 1.82+. Replaced with map_or(true, ...) which
compiles on all supported toolchain versions.
P1-4: print tee hint for TOML-filtered commands on failure
let _ = tee::tee_and_hint(...) silently discarded the hint path.
Now follows the same pattern as runner.rs: if let Some(hint) = ... {
println!("{}", hint); }. LLMs can now find the full output file.
P1-5: --require-all now runs integrity check
Previously, `rtk verify --require-all` branched away from
integrity::run_verify(). Now: filter-specific mode (--filter foo)
skips integrity, but default and --require-all always run it first.
P2-1: RTK_NO_TOML checks value not presence
.is_ok() matched any value including RTK_NO_TOML=0.
Replaced with .ok().as_deref() == Some("1").
P2-5: terraform-plan regex — add word boundary after "plan"
^terraform\s+plan → ^terraform\s+plan(\s|$) to prevent matching
"terraform planning" or other plan-prefixed subcommands.
P2-6: iptables regex — \b → (\s|$)
^iptables\b matched iptables-save and iptables-restore (\b fires at
hyphen). Fixed to ^iptables(\s|$).
All 715 tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Introduces a TOML-driven filter DSL (engine + built-in rules) that allows RTK to filter previously-unknown commands via run_fallback(), plus a rtk verify mode to execute inline TOML tests.
Changes:
- Added
src/toml_filter.rsimplementing TOML parsing/compilation, filter application pipeline, registry lookup, and inline test runner. - Added
src/builtin_filters.tomlwith built-in filters + inline test cases, and addedrtk verifysupport viasrc/verify_cmd.rs+main.rswiring. - Updated fallback execution to route unknown commands through TOML filters when matched, plus documentation/version and ignore-list updates.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/toml_filter.rs |
Core TOML filter engine, registry, pipeline primitives, and inline test execution. |
src/builtin_filters.toml |
Shipped built-in filters + inline tests for verification. |
src/verify_cmd.rs |
rtk verify implementation for running TOML inline tests and enforcing --require-all. |
src/main.rs |
CLI wiring for verify options and TOML-based routing inside run_fallback(). |
README.md |
Updated install verification snippet version string. |
CLAUDE.md |
Updated install verification snippet version string. |
CHANGELOG.md |
Documented new TOML DSL feature set and built-in filters. |
ARCHITECTURE.md |
Updated module counts/version references. |
.gitignore |
Added ignores for .vitals/ and .worktrees/, normalized claudedocs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+982
to
+988
| // TOML match: capture stdout for filtering | ||
| let result = std::process::Command::new(&args[0]) | ||
| .args(&args[1..]) | ||
| .stdin(std::process::Stdio::inherit()) | ||
| .stdout(std::process::Stdio::piped()) // capture | ||
| .stderr(std::process::Stdio::inherit()) // stderr always direct | ||
| .output(); |
There was a problem hiding this comment.
Using Command::output() for TOML-matched commands buffers stdout until the process exits, which regresses the streaming UX compared to the existing passthrough .status() path. For long-running commands like mvn/tofu this can look like the command is stuck. Consider switching to a spawned/streaming capture (read stdout incrementally into a buffer while forwarding to the terminal, then filter/print at the end), or at minimum emitting a stderr note in interactive terminals that output is being buffered for filtering.
Comment on lines
+1879
to
+1886
| if filter.is_some() { | ||
| // Filter-specific mode: run only that filter's tests | ||
| verify_cmd::run(filter, require_all)?; | ||
| } else { | ||
| // Default or --require-all: always run integrity check first | ||
| integrity::run_verify(cli.verbose)?; | ||
| verify_cmd::run(None, require_all)?; | ||
| } |
There was a problem hiding this comment.
rtk verify --filter <name> currently skips integrity::run_verify, but the command-level docstring says it verifies hook integrity and runs TOML tests. Either run the integrity check regardless of --filter, or update the CLI help text/flag semantics so users don’t get a partial verify unexpectedly.
Suggested change
| if filter.is_some() { | |
| // Filter-specific mode: run only that filter's tests | |
| verify_cmd::run(filter, require_all)?; | |
| } else { | |
| // Default or --require-all: always run integrity check first | |
| integrity::run_verify(cli.verbose)?; | |
| verify_cmd::run(None, require_all)?; | |
| } | |
| // Always run integrity check first, regardless of filter, so that | |
| // `rtk verify` consistently verifies hook integrity *and* runs tests. | |
| integrity::run_verify(cli.verbose)?; | |
| verify_cmd::run(filter, require_all)?; |
Comment on lines
+385
to
+391
| // 7. max_lines — absolute cap applied after head/tail (includes omit messages) | ||
| if let Some(max) = filter.max_lines { | ||
| if lines.len() > max { | ||
| let truncated = lines.len() - max; | ||
| lines.truncate(max); | ||
| lines.push(format!("... ({} lines truncated)", truncated)); | ||
| } |
There was a problem hiding this comment.
The max_lines stage comment says this is an “absolute cap”, but the implementation truncates to max and then appends a "... (N lines truncated)" line, resulting in max + 1 output lines. Either enforce a true cap (e.g., replace the last kept line with the truncation marker) or adjust the docs/primitive description so users understand the actual behavior.
Comment on lines
+460
to
+466
| let file: TomlFilterFile = match toml::from_str(content) { | ||
| Ok(f) => f, | ||
| Err(e) => { | ||
| eprintln!("[rtk] warning: TOML parse error during verify: {}", e); | ||
| return; | ||
| } | ||
| }; |
There was a problem hiding this comment.
collect_test_outcomes parses TOML for rtk verify but doesn’t validate schema_version like parse_and_compile does. This can make rtk verify pass/run tests for files that the runtime registry would reject (schema mismatch), leading to confusing inconsistencies. Add the same schema_version check here (and skip/err with a warning when unsupported).
Comment on lines
+91
to
+95
| [filters.tofu-plan] | ||
| description = "Compact OpenTofu plan output" | ||
| match_command = "^tofu\\s+plan" | ||
| strip_ansi = true | ||
| strip_lines_matching = [ |
There was a problem hiding this comment.
Several match_command patterns here aren’t anchored to a subcommand boundary (e.g., ^tofu\s+plan), so they will also match strings like tofu planner ... or any future subcommand starting with plan. Consider consistently using a boundary like ^(...)\s+plan(\s|$) (as done for terraform-plan and iptables) for tofu-* filters to avoid accidental matches.
ARCHITECTURE.md
Outdated
Comment on lines
+296
to
+301
| **Total: 59 modules** (38 command modules + 21 infrastructure modules) | ||
|
|
||
| ### Module Count Breakdown | ||
|
|
||
| - **Command Modules**: 34 (directly exposed to users) | ||
| - **Infrastructure Modules**: 18 (utils, filter, tracking, tee, config, init, gain, etc.) | ||
| - **Infrastructure Modules**: 20 (utils, filter, tracking, tee, config, init, gain, toml_filter, verify_cmd, etc.) |
There was a problem hiding this comment.
The module totals are internally inconsistent: the header says “59 modules (38 command + 21 infrastructure)”, but the breakdown immediately below says “34 command” and “20 infrastructure”. Please reconcile these counts so the doc is self-consistent (either update the breakdown numbers or the total).
Comment on lines
+994
to
+1003
| // Tee raw output BEFORE filtering on failure — lets LLM re-read if needed | ||
| let tee_hint = if !output.status.success() { | ||
| tee::tee_and_hint( | ||
| &stdout_raw, | ||
| &raw_command, | ||
| output.status.code().unwrap_or(1), | ||
| ) | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
In the TOML-match fallback path, tee output is only written on non-zero exit. This bypasses tee config modes like TeeMode::Always, and it also tees only captured stdout (stderr is inherited), so the printed "[full output: ...]" hint can be misleading/incomplete. Consider always calling tee::tee_and_hint(...) with a combined stdout+stderr buffer (or adjust the hint/tee logic to reflect that only stdout is captured here).
Suggested change
| // Tee raw output BEFORE filtering on failure — lets LLM re-read if needed | |
| let tee_hint = if !output.status.success() { | |
| tee::tee_and_hint( | |
| &stdout_raw, | |
| &raw_command, | |
| output.status.code().unwrap_or(1), | |
| ) | |
| } else { | |
| None | |
| }; | |
| // Tee raw output BEFORE filtering — lets LLM re-read if needed and | |
| // allows tee configuration (e.g., TeeMode::Always) to decide when to write. | |
| let tee_hint = tee::tee_and_hint( | |
| &stdout_raw, | |
| &raw_command, | |
| output.status.code().unwrap_or(0), | |
| ); |
|
|
||
| [filters.shopify-theme] | ||
| description = "Compact shopify theme push/pull output" | ||
| match_command = "^shopify\\s+theme\\s+(push|pull)" |
There was a problem hiding this comment.
match_command = "^shopify\\s+theme\\s+(push|pull)" should likely enforce an end/boundary after the subcommand (e.g. (\\s|$)) to avoid matching longer tokens like pushy/puller and to be consistent with other filters’ command-boundary regexes.
Suggested change
| match_command = "^shopify\\s+theme\\s+(push|pull)" | |
| match_command = "^shopify\\s+theme\\s+(push|pull)(\\s|$)" |
README.md
Outdated
|
|
||
| ```bash | ||
| rtk --version # Should show "rtk 0.27.x" | ||
| rtk --version # Should show "rtk 0.27.1" |
There was a problem hiding this comment.
Hard-coding the exact patch version in the install verification snippet is likely to become stale quickly. Prefer a looser check like "0.27.x" or "0.27.1 (or newer)" (as in CLAUDE.md) so the docs don’t require updates on every patch release.
Suggested change
| rtk --version # Should show "rtk 0.27.1" | |
| rtk --version # Should show "rtk 0.27.x" (or newer) |
Collaborator
Code Review — Local Testing ResultsTested PR branch locally ( Confirmed Issues1. tofu/mix regex overmatch (P2)
Verified with regex test: Affects: Fix: add match_command = "^tofu\\s+plan(\\s|$)"2. BTreeMap ordering — future shadow trap (P3)
No overlap exists in the 14 current filters, so this is safe today. Either switch to 3. max_lines = N produces N+1 lines (P3)
What's Good
|
Collaborator
Local review — round 3 (deep testing)Previously fixed (confirmed) ✅
🔴 Critical — must fix before mergeBUG-1: Missing trailing newline on ALL TOML-filtered output
Repro: Fix: BUG-2:
Repro: Fix: 🟡 Important — should fix
|
FlorianBruniaux
added a commit
that referenced
this pull request
Mar 9, 2026
- println! trailing newline on TOML-filtered output (BUG-1)
- RTK_NO_TOML check presence→value (as_deref==Ok("1")) (BUG-2)
- make filter: add on_empty="make: ok" + test (P2-3)
- TOML lookup: use basename of args[0] for absolute path support (P2-8)
- utils.rs: translate French docstrings to English (P2-9)
- main.rs: single error message on command-not-found, exit(127) (NEW)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FlorianBruniaux
added a commit
that referenced
this pull request
Mar 9, 2026
… clippy, docs macOS path - Add (\\s|$) word-boundary anchors to 6 tofu/mix match_command regexes (tofu-plan, tofu-init, tofu-validate, tofu-fmt, mix-format, mix-compile) - Add on_empty = "make: ok" to make.toml + update existing empty-output test - Fix build.rs clippy: map_or(false, ...) → is_some_and(..) - Add macOS alt path note to filter-workflow.md Mermaid diagram Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FlorianBruniaux
added a commit
that referenced
this pull request
Mar 9, 2026
…base Rebase on feat/toml-dsl-pr2 (post-PR-#349 fixes) caused the refactor commit to re-add dead filters that PR #351 had already removed: cargo-run, docker-compose-ps, docker-inspect, git-checkout, git-merge, git-remote, pnpm-build. These 7 files are removed to maintain the expected 36 filter count. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Collaborator
Re-review (round 3)LGTM ✅ — no issues found locally. |
Add a declarative TOML-based filter engine for zero-Rust command filtering, plus 14 built-in filters targeting open issues. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
P1-1: println! — add trailing newline to TOML-filtered output
print!("{}", filtered) → println!("{}", filtered). Fixes broken
pipe compatibility (rtk make all | wc -l returned 0 for 1 line).
P1-2: remove dead filters — git-checkout, git-remote, git-merge, cargo-run
Clap routes these commands to git.rs/cargo_cmd.rs before run_fallback
is reached, so these TOML filters never activate. Tests passed but
gave false confidence. Removed filter definitions and test sections.
P1-3: map_or instead of is_none_or (MSRV compatibility)
is_none_or requires Rust 1.82+. Replaced with map_or(true, ...) which
compiles on all supported toolchain versions.
P1-4: print tee hint for TOML-filtered commands on failure
let _ = tee::tee_and_hint(...) silently discarded the hint path.
Now follows the same pattern as runner.rs: if let Some(hint) = ... {
println!("{}", hint); }. LLMs can now find the full output file.
P1-5: --require-all now runs integrity check
Previously, `rtk verify --require-all` branched away from
integrity::run_verify(). Now: filter-specific mode (--filter foo)
skips integrity, but default and --require-all always run it first.
P2-1: RTK_NO_TOML checks value not presence
.is_ok() matched any value including RTK_NO_TOML=0.
Replaced with .ok().as_deref() == Some("1").
P2-5: terraform-plan regex — add word boundary after "plan"
^terraform\s+plan → ^terraform\s+plan(\s|$) to prevent matching
"terraform planning" or other plan-prefixed subcommands.
P2-6: iptables regex — \b → (\s|$)
^iptables\b matched iptables-save and iptables-restore (\b fires at
hyphen). Fixed to ^iptables(\s|$).
All 715 tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prevents overmatch on subcommands (e.g. `tofu planet` or `mix formats`). Mirrors the same `(\s|$)` pattern already used by terraform-plan. Fixes: tofu-plan, tofu-init, tofu-validate, tofu-fmt, mix-format, mix-compile Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- println! trailing newline on TOML-filtered output (BUG-1)
- RTK_NO_TOML check presence→value (as_deref==Ok("1")) (BUG-2)
- make filter: add on_empty="make: ok" + test (P2-3)
- TOML lookup: use basename of args[0] for absolute path support (P2-8)
- utils.rs: translate French docstrings to English (P2-9)
- main.rs: single error message on command-not-found, exit(127) (NEW)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fixes pre-push validation: README, CLAUDE.md, ARCHITECTURE.md were still referencing 0.27.1 (Cargo.toml is 0.27.2). Also updates module count from 59 to 60 to match main.rs (toml_filter + verify_cmd). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
709a4e6 to
c108c8f
Compare
FlorianBruniaux
added a commit
that referenced
this pull request
Mar 10, 2026
… clippy, docs macOS path - Add (\\s|$) word-boundary anchors to 6 tofu/mix match_command regexes (tofu-plan, tofu-init, tofu-validate, tofu-fmt, mix-format, mix-compile) - Add on_empty = "make: ok" to make.toml + update existing empty-output test - Fix build.rs clippy: map_or(false, ...) → is_some_and(..) - Add macOS alt path note to filter-workflow.md Mermaid diagram Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
FlorianBruniaux
added a commit
that referenced
this pull request
Mar 10, 2026
…base Rebase on feat/toml-dsl-pr2 (post-PR-#349 fixes) caused the refactor commit to re-add dead filters that PR #351 had already removed: cargo-run, docker-compose-ps, docker-inspect, git-checkout, git-merge, git-remote, pnpm-build. These 7 files are removed to maintain the expected 36 filter count. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pszymkowiak
pushed a commit
that referenced
this pull request
Mar 10, 2026
… clippy, docs macOS path - Add (\\s|$) word-boundary anchors to 6 tofu/mix match_command regexes (tofu-plan, tofu-init, tofu-validate, tofu-fmt, mix-format, mix-compile) - Add on_empty = "make: ok" to make.toml + update existing empty-output test - Fix build.rs clippy: map_or(false, ...) → is_some_and(..) - Add macOS alt path note to filter-workflow.md Mermaid diagram Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pszymkowiak
pushed a commit
that referenced
this pull request
Mar 10, 2026
…lates, 4 new built-in filters (#351) * docs: update module count to 58 (toml_filter + verify_cmd) * docs: bump version refs to 0.27.1, fix module count to 59 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat: TOML DSL PR 2 — user-global config, shadow warning, init templates, 4 new filters - toml_filter: add ~/.config/rtk/filters.toml (priority 2, between project and built-in) - toml_filter: shadow warning when match_command overlaps a Rust-handled command - init: rtk init generates .rtk/filters.toml template (local) and ~/.config/rtk/filters.toml (global) - builtin_filters: add pre-commit, helm, gcloud, ansible-playbook (46/46 inline tests) - README: add "Custom Filters" section with lookup table, primitives, examples, built-in list Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(filters): add quarto-render, pnpm-build, trunk-build from rtk discover data (52/52 tests) * feat(filters): add docker-inspect, sops, docker-compose-ps built-in filters Resolves issues #279, #277, #276 as TOML-native filters (no Rust required). - docker-inspect: strip_ansi + truncate_lines_at=120 + max_lines=60 - sops: strip_ansi + strip_lines_matching blank lines + max_lines=40 - docker-compose-ps: strip_ansi + truncate_lines_at=120 + max_lines=40 6 inline tests added (2 per filter), all passing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(filters): split monolithic builtin_filters.toml into 28 individual files Replace the 848-line src/builtin_filters.toml monolith with individual files under src/filters/<name>.toml (1 filter + tests per file). Files are concatenated alphabetically by build.rs at compile time into OUT_DIR/builtin_filters.toml, then embedded via a BUILTIN_TOML constant in toml_filter.rs. The build step validates TOML syntax and detects duplicate filter names across files. Benefits: - Zero merge conflicts when multiple PRs add filters (different files) - Clear PR diffs: "+1 file of 30 lines" vs "+30 lines in 850-line file" - Easy onboarding: copy any .toml, rename, edit 3 fields — done - Build-time TOML validation catches syntax errors before tests run Changes: - build.rs (new): concat + validate src/filters/*.toml - Cargo.toml: add [build-dependencies] toml = "0.8" - src/filters/*.toml (28 files): split from monolith - src/filters/README.md (new): contributor guide - src/toml_filter.rs: BUILTIN_TOML const + 5 include_str replacements - src/builtin_filters.toml: deleted Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(filters): add 5 new unit tests for multi-file architecture + wget filter Unit tests added (toml_filter.rs): - test_builtin_toml_has_schema_version: ensures build.rs injects schema_version - test_builtin_all_expected_filters_present: guards against accidental file deletion - test_builtin_filter_count: exact count check (fails if filter added/removed without update) - test_builtin_all_filters_have_inline_tests: prevents shipping filters with no tests - test_new_filter_discoverable_after_concat: simulates build.rs concat, verifies lookup New built-in filter: - src/filters/wget.toml: compact wget download output (strips connection/resolution noise, short-circuits on 'saved [' to 'ok (downloaded)', 2 inline tests) Test results: 661/661 unit tests, 60/60 inline TOML filter tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: add filter-workflow.md with Mermaid diagrams Build pipeline + runtime lookup priority, both as Mermaid flowcharts. Shows the full path from src/filters/*.toml to binary to execution. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove dead TOML filters — wget, git-checkout, git-merge, git-remote, cargo-run These filters were never reachable: Clap routes rtk git/cargo commands to their dedicated Rust modules before run_fallback is called. The TOML engine only fires for unknown commands. Shipping dead filters gives false confidence (inline tests pass, but filters never activate in production). Also fixes P1-1 from the PR #386 review: wget was in RUST_HANDLED_COMMANDS and the shadow warning fired at compile time but did not block the build. Removed: cargo-run.toml, git-checkout.toml, git-merge.toml, git-remote.toml, wget.toml Updated test guards: count 29→24, expected list -5 names, concat test 30→25. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: remove 3 dead TOML filters + update guards (PR #351) docker-inspect, docker-compose-ps, and pnpm-build are handled by container.rs and pnpm_cmd.rs before run_fallback is reached — their TOML filters never fire. Remove the files to avoid false documentation. Update toml_filter.rs guards: expected list -3 names, count 24→21, concat test 25→22. Update CONTRIBUTING.md example count to match. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: rebase on PR #349 — apply regex anchors, make on_empty, build.rs clippy, docs macOS path - Add (\\s|$) word-boundary anchors to 6 tofu/mix match_command regexes (tofu-plan, tofu-init, tofu-validate, tofu-fmt, mix-format, mix-compile) - Add on_empty = "make: ok" to make.toml + update existing empty-output test - Fix build.rs clippy: map_or(false, ...) → is_some_and(..) - Add macOS alt path note to filter-workflow.md Mermaid diagram Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
pszymkowiak
pushed a commit
that referenced
this pull request
Mar 10, 2026
…base Rebase on feat/toml-dsl-pr2 (post-PR-#349 fixes) caused the refactor commit to re-add dead filters that PR #351 had already removed: cargo-run, docker-compose-ps, docker-inspect, git-checkout, git-merge, git-remote, pnpm-build. These 7 files are removed to maintain the expected 36 filter count. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pszymkowiak
pushed a commit
that referenced
this pull request
Mar 10, 2026
…et, swift, shellcheck, hadolint, poetry, composer, brew, df, ps, systemctl, yamllint, markdownlint, uv) (#386) * docs: bump version refs to 0.27.1, fix module count to 59 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(filters): split monolithic builtin_filters.toml into 28 individual files Replace the 848-line src/builtin_filters.toml monolith with individual files under src/filters/<name>.toml (1 filter + tests per file). Files are concatenated alphabetically by build.rs at compile time into OUT_DIR/builtin_filters.toml, then embedded via a BUILTIN_TOML constant in toml_filter.rs. The build step validates TOML syntax and detects duplicate filter names across files. Benefits: - Zero merge conflicts when multiple PRs add filters (different files) - Clear PR diffs: "+1 file of 30 lines" vs "+30 lines in 850-line file" - Easy onboarding: copy any .toml, rename, edit 3 fields — done - Build-time TOML validation catches syntax errors before tests run Changes: - build.rs (new): concat + validate src/filters/*.toml - Cargo.toml: add [build-dependencies] toml = "0.8" - src/filters/*.toml (28 files): split from monolith - src/filters/README.md (new): contributor guide - src/toml_filter.rs: BUILTIN_TOML const + 5 include_str replacements - src/builtin_filters.toml: deleted Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(filters): add 15 TOML filters (ping, rsync, dotnet, swift, shellcheck, hadolint, poetry, composer, brew, df, ps, systemctl, yamllint, markdownlint, uv) Tier 1 (high value): - ping: strip per-packet lines, tail_lines=4 keeps summary only (70%+) - rsync: match_output on "total size is" → "ok (synced)" (80%+) - dotnet-build: match_output on "Build succeeded" → "ok (build succeeded)" (70%+) - swift-build: match_output on "Build complete!" → "ok (build complete)" (70%+) - shellcheck: strip blank lines + caret indicator lines, max_lines=50 (60%+) - hadolint: strip blank lines, truncate_lines_at=120, max_lines=40 (60%+) - poetry-install: strip downloads, match_output on "No dependencies to install" (75%+) - composer-install: strip downloads, match_output on "Nothing to install" (75%+) - brew-install: strip Downloading/Pouring, match_output on "already installed" (70%+) Tier 2 (medium value): - df: truncate_lines_at=80, max_lines=20 (60%+) - ps-aux: truncate_lines_at=120, max_lines=30 (60%+) - systemctl-status: strip blank lines, max_lines=20 (65%+) - yamllint: strip blank lines, max_lines=50, truncate_lines_at=120 (60%+) - markdownlint: strip blank lines, max_lines=50, truncate_lines_at=120 (60%+) - uv-sync: strip downloads, match_output on "Audited"/"Resolved" (70%+) Updates test guards: count 29→44, expected list +15 names, concat test 30→45. All 90 inline TOML tests pass. 720 unit tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(filters): address PR #386 code review (P1 + P2 items) P1-2: rsync.toml — remove dead strip rule "^total size is \d" match_output fires at stage 3, strip at stage 4 — the rule was unreachable on success and removed a useful line on error paths. P2-1: ps-aux.toml renamed to ps.toml, match_command ^ps\b → ^ps(\s|$) Prevents false matches on pstree, psql, pstack. Filter name and test sections updated from ps-aux to ps. Guard list updated. P2-2: df.toml match_command ^df\b → ^df(\s|$) Prevents false matches on dfs, dfx. P2-4: shellcheck.toml — keep caret indicator lines (^-- SC...) Stripping them lost the exact character position of the error. Only blank lines are now stripped. P2-7: swift-build.toml — remove dead strip rule "^Build complete!" match_output short-circuits before strip runs on success paths. The rule was unreachable and confusing. All 90 inline tests pass. 720 unit tests pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: address PR #386 code review — 4 filter fixes - uv-sync: remove Resolved short-circuit (fires before package list) - dotnet-build: short-circuit only on 0 warnings + 0 errors (not on any "Build succeeded" — catches builds with warnings too) - poetry-install: support Poetry 2.x bullet syntax (• vs -) and "No changes." message; add Poetry 2.x test - ping: add Windows format support (Pinging + Reply from patterns); add Windows test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs(changelog): document round 2 fixes (PR #351 + #386) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix: rebase on updated PR #351 — remove 7 dead filters re-added by rebase Rebase on feat/toml-dsl-pr2 (post-PR-#349 fixes) caused the refactor commit to re-add dead filters that PR #351 had already removed: cargo-run, docker-compose-ps, docker-inspect, git-checkout, git-merge, git-remote, pnpm-build. These 7 files are removed to maintain the expected 36 filter count. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * feat(dsl): add 'unless' to match_output — prevent short-circuit when errors present Fixes two match_output bugs where success short-circuit fires before strip_lines_matching can surface errors in the full output blob. Changes: - Add optional `unless` field to MatchOutputRule (serde default = None) - Compile `unless` into Option<Regex> in CompiledMatchOutputRule - In apply_filter stage 3: if `unless` matches the blob, skip the rule and continue to the next match_output rule (or fall through to the pipeline) - rsync.toml: add `unless = "error|failed|No such file"` to the "total size is" rule; add test verifying errors are not swallowed - swift-build.toml: add `unless = "warning:|error:"` to the "Build complete!" rule; add test verifying warnings are not swallowed - CONTRIBUTING.md: document match_output[].unless in the field reference table - Unit tests: 5 new tests covering unless blocks/allows/falls-through/ no-regression/invalid-regex Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: fix README version string 0.27.1 → 0.27.2 after rebase Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * docs: fix version 0.27.1→0.27.2 and module count 59→60 in ARCHITECTURE.md + CLAUDE.md Version strings and module count were stale after rebase onto updated feat/toml-dsl-pr2. validate-docs.sh now passes clean. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This was referenced Mar 10, 2026
ahundt
added a commit
to ahundt/rtk
that referenced
this pull request
Mar 11, 2026
Incorporates 52 upstream commits (v0.27.0 → v0.28.2): - TOML filter DSL engine + 30 built-in filters (PRs rtk-ai#349, rtk-ai#351, rtk-ai#386) - Graphite CLI support (PR rtk-ai#290) - git commit -am/--amend fix via trailing_var_arg (PR rtk-ai#327) - restore_double_dash for cargo (PR rtk-ai#326) - gh -R/--repo passthrough, pr edit/comment fix (PRs rtk-ai#328, rtk-ai#332) - docker compose subcommand filtering (PR rtk-ai#336) - Telemetry tokens_saved + install_method (PRs rtk-ai#462, rtk-ai#469, rtk-ai#471) - proxy streaming (PR rtk-ai#268) - Diff limits increased (100→500 lines, 10→30 hunk lines) Conflict resolution (5 files): - cargo_cmd.rs: adopted upstream restore_double_dash, adapted streaming run_test() to use it, converted old split_at_double_dash tests - git.rs: adopted upstream simplified Commit unit variant (fixes -am), adapted all commit tests to flat args API, added 6 new edge case tests - init.rs: added TOML template generation alongside hook manifest - main.rs: merged both upstream (gt, toml_filter, verify) and hooks-v2 (cmd, hook, stream, pipe) modules, kept all tests from both sides - utils.rs: kept hooks-v2 command_in_path/which_command + upstream English docs Hook engine additions during merge: - Added gt to hook_lookup() whitelist with 4 routing test cases All 5 hook bug fixes from issue rtk-ai#361 preserved: 1. Streaming (stream.rs BufReader) 2. Handler coordination (parallel-merge + run_manifest_handlers on both paths) 3. Stderr deny (exit 2) 4. Routing whitelist (hook_lookup) 5. Vitest run injection 1182 tests pass (1 environment-dependent upstream test excluded).
ahundt
added a commit
to ahundt/rtk
that referenced
this pull request
Mar 11, 2026
Incorporates 52 upstream commits (v0.27.0 → v0.28.2): - TOML filter DSL engine + 30 built-in filters (PRs rtk-ai#349, rtk-ai#351, rtk-ai#386) - Graphite CLI support (PR rtk-ai#290) - git commit -am/--amend fix via trailing_var_arg (PR rtk-ai#327) - restore_double_dash for cargo (PR rtk-ai#326) - gh -R/--repo passthrough, pr edit/comment fix (PRs rtk-ai#328, rtk-ai#332) - docker compose subcommand filtering (PR rtk-ai#336) - Telemetry tokens_saved + install_method (PRs rtk-ai#462, rtk-ai#469, rtk-ai#471) - proxy streaming (PR rtk-ai#268) - Diff limits increased (100→500 lines, 10→30 hunk lines) Conflict resolution (5 files): - cargo_cmd.rs: adopted upstream restore_double_dash - git.rs: adopted upstream simplified Commit variant (fixes -am), fixed test_git_status_not_a_repo via GIT_DIR env override - init.rs: added TOML template generation alongside hook manifest, made resolve_claude_dir pub(crate) for config/mod.rs - main.rs: merged upstream (gt, toml_filter, verify) and multi-platform (cmd, hook, stream, safety, gemini) modules - utils.rs: accepted English doc comments Recovery edits (safety integration restored after incorrect overwrite): - hook/mod.rs: restored config::rules::try_remap(), safety::check_raw(), safety::check() per-command, FORMAT_PRESERVING/TRANSPARENT_SINKS pub(crate), basename extraction, safety-dependent tests - discover/registry.rs: updated 3 wc tests for upstream IGNORED_PREFIXES All hook engine + safety + gemini features preserved. 1332 tests pass, 0 failures, 5 ignored.
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.
What this PR does
Adds a declarative TOML-based filter engine that lets anyone add a new command filter without writing Rust. A TOML block in
src/builtin_filters.toml(or the user's.rtk/filters.toml) is enough for commands with stable, predictable output.This is a foundational change: it lowers the contribution bar for simple filters from ~200 LOC of Rust to a ~10-line TOML block.
Architecture
New files
src/toml_filter.rsrun_fallback()integrationsrc/builtin_filters.tomlinclude_str!)src/verify_cmd.rsrtk verifycommand — runs inline TOML tests, supports--require-allHow it works
User filters in
.rtk/filters.tomlare loaded at startup and merged with built-ins. User filters take precedence over built-ins.Filter pipeline
TOML vs Rust — decision guide
Filter primitives (8 total)
strip_ansistrip_lines_matchingkeep_lines_matchingreplacematch_outputtruncate_lines_athead_lines/tail_linesmax_lineson_empty14 new built-in filters
#240 — OpenTofu (4 filters)
Mirror of the existing
terraform-*filters, targetingtofuCLI.tofu-plantofu plantofu-inittofu inittofu-validatetofu validateSuccess!tofu-fmttofu fmton_emptyfor no changes#284 —
du(1 filter)Strips blank lines, truncates long paths at 120 chars, caps at 40 lines.
#281 —
fail2ban-client(1 filter)Strips blank lines, caps at 30 lines. Note: no
sudoprefix — the hook rewrite does not capturesudo, so the prefix would be dead code.#282 —
iptables(1 filter)Defensive regex: strips only Docker-autogenerated chains (
^Chain DOCKER,^Chain BR-). Does not strip the rule lines that follow those chains. Caps at 50 lines, truncates at 120 chars.#310 — Elixir
mix(2 filters)mix-formatmix formaton_empty→mix format: okmix-compilemix compileCompiling N files,Generated, blanks; preserves warnings/errors#280 — Shopify Theme (1 filter)
Strips
Uploading/Downloadinglines, keeps last 5 lines (the summary) viatail_lines, caps at 15.#231 — PlatformIO (1 filter, partial)
pio run: strips build noise (CONFIGURATION, LDF, Compiling, Linking, Building, Checking size). Errors and the final memory summary are preserved.Note: this closes the
pio runcase. Other commands from #231 (npm test,python,stty) remain open.#338 — Maven (1 filter, partial)
mvn compile|package|clean|install: usesstrip_lines_matching(notkeep) to strip[INFO] ---,[INFO] Building, download lines, and blanks. This preserves stacktraces, plugin warnings, and any unexpected output — only known noise is stripped.Note:
mvn test(Surefire state machine) is out of scope for TOML; it requires a Rust module.Inline test suite
Every filter has at minimum 2 inline tests (realistic fixture + edge case). Run with:
Test results at time of this PR: 38/38 passed.
Issues closed / addressed
Closed by this PR
rtkmust pass through unrecognized commands — resolved byrun_fallback()+ TOML routingdusupport — filter addedfail2ban-clientsupport — filter addediptablessupport — filter addedmix formatandmix compile— 2 filters addedPartially addressed
pio runfilter addednpm test,python,stty, other 7 commands from the issueshopify theme push/pullfilter addedmvn compile/package/clean/installfilter addedmvn test(needs Rust state machine for Surefire)Not addressed (documented reasons)
docker execstatstat -fvs Linux format divergence; savings < 60%; low utilitysshjjPRs with technical coordination needed
Based on the pre-merge impact analysis (
claudedocs/toml-impact-triage-2026-03-05.md):Conflicts (should merge before this PR ideally)
lazy_static→LazyLockmigrationlazy_static!intoml_filter.rs. If #306 merges first, we switch toLazyLock— cleaner result.run_fallback()stderr/--separatorrun_fallback(). Merging #326 first avoids a painful rebase.Architectural note (no code conflict)
Stdio::pipedfor matched commands; streaming behavior for unmatched commands (inherit) is unchanged. No conflict, but related architecturally.PRs NOT affected by this PR
All 7 PRs deep-dived in the impact analysis are Rust-required — they use state machines, JSON aggregation, or HashMaps that TOML primitives cannot express:
serde_json::Value, metrics aggregationBazelBuildState/BazelTestState, BTreeMap, streamingtest/connected/depssubcommands = state machinesin_failure_blockfailure trackingserde_json+HashMap<String, usize>aggregationThe ~35 other open PRs (Rust filters, bug fixes, docs, infra, hooks) are unaffected.
Out of scope (documented for follow-up)
rtk <cmd>. Addingtofu,du,fail2ban-client,iptables,mix,shopify,pio,mvnto the hook is a separate PR.rtk discoverTOML recommendations:rtk discovercould suggest TOML filters for unhandled commands — separate issue.Test plan
🤖 Generated with Claude Code