[vdufloth]

Closes #1537.

Review response (PR review by @pszymkowiak, 2026-05-21)

Three commits address the review:

• cc152cd — fixes the blocking Surefire 3.x close-line regex, regenerates all five Surefire-touching fixtures from a real apache/commons-cli build inside maven:3.9-eclipse-temurin-21, adds a failure_trail state to retain the exception + user-code frames Surefire 3.x emits after the close line, and locks 2.x compatibility via a dedicated unit test (surefire_matches_legacy_2x_close_line).
• 77e28d0 — removes production duration normalisation entirely (went a step further than the suggested test-only gating because there are no insta snapshots in the module). Real Time elapsed / Total time ship to the user/LLM untouched.
• 97dbf98 — adds mvn -q quiet-mode filtering. Green run emits zero bytes (no overhead); failure run keeps the close-line, exception, user-code stack, summary block, and goal terminator while dropping framework frames + post-failure help boilerplate. 51.1% measured savings on the real fail fixture.

Reviewer-flagged follow-ups documented under Out of scope below: detect_phase ↔ hook regex disagreement on space-valued flags, parallel -T<N>C per-FQN keyed buffer, dependency:tree filter, mvnw.bat wrapper detection.

Summary

Comprehensive Apache Maven support replacing the stateless src/filters/mvn-build.toml (4 phases, 50-line cap) with a stateful Rust module in src/cmds/jvm/mvn_cmd.rs (alongside the existing gradlew_cmd.rs).

The TOML filter could not handle Maven's dominant noise pattern: expected-exception stack traces inside passing Surefire tests (assertThrows). Dropping those requires deciding the fate of a block based on its closing Tests run: N, Failures: 0, Errors: 0 line — a stateful decision the TOML DSL cannot express. Rust module is the right vehicle per CONTRIBUTING § "TOML vs Rust".

Also addresses the hook-routing bug in #1537: the previous regex ^mvn\s+(compile|package|clean|install)\b only matched when the goal was the first token after mvn, so mvn -B install, ./mvnw test, mvn -q -Dtest=Foo test, etc. never rewrote. The reporter (VictorAurelius) measured 270+ unfiltered mvn/mvnw invocations over 30 days in a Spring Boot project.

Relationship to in-flight PRs

Two other PRs landed in parallel targeting the same #1537 gap:

	#1907 (Tyler)	#1921 (Ehsan)	this PR (#1956)
Approach	TOML filter	TOML filter + engine extension (match_command_excludes)	Rust module
Size	+124 LoC, 3 files	+403 LoC, 8 files	+1183 LoC, 17 files
Stateful assertThrows collapse	❌ TOML DSL cannot express	❌ TOML DSL cannot express	✅
Locale guard (non-English)	❌	❌	✅
Verbose bypass (-X/-e)	❌	❌	✅
Wrapper (./mvnw/mvnw.cmd) detection	❌	❌	✅
Flag-skip goal matching	❌ first-token only	✅ lazy quantifier	✅ lazy quantifier
CRLF explicit test	❌	✅	❌
Replaces mvn-build.toml	preserved	preserved + routing-excluded	deleted
Savings (passing fixture, real commons-cli)	~85%	~95%	98.0%

Reviewer decision required: TOML approach (cheaper review, smaller surface, but misses assertThrows collapse — the dominant noise pattern on healthy projects) vs Rust approach (this PR — bigger surface but covers the pattern TOML cannot). Happy to defer to maintainer judgment; will close this PR if a TOML approach is preferred.

Token savings (measured on real Maven output)

Reference build: apache/commons-cli (master) inside maven:3.9-eclipse-temurin-21, default toolchain → maven-surefire-plugin:3.5.5. Fixtures are the raw bytes captured from that build — exactly what the binary receives via its child process.

Filter	Raw tokens	Filtered tokens	Savings
mvn test (full fixture)	1896	38	98.0%
mvn install (full fixture)	2021	78	96.1%
mvn -q test (fail fixture)	174	85	51.1%
mvn -q test (green run)	0	0	n/a (zero-byte input)

Why the original 98.9% / 97.5% numbers were wrong: the original fixtures were synthetic Surefire 2.x shape (- in separator). Surefire 3.x emits -- in, so the CLOSE regex never matched against real 3.x output; every [INFO] Running … flushed the previous block as kept. Reviewer measured 27.1% real savings; that has been reproduced on the pre-fix binary and corrected. 2.x close-line shape is preserved by the \s+--?\s+in separator and locked by a unit test.

Commands handled

Command	Filter	Tier
rtk mvn test	Surefire block collapse	≥90%
rtk mvn integration-test	Same (Failsafe shape)	≥90%
rtk mvn compile / test-compile	[ERROR] + indent + [WARNING] dedup	≥75%
rtk mvn package / install / verify / deploy	Compile + Surefire pipeline + artifact line	≥85%
rtk mvn -q <phase>	Quiet-mode filter (filter_quiet)	≥50% on fail; 0 → 0 on green
rtk mvn clean / site / dependency:* / --version / --help	passthrough	0% (no overhead)

