Skip to content

fix(gh): show fallback note for filtered body; fix api truncation message - #235

Closed
polaminggkub-debug wants to merge 1 commit into
rtk-ai:masterfrom
polaminggkub-debug:fix/gh-body-and-api-truncation
Closed

polaminggkub-debug wants to merge 1 commit into
rtk-ai:masterfrom
polaminggkub-debug:fix/gh-body-and-api-truncation

Conversation

@polaminggkub-debug

Copy link
Copy Markdown
Contributor

Problems

Two related issues in `gh_cmd.rs`, same category as the body-display fix from #214.

1. Silent body loss in `view_pr()` and `view_issue()`

When `filter_markdown_body()` removes all content (body contains only badges, images, or HTML comments), the body section is silently skipped — no output, no indication that content existed.

Example body that disappears completely:
```markdown

CI
screenshot

```

The user sees PR metadata but has no way to know a body was present and filtered.

Fix: Show `(body contained only badges/images/comments)` so the user is aware.

2. Naive `.take(20)` with double iteration in `run_api()`

The non-JSON fallback in `run_api()` used the same naive truncation pattern that #214 fixed for `view_pr`:

  • Calls `raw.lines()` twice (once to take 20, once to count total)
  • Shows `"... (truncated)"` with no line count

Fix: Single pass, message shows exact count: `"... N more lines (use gh api for full output)"`, consistent with `list_prs()` and the rest of the module.

Testing

All 412 existing tests pass.

🤖 Generated with Claude Code

Two fixes in gh_cmd.rs:

1. view_pr() and view_issue(): when filter_markdown_body() returns an
   empty string (body contained only badges, images, or HTML comments),
   the body was silently skipped. Now shows a note so the user knows
   content existed but was filtered.

2. run_api() non-JSON fallback: replaced naive .take(20) + double
   iteration with a single pass that shows line count in the truncation
   message ("... N more lines"), consistent with the rest of the module.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@pszymkowiak

Copy link
Copy Markdown
Collaborator

Hi,

The gh_cmd.rs fix looks good — showing a fallback note when the body is fully filtered and improving the API truncation display are both useful improvements.

However, this PR was branched from an older version of master and would unintentionally delete features merged since then:

  • wrangler_cmd.rs (Cloudflare Wrangler support, 709 lines)
  • grep -n flag support
  • Playwright JSON parser fixes

Please rebase on current master — the gh_cmd.rs changes should apply cleanly with no real conflicts.

@FlorianBruniaux

Copy link
Copy Markdown
Contributor

Hi @polaminggkub-debug, the underlying logic is sound. The run_api() single-pass refactor is cleaner than before, and the fallback note for badge-only bodies is genuinely useful.

Three things to sort out:

  1. CI is showing 2/3 checks passing with one failing. We can't merge with a known red CI regardless of local test results. Please identify and fix the failing check.

  2. The two new else branches (one in view_pr, one in view_issue) have no tests. A fixture with a badge-only body would cover the new path.

  3. The " (body contained only badges/images/comments)\n" string is duplicated identically in both branches. Extracting it to a module-level const would make it a one-liner to change later.

@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@pszymkowiak

Copy link
Copy Markdown
Collaborator

Hi! Two things needed before we can review:

  1. Retarget to develop — this PR targets master, but all PRs should target develop. You can change the base branch in the PR settings (right sidebar).
  2. Sign the CLA — if not already done, please sign at https://cla-assistant.io/rtk-ai/rtk

Thanks!

@aeppling

Copy link
Copy Markdown
Contributor

Hey

We are cleaning up the codebase and improving the project structure for better onboarding. As part of this effort, PR #826 reorganizes src/ from a flat layout into subfolders.

No logic changes — only file moves and import path updates.

What you need to do

Rebase your branch on develop when receiving this comment:

git fetch origin && git rebase origin/develop

Git detects renames automatically. If you get import conflicts, update the paths:

use crate::git;        // now: use crate::cmds::git::git;
use crate::tracking;   // now: use crate::core::tracking;
use crate::config;     // now: use crate::core::config;
use crate::init;       // now: use crate::hooks::init;
use crate::gain;       // now: use crate::analytics::gain;

Need help rebasing? Tag @aeppling

@pszymkowiak

Copy link
Copy Markdown
Collaborator

Hi @polaminggkub-debug — closing as stale, but we'll re-implement
your fix on develop with Co-authored-by: polaminggkub-debug
attribution.
The bugs you spotted are still live in v0.42.0, so the
work won't be lost.

What's making a fresh PR cleaner than rebasing:

Your fix logic carries over cleanly:

  1. format_pr_view (src/cmds/git/gh_cmd.rs:411-421) and
    format_issue_view (:658) need an else branch when
    body_filtered.is_empty() to push the
    (body contained only badges/images/comments) note.
  2. run_api truncation message gets the single-pass refactor +
    ... N more lines (use gh api for full output) formatting.

Closing as superseded-by-reimplementation. Thanks for finding both
bugs — your run_api single-pass version is genuinely cleaner than
the original double-iteration.

KuSh pushed a commit that referenced this pull request May 31, 2026
When `filter_markdown_body()` strips a PR or issue body to empty
(body contained only badges, images, HTML comments, or horizontal
rules), `format_pr_view` and `format_issue_view` previously skipped
the body section silently. Users had no indication that body content
had been present and filtered.

Now both views emit a fallback note when the filtered body is empty
but the raw body was not:

  PR view:    (body contained only badges/images/comments)
  Issue view: Description: (body contained only badges/images/comments)

The 4 added tests cover:
- PR body with only badges/images/comments -> fallback note appears
- PR body with real content -> no fallback note (sanity)
- PR body empty (raw) -> no fallback note (no signal to give)
- Issue body badges-only -> fallback note appears

Closes #235

Co-authored-by: polaminggkub-debug <polaminggkub-debug@users.noreply.github.com>
thehoff added a commit to thehoff/contextcrawler that referenced this pull request Jul 14, 2026
)

A store created by a pre-rtk-ai#219 release (or under a permissive umask) can be
0755/0644, which the strict 0o077 validator rejects — emitting "trust store
unreadable ... treating all filters as untrusted" on every command and silently
disabling custom filter trust (built-in filters and the security gates are
unaffected).

The 0o077 check is correct and is NOT relaxed. Instead, on read, an owner-owned
store dir/file with group/other bits is tightened to 0700/0600 before
validation, best-effort and fully descriptor-based (no TOCTOU): the file via its
already O_NOFOLLOW-open fd (fchmod), the directory via a fresh
O_NOFOLLOW|O_DIRECTORY open then fstat+fchmod on that fd, so a symlink swap
cannot redirect the chmod. Foreign-owned paths and symlinks are never touched —
the validator still rejects those; permissions are only ever tightened.
Creation already writes private, so this only migrates legacy stores.

Regression test: a 0755 dir + 0644 file store reads cleanly and ends up
0700/0600. Council (deepseek+mmax) SHIP; the raised dir-chmod TOCTOU is closed
by the descriptor-based approach. Full lib suite 3290 pass/0 fail; clippy clean.

