Skip to content

fix: add is_unsupported_shape() guard before rewrite_compound() - #2903

Closed
Ev3lynx727 wants to merge 1 commit into
rtk-ai:developfrom
Ev3lynx727:fix/command-shape-validation-2902
Closed

Ev3lynx727 wants to merge 1 commit into
rtk-ai:developfrom
Ev3lynx727:fix/command-shape-validation-2902

Conversation

@Ev3lynx727

@Ev3lynx727 Ev3lynx727 commented Jul 9, 2026 •

Copy link
Copy Markdown

Summary

Fixes #2902 — which is the root cause of #2792, #2821, #2892.

rewrite_command() attempts to rewrite unsupported command shapes without early validation, producing malformed rtk invocations that fail at runtime.

Changes

Adds is_unsupported_shape() function that returns early before expensive tokenization/regex matching:

  • find / fd — excluded entirely because rtk find/rtk fd only support -name, -iname, -type, -maxdepth. Any other predicate (-not, -exec, -newermt, -path, etc.) silently produces wrong/empty results.
  • rtk find / rtk fd — already-routed to a subcommand rtk can't rewrite further
  • Unattestable constructs — backticks, $((, <( — output shape is unpredictable

User-facing impact

Before this fix, commands like find . -name "*.log" or fd -e rs were rewritten to rtk find ... / rtk fd ..., which either exit with code 3 (ask) or silently produce wrong results when complex predicates are involved.

After this fix, unsupported command shapes pass through to the native command unchanged — no rewrite, no malformed invocation, no silent corruption.

Changelog

- fix: add is_unsupported_shape() guard to skip rewrite for find/fd, already-routed rtk subcommands, and unattestable shell constructs (#2903)
- fix: prevent malformed rtk find/fd invocations from unsupported predicates (#2902, #2792, #2821, #2892)

Verification

  • cargo fmt --all — clean
  • cargo clippy --all-targets — no new warnings
  • cargo test --all — 2399 passed, 0 failed, 8 ignored (baseline)

Related

Three reported issues (rtk-ai#2792, rtk-ai#2821, rtk-ai#2892) share the same root cause:
rewrite_command() attempts to rewrite unsupported command shapes without
early validation, producing malformed rtk invocations that fail at runtime.

Add is_unsupported_shape() that rejects find/fd, rtk find/rtk fd, and
unattestable constructs before expensive tokenization. find/fd are excluded
entirely because rtk find/fd only support a limited flag set (-name, -iname,
-type, -maxdepth); any other predicate silently produces wrong/empty results.

Tests: 6 new + 2 updated. 2399 pass, 0 fail, 8 ignored.
Ev3lynx727 added a commit to Ev3lynx727/server-commands-rtk that referenced this pull request Jul 9, 2026
Stores the patch from PR #2903 (rtk-ai/rtk#2903) in patches/ for local
reconstruction if upstream does not merge the change. Apply with:

  cd <rtk-repo> && git am /path/to/patches/rtk+0001-*.patch
  # or:  patch -p1 < /path/to/patches/rtk+0001-*.patch

Relates to: Ev3lynx727/server-commands-rtk executor.ts fix that checks
rtk rewrite before auto-prepending rtk prefix.
@Ev3lynx727

Copy link
Copy Markdown
Author

Thanks for the review @tapheret2 — both nits addressed:

  1. User-facing impact — added to PR description (before/after: unsupported shapes now passthrough instead of producing malformed invocations)
  2. Changelog — two entries added to PR description, ready for the next release tag

Appreciate the independent pass!

@TheCookieLab

Copy link
Copy Markdown

I reproduced the compound-find failure on v0.44.0 and current develop. A narrower prototype avoids disabling useful simple find rewrites: expose the runtime parser's unsupported-token predicate from find_cmd, have the discovery registry return Ignored only when those tokens are present, and cover both sides (compound/action forms produce no rewrite; find . -name '*.rs' remains rewriteable). The shared predicate also prevents the hook and runtime support lists from drifting. Tested cases include -o, -not, -exec, -delete, -print, and -print0. I can turn that focused prototype into a PR if maintainers prefer selective fallback over skipping every native find/fd command.

@KuSh

KuSh commented Sep 26, 2026

Copy link
Copy Markdown
Collaborator

Thank you for tracking this down. The find part was fixed differently in #3603 (merged 2026-08-26): rtk find now follows find's own grammar and hands predicates it does not model to the real find, so find commands can keep being rewritten safely instead of being skipped. The other shapes in this PR are already left alone on develop: fd is not rewritten, an existing rtk find command comes back unchanged, and commands with backticks, $(( or <( are never rewritten. If a case here still misbehaves on the latest version, please comment or reopen with the command.

@KuSh KuSh closed this Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment