Review feedback on #2550: the flag-shape tests all used standalone flags, leaving the exact #2543 repro -- a bundled -rln/-ln files-with-matches cluster -- unexercised. Assert rtk grep -rln <pat> <dir> lists matching files instead of a false '0 matches'.
Per review: avoid signal loss that pushes agents to retry with
`rtk proxy`. pulumi-stack now keeps the full stack identity block
(Owner:, Pulumi version used:) alongside the header and Last updated:.
Only interactive prompt and help-footer noise is stripped.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The grep argument-parsing rework routed many flag shapes through the
grouping reparse, which silently dropped any line it could not parse and
returned a success exit. Common invocations produced wrong or empty
output -- and grep-only syntax like `--include` failed outright -- which
pushed agents back to unfiltered `rtk proxy grep`.
Every valid grep invocation now produces correct output:
- Context (`-A`/`-B`/`-C`) is grouped and compressed instead of dropped;
the header counts matches only and context lines are shown.
- Output the reparse cannot represent (`-N`, `-I`, `--color`, `-p`,
`--heading`, binary-file notices, ...) is passed through verbatim
instead of reported as a false "0 matches".
- Format/shape flags (`-c`, `-l`, `-o`, `-Z`, `--column`, `-b`,
`--vimgrep`, `--null-data`) pass through cleanly with no NUL leak.
- grep-only syntax (`--include`, `--exclude`, ...) works: ripgrep is the
fast path and rtk falls back to system grep when ripgrep rejects a flag.
- rtk grep never emits more than plain grep: if grouping is not smaller
than `file:line:content`, the raw form is shown.
Resolves#2543.
Stop stripping `Current stack is X:` and `Last updated:`. pulumi stack
is read-only (exit 0) so there's no tee fallback; dropping the active
stack identity was irreversible signal loss. Keeping the header also
fixes the orphaned indented `Managed by <user>` line.
Owner: and Pulumi version used: remain stripped as noise.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
test_needs_raw_fallback dropped the raw stdout prepend whenever every
parsed failure carried detail, but never checked the list was complete
against summary.failed. A run reporting 5 failures but parsing only 3
detailed blocks would silently lose the 2 unparsed failures.
Add a `failed_tests.len() < summary.failed` clause so the raw fallback is
kept when the structured section is numerically incomplete. Add a
regression test and fix an inconsistent existing fixture (failed: 5 with a
single parsed entry) that the new clause correctly flagged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Explicit Vitest reporters can produce successful output that exceeds the passthrough cap. Track truncation as structured state and use force_tee_hint so the full verbose output remains recoverable.
On failing `dotnet test` runs the orchestrator prepended the full raw
stdout ahead of the filtered summary. The `Failed Tests:` section already
reproduces each failure (name + message + clipped stack) parsed from
TRX/console, so every failure was printed twice — inflating output +65%
vs raw, scaling linearly with failure count (issue #2501).
Gate the raw-stdout prepend behind `test_needs_raw_fallback`: skip it when
the structured section carries detail, keep it when the filter is blind
(no failures parsed, or a parsed failure has empty detail — e.g. a
self-closing <UnitTestResult> with no <ErrorInfo>, or a build failure /
crash where nothing reaches failed_tests).
Extracts two pure, unit-tested fns (`test_needs_raw_fallback`,
`compose_failure_output`) so the orchestration decision is covered without
running dotnet.
Closes#2501
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address KuSh review on #2465:
- Move the `exit_code >= 2` decision into a pure `is_grep_error_exit`
function and unit-test it directly (0/1 = normal, >=2 = error),
instead of faking the rg binary in an integration test.
- Drop the GREP_ERROR_EXIT const; the doc comment on the function
conveys the grep/rg exit convention.
- Remove tests/grep_error_test.rs (faked binary) in favour of the
unit test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Wrap the rtk binary path expression per rustfmt to satisfy
`cargo fmt --all -- --check` on CI.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The test simulates a broken `rg` via a chmod'd shell script (from_mode,
PermissionsExt) which is unix-only. The unconditional `use std::os::unix`
broke compilation of the integration-test crate on the Windows CI target.
Gate the whole file so Windows stays green while preserving unix coverage.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exit codes >= 2 signal real errors in grep/rg (bad regex, tool
crash, missing binary). rtk was only checking the narrow case
exit_code == 2 with non-empty stderr, letting any other error
exit masquerade as a clean "0 matches" false negative.
Now any exit code >= 2 surfaces the error to stderr instead of
printing a false zero-match result.
Fixes#2461
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The system-grep fallback (used when ripgrep is not installed) passed
-rnHZ, relying on -Z for the NUL filename separator the match parser
requires. -Z only means --null on GNU grep; on BSD/macOS grep it is an
alias for --decompress (zgrep mode), so output is plain
file:line:content with no NUL. parse_match_line() then matches zero
filenames and every result collapses into "N matches in 0 files" with
all lines hidden behind [+N more].
Use the long option --null instead, which both GNU and BSD grep define
as "print a zero-byte after the file name".
Related to #2310
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The left-to-right scanning logic now lives entirely in parse_cluster(), which
returns a ClusterResult enum (Boolean or ValueTaking). extract_pattern_path
just dispatches on the result — no loop logic inline.
parse_cluster() is the only place that touches cluster bytes, so the full
contract is testable in isolation:
parse_cluster("ecarrot") → ValueTaking { prefix: None, flag: 'e', inline: "carrot" }
parse_cluster("rne") → ValueTaking { prefix: Some("n"), flag: 'e', inline: "" }
parse_cluster("g*.rs") → ValueTaking { prefix: None, flag: 'g', inline: "*.rs" }
parse_cluster("rn") → Boolean(Some("n"))
strip_r() remains as the focused r/R-stripping helper used internally by
parse_cluster (and tested directly to document the "carrot"→"caot" danger).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The left-to-right cluster scan now calls strip_r() at emit time on the
accumulated flag-letter prefix (raw_prefix), never on inline value bytes.
This makes the safety contract explicit and testable: strip_r("carrot")
produces "caot", documenting exactly why it must not touch value bytes.
Also fix pre-existing unused-mut in aws_cmd.rs (has_output_flag).
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Five regressions on grep_arg_parsing vs develop:
- `-ecarrot` matched "caot" — strip_r was applied to the inline value bytes.
Fix: scan cluster left-to-right; stop at first value-taking letter and
treat the remainder as the inline value without r/R stripping.
- `-g*.rs` glob became `*.s` — same root cause, inline glob value r-stripped.
- `--glob '*.md'` took `*.md` as a path — no VALUE_FLAGS_LONG existed; the
value token fell through to the positional collector.
Fix: add VALUE_FLAGS_LONG covering all rg long flags that take a value.
- `--max-count 1 fn file` took `1` as the pattern — same missing long flag.
- VALUE_FLAGS_SHORT expanded: add t/T (--type/--type-not), d (--max-depth),
M (--max-columns) so inline short forms and space-separated forms parse
correctly without mis-routing values to patterns or paths.
- --regexp (long form of -e) now routes to patterns like -e does.
- has_format_flag: add --count-matches, --json, --passthru, --files so
these output-format modes get passthrough treatment instead of silent
"0 matches in 0 files".
2183 tests pass.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
rtk diff treated files whose changes were all classified as modified
(similar lines, e.g. "a: 1" vs "a: 2" in YAML/JSON) as identical,
because the identical check only looked at added/removed counts.
Report any non-empty change set as a difference, and exit 1 when
files differ per diff convention (0 when identical).
Fixes#2364
A combined cluster like -rne PATTERN or -rA 2 had the last flag's
value consumed only if the cluster was exactly 2 chars. Longer
clusters (e.g. -rne) fell through to process_flag, which stripped
-r and emitted -ne — then -e ended up in flags without a value,
causing rg to error.
New unified short-flag handler: for any single-dash cluster, inspect
the last byte. If it is -e or a VALUE_FLAGS_SHORT member, strip the
r/R chars from the prefix, emit the prefix as a flag (if non-empty),
and consume the next token as the value (pattern for -e, flag value
otherwise). Long flags (--foo) are still routed through process_flag.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Four bugs in `extract_pattern_path` caused `rtk grep` to misparse
common invocations:
- `-A 2 error src` → `2` was taken as the pattern (value-taking
flags not recognised)
- `-rn foo src` → `n` leaked to rg as `--replace` value (only
standalone `-r` was stripped, not combined clusters like `-rn`)
- `TODO src tests` → second path silently dropped (only one path
kept)
- `-i x agent -g '*.md'` → rg regex error (`*.md` became positional
before `-g` could consume it)
New implementation:
- `VALUE_FLAGS_SHORT`: explicit byte-set of 2-char flags that consume
one following token (`A`, `B`, `C`, `g`, `f`, `j`, `m`)
- `process_flag()`: strips `r`/`R` from any single-dash cluster,
drops `--recursive`; replaces the old inline `if arg == "-r"` loop
- `extract_pattern_path` returns `(Vec<String>, Vec<String>, Vec<String>)`
(patterns, paths, flags) — no more `Option<String>` for pattern
- `-e` values collected separately; if any `-e` is present, all
positionals are paths; otherwise first positional is the pattern
- All patterns forwarded to rg as `-e` flags (BRE `\|`→`|` inline)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- restore migration Outdated check in hook_check (binary registered +
legacy script present still reports Outdated until cleanup)
- add error context to resolve_claude_dir call sites (provider, integrity)
- use CLAUDE_DIR const instead of literal in telemetry + provider test
- simplify settings_path construction in print_manual_instructions
Refs #2087
rtk init -g ignored CLAUDE_CONFIG_DIR and always targeted ~/.claude.
Honor the env var Claude Code uses to relocate its config directory.
Priority: RTK_CLAUDE_DIR > CLAUDE_CONFIG_DIR > $HOME/.claude.
Empty values are treated as unset. Manual-step instructions now
print the resolved path instead of a hardcoded ~/.claude.
All callsites that previously hardcoded home.join(CLAUDE_DIR) now use
the shared resolve_claude_dir(): hook_check (rtk gain), integrity
(rtk verify), telemetry, and discover (rtk discover).
Closes#633
Match the join_with_overflow convention (ellipsis char, '+N more' label)
used by container, aws, gh and glab so the LLM sees one consistent
overflow style. The tee '[full output: …]' recovery hint already comes
from core tee_and_hint via RunOptions::with_tee, so capped failure output
carries the canonical recovery path unchanged.
Replace the per-filter LimitsConfig::mvn_max_failures field with a local
const MAX_MVN_FAILING_CLASSES = CAP_WARNINGS, matching the canonical cap
binding used by pytest/rspec/rake/runner. Align cap = 0 semantics with the
core policy (summary-only: no blocks emitted, tail still counts) instead
of the old opt-out meaning, so the future config surface lands without a
special case.
aesoft