Refs: rtk-ai#235
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K3SAimSBwPUp4MkRXaYXSU
thehoff added a commit to thehoff/contextcrawler that referenced this pull request Jul 14, 2026
Migrate a pre-rtk-ai#219 0755/0644 store to 0700/0600 on read (descriptor-based, no
TOCTOU) instead of warning + untrusting all filters. 0o077 check unchanged.
Council SHIP. Refs: rtk-ai#235
thehoff added a commit to thehoff/contextcrawler that referenced this pull request Jul 14, 2026
rtk-ai#235 — self-heal a legacy-permissioned (0755/0644) filter trust store to
0700/0600 on read (descriptor-based, no TOCTOU) instead of warning and
untrusting all custom filters. The owner-only requirement is unchanged. See
CHANGELOG.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01K3SAimSBwPUp4MkRXaYXSU
ryanmurf added a commit to murphytek/rtk that referenced this pull request Jul 25, 2026
* fix(security): replace insecure tmp, lock git workflow perm

* fix(security): pin workflow actions to SHA, clean up tempfile on failure

* Update pr-target-check.yml

* fix(cicd): set release-please target-branch to master [skip ci]

* chore(master): release 0.40.0

* fix(docker): forward --tail flag in compose logs

Previously `rtk docker compose logs --tail=20 web` would fall through to
the passthrough handler because clap had no `--tail` field on the `Logs`
subcommand, resulting in 0% token savings. Additionally, `run_compose_logs`
always fetched 100 lines regardless of user input.

- Add `#[arg(long, default_value_t = 100)] tail: u32` to `ComposeCommands::Logs`
- Thread the value through to the docker invocation
- Default unchanged (100 lines), so bare `rtk docker compose logs` behaves as before

Ports the core fix from #580, retargeted onto current develop layout
(`src/cmds/cloud/container.rs`). Relates to #578.

Co-authored-by: Mihir Dash <137862945+slice-mihird@users.noreply.github.com>

* fix(filters): address adversarial test-suite findings on aggressive filtering

Several filters dropped or rewrote information an agent needs, producing
output that was misleading rather than merely compressed. This fixes the
clear-cut cases surfaced by the TheDecipherist/rtk-test suite:

- ls: stop hiding the `.env` file (removed `.env`/`env` from NOISE_DIRS).
- git status: restore the explicit "HEAD detached at <sha>" line instead of
  the opaque "* HEAD (no branch)".
- log/docker logs: recognise CRITICAL/FATAL/ALERT/EMERGENCY/SEVERE (and
  NOTICE) as severities so those lines are no longer silently filtered;
  expose `log` as a `rtk pipe` filter and accept a positional filter name.
- wc: forward stdin to the child so `cat file | rtk wc` counts the pipe
  instead of reporting 0 (new RunOptions::inherit_stdin).
- jest/vitest: include skipped/pending test count in the compact summary.
- pytest: surface xfailed/xpassed counts and list XFAIL/XPASS entries with
  their reasons (adds `-rxX`).
- ruff/eslint: list individual violations with file:line:col, not just
  rule/file group counts.
- pnpm/npm list: render the actual package list instead of the false-positive
  "All packages up-to-date" when there's no upgrade info.
- git add: stay silent on a no-op instead of printing an ambiguous "ok".
- git stash: pass git's own message through instead of collapsing to "ok".
- docker ps: keep the Status column (health/restart info) and list
  stopped/exited containers; docker images: show all images with full names;
  docker compose ps: use `-a` so crashed services stay visible.

All 1870 unit tests pass; cargo fmt + clippy clean.

* chore(filters): remove filter-level annotations and restore compose logs tail arg

* fix(filters): add test for aggressive filter batch fix

* fix(git): stream push output via FilterMode::Streaming (#963)

## Problem (#963)

`rtk git push` reportedly times out: users see
`bash tool terminated command after exceeding timeout 30000 ms` while
plain `git push` to the same remote completes fine. P1-critical because
every Claude Code git push goes through rtk.

## Root cause

`run_push` used `cmd.stdin(Stdio::inherit()).output()`. `Command::output()`
captures both stdout and stderr until the child exits. Git push prints
its progress (`Counting objects` / `Compressing objects` / `Writing
objects`) to stderr and may prompt for SSH passphrases or HTTPS
credentials. With stderr captured, Claude Code's bash tool saw zero
output for 30+ seconds and killed the command — exactly the 30000 ms
message in the issue.

## Fix

Rewrite `run_push` on top of the streaming infrastructure that already
exists for this exact purpose (`stream::run_streaming` +
`FilterMode::Streaming`, added in 0.37.0).

Add a counterpart to `BlockStreamFilter<H: BlockHandler>` in
`src/core/stream.rs`: `LineStreamFilter<H: LineHandler>`. Where
`BlockStreamFilter` defaults to DROP and emits only collected blocks,
`LineStreamFilter` defaults to KEEP and lets handlers opt into dropping
noise. Trait surface mirrors `BlockHandler`:

- `should_skip(&mut self, line: &str) -> bool` — default false
- `observe_line(&mut self, line: &str)` — default no-op
- `format_summary(&self, exit_code, raw) -> Option<String>`

This lets future streaming commands reuse the line-oriented pattern.

`GitPushLineHandler` then becomes a tiny `LineHandler` impl:

- `should_skip` drops the high-volume progress phases (Enumerating /
  Counting / Compressing / Writing objects, Delta compression, Total)
  and blank lines.
- `observe_line` captures the up-to-date sentinel and the first ref
  update target (e.g. `master`) for the summary.
- `format_summary` emits `ok <ref>` / `ok (up-to-date)` / `ok` on
  success; nothing on failure (raw error lines already flowed through).

Stdin is inherited (`StdinMode::Inherit`) so SSH passphrase and HTTPS
credential prompts still reach the user. Tracking now records the real
raw output and the filtered output.

## Test plan

- [x] `cargo fmt --all -- --check`
- [x] `cargo clippy --all-targets -- -D warnings` — clean
- [x] `cargo test --all` — 1880 passed, 0 failed, 6 ignored
- Six unit tests cover the push handler: progress-prefix drop,
  up-to-date summary, remote message passthrough, no-summary-on-failure,
  first-ref-wins, and token-savings (>=60% on a representative payload).
- Four unit tests cover the new `LineStreamFilter` trait:
  default-keep-all, skip-drops-matching, summary-propagates-exit-code,
  observe-only-called-for-kept-lines.

## Notes

- Behaviour change: users now see git's native output line-by-line
  (with progress phases stripped) plus a final `ok <ref>` summary,
  instead of just the compact summary. This matches plain `git push`
  more closely and is what the issue reporter expects.
- No regression for other filters: `run_pull`, `run_fetch`, `run_clone`
  are untouched; only `run_push` is modified.

Closes #963

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---
_Vibe Coded by Ousama Ben Younes_
_Developed With Ora Studio (Claude Code)_

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat(hints): add tail hints for tee & hints + address reviews

- fix signal truncation gaps + kush reviews
- added a new tee and hints function to give an hint with tail (avoid re-ingest of the head)
- extracted patterns in constants

* fix: re-add env python as noisy dir

* fix: '...' ascii to unicode, remove some comments

* fix(kubectl): compact get pods and services aliases

Co-authored-by: Cursor <cursoragent@cursor.com>

* feat(mvn)!: Rust module replacing TOML filter, adds test phase support

Replaces the stateless src/filters/mvn-build.toml (4 phases, 50-line cap) with
a Rust module under src/cmds/jvm/mvn_cmd.rs handling all common Maven lifecycle
phases with stateful Surefire/Failsafe block collapse.

Why TOML couldn't do this: the dominant noise on a healthy Maven project is
expected-exception stack traces inside passing tests (assertThrows). Collapsing
these requires deciding to drop a block based on its closing
`Tests run: N, Failures: 0, Errors: 0` line — a stateful decision the TOML DSL
cannot express.

Measured savings on synthetic full-shape fixtures (~1100 lines each, gzipped):
- mvn test:    3 422 tok → 36 tok (98.9% savings)
- mvn install: 3 460 tok → 85 tok (97.5% savings)

Coverage:

| Phase                              | Filter                                 |
|------------------------------------|----------------------------------------|
| test, integration-test             | filter_surefire (block collapse)       |
| compile, test-compile              | filter_compile (ERROR + indent + dedup)|
| package, install, verify, deploy   | filter_package (compile + surefire)    |
| clean, site, plugin goals, version | passthrough                            |

Key behaviours:
- ANSI strip first in every filter (real Maven output contains colour escapes).
- English-footer guard: if no `BUILD SUCCESS`/`BUILD FAILURE` line is present,
  return ANSI-stripped raw input unchanged — protects non-English locales.
- Verbose bypass: `-X`, `--debug`, `-e`, `--errors` skip filtering.
- Stack-frame deny-list strips framework frames (`at org.junit.`, `at java.util.`,
  `at sun.reflect.`, etc.); user-code frames (any other prefix) preserved.
- Duration normalisation (`Time elapsed: 2.341 s` → `Time elapsed: T s`) for
  deterministic test output.
- Wrapper detection: `./mvnw` / `mvnw.cmd` via string-literal `Command::new`
  (semgrep-safe); falls back to `resolved_command("mvn")`.

Hook regex changes (src/discover/rules.rs):
- Adds wrapper prefixes (./mvnw, mvnw.cmd, mvnw).
- Adds test, integration-test, verify, deploy to the alternation.
- Drops clean + site so bare invocations bypass RTK entirely (0 overhead).
- Lazy quantifier skips flags before the goal (`mvn -B clean install` now
  rewrites correctly; previously failed because the regex required the goal
  as the first token after `mvn`).
- Drops subcmd_savings (lazy match captures first phase, which would mis-tier
  `clean install` to clean); uses flat 82.0.

Tests:
- 33 unit tests in mvn_cmd.rs (phase detection, footer guard, framework deny,
  duration normalisation, ERROR continuation, WARNING dedup, install/jar lines).
- 2 inline savings assertions on gzipped synthetic full fixtures (~3 KB each)
  using flate2 already in Cargo.toml. Run as part of standard `cargo test --all`.
- 12 classifier/rewrite tests in src/discover/registry.rs under new Maven
  section.

Files changed:
- ADD src/cmds/jvm/mvn_cmd.rs (~810 LoC incl. tests)
- ADD src/cmds/jvm/README.md (documents whitelist omission decision)
- ADD tests/fixtures/mvn_*_raw.txt (6 inline) + 2 .gz synthetic full fixtures
- MOD src/main.rs (1 use, 1 Commands variant, 1 dispatch arm)
- MOD src/discover/rules.rs (replace mvn entry)
- MOD src/discover/registry.rs (12 classifier tests)
- MOD src/core/toml_filter.rs (drop mvn-build from expected-filters list,
  adjust the 2 count assertions)
- DEL src/filters/mvn-build.toml

Fixtures are entirely synthetic — generic `com.example.app.*` package names and
`com.example:myapp` Maven coordinates. Same SHAPE as real Maven output (block
structure, ANSI codes, framework stack frames, BUILD footer, plugin banners)
without any project-identifying content.

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). The whitelist is opt-in by design per the
comment at L2452-2454 — filter modules invoked through an already-verified
hook do not need a second integrity check on their own dispatch path.
Documented in src/cmds/jvm/README.md.

