feat(cli): unify check command input to positional args matching exec#96
feat(cli): unify check command input to positional args matching exec#96
check command input to positional args matching exec#96Conversation
Replace `--command` named option with trailing_var_arg positional arguments, matching the `exec` subcommand interface. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #96 +/- ##
=======================================
Coverage 89.92% 89.93%
=======================================
Files 29 29
Lines 6164 6167 +3
=======================================
+ Hits 5543 5546 +3
Misses 621 621
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Summary of ChangesHello @fohte, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refactors the Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
…e argument boundaries Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…serve compound command detection Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ E2E error-handling tests still use removed --command flag, silently testing wrong commands (tests/e2e/error_handling.rs:69)
The PR removed the --command named flag from CheckArgs and replaced it with trailing_var_arg positional args, but tests/e2e/error_handling.rs was not updated. Because CheckArgs now has #[arg(trailing_var_arg = true, allow_hyphen_values = true)], clap silently captures --command as a positional argument instead of erroring.
Detailed impact on each affected test
For example, ["check", "--command", "rm -rf /"] at line 69 is now parsed as command: vec!["--command", "rm -rf /"], which gets joined via shell_quote_join into "--command 'rm -rf /'". This string does not match the deny rule rm -rf /, so the decision becomes "ask" (default) instead of "deny". The test still passes (exit code 0) but no longer validates the deny path.
All four occurrences are affected:
- Line 9:
&["check", "--command", "git status"]— config is invalid, so exits 2 regardless. Test passes by coincidence. - Line 22:
&["check", "--command", "rm -rf /"]— config validation error occurs before evaluation. Test passes by coincidence. - Line 69:
&["check", "--command", "rm -rf /"]— most impactful: theexit_codes::case_2_check_denytest intends to verify a denied command returns exit 0, but now tests a default/ask case. If the deny→exit-0 path breaks, this test won't catch it. - Line 89:
&["check", "--command", "git status"]— expects"ask"decision; still gets"ask"since no rules match the mangled command. Passes by coincidence.
View 4 additional findings in Devin Review.
…l-command # Conflicts: # src/cli/mod.rs # src/cli/route.rs # src/main.rs # tests/e2e/check_generic.rs
There was a problem hiding this comment.
Devin Review found 1 new potential issue.
⚠️ 1 issue in files not directly in the diff
⚠️ E2E error-handling tests still use removed --command flag, silently testing wrong commands (tests/e2e/error_handling.rs:69)
The PR removed the --command named flag from CheckArgs and replaced it with trailing_var_arg positional args, but tests/e2e/error_handling.rs was not updated. Because CheckArgs now has #[arg(trailing_var_arg = true, allow_hyphen_values = true)], clap silently captures --command as a positional argument instead of erroring.
Detailed impact on each affected test
For example, ["check", "--command", "rm -rf /"] at line 69 is now parsed as command: vec!["--command", "rm -rf /"], which gets joined via shell_quote_join into "--command 'rm -rf /'". This string does not match the deny rule rm -rf /, so the decision becomes "ask" (default) instead of "deny". The test still passes (exit code 0) but no longer validates the deny path.
All four occurrences are affected:
- Line 9:
&["check", "--command", "git status"]— config is invalid, so exits 2 regardless. Test passes by coincidence. - Line 22:
&["check", "--command", "rm -rf /"]— config validation error occurs before evaluation. Test passes by coincidence. - Line 69:
&["check", "--command", "rm -rf /"]— most impactful: theexit_codes::case_2_check_denytest intends to verify a denied command returns exit 0, but now tests a default/ask case. If the deny→exit-0 path breaks, this test won't catch it. - Line 89:
&["check", "--command", "git status"]— expects"ask"decision; still gets"ask"since no rules match the mangled command. Passes by coincidence.
View 5 additional findings in Devin Review.
…tead of removed `--command` flag Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Why
checksubcommand used a different command specification method fromexec, causing inconsistent UXexecaccepts positional args (runok exec -- echo hello), butcheckrequired a named option (runok check --command "echo hello")What
checkcommand input to the same trailing_var_arg format asexecrunok check -- echo hellois now supported