Skip to content

fix(discover): rewrite dotnet test/restore/format, not just build#2142

Open
maxmilian wants to merge 1 commit into
rtk-ai:developfrom
maxmilian:fix/discover-dotnet-rewrite-subcommands
Open

fix(discover): rewrite dotnet test/restore/format, not just build#2142
maxmilian wants to merge 1 commit into
rtk-ai:developfrom
maxmilian:fix/discover-dotnet-rewrite-subcommands

Conversation

@maxmilian
Copy link
Copy Markdown

Summary

Root cause

The dotnet rewrite rule in src/discover/rules.rs used pattern: r"^dotnet\s+build\b", covering only build. DotnetCommands::Test/Restore/Format all dispatch to real handlers (src/main.rs:1709-1711), so the proxies work when invoked directly — only the rewrite registry was missing them.

Fix

Widen the rule's pattern to ^dotnet\s+(build|test|restore|format)\b. The output filters already handle all four; this is purely a routing fix (one regex).

Tests

  • test_rewrite_dotnet_subcommands — all four rewrite to rtk dotnet <sub>; dotnet publish / dotnet run (no proxy) are left untouched.
  • Full suite: 1947 passed, clippy clean, cargo fmt --check clean.

Manual verification

dotnet build    -> rtk dotnet build
dotnet test     -> rtk dotnet test
dotnet restore  -> rtk dotnet restore
dotnet format   -> rtk dotnet format
dotnet publish  -> (not rewritten)

Scope

Only the four subcommands with existing proxies. dotnet build and the three new subcommands now share one rule and behave identically (same permission verdict / exit code). The exit-code (ask vs auto-allow) is governed by a separate permission layer and is unchanged by this routing fix.

`rtk dotnet test`, `rtk dotnet restore`, and `rtk dotnet format` proxies
already exist and route to dedicated handlers, but the rewrite rule only
matched `dotnet build`, so the Claude Code hook silently passed the other
three through unfiltered. Extend the rule's subcommand set; the output
filters already handle all four — this is purely a routing fix.

Fixes rtk-ai#1830
@maxmilian maxmilian force-pushed the fix/discover-dotnet-rewrite-subcommands branch from b67f390 to c702065 Compare May 28, 2026 15:47
@maxmilian
Copy link
Copy Markdown
Author

Ran a fresh-context independent review (skeptical, diff + issue only). Verdict: no blockers. Two points addressed / noted:

  • Hardened the test (pushed): added a trailing-args case (dotnet test --filter Category=Unit → preserved) and a word-boundary negative (dotnet builder → not rewritten), confirming \b stops at the subcommand.
  • Kept the capturing group (build|test|restore|format) to stay consistent with the other multi-subcommand rules in rules.rs (git/gh/cargo/ruff/go), which capture for subcmd_savings lookup — and it leaves room for the follow-up below.

Follow-up (intentionally out of scope here): the rule's flat savings_pct: 70.0 now applies to all four subcommands. dotnet test likely compresses closer to ~90% (cf. cargo test/go test), so rtk discover will slightly undercount its savings. I left this out of a pure routing fix because I don't have reliable compression figures for restore/format and didn't want to guess. Happy to file a separate PR adding subcmd_savings overrides if maintainers want the report accuracy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[rewrite] dotnet test|restore|format should auto-rewrite to rtk dotnet

1 participant