Out-of-scope (follow-ups noted in module + README):
- Parallel mode `-T2C` (interleaved blocks across threads).
- Plugin goals (`mvn dependency:tree`, `mvn versions:*`).
- `mvnw.bat` legacy wrapper (Maven Wrapper 3.x emits `.cmd`).

BREAKING CHANGE: `rtk mvn <args>` output format changed. Previously: TOML
filter strips INFO/Downloading lines and caps at 50 lines. Now: state-machine
filter with Surefire block collapse + locale guard + verbose bypass. Different
output shape; significantly better savings tier; behaviour is otherwise a
superset (all previously-handled cases still handled, plus test/verify/etc.).

* fix(filters): split docker ps/-a paths, cap ruff violations at 50

- docker ps reverts to plain (no -a augmentation)
- docker ps -a / compose ps -a wired to the running/stopped split filter
- ruff Violations: capped at 50 + force_tee_tail_hint recovery
- tests for the ruff and pytest xfail caps

* fix(init): honor dry-run for Pi install and uninstall paths

* fix(tee): safe truncation caps and compose-ps tee content fix

Replace all bare magic offset literals (e.g. 21 for cap=20) with named
constants (MAX_XXX + 1) so offsets stay in sync if caps change.
Fix compose_ps to tee pre-formatted lines instead of raw tab-separated
input, so tail -n +N gives the agent readable content.
All 13 affected modules updated: container, dotnet, gh, glab, go, lint,
pnpm, pytest, rubocop, cargo, aws. 1884 tests pass.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix(rust): multi-line blocks used with tail hint

* docs(cmds): truncations hints and recovery guidelines

* test(hooks/init): add red test for copilot-instructions preservation

Adds a TDD red test that asserts pre-existing user content in
.github/copilot-instructions.md must survive 'rtk init --copilot'.
Currently fails because run_copilot() uses write_if_changed() which
truncates the file.

Refs #1964

* fix(hooks/init): preserve user content in copilot-instructions.md

run_copilot() previously called write_if_changed() which truncated any
pre-existing .github/copilot-instructions.md, destroying user-authored
Copilot rules.

This routes copilot-instructions.md through upsert_rtk_block() (the
same idempotent marker-block helper already used for CLAUDE.md and
AGENTS.md). The COPILOT_INSTRUCTIONS constant is wrapped in
<!-- rtk-instructions v2 --> ... <!-- /rtk-instructions --> markers
so subsequent inits replace only RTK-owned content; the rest of the
file is preserved verbatim. Malformed pre-existing blocks (opening
marker without closing) are refused with a remediation message
rather than silently rewritten.

Fixes #1964, #1891

* test(hooks/init): cover idempotency, stale-block, dry-run, fresh, malformed paths

Adds five additional tests around copilot-instructions.md upsert:
- idempotent re-init (no duplicated blocks, identical content)
- stale RTK block in-place update with surrounding user content preserved
- dry-run never creates or modifies the file
- fresh install creates file with RTK marker block when no file exists
- malformed pre-existing block (opening marker without closing) is
  refused with a clear error rather than silently rewritten

Refs #1964

* refactor(hooks/init): extract run_copilot_at for parallel-safe tests

run_copilot(ctx) is now a thin wrapper around run_copilot_at(base, ctx);
tests pass a TempDir path so they no longer mutate process-global cwd.
This eliminates flakes when cargo test runs in parallel (the default).

The public API is unchanged; only the internal seam moves.

Refs #1964

* docs(readme): fix license references to match LICENSE (Apache-2.0)

The README badge and footer claimed MIT, but the LICENSE file is
Apache License 2.0. Updates both references so the README matches
the authoritative LICENSE file.

Closes #1996

Co-Authored-By: Claude <noreply@anthropic.com>

* refacto(truncations): Set global CAPS for truncation

Following tee and hint refacto
Add global CAP constant to be inherited , to enable easier global configuration later

* refactor(hooks/init): share write_rtk_block, unify malformed handling

Address review feedback on #1976:

- Extract write_rtk_block() shared dispatcher: eliminates the duplicated
  4-arm RtkBlockUpsert match between run_claude_md_mode and the former
  upsert_copilot_instructions (now inlined). Both callers stay under the
  ~60-line guideline.
