Skip to content

feat: include user's OS and shell in LLM prompt#62

Open
JonathanRohland-TNG wants to merge 1 commit intomainfrom
feature/include-os-shell-in-prompt
Open

feat: include user's OS and shell in LLM prompt#62
JonathanRohland-TNG wants to merge 1 commit intomainfrom
feature/include-os-shell-in-prompt

Conversation

@JonathanRohland-TNG
Copy link
Copy Markdown
Collaborator

This PR is an alternative take on the idea from #44 based on my review comment there: #44 (comment)

…ommand generation

Signed-off-by: Jonathan Rohland <jonathan.rohland@tngtech.com>
@JonathanRohland-TNG
Copy link
Copy Markdown
Collaborator Author

bugbot run

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Bug: Shell Detection Fails in Unset Environments

When the $SHELL environment variable is unset or empty, basename "$SHELL" returns ".". This incorrect value is then passed to the LLM as the user's shell (e.g., "shell is '.'"), leading to the LLM generating inappropriate or incorrect command suggestions. This can occur in environments where $SHELL is not reliably set, such as containers or minimal shells.

please.sh#L33-L34

please-cli/please.sh

Lines 33 to 34 in 6a0045a

user_os="$(uname -s)"
user_shell="$(basename "$SHELL")"

Fix in Cursor


Was this report helpful? Give feedback by reacting with 👍 or 👎

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