GH#2993: Fix ShellCheck memory explosion regression — PATH shim for bash-language-server#2995
Conversation
…ion (#2993) bash-language-server resolves shellcheck via PATH lookup, bypassing the SHELLCHECK_PATH env var set by GH#2915. It finds /opt/homebrew/bin/shellcheck directly and invokes it with --external-sources, causing 9+ GB RAM usage. Fix: create ~/.aidevops/bin/shellcheck symlink pointing to shellcheck-wrapper.sh and prepend ~/.aidevops/bin to PATH in .zshenv and shell rc files. This ensures all processes (including non-interactive LSP children) find the wrapper first. The wrapper's existing self-detection (realpath comparison) correctly skips the symlink and finds the real binary, preventing infinite recursion. Closes #2993
Summary of ChangesHello, 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 resolves a critical memory consumption issue within the bash-language-server when interacting with shellcheck. The previous setup allowed the language server to bypass a protective shellcheck wrapper, leading to significant memory usage. By introducing a new PATH shim, this change ensures that the shellcheck wrapper is correctly invoked, effectively mitigating the memory explosion and establishing a more robust and efficient environment for shell script analysis. Highlights
Changelog
Activity
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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughImplements a multi-layer ShellCheck wrapper wiring in Changes
Sequence Diagram(s)sequenceDiagram
participant Installer as Setup Script
participant FS as Filesystem (shim)
participant Env as Shell Envs (.zshenv/.bashrc/fish)
participant Launchctl as macOS launchctl
participant Session as Current Session
participant LSP as bash-language-server
Installer->>Installer: validate wrapper executable & locate real shellcheck
alt valid
Installer->>FS: create/replace symlink ~/.aidevops/bin/shellcheck -> wrapper
Installer->>Env: prepend ~/.aidevops/bin to PATH, write SHELLCHECK_PATH
Installer->>Launchctl: set PATH/SHELLCHECK_PATH for launchctl (macOS)
Installer->>Session: export PATH and SHELLCHECK_PATH in current session
LSP->>Session: spawn shellcheck (resolves PATH)
Session->>FS: resolves to ~/.aidevops/bin/shellcheck (wrapper)
else invalid
Installer->>Installer: skip wiring (no changes)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Fri Mar 6 15:49:59 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request effectively addresses the issue of bash-language-server bypassing the shellcheck-wrapper.sh by introducing a PATH shim. The implementation is robust, handling the creation of the symlink idempotently and updating the necessary shell configuration files. My review focuses on improving debuggability by adhering to the project's guidelines on error handling, specifically by removing unnecessary stderr suppression. The changes are well-commented and clearly explained.
setup-modules/shell-env.sh
Outdated
| if grep -q 'SHELLCHECK_PATH' "$zshenv" 2>/dev/null; then | ||
| already_in="${already_in:+$already_in, }$zshenv" | ||
| else | ||
| touch "$zshenv" | ||
| echo "" >>"$zshenv" | ||
| echo "# Added by aidevops setup (GH#2915: prevent ShellCheck memory explosion)" >>"$zshenv" | ||
| echo "$env_line" >>"$zshenv" | ||
| added_to="${added_to:+$added_to, }$zshenv" | ||
| fi | ||
|
|
||
| # PATH prepend for ~/.aidevops/bin (GH#2993: shim must be on PATH) | ||
| if ! grep -q '\.aidevops/bin' "$zshenv" 2>/dev/null; then | ||
| echo "" >>"$zshenv" | ||
| echo "# Added by aidevops setup (GH#2993: shellcheck shim on PATH)" >>"$zshenv" | ||
| echo "$path_line" >>"$zshenv" | ||
| print_success "Prepended $shim_dir to PATH in .zshenv" | ||
| else | ||
| print_info "$shim_dir already on PATH in .zshenv" | ||
| fi |
There was a problem hiding this comment.
The script calls touch "$zshenv" before this block, and set -e is active, which should ensure the file exists. Suppressing stderr with 2>/dev/null can hide important errors like read permissions issues. To align with the project's general rules on error visibility for debugging, please remove 2>/dev/null from the grep commands.
| if grep -q 'SHELLCHECK_PATH' "$zshenv" 2>/dev/null; then | |
| already_in="${already_in:+$already_in, }$zshenv" | |
| else | |
| touch "$zshenv" | |
| echo "" >>"$zshenv" | |
| echo "# Added by aidevops setup (GH#2915: prevent ShellCheck memory explosion)" >>"$zshenv" | |
| echo "$env_line" >>"$zshenv" | |
| added_to="${added_to:+$added_to, }$zshenv" | |
| fi | |
| # PATH prepend for ~/.aidevops/bin (GH#2993: shim must be on PATH) | |
| if ! grep -q '\.aidevops/bin' "$zshenv" 2>/dev/null; then | |
| echo "" >>"$zshenv" | |
| echo "# Added by aidevops setup (GH#2993: shellcheck shim on PATH)" >>"$zshenv" | |
| echo "$path_line" >>"$zshenv" | |
| print_success "Prepended $shim_dir to PATH in .zshenv" | |
| else | |
| print_info "$shim_dir already on PATH in .zshenv" | |
| fi | |
| if grep -q 'SHELLCHECK_PATH' "$zshenv"; then | |
| already_in="${already_in:+$already_in, }$zshenv" | |
| else | |
| echo "" >>"$zshenv" | |
| echo "# Added by aidevops setup (GH#2915: prevent ShellCheck memory explosion)" >>"$zshenv" | |
| echo "$env_line" >>"$zshenv" | |
| added_to="${added_to:+$added_to, }$zshenv" | |
| fi | |
| # PATH prepend for ~/.aidevops/bin (GH#2993: shim must be on PATH) | |
| if ! grep -q '\.aidevops/bin' "$zshenv"; then | |
| echo "" >>"$zshenv" | |
| echo "# Added by aidevops setup (GH#2993: shellcheck shim on PATH)" >>"$zshenv" | |
| echo "$path_line" >>"$zshenv" | |
| print_success "Prepended $shim_dir to PATH in .zshenv" | |
| else | |
| print_info "$shim_dir already on PATH in .zshenv" | |
| fi |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
setup-modules/shell-env.sh
Outdated
| if grep -q 'SHELLCHECK_PATH' "$rc_file" 2>/dev/null; then | ||
| already_in="${already_in:+$already_in, }$rc_file" | ||
| continue | ||
| else | ||
| echo "" >>"$rc_file" | ||
| echo "# Added by aidevops setup (GH#2915: prevent ShellCheck memory explosion)" >>"$rc_file" | ||
| echo "$env_line" >>"$rc_file" | ||
| added_to="${added_to:+$added_to, }$rc_file" | ||
| fi | ||
|
|
||
| echo "" >>"$rc_file" | ||
| echo "# Added by aidevops setup (GH#2915: prevent ShellCheck memory explosion)" >>"$rc_file" | ||
| echo "$env_line" >>"$rc_file" | ||
| added_to="${added_to:+$added_to, }$rc_file" | ||
| # PATH prepend for ~/.aidevops/bin (GH#2993) | ||
| if ! grep -q '\.aidevops/bin' "$rc_file" 2>/dev/null; then | ||
| echo "" >>"$rc_file" | ||
| echo "# Added by aidevops setup (GH#2993: shellcheck shim on PATH)" >>"$rc_file" | ||
| echo "$path_line" >>"$rc_file" | ||
| fi |
There was a problem hiding this comment.
Similar to the .zshenv block, touch "$rc_file" is called before this, ensuring the file exists. To improve debuggability by making potential read errors visible, please remove 2>/dev/null from the grep commands, in accordance with the project's general rules.
| if grep -q 'SHELLCHECK_PATH' "$rc_file" 2>/dev/null; then | |
| already_in="${already_in:+$already_in, }$rc_file" | |
| continue | |
| else | |
| echo "" >>"$rc_file" | |
| echo "# Added by aidevops setup (GH#2915: prevent ShellCheck memory explosion)" >>"$rc_file" | |
| echo "$env_line" >>"$rc_file" | |
| added_to="${added_to:+$added_to, }$rc_file" | |
| fi | |
| echo "" >>"$rc_file" | |
| echo "# Added by aidevops setup (GH#2915: prevent ShellCheck memory explosion)" >>"$rc_file" | |
| echo "$env_line" >>"$rc_file" | |
| added_to="${added_to:+$added_to, }$rc_file" | |
| # PATH prepend for ~/.aidevops/bin (GH#2993) | |
| if ! grep -q '\.aidevops/bin' "$rc_file" 2>/dev/null; then | |
| echo "" >>"$rc_file" | |
| echo "# Added by aidevops setup (GH#2993: shellcheck shim on PATH)" >>"$rc_file" | |
| echo "$path_line" >>"$rc_file" | |
| fi | |
| if grep -q 'SHELLCHECK_PATH' "$rc_file"; then | |
| already_in="${already_in:+$already_in, }$rc_file" | |
| else | |
| echo "" >>"$rc_file" | |
| echo "# Added by aidevops setup (GH#2915: prevent ShellCheck memory explosion)" >>"$rc_file" | |
| echo "$env_line" >>"$rc_file" | |
| added_to="${added_to:+$added_to, }$rc_file" | |
| fi | |
| # PATH prepend for ~/.aidevops/bin (GH#2993) | |
| if ! grep -q '\.aidevops/bin' "$rc_file"; then | |
| echo "" >>"$rc_file" | |
| echo "# Added by aidevops setup (GH#2993: shellcheck shim on PATH)" >>"$rc_file" | |
| echo "$path_line" >>"$rc_file" | |
| fi |
References
- Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@setup-modules/shell-env.sh`:
- Around line 503-504: The current check that skips adding .aidevops/bin if any
mention exists is too permissive; update the logic that uses the path_line
variable (local path_line='export PATH="$HOME/.aidevops/bin:$PATH"') and the
rc-file loop to check for an exact match of that whole line (not just
substring), e.g. use a fixed-string exact-line grep/awk (grep -Fx or grep -F -x)
against each rc file before appending, or implement a managed block (delimited
begin/end markers) and replace the block atomically; ensure you change both the
initial presence check and the rc-file loop so only the exact prepend form
short-circuits, preventing stale append-form entries from remaining earlier in
PATH.
- Around line 471-480: The launchctl section only sets SHELLCHECK_PATH but not
PATH, so GUI-launched apps can still resolve the real shellcheck; update the
launchctl block to also set the PATH environment via launchctl setenv PATH to
prepend ~/.aidevops/bin (e.g. compute new_path="$HOME/.aidevops/bin:$PATH"
safely expanding $HOME) alongside launchctl setenv SHELLCHECK_PATH, and ensure
you use the same symbol names as in the file (launchctl setenv, SHELLCHECK_PATH)
and handle idempotency so repeated runs don't duplicate entries.
- Around line 583-598: get_all_shell_rcs() currently treats fish rc files like
bash/zsh and appends $env_line/$path_line (bash-style exports) to $rc_file which
breaks fish; update the logic around the blocks that append $env_line and
$path_line to detect when $rc_file is the fish config (e.g.,
~/.config/fish/config.fish) and instead write fish-compatible lines using the
same pattern used by alias_block_fish (use "set -gx" syntax and fish-specific
comment header), leaving bash/zsh behavior unchanged for other rc files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7e0d12b0-5a40-4923-8d53-69566236b1e7
📒 Files selected for processing (1)
setup-modules/shell-env.sh
- Add launchctl setenv PATH alongside SHELLCHECK_PATH so GUI-launched editors (Dock/Spotlight) find the shim via PATH lookup - Use case guard in path_line for runtime idempotency (no duplicate PATH entries in nested shells) - Use grep -Fq with exact path_line for detection instead of loose substring match that could match comments or append-form entries - Handle fish shell config.fish with set -gx syntax instead of bash export (get_all_shell_rcs can return fish config on Linux) - Remove unnecessary 2>/dev/null from grep commands where file is guaranteed to exist (touch called before grep) Addresses: CodeRabbit CHANGES_REQUESTED (3 actionable comments) Also addresses: Gemini Code Assist suggestions (2 comments) Closes #2993
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Fri Mar 6 16:11:20 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Fri Mar 6 16:21:50 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
Worker killed after 6h47m with 0 commits (struggle_ratio: 19764). Re-dispatching with more context. The issue is a ShellCheck PATH shim fix — CodeRabbit requested changes. New worker will read the review comments and address them. |
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
setup-modules/shell-env.sh (1)
643-645: Consider adding idempotency guard to current-session PATH export.The current-session export at line 645 is unconditional, while the persisted
path_line(line 505) uses acaseguard to prevent duplicates. Ifsetup_shellcheck_wrapper()is called multiple times in the same session (e.g., during testing), PATH would accumulate duplicates.♻️ Optional: Add consistency with persisted path_line
# Also export for current session export SHELLCHECK_PATH="$wrapper_path" - export PATH="$HOME/.aidevops/bin:$PATH" + if [[ ":$PATH:" != *":$HOME/.aidevops/bin:"* ]]; then + export PATH="$HOME/.aidevops/bin:$PATH" + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@setup-modules/shell-env.sh` around lines 643 - 645, The current-session PATH export in setup_shellcheck_wrapper() unconditionally prepends "$HOME/.aidevops/bin" causing duplicates if called multiple times; change the in-session export to check whether "$HOME/.aidevops/bin" is already in PATH before modifying it (match the persisted path_line behavior), and ensure SHELLCHECK_PATH is still exported; locate the block that sets export SHELLCHECK_PATH and export PATH and wrap the PATH change with a guard (e.g., using a case or conditional that tests $PATH for the directory) so repeated calls do not accumulate duplicates.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@setup-modules/shell-env.sh`:
- Around line 643-645: The current-session PATH export in
setup_shellcheck_wrapper() unconditionally prepends "$HOME/.aidevops/bin"
causing duplicates if called multiple times; change the in-session export to
check whether "$HOME/.aidevops/bin" is already in PATH before modifying it
(match the persisted path_line behavior), and ensure SHELLCHECK_PATH is still
exported; locate the block that sets export SHELLCHECK_PATH and export PATH and
wrap the PATH change with a guard (e.g., using a case or conditional that tests
$PATH for the directory) so repeated calls do not accumulate duplicates.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ff216773-0162-4b05-82e2-3a2e546a9255
📒 Files selected for processing (1)
setup-modules/shell-env.sh
1 participant
Summary
shellcheck-wrapper.shby resolvingshellcheckvia PATH to/opt/homebrew/bin/shellcheckinstead of the wrapper~/.aidevops/bin/shellchecksymlink pointing toshellcheck-wrapper.shand prepends~/.aidevops/binto PATH in.zshenvand shell rc files--external-sourcesflag that the language server hardcodesRoot Cause
GH#2915 set
SHELLCHECK_PATHenv var, but bash-language-server ignores it — it does a standard PATH lookup forshellcheck, finding/opt/homebrew/bin/shellcheckdirectly. The wrapper's--external-sourcesstripping andulimitprotections never applied.Fix
Added Layer 0 (PATH shim) to
setup_shellcheck_wrapper():~/.aidevops/bin/shellcheckas a symlink toshellcheck-wrapper.sh~/.aidevops/binto PATH in.zshenv(all zsh processes) and shell rc filesrealpathself-detection correctly skips the symlink, finds the real binary — no recursionLayers 1-3 (launchctl, .zshenv SHELLCHECK_PATH, rc files) are retained for tools that honour
SHELLCHECK_PATH.Testing
Verified locally:
which shellcheckwith~/.aidevops/binon PATH → finds the shim--versionworks, finds real binary)--external-sourcesis stripped (no memory explosion)realpathcomparison)Closes #2993
Summary by CodeRabbit