- Unify malformed handling: both paths now bail!() with a diagnostic and
  the exact recovery command. CLAUDE.md previously warned and exited 0,
  silently skipping the OpenCode plugin step; behaviour is now consistent.
- Reorder run_copilot_at: upsert copilot-instructions.md BEFORE writing
  the hook config so a malformed file aborts the install without leaving
  a stale .github/hooks/rtk-rewrite.json on disk.

Regression coverage:
- test_claude_md_mode_refuses_malformed_block mirrors the existing
  copilot malformed test against the shared dispatcher contract.
- test_copilot_init_malformed_leaves_no_hook_on_disk pins the new write
  order so a future re-order regression is caught.

cargo fmt / clippy --all-targets / test --bin rtk: clean (1909 passed).

* chore(cargo): declare MSRV via rust-version = "1.91"

rtk uses `str::floor_char_boundary()` (src/cmds/system/pipe_cmd.rs:140)
which was stabilized in Rust 1.91.0. Without `rust-version` in
Cargo.toml, packagers on older toolchains hit a confusing
`use of unstable library feature` error from rustc.

With this declaration, cargo gives the expected friendly diagnostic
up-front:

    error: rustc 1.88.0 is not supported by the following packages:
      rtk@0.34.3 requires rustc 1.91

Verified locally by switching toolchains:
- cargo +1.88 check → fails with the new clear MSRV error (intended)
- cargo +1.93 fmt/clippy/test → 1909 passed, zero warnings

Fixes #1402

* fix(truncate): global caps reduce (avoid underflow and 0 results)

- Avoid underflow + 0 results caused by overwrite of global constant CAPS
- Soon user will be able to apply global configuration, this will change global CAPS and should not cause any underflow or 0 results, if overwrite sub >= CAPS -> use CAPS.

* fix: honor explicit -n N limit for git log on merge commits

When user runs 'git log -1 --format='%H' HEAD' where HEAD is a merge
commit, rtk was adding --no-merges which filtered out the merge commit
itself and returned the second parent instead. This made 'git log -1'
return wrong SHAs for merge commits.

Fix: don't add --no-merges when user explicitly passes -n N or
--max-count=N. When a user specifies an exact count they expect exactly
that many commits, not filtered results. Also skip --no-merges if user
already passed --merges or --no-merges explicitly.

Fixes rtk-ai/rtk#2009.

* chore: regroup agent init tests by agent section

* fix(mvn): match Surefire 3.x close lines, preserve failure stack trail

CLOSE regex previously required ` - in ` (single-dash, Surefire 2.x).
Surefire 3.x emits ` -- in ` (double-dash), so close lines never matched
on Maven 3.9 default toolchains: blocks stayed open, every new
`[INFO] Running …` flushed the prior block as kept, and real-world
savings dropped to ~27% on commons-cli.

Changes:
- CLOSE separator widened to `\s+--?\s+in ` (matches 2.x and 3.x).
- CLOSE prefix widened to `INFO|ERROR|WARNING` (3.x emits WARNING for
  classes whose only tests are skipped).
- `filter_surefire` and `filter_package` gain a `failure_trail` flag:
  Surefire 3.x emits the exception class and stack frames *after* the
  CLOSE line; keep them (stripping framework frames) until the next
  blank line. Without this, the failing-test signal was silently dropped.

Fixtures replaced with output from a real `apache/commons-cli` build in
`maven:3.9-eclipse-temurin-21`. Synthetic 2.x shape removed; 2.x compat
locked by a dedicated unit test using the single-dash separator.

Measured token savings on commons-cli fixtures:
- `mvn test` (full):    1896 -> 38 tokens (98.0%)
- `mvn install` (full): 2021 -> 78 tokens (96.1%)

Refs: pszymkowiak review on PR #1956

* refactor(mvn): drop production duration normalisation

`normalise()` rewrote real `Time elapsed: N s` and `Total time: N s`
values to `T s` in the shipped output. That was only ever useful for
keeping snapshot tests deterministic and never belonged in production:
test perf signal is exactly the kind of data the LLM needs to spot a
regression, and there are no insta snapshots in this module to protect.

- Remove `normalise()` and its `TIME_DURATION` / `TOTAL_TIME` regexes.
- All `filter_surefire`, `filter_compile`, `filter_package` emit sites
  pass the raw line through.
- Renamed and inverted `surefire_normalises_durations` →
  `surefire_preserves_real_durations`: asserts `2.341 s` and
  `Total time:  4.567 s` survive filtering.

Refs: pszymkowiak review on PR #1956 (E.1)

* feat(mvn): filter mvn -q quiet-mode output