clean and site deliberately bypass RTK (hook regex doesn't match them) so no subprocess fork for zero benefit.

Features

• ANSI strip first in every filter — real Maven output contains colour escapes everywhere ([[1;34mINFO[m]).
• English-footer guard: if neither BUILD SUCCESS nor BUILD FAILURE appears as a trimmed line suffix, return ANSI-stripped raw input unchanged. Protects non-English locales (French BUILD ÉCHEC, Chinese 生成成功, etc.) — no mangling.
• Verbose bypass: -X, --debug, -e, --errors skip filtering. User asked for detail; respect it.
• Surefire block collapse: buffer [INFO] Running <FQN> … [INFO] Tests run: … blocks, drop silently if Failures: 0 && Errors: 0, else emit with framework stack frames stripped.
• Stack-frame deny-list: drops at org.junit., at junit., at org.apache.maven.surefire., at sun.reflect., at jdk.internal.reflect., at jdk.proxy, at java.base/, at java.lang.reflect., at java.util. (and stream/concurrent). Keeps user-code frames in any namespace.
• Raw durations preserved: Time elapsed: N s and [INFO] Total time: N s ship through unchanged — the user/LLM needs the real numbers to diagnose performance regressions. (Earlier revisions normalised these to T s in production; removed per reviewer feedback.)
• Failure-trail retention (Surefire 3.x): 3.x emits the exception class and stack frames after the per-class close line. filter_surefire and filter_package enter a failure_trail state on any <<< FAILURE! close and keep subsequent non-framework lines until the next blank line. 2.x compat preserved.
• Quiet-mode filtering (mvn -q): detect -q / --quiet in argv → bypass the English-footer guard and route to filter_quiet. Green run = 0 → 0 (no overhead). Failure run keeps signal lines (close, exception, user frames, summary, goal terminator); drops framework frames + post-failure help boilerplate. Safety net: unclassified [ERROR] lines are kept.
• [WARNING] dedup: strips file coordinates (/path/Foo.java:[L,C]), keeps first occurrence per normalised message.
• Wrapper detection: ./mvnw (POSIX) / mvnw.cmd (Windows) via string-literal Command::new (semgrep dynamic-command-execution-safe); falls back to resolved_command("mvn").
• Hook regex fix (addresses Hook doesn't auto-route mvn / mvnw despite handler existing #1537): previous pattern ^mvn\s+(compile|package|clean|install)\b required goal as first token — mvn -B install did not rewrite. New pattern uses lazy quantifier (?:\s+\S+)*? to skip flags. mvn -B clean install now rewrites correctly. Also adds ./mvnw, mvnw.cmd, mvnw prefixes from the reporter's reproduction.
• Tee recovery: every filtered path uses RunOptions::with_tee("mvn_<phase>") so raw output is recoverable on BUILD FAILURE.

Architecture

• Phase detection (detect_phase): scan args left-to-right, skip flag tokens (- prefix + -D…), pick the last lifecycle token; route to Test / Compile / Package / Passthrough.
• Quiet-mode short-circuit: is_quiet(args) runs before phase routing. If set, non-passthrough phases route to filter_quiet instead of filter_surefire / filter_compile / filter_package because those depend on [INFO]-prefixed markers that -q suppresses.
• Buffered single-pass filters via runner::run_filtered + stateful Fn(&str) -> String. NOT BlockHandler/BlockStreamFilter — run_filtered takes a plain closure (verified at src/core/runner.rs:152-169). Same pattern as gradle's filter_test at gradlew_cmd.rs:229-301.
• Three filters share helpers: strip_ansi, has_english_footer, keep_outside_block, is_framework_frame, flush_block_as_keep, emit_block.
• Single PR delivery (not split) — adding Commands::Mvn removes mvn from the parse-error path that consults the TOML filter, so a partial PR would regress compile/install/package (currently ~70% via TOML) to 0% savings until a follow-up landed.

Quality checks

• cargo fmt --all -- --check — clean
• cargo clippy --all-targets -- -D warnings — no warnings
• cargo test --all — 1925 passed, 7 ignored (pre-existing in git.rs/read.rs), 0 failed
• 44 mvn-specific unit tests (43 active + 1 #[ignore] summary helper) including: phase detection, English-footer guard, framework deny-list, ERROR continuation, WARNING dedup, install/jar lines, 2.x ↔ 3.x close-line compat (\s+--?\s+in), [WARNING]-prefixed close lines (3.x skip-only classes), failure-trail retention, raw duration preservation, -q detection (short/long/false-positive), -q green run is empty, -q fail strips framework + boilerplate, -q ≥50% savings target, -q unclassified-line safety net
• 2 inline savings assertions on gzipped full commons-cli fixtures (no #[ignore] gating — they run in standard cargo test --all)
• 12 classifier/rewrite tests in src/discover/registry.rs under new // --- Maven --- section
• Token savings verified ≥90% on mvn test, ≥85% on mvn install (actual: 98.0% and 96.1%)
• Hook regex updated with wrapper prefixes + test/integration-test/verify/deploy + flag-skip
• src/filters/mvn-build.toml deleted, src/core/toml_filter.rs:1585 scrubbed, count assertions updated

Manual testing

• rtk mvn -B clean — bypass via hook (regex excludes clean); raw mvn output
• rtk mvn -B compile against a project with intentional compile error: [ERROR] blocks with file:line + indented continuation preserved, BUILD FAILURE footer preserved
• rtk mvn --version — passthrough; raw mvn version banner
• Verbose bypass: rtk mvn -B -X compile → raw mvn output (X flag)
• Locale guard via fixture: French BUILD ÉCHEC preserved unchanged
• No-pom fixture (mvn install outside any project) passthroughs the [ERROR] no POM line — never blocks
• Startup time: hyperfine 'target/release/rtk --help' = 1.7 ms ± 0.5 ms (well under 10 ms target)
• Release binary size: 7.8 MB (unchanged from develop)

Breaking change

The TOML filter previously covered compile|clean|install|package with simple line-strip + 50-line cap. The new Rust module covers a superset of phases with state-machine collapse + locale guard + verbose bypass. Output shape is different (different lines kept; significantly better savings tier). All previously-handled cases still handled, plus test/integration-test/verify/deploy/-q.

The ! in the commit type ensures release-please surfaces this in the CHANGELOG.

Out of scope (deferred)

Documented here so they aren't lost after merge — happy to convert any into formal issues if you'd prefer to track them in the tracker:

• detect_phase ↔ hook regex disagreement on space-valued flags (-pl core, -T 4, -pl :core)
• Parallel -T<N>C builds (per-FQN keyed buffer)
• dependency:tree filter (reviewer flagged as "common and verbose")
• mvnw.bat legacy Windows wrapper detection

Integrity-check whitelist

Commands::Mvn is intentionally omitted from is_operational_command at src/main.rs:2455-2501, matching the gradle precedent (Commands::Gradlew also omitted). Per the comment at L2452-2454, the whitelist is opt-in by design and a forgotten command fails open. Filter modules invoked via the hook do not require SHA-256 integrity verification of their own binary path — the hook itself is integrity-checked. Documented in src/cmds/jvm/README.md.

[CLAassistant]

All committers have signed the CLA.

[pszymkowiak]

Review — validated end-to-end against real Maven, found a blocking issue

Thanks for this — the architecture is genuinely clean: the phase state machine, the English-footer locale guard, the -X/-e verbose bypass, the framework-frame deny-list and the tee recovery are all the right calls, and the module mirrors gradlew_cmd.rs nicely.

I built the branch into Docker (rust:1-bookworm → maven:3.9-eclipse-temurin-21) and ran it against a real project (apache/commons-cli, warm .m2). The headline feature doesn't trigger on real Surefire output.

Blocking: the Surefire close-line regex misses the -- separator

Maven Surefire 3.x emits a double dash before in:

RAW (Surefire 3.x):  [INFO] Tests run: 115, ... Time elapsed: 0.543 s -- in org.apache.commons.cli.OptionValidatorTest
CLOSE regex (mvn_cmd.rs:26): ... Time elapsed: [^ ]+ s (?:<<< FAILURE!)? - in (.+)$

The regex expects - in (single dash), so CLOSE never matches. Blocks never close, and each new [INFO] Running … flushes the previous block as kept (mvn_cmd.rs:175-177). On a green run, rtk keeps 47 Running + 48 Tests run: lines instead of dropping them.

Measured savings (warm cache, commons-cli):

command	real savings	PR claim
mvn test (green)	27.1%	98.9%
mvn clean compile	76.1%	≥75%
mvn install -DskipTests	78.3%	≥85%

compile / install are fine — they rely on prefix keep-lists, not CLOSE. Only the Surefire path (the core of this PR) is affected; the same break hits verify/package once tests run.

Why CI is green

The .gz fixtures use - in (single dash), so the 33 unit tests + the gzipped savings asserts validate against output Maven never produces. The fixtures need regenerating from real mvn output — that's what masked this.

Suggested fix: tolerate both separators, e.g. \s+--?\s+in (keeps - for Surefire 2.x).

Goal coverage

Filtered: compile, test-compile, test, integration-test, package, install, verify, deploy — a clean superset of the old TOML. Verified passthrough (0%, by design): clean, site, and all plugin goals (dependency:tree, dependency:analyze, versions:*, …). That's reasonable, but dependency:tree is common and verbose — please follow through on the follow-ups you already documented in the PR description (parallel -T2C, plugin goals like dependency:tree, mvnw.bat); they shouldn't be lost after merge.

Secondary

• Durations normalized in production (mvn_cmd.rs:120): normalise() rewrites Time elapsed: … s → T s and Total time: … s → T s in shipped output, not just snapshots — real timings are wiped for the user/LLM. Worth gating to test-only.
• mvn -q suppresses the BUILD SUCCESS line, so the footer guard passes through with 0 savings. Safe fallback, but -q is a common agent invocation — worth a note or handling.
• Phase detection picks the last non-flag token (mvn_cmd.rs:69), so a space-valued flag after the goal (mvn install -pl core, mvn test -T 4) resolves to Passthrough while the hook regex still rewrites on the first goal — they can disagree. (Observed by reading; not in the runs above.)

Path to merge

1. Fix the CLOSE separator.
2. Regenerate all fixtures from real mvn output and re-assert savings.
3. Address the documented follow-ups, plus duration normalization (test-only) and the -q case.

Happy to share the Docker harness. The approach is right — it just needs to meet real Surefire output.

[vdufloth]

Thanks for the detailed review and for sharing the harness — re-ran it against apache/commons-cli in maven:3.9-eclipse-temurin-21, captured the raw output, and replaced all fixtures with it. Three commits on feat/mvn-rust-module:

cc152cd — CLOSE regex + failure-trail state (src/cmds/jvm/mvn_cmd.rs:26-30, :170-222, :344-401)

• Separator widened to \s+--?\s+in per your suggestion — matches Surefire 2.x (- in) and 3.x (-- in) in a single regex.

• Prefix widened to INFO|ERROR|WARNING. On commons-cli Surefire 3.x emits [WARNING] for classes whose only tests are skipped, e.g.

  [WARNING] Tests run: 5, Failures: 0, Errors: 0, Skipped: 5, Time elapsed: 0.010 s -- in org.apache.commons.cli.BasicParserTest
  

  Without that the block stayed open and got flushed as kept on the next Running line.

• Added a failure_trail state in filter_surefire (:170-222) and filter_package (:344-401): Surefire 3.x emits the exception class + stack frames after the close line, not inside the block as 2.x default-config did. Raw shape on a 3.x failure:

  [ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.543 s <<< FAILURE! -- in org.apache.commons.cli.RtkInducedFailTest
  
  org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
  	at org.apache.commons.cli.RtkInducedFailTest.rtkInducedFailure(...)
  	at org.junit.jupiter.api.AssertionUtils.fail(...)
  	...
  

  Without the trail those lines fell through keep_outside_block and got silently dropped. Now retained (framework frames stripped, user frame preserved) until the next blank line.

• All five Surefire-touching fixtures regenerated from a real apache/commons-cli build inside maven:3.9-eclipse-temurin-21: mvn_test_pass_slice_raw.txt, mvn_test_fail_slice_raw.txt, mvn_install_slice_raw.txt, mvn_compile_error_slice_raw.txt, and the two gzipped full fixtures. All synthetic 2.x-shape content is gone. 2.x compat locked by surefire_matches_legacy_2x_close_line hitting the single-dash separator directly.

• Measured savings on the new full fixtures (same maven:3.9-eclipse-temurin-21 toolchain as your harness): mvn test 98.0% (1896 → 38 tokens), mvn install 96.1% (2021 → 78 tokens). These are the input bytes your harness pipes to the binary, so the number that answers your 27.1% is 98.0% on green commons-cli mvn test.

77e28d0 — production duration normalisation

Went a step further than the test-only gating you suggested: normalise() and the TIME_DURATION / TOTAL_TIME regexes are deleted outright. There are no insta snapshots in the module today, and surefire_preserves_real_durations asserts the inverse, so a #[cfg(test)] version would be a #[allow(dead_code)] helper with no callers. Production now ships raw Time elapsed: 2.341 s and [INFO] Total time: 4.567 s untouched. Happy to put it back behind a cfg guard if you'd rather keep it available for future snapshot work.

97dbf98 — mvn -q filtering

Implements the -q handling you flagged. Behaviour captured from mvn -q test on Maven 3.9.9 + Surefire 3.5.5:

• Green run emits zero bytes — filter_quiet returns empty (0 → 0, no overhead).
• Failure run emits the per-class CLOSE line, exception + stack, [ERROR] Failures: summary, aggregate, [ERROR] Failed to execute goal terminator, followed by ~10 lines of [ERROR] See … / [Help 1] / Re-run Maven / To see the full stack trace boilerplate pointing at log files and help URLs.

filter_quiet keeps the failure signal and user-code frames, drops framework frames (existing deny-list) and the post-failure boilerplate block. Routing: is_quiet(args) (-q / --quiet) short-circuits the standard phase routing; passthrough phases (clean, site, plugin goals) remain passthrough. Measured savings on the captured fail fixture: 51.1% (174 → 85 tokens). Safety net: unclassified [ERROR] lines are kept rather than dropped — better to leak a line than hide signal.

On the remaining secondary items

• detect_phase ↔ hook regex disagreement on space-valued flags — captured in the PR body's Out of scope section. Confirmed via a shim mvn on PATH that mvn install -pl core, mvn test -T 4, mvn install -pl :core all return Passthrough from detect_phase while the hook regex at src/discover/rules.rs:693 still rewrites. Latent, not a regression: today those commands route to Passthrough → raw mvn output ships unchanged, same as if the hook hadn't matched. Follow-up tracks the gain opportunity, not a bug.
• Parallel -T<N>C, dependency:tree, mvnw.bat — also documented under Out of scope in the PR body. Happy to file formal issues for any of these if you'd rather have them in the tracker.

Quality gate on all three commits: cargo fmt --all && cargo clippy --all-targets -- -D warnings && cargo test --all (1925 passed, 7 ignored pre-existing in git.rs/read.rs, 0 failed). 10 new mvn-specific tests across the three commits (2.x compat, WARNING-prefix close, 3.x failure trail, raw-duration preservation, is_quiet, -q empty green, -q fail strip, -q savings, -q safety net) bring the module total to 44. hyperfine 'target/release/rtk --help' measures 1.7 ms ± 0.5 ms.

Happy to take another pass on anything else, @pszymkowiak.

[aeppling]

Verified the branch end-to-end against the real commons-cli fixtures through the built binary. Remaining items:

P0: compile errors lose detail on mvn test
filter_surefire (mvn_cmd.rs:160) lacks the keep_continuation branch that filter_compile (:302) and filter_package (:408) have. Verified: rtk mvn test keeps cannot find symbol but drops the following symbol: / location: lines; rtk mvn install on the same input keeps them. Since the test lifecycle compiles first, this hits the headline command on a common failure. Please mirror the continuation handling into filter_surefire and add a compile-fail-under-test test.

P1: no output cap / config::limits() unused
The module's only bound is the global RAW_CAP (10 MiB), which guards the raw/tee buffer, not the filtered output, so a BUILD FAILURE with many failures isn't capped (unlike git/cargo/gradle, which cap and emit a "+N more" tail). Cap failing classes (and the [ERROR] Failures: block) with a tail line, ideally via a tunable mvn_max_failures on LimitsConfig.

P2: medium

• Multi-module reactor: keep_outside_block (:127) drops the Reactor Summary rows; the failing module survives only via the [ERROR] Failed to execute goal … line. Keep the rows, and add a multi-module fixture (all current fixtures are single-module).
• The Surefire block state machine is duplicated between filter_surefire (:177) and filter_package (:352). Extract a shared helper (also prevents the P0-style drift).

P3: minor

• Idioms: block buffers (:167, :340) can borrow &str rather than clone; &line["[WARNING] ".len()…] (:309, :415) reads cleaner as strip_prefix(...).unwrap_or(line).
• CRLF is handled by str::lines() but isn't asserted by a test.

Mechanics: Please drop Cargo.lock from the PR.

Open to discuss about those if needed

[vdufloth]

Thanks @aeppling — all six points addressed, one commit per ask, P0 → P1 → P2 → P3 → mechanics. Quality gate (cargo fmt --all && cargo clippy --all-targets && cargo test --all) green on every step; total tests went from 1925 to 1934 (+9 new).

P0 — filter_surefire continuation drop (97bd2a7)

Confirmed and fixed at src/cmds/jvm/mvn_cmd.rs:441 (filter_surefire). Added a keep_continuation flag with the same predicate filter_package uses (set on any kept [ERROR] that isn't Tests run: / Failures: / Errors:), and a passthrough branch for indented continuation lines before the outside-block keep-list check.

Fixture tests/fixtures/mvn_test_compile_fail_slice_raw.txt is captured from a deliberate BogusType insertion under src/main/java in a commons-cli checkout; it goes through filter_surefire because phase routing pins on the last non-flag arg (test), and shows the exact cannot find symbol → symbol: → location: triple. Two new tests: surefire_keeps_compile_continuation_on_test_phase (the bug) and package_still_keeps_compile_error_continuation_after_refactor (drift guard on the package path).

Savings on the new fixture: 122 → 70 tokens, 42.6 % savings. (Slice is small — the absolute numbers are modest, but the signal is now preserved end-to-end.)

P1 — bounded failing-class output (be6c812)

Added LimitsConfig::mvn_max_failures at src/core/config.rs:138, default 25 (matching the grep_max_per_file: 25 / status_max_files: 15 convention in the same struct), sentinel 0 = opt-out. Two cap sites:

1. Failing test classes. The shared SurefireBlock::step() machine (Commit B) yields FailingClose once per failing class; the outer loop in filter_*_with_cap commits the first N and calls a new SurefireBlock::drop_failing() (mvn_cmd.rs:294) for the rest. drop_failing flips a drop_trail flag so the post-close per-test subline, exception, and user frames are consumed silently until the next blank line — otherwise keep_outside_block would still preserve the [ERROR] subline for dropped classes. After the loop, \n... +N more failing test classes\n is appended when any were dropped.
2. [ERROR] Failures: summary entries. New FailuresSummaryCap helper (mvn_cmd.rs:363) tracks the summary header, caps [ERROR] (three-space-indent) entries at the same limit, and pre-emits \n... +N more failures\n immediately before the aggregate [ERROR] Tests run: line.

Tail wording is verbatim cargo_cmd.rs:1035 (\n... +N more failures\n) — no [INFO] prefix, no parenthetical hint.

Behaviour-change note: with the default cap of 25 in place, users without a custom config now get bounded output on large failure sets. Existing fixtures all have <25 failures so no current test diffs; happy to flip the default to 0 if you'd prefer opt-in.

Cap demo on a synthetic 50-failing-class input: uncapped 1761 → capped 892 tokens, 49.3 % extra savings on top of the base filter.

Three new tests: surefire_caps_failing_blocks_emits_tail (cap=3, 5 failures, asserts blocks 1-3 emitted + 4-5 dropped + tail), surefire_cap_zero_disables_capping, and failures_summary_block_is_capped (verifies tail precedes aggregate).

Documented in src/cmds/jvm/README.md alongside the other key behaviours.

P2a — multi-module Reactor Summary (4609102)

Added REACTOR_SUMMARY regex and reactor_summary_keep helper at src/cmds/jvm/mvn_cmd.rs:140. The helper toggles in_reactor_summary on the [INFO] Reactor Summary for header and clears it on BUILD SUCCESS / BUILD FAILURE. While set, it returns true for every line so the per-module rows ([INFO] child-a ...... SUCCESS [ 1.676 s]), in-summary [WARNING] activated-profile notices, and the dashes-only HR all survive. Both filters call it before keep_outside_block so the BUILD_FOOT-clears-flag side effect always runs regardless of || short-circuit.

Fixtures captured against a hand-rolled 2-module reactor checked in under tests/fixtures/multi-module-skeleton/: parent pom + child-a + child-b with Empty.java stubs. The pass fixture is mvn install on the skeleton as-is; the fail fixture is captured by temporarily swapping child-b/src/main/java/Empty.java to one referencing BogusType. Reproducible from the committed skeleton — cd tests/fixtures/multi-module-skeleton && mvn install.

Savings on the new fixtures: reactor-pass 330 → 98 tokens, 70.3 %; reactor-fail 395 → 200 tokens, 49.4 %. Two new tests: reactor_summary_kept_on_multi_module_pass, reactor_summary_kept_on_multi_module_fail.

P2b — extract shared Surefire block state machine (5459c6d)

Did this before P1 so the cap logic only had to land in one place. Introduced SurefireBlock at src/cmds/jvm/mvn_cmd.rs:174 owning the inner state machine: PLUGIN_BANNER skip, RUNNING (with prior-block flush on truncated output), in-block buffering, CLOSE detection, and the post-close failure-trail with framework-frame stripping.

step() returns SurefireStep::{Consumed, FailingClose { … }, Passthrough}. The FailingClose variant defers the actual write to the caller via commit_failing() (mvn_cmd.rs:302) — this is the seam P1 uses to enforce the cap without duplicating the cap logic.

The outer outside-block keep stays inside each filter unchanged (MODULE_BANNER || keep_outside_block, [WARNING] dedup, keep_continuation). Byte-identical filter output — all pre-existing surefire_* / package_* / token-savings tests still pass.

P3a, P3b, P3c — polish (92a9218)

• strip_prefix: replaced both &line["[WARNING] ".len().min(line.len())..] defensive slices with strip_prefix("[WARNING] ").unwrap_or(line). Both sites are already gated by line.starts_with("[WARNING]") so the min was dead defence.
• Borrow refactor: SurefireBlock<'a> and SurefireStep<'a>::FailingClose now hold Vec<&'a str> / Option<&'a str> / &'a str close, tied to the post-ANSI-strip stripped local. No .to_string() allocations on the per-line hot path. Lifetime propagation through the shared helper was clean — 'a only appears on the struct/enum and the step() signature; call sites don't need to mention it. Gzipped full-fixture savings still hit ≥90 % / ≥85 %.
• CRLF tests: surefire_handles_crlf_line_endings and package_handles_crlf_line_endings convert an LF fixture to CRLF, filter both, normalise the CRLF output back to LF, and assert byte-equality. Catches the class of bug where exact-equality line checks silently miss CRLF input because str::lines() strips \n but keeps trailing \r.

Mechanics — Cargo.lock (f58333c)

develop's lock is internally stale (rtk-package entry pinned at 0.36.0 while Cargo.toml says 0.34.3) — when cargo build ran on this branch it re-pinned the lock to 0.34.3, producing the diff you flagged. Reverted to develop's baseline so the PR's committed diff no longer touches Cargo.lock. cargo build will keep rewriting the working-tree lock on every invocation; that's a local artifact, not part of the PR.

---

If anything looks off, happy to iterate on individual commits before merge.

[aeppling]

Tested this against real Maven (Surefire 3.2.5) by building the binary and running live builds, not just the fixtures. The earlier review items all check out: separator fix, duration preservation, -q, the compile-continuation fix, the failure cap, reactor summary, shared block machine. Passing builds, single failures, compile errors and quiet mode all behave correctly.

One blocker though. When a single test class has more than one failing test, the filter drops the exception message and leaks the framework stack for every failure after the first.

Surefire 3.x prints one blank-separated detail block per failing test under a single CLOSE line. failure_trail in SurefireBlock::step() ends at the first blank line and never re-arms, so only the first failure in a class goes through the framework-frame stripping. The rest fall through to the generic keep_continuation path. Real mvn test with two failures in one class:

[ERROR] CalcTest.failTwo -- ... <<< FAILURE!
	at org.junit.jupiter.api.AssertionFailureBuilder.build(...)
	at java.base/java.util.ArrayList.forEach(...)

The AssertionFailedError: expected... but was... line is gone and the full junit/jdk stack is kept, the opposite of what the module is supposed to do. A class with 20 failures dumps a couple hundred lines of framework noise. The summary block at the bottom still has a one-liner per failure so it's not a total signal loss, but the inline output is wrong and savings tank on a common pattern.

CI doesn't catch it because every *_fail* fixture has exactly one failing test per class. Worth noting filter_quiet already handles this, it re-arms failure_trail on each subline (mvn_cmd.rs:730), but filter_surefire/filter_package don't. The fix is to do the same re-arm on [ERROR] <FQN> ... <<< FAILURE!/ERROR! sublines and run is_framework_frame stripping there, plus a fixture with two failures in one class.

Minor, not blocking: the non-quiet failure path keeps the post-failure help boilerplate (-> [Help 1], Re-run Maven, etc.) that filter_quiet strips. Could reuse QUIET_BOILER_PREFIXES.

Otherwise solid. Just needs the multi-failure path sorted before merge.

[aeppling]

Also all ci checks should pass for this to be merged

[qingmo]

wait for this feature

[vdufloth]

Thanks @aeppling — all three points addressed, one commit per ask as before. Quality gate green on every step; 1934 → 1943 tests.

Blocker — multi-failure trail re-arm (1050cfe)

Fixed along the lines you suggested. When a trail ends at a blank line, SurefireBlock now remembers the keep/drop decision (trail_rearm), and the next per-test subline (<<< FAILURE! or <<< ERROR!, shared is_per_test_subline() also used by filter_quiet) re-enters the trail with that same decision. So every failure in a class gets framework-frame stripping, and a class capped by mvn_max_failures drops all of its blocks, with the tail still counting classes. Any other non-blank line disarms re-entry, and RUNNING clears it, so a later class can't inherit the decision.

Fixture is captured from a real mvn test run (Maven 3.9.9, Surefire 3.5.5): new tests/fixtures/multifail-skeleton/ with one class containing an assertion failure plus a thrown exception, and an errors-only class. One note from the capture: 3.5.5 prints <<< FAILURE! on the class close line even for errors-only classes; ERROR! only shows up on per-test sublines. The CLOSE regex accepts both anyway, since detection keys off the counts.

8 new tests, including a byte-exact pin of the single-failure path (output unchanged) and a capped-class test asserting nothing from the second block leaks. Also ran the release binary against the skeleton: both exception messages survive, no junit/jdk frames, and with mvn_max_failures = 1 the capped class disappears entirely behind ... +1 more failing test classes.

CI — windows-latest CRLF failures (be28a51)

Root cause was checkout, not the filter: with no .gitattributes, the Windows runner (core.autocrlf=true) checks fixtures out with CRLF, so the CRLF tests' replace('\n', "\r\n") produces \r\r\n. str::lines() strips only the final \r\n of each pair, and the stray \r makes the $-anchored MODULE_BANNER/BUILD_FOOT regexes miss — which is why only the synthesized CRLF leg failed and everything else passed on Windows.

Fix: tests/fixtures/** -text in a new .gitattributes, plus the two CRLF tests normalize the fixture back to LF before synthesizing, so they stay correct on zip downloads or pre-existing autocrlf clones. Reproduced the failure mode locally with a CRLF-converted fixture copy to confirm. No filter change needed — real Maven CRLF output was always handled.

Minor — non-quiet help boilerplate (df76528)

Done. QUIET_BOILER_PREFIXES is now a shared BOILER_PREFIXES (also covering the bare [ERROR] divider lines, matching filter_quiet), dropped before the [ERROR] catch-alls in test/package. I extended the same stripping to filter_compile for consistency — happy to revert if you'd rather keep it minimal.

One thing I kept on purpose: the resume hint ([ERROR] After correcting the problems… / [ERROR] mvn <args> -rf :child-b). It's the only actionable line in the block, so it stays alongside Failed to execute goal.

Multifail-slice savings went from 19.9% to 42.3% with the boilerplate stripped; full-fixture targets unchanged (≥90% test, ≥85% install).

---

The CI run for df76528 is waiting on workflow approval — could you approve it when you get a chance? Local gate is green and the Windows failure mode is reproduced and fixed, so it should come back green including test (windows-latest).

[aeppling]

Hello @vdufloth, and thanks for the very thorough work across the previous rounds — the trail re-arm, CRLF fix, reactor summary, and shared SurefireBlock are all clean, and CI is green now. Just one last architectural ask before merge.

Could you reuse the existing cap infrastructure instead of the new LimitsConfig::mvn_max_failures field?

src/core/truncate.rs defines the canonical caps (CAP_ERRORS, CAP_WARNINGS, CAP_LIST, CAP_INVENTORY). The README at src/core/README.md documents the policy: every filter binds a local const MAX_* to the right CAP. Every other test-failure filter already does this:

• pytest_cmd.rs:9 MAX_PYTEST_FAILURES = CAP_WARNINGS
• rspec_cmd.rs:17 MAX_RSPEC_FAILURES = reduced(CAP_WARNINGS, 5)
• rake_cmd.rs:12 MAX_RAKE_FAILURES = CAP_WARNINGS
• runner.rs:10 MAX_RUNNER_FAILURES = CAP_WARNINGS

The README also notes the CAPs are the staging point for the user-config surface that's coming — overriding CAP_WARNINGS once in ~/.config/rtk/config.toml should tune every test-failure filter in one place. A per-filter mvn_max_failures knob would force a special-case migration when that lands.

Suggested change:

use crate::core::truncate::CAP_WARNINGS;
const MAX_MVN_FAILING_CLASSES: usize = CAP_WARNINGS;

If classes warrant a deviation from per-test failure (one class hiding N tests), use truncate::reduced(CAP_WARNINGS, N) with the one-line comment the README's deviation rule asks for. And drop the new field from LimitsConfig.

While you're at it, the tail format would ideally match the canonical … +N more {label} shape from src/core/utils.rs:148 (join_with_overflow) — same convention used by container, aws, gh, glab. The mvn tail is mid-stream so the helper may not slot in directly, but the format string (ellipsis + label) should match so the LLM sees one consistent overflow style. Same idea for the tee [full output: …] recovery hint when content is capped.

With this lined up, the PR should be ready. Thanks again for the patience on the back-and-forth.

[vdufloth]

Thanks @aeppling — both points addressed, one commit per ask as before.

One note up front: the branch was cut before src/core/truncate.rs landed on develop, so the first step was a sync merge (no conflicts). Suite green at the merge point before any change — 2145 tests; the jump from last round's 1943 is develop's own additions coming in.

Cap infrastructure (3c0ee94)

Done as suggested:

use crate::core::truncate::CAP_WARNINGS;
const MAX_MVN_FAILING_CLASSES: usize = CAP_WARNINGS;

LimitsConfig::mvn_max_failures is gone — with the field dropped and develop merged in, the PR no longer touches src/core/config.rs at all. Both consumers (failing-class blocks and the [ERROR] Failures: summary entries) bind to the shared const. No reduced() deviation: by the time a class block is emitted, framework frames are already stripped, so blocks are compact — per-test sublines plus exception messages and user frames. If real-world use shows classes are still too verbose, reduced(CAP_WARNINGS, N) is a one-line follow-up.

While dropping the knob I also aligned the cap == 0 semantics with the core policy in src/core/README.md: 0 now means summary-only (no blocks, tail still counts every dropped class) instead of the old "opt out of the cap" meaning. Production can't hit 0 today since the const is CAP_WARNINGS, but this way the config surface lands on mvn without the special-case migration you flagged. The cap-0 test flipped accordingly (surefire_cap_zero_emits_summary_only).

Tail format (f8bc856)

The tails are now … +N more failing test classes / … +N more failures — ellipsis plus +N more {label}, matching join_with_overflow. As you anticipated, the helper itself doesn't slot in (the summary tail is pre-emitted mid-stream before the Tests run: aggregate), but the format string is identical so the output shows one overflow style.

On the tee hint: mvn's filtered phases already route through RunOptions::with_tee, so capped output rides core's tee_and_hint and gets the canonical [full output: …] line — no custom format to align. The one gap is -Dmaven.test.failure.ignore=true (failing tests, exit 0): with the default tee.mode = "failures" no file is written, so a capped-but-green run has no recovery hint. Happy to wire force_tee_hint on the dropped-content path if you want that covered; left it out to keep this round minimal.

Quality gate green on both commits (fmt, clippy, cargo test --all). Savings targets unchanged (≥90% test, ≥85% install) with the cap at 10.

Same as last time, the CI run is waiting on workflow approval — could you approve it when you get a chance?

[aeppling]

Hello @vdufloth — quick correction on my earlier message, since you offered to wire this up.

The canonical function isn't force_tee_hint — it's force_tee_tail_hint in src/core/tee.rs:225. It emits [see remaining: tail -n +{N} ~/path] so the LLM can read only the dropped section via tail -n +N, not the whole raw file. Far better token recovery, and it bypasses the tee.mode = "failures" gate (via force_tee_path), so it also covers the -Dmaven.test.failure.ignore=true green-with-capped-failures case in one go.

Every other capped-tail filter in the codebase already uses it — container.rs, gh_cmd.rs, dotnet_cmd.rs, go_cmd.rs, lint_cmd.rs, pnpm_cmd.rs, cargo_cmd.rs. The shape is:

if dropped > 0 {
    out.push_str(&format!("\n… +{} more failing test classes\n", dropped));
    if let Some(hint) = crate::core::tee::force_tee_tail_hint(
        raw,                            // the full raw output
        "mvn-test-failures",            // slug
        MAX_MVN_FAILING_CLASSES + 1,    // line offset where the dropped section starts
    ) {
        out.push_str(&format!("{}\n", hint));
    }
}

Reference call sites you can crib from:

• src/cmds/cloud/container.rs:295-300 (docker images)
• src/cmds/git/gh_cmd.rs:281 (gh prs)
• src/cmds/python/pytest_cmd.rs:281 uses force_tee_hint (the non-tail variant) but the pattern of pairing with the cap is the same.

In mvn_cmd.rs this needs wiring at the two cap sites you already have: the failing-class tail (… +N more failing test classes) and the FailuresSummaryCap tail (… +N more failures). The line_offset argument is the line in the raw output where the dropped section begins, so the tail -n +N command reads only the dropped lines — not the slug-naming, the offset, is what matters for correctness here.

With this added, the cap surface is fully consistent with the rest of the codebase and the recovery path works on both red and green capped runs. After that, I think we're truly done.

[vdufloth]

Thanks @aeppling — happy to wire it. One thing I want to confirm first, because I think the tail variant may not fit mvn's output shape and I'd rather get it right than ship a hint that points at the wrong lines.

force_tee_tail_hint's tail -n +N only lands on the dropped items when each item is a single line — that's how docker-images (MAX_IMAGES + 2) and gh-prs (MAX_LIST + 1) work: they tee a flat one-line-per-item buffer, so line K is item K. mvn's dropped failing classes are multi-line blocks (close line + per-test sublines + exception + user frames), interleaved through the raw output, so a fixed MAX_MVN_FAILING_CLASSES + 1 offset against raw would land on [INFO] Scanning for projects… rather than the 11th failure. The summary entries ([ERROR] Class.test:line …) are one-liners, but they're not contiguous in raw either.

That's the case the src/cmds/README.md table routes to force_tee_hint — multi-line "test failures, build error blocks," where "a line offset is meaningless" — and it's why the pytest path you pointed at (pytest_cmd.rs:281) uses force_tee_hint(&all_failures, …) rather than the tail variant.

So my plan was force_tee_hint(raw, "mvn-test-failures") paired with the failing-class tail: full raw output for recovery, and since force_tee_path bypasses the tee.mode = "failures" gate it also covers the -Dmaven.test.failure.ignore=true green case you mentioned. Does that work, or did you have a flat-list buffer in mind that I'm not seeing?

[aeppling]

Hello @vdufloth, my bad on the back-and-forth around the tee hint, that was a wrong direction. The PR is ready as it is. Thanks for the thorough work across all the rounds (trail re-arm, CRLF fix, reactor summary, cap binding to CAP_WARNINGS, tail format alignment), and for the patience.

Approving and mergin, thanks again for this contribution ! Will be available in next release