Skip to content

fix(command_parser): handle negated_command as a transparent container#266

Merged
fohte merged 2 commits intomainfrom
fohte/fix-negated-command
Mar 31, 2026
Merged

fix(command_parser): handle negated_command as a transparent container#266
fohte merged 2 commits intomainfrom
fohte/fix-negated-command

Conversation

@fohte
Copy link
Copy Markdown
Owner

@fohte fohte commented Mar 28, 2026

Why

  • Commands containing the shell negation operator ! were not evaluated correctly, causing commands that should be allowed to return ask instead
    • e.g., if ! grep -q test /dev/null; then echo no; fi did not match the grep rule

What

  • negated_command is now handled as a transparent container, allowing inner commands to be correctly extracted and evaluated
    • tree-sitter-bash parses ! cmd as a negated_command node, but collect_commands() did not handle this node type

Before:

$ runok check --verbose -- 'if ! grep -q test /dev/null; then echo no; fi'
[verbose] Evaluating command: "if ! grep -q test /dev/null; then echo no; fi"
[verbose] Compound command detected (2 sub-commands)
[verbose]   sub-command 1: "! grep -q test /dev/null"
[verbose]   sub-command 2: "echo no"
[verbose] No rules matched
[verbose]   sub-command "! grep -q test /dev/null": Ask(None)
[verbose] Rule matched: allow 'echo *' (matched tokens: ["no"])
[verbose]   sub-command "echo no": Allow
[verbose] Compound evaluation result: Ask(None)
ask

After:

[verbose] Evaluating command: "if ! grep -q test /dev/null; then echo no; fi"
[verbose] Compound command detected (2 sub-commands)
[verbose]   sub-command 1: "grep -q test /dev/null"
[verbose]   sub-command 2: "echo no"
[verbose] Rule matched: allow 'grep *' (matched tokens: ["-q", "test", "/dev/null"])
[verbose]   sub-command "grep -q test /dev/null": Allow
[verbose] Rule matched: allow 'echo *' (matched tokens: ["no"])
[verbose]   sub-command "echo no": Allow
[verbose] Compound evaluation result: Allow
allow

Open with Devin

…correctly

intent(command_parser): shell negation operator `!` (negated_command) was not
  recognized as a transparent container, causing `! grep -q test /dev/null` to
  be treated as an opaque string that failed to match rules
learned(command_parser): tree-sitter-bash parses `! cmd` as a `negated_command`
  node wrapping the inner command — adding it to the transparent container list
  (alongside if_statement, while_statement, etc.) is sufficient to fix the issue
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes an issue where negated shell commands (e.g., ! grep) were not being parsed correctly, causing them to fail rule matching. The negated_command node is now handled as a transparent container in the command parser, allowing the inner command to be evaluated. The changes include documentation updates and new test cases in both unit and integration tests. A review comment suggests adding a test case for negated pipelines within subshells to ensure robust handling of nested command structures.

Comment thread src/rules/command_parser.rs
devin-ai-integration[bot]

This comment was marked as resolved.

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 28, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.50%. Comparing base (8a51b4c) to head (b63ffc3).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #266   +/-   ##
=======================================
  Coverage   89.50%   89.50%           
=======================================
  Files          52       52           
  Lines       10730    10731    +1     
=======================================
+ Hits         9604     9605    +1     
  Misses       1126     1126           
Flag Coverage Δ
Linux 89.30% <100.00%> (+0.01%) ⬆️
macOS 90.85% <100.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

intent(command_parser): add negated pipeline in subshell test case
  per Gemini review suggestion to cover nested container interaction
intent(docs): add PR link to release notes entry per project convention
@fohte fohte merged commit 3aebc80 into main Mar 31, 2026
10 checks passed
@fohte fohte deleted the fohte/fix-negated-command branch March 31, 2026 11:54
@fohte-bot fohte-bot Bot mentioned this pull request Mar 31, 2026
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.

1 participant