Under `mvn -q`, Maven 3.x suppresses all `[INFO]` lines: no
`BUILD SUCCESS` footer, no `[INFO] Running` markers, no module
banners. The existing surefire/compile/package filters all key off
the English-footer guard and `[INFO] Running`/CLOSE state machine,
so under `-q` they fell through and shipped the raw output unchanged
(0% savings on PR #1956 reviewer's measurement).

Reviewer (@pszymkowiak on PR #1956) listed `-q` handling in the
path-to-merge as "worth a note or handling". This commit implements
the handling.

Behavior captured from `mvn -q test` on Maven 3.9.9 + Surefire 3.5.5
with JUnit 5:

  - Green run emits ZERO bytes — filter returns empty output.
  - Failure run emits ~27 lines: per-class CLOSE line, exception +
    stack trace, `[ERROR] Failures:` summary, aggregate
    `[ERROR] Tests run: N, Failures: F, ...`, and the
    `[ERROR] Failed to execute goal` terminator, followed by a
    ~10-line `[ERROR] See .../[Help 1].../Re-run Maven .../To see
    the full stack trace .../For more information ...` boilerplate
    block pointing at log files and help URLs.

`filter_quiet` keeps the failure-signal lines and the user-code
stack frames; drops the framework frames (existing deny-list) and
the post-failure boilerplate block.

Routing: `is_quiet` checks argv for `-q` or `--quiet`. When set,
`run()` short-circuits the phase-based filter selection and routes
non-passthrough phases to `filter_quiet`. Passthrough phases
(`clean`, `site`, plugin goals) remain passthrough under `-q`.

Measured savings on the captured fail fixture: 51.1%
(174 -> 85 tokens). Green run: 0 -> 0 (no overhead).

Safety net: unclassified `[ERROR]` lines are kept rather than
dropped — better to leak a line than hide signal.

Tests added (7):
  - `quiet_detects_short_flag` / `quiet_detects_long_flag` /
    `quiet_does_not_match_unrelated_flags` for `is_quiet`.
  - `quiet_green_run_is_empty` — empty input contract.
  - `quiet_fail_strips_framework_and_boilerplate` — kept-list and
    drop-list assertions on the real fixture.
  - `savings_mvn_quiet_fail` — >=50% savings target on the fixture.
  - `quiet_unknown_error_line_kept_as_safety_net` — unclassified
    `[ERROR]` not silently dropped.

New fixture: `tests/fixtures/mvn_quiet_fail_raw.txt` (27 lines,
770 bytes) captured from a real `mvn -q test` run against a
JUnit 5 / Surefire 3.5.5 project with one failing test.

* fix(git): drop -uall from compact status so output never exceeds raw

The compact `git status` path ran `git status --porcelain -b -uall`. The
`-uall` flag expands fully-untracked directories into every file, while raw
`git status` collapses them (e.g. `node_modules/`). This made rtk output
larger than raw — measured ~29x on a 200-file untracked dir (5500B vs 191B) —
violating RTK's compress-or-match-raw invariant and inflating tokens.

Remove `-uall` so untracked directories collapse exactly like raw. This keeps
#991's actual fix intact: all modified/staged/renamed/conflict paths are still
shown with no grouped summaries or `... +N more` overflow markers (`-uall`
never affected those lines). Untracked files in partially-tracked dirs and any
paths git itself expands are still preserved by the formatter.

Measured (rtk vs raw git status):
- node_modules/ (200 files): 5500B (-2780%) -> 25B (+87% savings)
- normal (8 mod + 2 untracked): +71% -> +76%
- 17 modified (#991 case): unchanged at +58%, all 17 shown, 0 overflow markers

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* Update git.rs

* docs(git): sync status README with --porcelain -b (drop -uall)

* chore(master): release 0.41.0

* feat: address pr suggestions

* chore: sync with pi api

* chore: minor cleanup

* fix(grep): parse single-file output containing colons

Single-file `rtk grep` returned synthetic `[file] N` buckets when a
matching line contained `:`. ripgrep emits `<line>:<content>` for
single-file searches; the parser used `splitn(3, ':')` length to
disambiguate, which fails when content has `:` (e.g. `Foo::bar(...)`).

Force `--with-filename` on rg (`-H` on the grep fallback) so output is
always `file:line:content`, and replace the inline parser with a
regex-based `parse_match_line` that also handles paths with embedded
`:` (Windows drive letters).

Fixes the parser/rendering portion of #1436. The BRE-translation
portion is addressed in a follow-up PR.

Signed-off-by: Artiom Tofan <arto@queue-it.com>

* fix(grep): NUL-separate file from line:content per review

- rg: switch to `-nH --null` for robust file/content separation
- grep fallback: switch to `-rnHZ` to match rg's --null contract
- Parser regex: `^([^\x00]+)\x00(\d+):(.*)$` so filenames or content
  containing `:digits:` (e.g. `badly_named:52:file.txt`,
  `debug: counter is :42: now`) can no longer fool the parser
- Use `Captures::extract` for cleaner destructuring

Adds tests for the badly-named-filename and digit-colons-in-content
cases; updates existing tests to use NUL-separated fixtures.

Addresses review on #1554.

Signed-off-by: Artiom Tofan <arto@queue-it.com>

* chore(master): release 0.42.0

* fix(hook): collapse bash line continuations before matching (#1564)

Closes #1564.

`rtk hook check` rejected commands that began with a bash line
continuation (`\<NL>`) because the matcher saw the literal `\`
as the first token and bailed out:

    $ rtk hook check $'\\\ngit diff HEAD~1'
    No rewrite for: \\ngit diff HEAD~1

Bash collapses `\<NL>` (and `\<CRLF>`) plus surrounding horizontal
whitespace to a single space before dispatching the command, so
the matcher should do the same. Internal continuations (e.g.
`git diff \<NL>HEAD~1`) are equally common when Claude Code
formats long invocations.

Add a small `collapse_line_continuations` helper that runs a
single regex over the input before `cmd.trim()`. The regex eats
the trailing whitespace on the previous line and the leading
indentation on the next line so the no-double-space invariant
holds. The fast path returns `Cow::Borrowed` when no continuation
is present, so commands without `\<NL>` allocate nothing.

Six regression tests pin: leading \\<NL\>, leading \\<CRLF\>, internal
\\<NL\> between subcommand and args, \\<NL\> followed by indentation,
the no-op fast path, and the helper-level Cow::Borrowed contract.

* fix(pkg): rtk is Apache 2.0 and no MIT

* fix(docs): replace remaining MIT license references with Apache 2.0

* fix(ls): preserve permission info as octal when -l/-la is passed

Closes #1672

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* feat: prefer short args and cleanup comments

* fix: handle GNU `--format=long` and `--format=verbose` long args

* fix(hook): use maintainer regex suggestion for line continuation

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>

* fix(tests): use rewrite_command_no_prefixes in line-continuation tests

Signed-off-by: Sai Asish Y <say.apm35@gmail.com>

* fix: remove remaining noise and rework comments

* fix(mvn): preserve compile-error continuation in filter_surefire

When `mvn test` fails at the compile step before Surefire runs, the
`[ERROR]` block's indented `symbol:` / `location:` / `^` continuation
lines were being dropped. `filter_surefire` had no `keep_continuation`
state, unlike `filter_compile:302` and `filter_package:408`.

Add the missing state machine: reset in the RUNNING branch, set on each
kept `[ERROR]` line (except `Tests run:` / `Failures:` / `Errors:`
headers — same predicate as `filter_package:402-405`), and pass through
indented continuation lines before the outside-block keep-list check.

Fixture `mvn_test_compile_fail_slice_raw.txt` captures the
`mvn test` slice for a project with a deliberate `cannot find symbol`
in a `src/main/java` source. New tests:
- `surefire_keeps_compile_continuation_on_test_phase` — the bug.
- `package_still_keeps_compile_error_continuation_after_refactor` —
  drift guard on the `install`/`verify` path.

* refactor(mvn): extract shared SurefireBlock state machine

`filter_surefire` and `filter_package` had a ~30-line duplicated state
machine for Surefire/Failsafe per-class blocks (RUNNING / in-block
buffer / CLOSE branching / failure-trail). The duplication is what let
the P0 continuation bug land — the fix had to be applied in only one of
the two filters.

Introduce `SurefireBlock` owning that inner 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.

Each `step()` returns `SurefireStep::{Consumed, FailingClose, Passthrough}`.
The `FailingClose` arm defers the actual write to the caller via
`commit_failing()` — this is the seam Commit D will use to enforce the
`mvn_max_failures` cap without duplicating the cap logic.

Outside-block keep logic stays inside each filter unchanged
(`MODULE_BANNER || keep_outside_block`, `[WARNING]` dedup, the
`keep_continuation` flag from Commit A). Byte-identical filter output —
all existing `surefire_*` / `package_*` / token-savings tests pass.

The previous free helpers `flush_block_as_keep` and `emit_block` are
removed; their behaviour now lives in `SurefireBlock::flush_open_block_as_keep`
and `SurefireBlock::commit_failing`.

* fix(mvn): keep multi-module Reactor Summary rows

In a reactor build (parent pom with `<modules>`), Maven appends a
`Reactor Summary for <root>` block at the end listing each module's
status:

    [INFO] Reactor Summary for multi-module-skeleton 1.0.0-SNAPSHOT:
    [INFO]
    [INFO] multi-module-skeleton ...... SUCCESS [  0.353 s]
    [INFO] child-a .................... SUCCESS [  1.676 s]
    [INFO] child-b .................... FAILURE [  0.940 s]

`filter_surefire` / `filter_package` were dropping these rows because
`keep_outside_block` only keeps `[INFO]` lines that match a specific
prefix (`Building`, `Installing`, `Total time:`, etc.). The per-module
status rows match none of them.

Add a `REACTOR_SUMMARY` regex and `reactor_summary_keep` helper that
toggles a flag on the header and clears it on `BUILD SUCCESS` /
`BUILD FAILURE`. While the flag is set the helper returns `true` so
the rows survive — including `[WARNING]` (activated-profile notices)
and the in-summary horizontal rule. Caller invokes the helper before
`keep_outside_block` so the clears-flag side effect runs regardless
of `||` short-circuit.

Fixtures captured against a hand-rolled 2-module skeleton checked in
under `tests/fixtures/multi-module-skeleton/` (parent pom + child-a
+ child-b with `Empty.java` stubs). The fail fixture is captured by
temporarily replacing child-b's `Empty.java` with one that references
a `BogusType` symbol. Reproducible from the committed skeleton.

* feat(mvn): cap failing-class blocks and Failures summary entries

`filter_surefire` / `filter_package` emitted unbounded output on builds
with hundreds of failures — the per-class failing blocks (with their
post-close exception trail) plus every `[ERROR]   ClassA.test:N …`
entry in the `[ERROR] Failures:` summary were all kept. Other RTK
filters (git/cargo/lint) already cap with `... +N more` tails; bring
mvn in line.

Add `LimitsConfig::mvn_max_failures` (default 25, matching the
`grep_max_per_file: 25` convention in the same struct; `0` = opt out)
and wire it into two cap sites:

1. Failing-class blocks. The shared `SurefireBlock::step()` machine
   yields `FailingClose` once per failing class; the outer loop now
   commits the first N and calls a new `drop_failing()` for the rest.
   `drop_failing()` enables a `drop_trail` flag so the post-close
   per-test subline, exception, and user frames are consumed silently
   until the next blank line. After the loop, if any were dropped,
   `\n... +N more failing test classes\n` is appended.

2. `[ERROR] Failures:` summary block. New `FailuresSummaryCap` helper
   tracks the header, caps `[ERROR]   ` entries at the same limit, and
   pre-emits `\n... +N more failures\n` immediately before the
   aggregate `[ERROR] Tests run: …` line.

Tail wording follows the existing repo convention — no `[INFO]`
prefix, no parenthetical hint (`cargo_cmd.rs:1035` uses the same
`\n... +N more failures\n` verbatim).

Tests:
- `surefire_caps_failing_blocks_emits_tail` — synthetic 5-failure
  fixture with `cap = 3`; asserts blocks 1-3 emitted, 4-5 dropped,
  tail emitted.
- `surefire_cap_zero_disables_capping` — same fixture with `cap = 0`;
  all 5 blocks kept, no tail.
- `failures_summary_block_is_capped` — 5-entry summary with `cap = 3`;
  asserts entries 1-3 kept, 4-5 dropped, tail emitted *before* the
  aggregate.

Behaviour-change note: the default cap of 25 means users without a
custom config get a bounded output for the first time. Existing
fixtures all have <25 failures so no current test diffs.

* refactor(mvn): polish — strip_prefix, borrow buffers, CRLF tests

Three small reviewer-suggested cleanups:

- `[WARNING] ` prefix slicing in `filter_compile` / `filter_package`
  was written defensively as `&line["[WARNING] ".len().min(line.len())..]`.
  Both call sites are already gated by `line.starts_with("[WARNING]")`
  so the `min` is dead defence. Use `strip_prefix(...).unwrap_or(line)`
  which reads identically to the rest of the module.

- `SurefireBlock` and `SurefireStep::FailingClose` now borrow lines
  from the post-ANSI-strip `String` (`Vec<&'a str>` / `Option<&'a str>`
  / `&'a str` close), tied to the `stripped` local in
  `filter_surefire_with_cap` / `filter_package_with_cap`. No
  `.to_string()` allocations on the hot per-line path; the slice
  fixtures' savings tests are unaffected and the gzipped full-fixture
  tests still hit ≥90%/≥85%.

- New `surefire_handles_crlf_line_endings` and
  `package_handles_crlf_line_endings` tests assert byte-equality
  between an LF-filtered fixture and a CRLF-converted-then-LF-normalised
  one. Catches the class of bug where exact-equality line checks
  silently miss CRLF input because `str::lines()` strips `\n` but
  keeps trailing `\r`.

* chore(mvn): drop unrelated Cargo.lock churn from PR

`develop`'s Cargo.lock has its rtk-package entry pinned at `0.36.0`
while `Cargo.toml` says `0.34.3` — the lock is internally stale on
develop. When `cargo build` ran on this branch it re-pinned the lock
to `0.34.3` to match `Cargo.toml`, producing the diff the reviewer
flagged on PR #1956.

Revert the lock to `develop`'s baseline so the PR's committed diff
no longer touches Cargo.lock. `cargo build` will rewrite the
working-tree lock on every invocation; that's a local artifact, not
part of the committed PR.

* fix(cicd): MIT to Apache 2.0

* fix(hook): emit Copilot CLI-compatible hook config + strip BOM on Windows

* feat(hook): wire Copilot into uninstall, telemetry, and discover

* fix(hook): semgrep marker + regression tests for Copilot hook isolation

* feat(hook): --global Copilot install + transparent rewrite via modifiedArgs

* fix(hook): default Copilot CLI rewrite to 'ask' to preserve permission prompts

* fix(hook): preserve Copilot CLI toolArgs metadata when rewriting command

* fix(hook): omit permissionDecision so Copilot prompts for the rewritten command

* fix(gh,glab): don't pre-reject view/checks/run subcommands missing the id

gh and glab both accept several subcommands without an explicit id and
resolve it from the current branch (gh pr view, gh pr checks, glab mr
view) or open an interactive picker (gh run view). RTK was pre-rejecting
with "X number required" before the underlying tool ever ran.

Six call sites now treat the identifier as optional and forward the
request unchanged, letting gh/glab decide:

- gh pr view, gh pr checks, gh issue view, gh run view
- glab mr view, glab issue view

A new `parse_optional_identifier` helper in each file captures the
"(Option<id>, Vec<extra>)" parse so the no-id branch is unit-testable.
format_run_view also drops the dangling `#` from its header when the
run id is empty (interactive-picker passthrough).

* docs(readme): add Portuguese translation

* fix(aws): preserve values in JSON output for unsupported subcommands

The generic AWS handler (used for subcommands outside the hardcoded
specialized list) called json_cmd::filter_json_string, which extracts
a type-only schema and discards every value.

    $ rtk aws backup describe-global-settings --output json
    {
      GlobalSettings:
      {
        isCrossAccountBackupEnabled: string,
        isDelegatedAdministratorEnabled: string,
        isMpaEnabled: string
      }
      LastUpdateTime: string
    }

Callers got the schema instead of the data they asked for.

Swap to json_cmd::filter_json_compact, which preserves values while
still applying depth, string-length, and array-size truncation:

    $ rtk aws backup describe-global-settings --output json
    {
      GlobalSettings:
      {
        isCrossAccountBackupEnabled: "false",
        isDelegatedAdministratorEnabled: "false",
        isMpaEnabled: "false"
      }
      LastUpdateTime: "2026-05-28T09:52:17.525000+02:00"
    }

Affects every AWS subcommand not on the hardcoded list (backup,
route53, kms, ssm, apigateway, ...) when output is valid JSON
(explicit `--output json` or auto-injected for describe-/list-/get-/scan).

* missing bracket

* missing test tag

* fix(copilot-cli): use user-level instruction for -g

-g was missing user level instructions which are available for copilot

* fix: fmt

* Update main.rs

* allow unsafe code clippy + semgrep

* Update main.rs

* Update main.rs

* fix(provider): sanatize more chars when encoding claude code project pathes

Fixes #2150

* doc(init): fix documentation inconsistencies arount `rtk init`

Fixes #213

* fix(curl): passthrough binary downloads to prevent UTF-8 corruption

`rtk curl <binary-url>` silently corrupted any binary download
(tarballs, zip, png, pdf, ELF, ...) because the response body was
captured via `String::from_utf8_lossy` in `core::stream::exec_capture`.
Every non-UTF-8 byte got replaced with U+FFFD (3 bytes: 0xEF 0xBF 0xBD),
so gzip magic 1f 8b 08 00 arrived at downstream consumers as
1f ef bf bd 08 00 and `tar -xzf` complained "not in gzip format".

Now `rtk curl` runs `Command::output()` directly to keep stdout as
`Vec<u8>`, then checks `std::str::from_utf8(&bytes).is_err()`. If the
response is not valid UTF-8 (i.e. the lossy conversion would corrupt
it), raw bytes are written through to stdout via `write_all` and
tracking is recorded as passthrough (0% savings — token counts over
binary content have no meaning). The text/JSON code path is unchanged.

Verified live on 18 real binary formats (rtk's own release artifacts,
ripgrep, bat, fd, hyperfine, gh, tokei, kubectl ELF 51MB, rustup-init
20MB, W3C PDF, ISO 9899 PDF 4MB, GitHub avatar PNG, Wikimedia GIF,
WebP sample, MP3/MP4/WAV samples) — all byte-identical to raw curl.
10 text regression tests (JSON/HTML/Markdown/Cargo.toml/RFC) confirm
the text path keeps its existing behavior.

Closes #1087

* chore(args): introduce a more generic solution for restoring double dashes in args

Fixes #1669

* fix(init): preserve settings.json symlink during atomic write

atomic_write() replaced symlinked settings.json with a regular file.
Now resolves symlink targets before the tempfile+rename so the link
itself is preserved and the real file gets updated.

Covers both absolute and relative symlink targets.

Closes #653

* fix(init): use fs::canonicalize for symlink resolution

Simplify resolve_atomic_target() per review feedback — fs::canonicalize
handles chained symlinks, relative paths, and all edge cases.

Co-authored-by: Roopesh <roopesh1724989@gmail.com>

* fix(gh): show fallback note when PR/issue body is filtered to empty

When `filter_markdown_body()` strips a PR or issue body to empty
(body contained only badges, images, HTML comments, or horizontal
rules), `format_pr_view` and `format_issue_view` previously skipped
the body section silently. Users had no indication that body content
had been present and filtered.

Now both views emit a fallback note when the filtered body is empty
but the raw body was not:

  PR view:    (body contained only badges/images/comments)
  Issue view: Description: (body contained only badges/images/comments)

The 4 added tests cover:
- PR body with only badges/images/comments -> fallback note appears
- PR body with real content -> no fallback note (sanity)
- PR body empty (raw) -> no fallback note (no signal to give)
- Issue body badges-only -> fallback note appears

Closes #235

Co-authored-by: polaminggkub-debug <polaminggkub-debug@users.noreply.github.com>

* fix(go): respect build failure exit status

Gate go build success output on the child exit code and surface unrecognized non-zero build output instead of reporting success.

Closes #2185.

* fix(grep): command token duplication in output withsame value filename or positional arg

* fix(openclaw): no execSync to avoid async dangerous cmds

* chore(master): release 0.42.1

* fix(permissions): never auto-allow not evaluable + defer to the agent

- Decompose compound commands for permission checks (newline, background &, subshell `( )`) in addition to &&, ||, ;, | so hidden segments are checked.
- `contains_unattestable_construct`: flag command/process substitution and file-target redirects (fd-dup `2>&1` and /dev/null exempt) — RTK can't decompose these, so they are never auto-allowed.
- Route every host hook (Claude, VS Code, Gemini, Cursor, Copilot CLI) through a single decision flow. Precedence: Deny → (defer if unattestable) → Allow → Ask → Default. Auto-allow only on a positive Allow; otherwise defer to the host's own engine.
- Gemini: ask_user instead of hardcoded allow. Cursor: empty `{}` delegation, since permission:"ask" is not enforced on its sandboxed shell.

* fix(permissions): cursor and gemini use correct permissions settings file

-

* fix(permissions): add test for cursor and gemini settings perm

* refacto(permissions): fmt

* fix(permissions): project-first config lookup for Gemini/Cursor

* fix(permissions):  >&file redirect no allow + scope Gemini/Cursor config

* chore(master): release 0.42.2

* fix(permissions): never auto-allow not evaluable + defer to the agent

- Decompose compound commands for permission checks (newline, background &, subshell `( )`) in addition to &&, ||, ;, | so hidden segments are checked.
- `contains_unattestable_construct`: flag command/process substitution and file-target redirects (fd-dup `2>&1` and /dev/null exempt) — RTK can't decompose these, so they are never auto-allowed.
- Route every host hook (Claude, VS Code, Gemini, Cursor, Copilot CLI) through a single decision flow. Precedence: Deny → (defer if unattestable) → Allow → Ask → Default. Auto-allow only on a positive Allow; otherwise defer to the host's own engine.
- Gemini: ask_user instead of hardcoded allow. Cursor: empty `{}` delegation, since permission:"ask" is not enforced on its sandboxed shell.

* fix(permissions): cursor and gemini use correct permissions settings file

-

* fix(permissions): add test for cursor and gemini settings perm

* refacto(permissions): fmt

* fix(permissions): project-first config lookup for Gemini/Cursor

* fix(permissions):  >&file redirect no allow + scope Gemini/Cursor config

* fix(openclaw): no execSync to avoid async dangerous cmds

* test(hook): Copilot CLI acceptance tests

* fix: semgrep markers on test-fixture sensitive paths

* refacto(cmds): strip decorator noise from filter output

Separator lines (═══, ---) and an emoji status marker cost tokens
without adding signal for the LLM — RTK output must never add noise
over raw. Semantic labels are kept; the emoji is swapped for plain
monochrome unicode.

* chore(master): release 0.42.3

* fix(vitest): preserve explicit reporters

* fix(mvn): re-arm failure trail on per-test sublines

Surefire 3.x emits one blank-separated detail block per failing test
under a single class close line. The trail previously ended at the
first blank line and never re-armed, so failures after the first lost
their exception message (unindented -> dropped) and leaked the full
junit/jdk framework stack (indented -> kept by keep_continuation).

SurefireBlock now remembers the trail's keep/drop decision when it
ends at a blank line (trail_rearm) and re-enters the trail on the next
per-test subline with the same decision — a capped class drops all its
per-test blocks, not just the first. Any other non-blank line disarms
re-entry; RUNNING/commit/drop clear it so an unrelated class can never
inherit the decision. Extra blanks between per-test blocks stay armed.

Per-test sublines use '<<< ERROR!' for thrown (non-assertion)
exceptions — accept both markers via a shared is_per_test_subline()
(also reused by filter_quiet, which already re-armed correctly), and
widen the CLOSE regex to tolerate an ERROR! marker defensively
(Surefire 3.5.5 emits FAILURE! even for errors-only classes, per the
fixture capture; detection keys off the counts, not the marker).

Fixture captured from real Maven 3.9.9 / Surefire 3.5.5 output against
the new tests/fixtures/multifail-skeleton/ (CalcTest: assertion failure
+ thrown exception in one class; BoomTest: errors-only class). A
byte-exact pin locks the single-failure path unchanged.

* fix(ci): pin fixture line endings, harden CRLF tests

test (windows-latest) failed on the two CRLF tests: with no
.gitattributes, the Windows runner's core.autocrlf=true checks fixtures
out with CRLF, include_str! embeds \r\n, and the tests'
replace('\n', "\r\n") synthesis produces \r\r\n. str::lines() strips
only the final \r\n of each pair, leaving a stray \r that $-anchored
regexes (MODULE_BANNER, BUILD_FOOT) reject — dropping the module banner
and BUILD SUCCESS in the CRLF leg only. Real Maven CRLF output (single
\r\n) was always handled; only the synthesized double-CR leg broke.

Root-cause fix: tests/fixtures/** -text — fixtures are byte-exact
filter inputs; eol conversion must never touch them. Defense-in-depth:
the two CRLF tests normalize the embedded fixture back to LF before
synthesizing, so they stay correct even on checkouts without
attributes (zip downloads, pre-existing autocrlf clones).

Also fixes the test doc comment, which claimed str::lines() keeps the
trailing \r of a \r\n pair — it strips it; the hazard is the doubled
\r\r\n, not single CRLF.

* fix(mvn): strip post-failure help boilerplate in non-quiet mode

filter_quiet already dropped the help block Maven emits after
'Failed to execute goal' (See ..., -> [Help 1], Re-run Maven, help
URLs, bare [ERROR] dividers); the non-quiet paths kept it via the
[ERROR] catch-alls — keep_outside_block for test/package and
filter_compile's own [ERROR] branch.

Promote QUIET_BOILER_PREFIXES to a shared BOILER_PREFIXES (+
is_boilerplate(), which also covers the bare [ERROR] divider lines,
matching filter_quiet's existing drop) and reject boilerplate before
each [ERROR] catch-all. The list deliberately keeps the resume hint
([ERROR] After correcting the problems... / [ERROR]   mvn <args>
-rf :module) — actionable signal for resuming a multi-module build —
and the Failed to execute goal terminator.

keep_continuation hygiene (defensive parity, not a demonstrated bug):
filter_surefire only reset the flag inside its kept branch, so a
dropped [ERROR] line would leave it stale and a following indented
line could be wrongly kept. No committed fixture exhibits the leak;
add the same end-of-loop fall-through reset filter_package already
has. filter_compile resets on its new boilerplate drop path.

Multifail-slice savings rise from 19.9% to 42.3%; threshold pinned
at >=30% to match the reactor-fail precedent. README: document the
stripping and drop the stale duration-normalisation bullet (that
behaviour was removed in 77e28d0).

* refactor(mvn): bind failure cap to truncate::CAP_WARNINGS, drop config knob

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.

* style(mvn): align overflow tail with canonical '… +N more {label}' shape

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.

* fix(init): respect CLAUDE_CONFIG_DIR for global paths

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

* refactor(init): address review feedback on CLAUDE_CONFIG_DIR PR

- 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

* review: cleanup code, comments and uneeded undocumented RTK_CLAUDE_DIR now that we have CLAUDE_CONFIG_DIR

* fix: minor print_manual_instructions regression

* fix(grep): stabilize argument parsing

* fix(grep): fix value-taking flags, -rN clusters, multi-path, and -e patterns

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>

* fix(grep): handle value-taking flags in tail position of short clusters

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>

* fix(grep): code and test cleanup

* fix(diff): report modified-only diffs and follow diff exit convention

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

* fix(security): harden installer checksum, filter-trust, meta-command fallthrough

* fix(security): harden meta command list check

fail in cicd before letting meta command omission reach production

* chore(master): release 0.42.4

* fix(grep): left-to-right cluster scan, long value flags, format passthrough

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>

* fix(grep): restore strip_r as explicit testable helper + pre-existing clippy fix

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>

* refactor(grep): extract parse_cluster to make cluster parsing directly testable

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>

* feat(grep): sort content alphabetically

* fix(grep): use portable --null in system grep fallback (BSD/macOS)

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>

* feat(oc): add Openshift CLI support with shared k8s filtering

* fix(hook): treat uv run as a transparent prefix

* fix(grep): surface error on exit code >= 2 instead of false "0 matches"

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>

* test(grep): gate grep_error_test with #![cfg(unix)] for Windows CI

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>

* style(test): cargo fmt grep_error_test.rs

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>

* refactor(grep): extract error-exit decision into pure unit-tested fn

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>

* fix(git): propagate exit code when commit fails instead of reporting ok

run_commit had a branch that printed 'ok (nothing to commit)' and fell
through to Ok(0) whenever git exited non-zero with a 'nothing to commit'
message — so a no-op or hook-aborted commit was reported as success with
exit 0. Gate strictly on output.status.success() via a CommitOutcome
helper: any failure surfaces git's message and propagates the real exit
code, matching native git. Same bug class as #1581 (push) and #1535
(stash).

Fixes #2494

* fix(git): propagate exit code on git status failure in compact path

The compact status path only caught the "not a git repository" error and
let every other failure (corrupt index, lock contention, broken refs)
fall through to a formatted "Clean working tree" + Ok(0), masking a real
git error. The non-compact path already guards on result.success(); apply
the same guard to the compact path, keeping the friendly not-a-repo
message and propagating git's real exit code otherwise.

Part of #2497

* fix(git): propagate exit code on git worktree list failure

run_worktree list mode never checked result.success() — a failed
`git worktree list` (e.g. run outside a repo) was flattened to empty
output + exit 0. The has_action branch already guards on success; apply
the same guard to list mode and surface git's error with its exit code.

Part of #2497

* fix(dotnet): stop duplicating failures on failing test runs

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>

* test(main): classify oc subcommand in PASSTHROUGH allowlist

* fix(vitest): add passthrough recovery hint

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.

* fix(dotnet): keep raw fallback when parsed failures incomplete

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>

* fix(pulumi): keep stack identity in pulumi-stack filter

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>

* fix(grep): correctly handle all flag shapes and never exceed raw output

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.

* fix(pulumi): keep Owner and version in pulumi-stack filter

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>

* test(grep): cover bundled -rln files-with-matches cluster (#2543)

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'.

* fix(core): never-worse output guard so RTK never exceeds the raw command

RTK could emit more tokens than the underlying command on small inputs:
filters that add headers, summaries, re-indentation, or a tee hint, plus
synthetic no-result messages ("0 matches", "No stashes", "[docker] 0
containers") printed where the raw command emitted nothing. Both break the
Transparency principle and inflate tokens instead of saving them.

- core::guard::never_worse(raw, filtered) returns raw when the filtered form
  has more tokens (reuses tracking::estimate_tokens), so RTK output is never
  larger than the real command.
- runner::emit_guarded(filtered, hint, raw) composes body + tee hint, guards
  the whole, prints, and returns what was shown so printed == tracked.
- run_captured_filter guards the run_filtered* family centrally; per-site
  guards cover the remaining single-string filters.
- On empty raw, emit empty and preserve the exit code instead of a synthetic
  no-result message (the messages were cosmetic with no dependents; #2461
  reports the grep one as actively harmful).
- git stash show now propagates its exit code instead of masking a real
  failure as Ok(0).

Resolves #2551.

* fix(env): clean up feature from secrets rewrite

Masking env secrets is not what core RTK (OSS) is for -- it compresses command
output, it does not redact what the command itself already reveals. It also
broke raw-vs-filtered consistency, which the never-worse guard relies on.

* fix(pipe): apply the never-worse guard

pipe filters piped stdin but bypassed the guard; route its output through
never_worse like every other compressor.

* fix(read): make guard baseline faithful to cat/cat -n output

Compare filtered output against the raw the agent's command would
emit (numbered for -n), not against unnumbered content, so the
guard keeps requested line numbers instead of dropping to plain.

* fix(docker): make the agent's command authoritative for the guard baseline

docker ps/ps -a/images ran a plain command for the raw baseline and a
separate --format command for parsing, capturing the plain run with
unwrap_or_default(). A failed plain run while --format succeeded left
raw empty, and never_worse("", summary) dropped real output.

Run the agent's actual command first as the source of truth: on failure
show its output/exit faithfully; on succes…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants