fix(backend): avoid misclassifying shell aliases as env conflicts; detect bun-installed CLIs#2671
fix(backend): avoid misclassifying shell aliases as env conflicts; detect bun-installed CLIs#2671geminiwen wants to merge 2 commits into
Conversation
… detect bun-installed CLIs - env_checker: only treat lines as env assignments when the LHS is a valid POSIX identifier ([A-Za-z_][A-Za-z0-9_]*). This prevents shell aliases like `alias gemini="..."` from being parsed into a var named `alias gemini` and then matched against the GEMINI keyword. - misc/scan_cli_version: add `~/.bun/bin` to the home-relative search paths so CLIs installed via bun (e.g. gemini, codex) are detected when the shell -lic invocation can't resolve them.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afce71f2c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !is_valid_env_name(var_name) { | ||
| continue; |
There was a problem hiding this comment.
When a shell config uses declare -x OPENAI_API_KEY=... (bash) or typeset -gx GEMINI_API_KEY=... (zsh), this new identifier check rejects the line because the parsed name includes the command prefix. Those forms create exported environment variables (bash help declare documents -x as exporting names), and the previous scanner at least reported them, which matters when the GUI app is not launched from that shell and relies on file scanning rather than the process environment. Consider recognizing export builtins like declare -x/typeset -gx and extracting the actual variable name before this guard.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — pushed b6a6aead to address this.
Refactored the parser into extract_exporting_assignment which now:
- recognizes
export NAME=VALUE - recognizes
declare/typesetonly when the flags include-x(sodeclare -x FOO=bar,typeset -gx FOO=bar, and combined forms likedeclare -rx FOO=barall parse), strips the prefix + flags, and feeds the innerNAME=VALUEthrough the same identifier guard - skips
declare FOO=bar/typeset -g FOO=barbecause those don't export
Also added unit tests covering each case (including declared=foo not being mis-stripped as declare).
The previous identifier guard rejected real exporting builtins like `declare -x OPENAI_API_KEY=...` (bash) and `typeset -gx GEMINI_API_KEY=...` (zsh) because the parsed name included the command prefix. Extract the inner NAME=VALUE before validating the identifier, and only treat declare/typeset as exporting when their flags include `-x` (otherwise they create function-local, non-exported variables). Per review feedback on PR farion1231#2671.
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
Summary
Two related fixes in the backend env/tool detection layer.
1. Shell alias misclassified as env-var conflict
env_checker::check_shell_configsparsed any line containing=as an env assignment, then matched the LHS against per-app keywords. A line like:was parsed into
var_name = "alias gemini", uppercased toALIAS GEMINI, which contains theGEMINIkeyword — so the alias surfaced as a conflicting env var on app startup.Fix: require the LHS to be a valid POSIX env-var identifier (
[A-Za-z_][A-Za-z0-9_]*) before treating the line as an assignment. Aliases, function definitions, and lines with embedded spaces are now skipped. Added unit tests foris_valid_env_name.2.
~/.bun/binmissing from CLI scan pathsscan_cli_version(the path-scan fallback after$SHELL -lic 'tool --version'fails) checked~/.local/bin,~/.npm-global/bin,~/n/bin,~/.volta/bin, and the various nvm/fnm/volta layouts — but not~/.bun/bin. CLIs installed viabun install -g(gemini, codex, etc.) were therefore reported as not installed when the shell invocation couldn't resolve them (e.g. when the alias is in.bash_profilebut the user's shell is zsh).Fix: add
~/.bun/binto the home-relative search paths.Test plan
cargo test --lib env_checkerpasses (existing + newtest_is_valid_env_name).cargo build --libclean.alias gemini="..."in.bash_profile, the env-conflict banner no longer flags it.bun install -g @google/gemini-cli, the version is detected.🤖 Generated with Claude Code