Fix detection of shell built-in commands (e.g. pwd) in interactive CLI#1252
Conversation
Greptile SummaryThis PR adds a Confidence Score: 4/5The fix is functionally correct for the core built-ins case, but the inclusion of non-built-in tools warrants addressing before merge. A P1 regression (non-built-in tools always matching even when not installed) was identified in a previous review thread and has not been addressed in the current diff; this caps the score at 4. app/cli/interactive_shell/agent_actions.py — specifically the contents and placement of COMMON_SHELL_COMMANDS. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User input text] --> B[_looks_like_direct_shell_command]
B --> C{first token is None?}
C -->|Yes| D[return False]
C -->|No| E{in NON_COMMAND_STARTS?}
E -->|Yes| D
E -->|No| F{in COMMON_SHELL_COMMANDS? NEW}
F -->|Yes| G[return True - dispatch as shell action]
F -->|No| H{starts with ./ ../ /}
H -->|Yes| I{Path.exists?}
I -->|Yes| G
I -->|No| D
H -->|No| J{shutil.which finds it?}
J -->|Found| G
J -->|Not Found| D
Reviews (2): Last reviewed commit: "Fix detection of shell built-in commands..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| COMMON_SHELL_COMMANDS = { | ||
| "pwd", "ls", "cd", "echo", "cat", "touch", | ||
| "mkdir", "rm", "cp", "mv", "grep", "find", | ||
| "kubectl", "docker", "git" | ||
| } |
There was a problem hiding this comment.
COMMON_SHELL_COMMANDS cause false positives
kubectl, docker, and git are external executables, not shell built-ins. They are already discoverable via shutil.which on systems where they are installed, so listing them here is redundant there. On systems where they are not installed, this change causes a regression: any input whose first token is "docker", "kubectl", or "git" will now always be classified as a shell command, bypassing the LLM fallback. For example, "git is a version control tool" typed on a machine without Git installed would be incorrectly dispatched as a shell action instead of being sent to the LLM.
The fix should be limited to genuine shell built-ins that shutil.which cannot discover (e.g. pwd, cd, echo in some shells).
| COMMON_SHELL_COMMANDS = { | |
| "pwd", "ls", "cd", "echo", "cat", "touch", | |
| "mkdir", "rm", "cp", "mv", "grep", "find", | |
| "kubectl", "docker", "git" | |
| } | |
| COMMON_SHELL_BUILTINS: frozenset[str] = frozenset( | |
| { | |
| "pwd", "cd", "echo", "export", "unset", | |
| "alias", "type", "source", "exit", "history", | |
| } | |
| ) |
|
pls open a github issue and describe the scenario/context for this |
|
🧑💻 @aksKrIITK has entered the contributor hall of fame. Merged. Done. Shipped. Go touch grass (then come back with another PR). 🌱
👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome. |
3 participants
Fixes #
Describe the changes you have made in this PR -
This PR fixes detection of direct shell commands (e.g.,
pwd) in the interactive CLI.Previously,
_looks_like_direct_shell_commandrelied onshutil.which, which only detects executables available in the system PATH. However, many common shell commands likepwdandcdare shell built-ins and are not discoverable viashutil.which, especially on Windows.As a result, direct commands such as
"pwd"were not recognized as valid shell actions, causingplan_terminal_tasks("pwd")to return an empty list instead of["shell"].Demo/Screenshot for feature changes and bug fixes -
Before fix:
After fix:
All related CLI tests now pass:
Code Understanding and AI Usage
Did you use AI assistance (ChatGPT, Claude, Copilot, etc.) to write any part of this code?
If you used AI assistance:
Explain your implementation approach:
Problem: Direct shell commands like
"pwd"were not detected becauseshutil.whichdoes not recognize shell built-ins.Approach:
I extended
_looks_like_direct_shell_commandby adding a fallback check for common shell built-in commands (e.g.,pwd,cd,ls).Alternative considered:
Modifying
_plan_clause_actionsor_extract_shell_command, but that would break abstraction and duplicate logic.The correct layer to fix was
_looks_like_direct_shell_command.Why this approach:
Key change:
Added a safe fallback for known shell commands before
shutil.whichcheck.Checklist before requesting a review