Skip to content

t1491: fix Bash 3.2 bad substitution in config_get indirect expansion#4931

Merged
marcusquinn merged 1 commit intomainfrom
bugfix/t1491-bash32-config-helper
Mar 15, 2026
Merged

t1491: fix Bash 3.2 bad substitution in config_get indirect expansion#4931
marcusquinn merged 1 commit intomainfrom
bugfix/t1491-bash32-config-helper

Conversation

@alex-solovyev
Copy link
Collaborator

@alex-solovyev alex-solovyev commented Mar 15, 2026

Summary

  • Replace ${!env_var:-} with eval "env_val=\${$env_var:-}" at 3 locations in config-helper.sh (lines 333, 527, 732)
  • The ${!var:-default} indirect expansion syntax causes "bad substitution" on macOS /bin/bash 3.2.57
  • This caused config_get to fail silently during pulse-wrapper.sh startup, making MAX_WORKERS_CAP and QUALITY_DEBT_CAP_PCT fall back to hardcoded defaults instead of reading from settings.json

Verification

  • ShellCheck clean (zero violations)
  • Tested config_get with env var override set → returns override value
  • Tested config_get with env var unset → falls through to JSONC config value
  • No remaining ${!env_var:-} instances in the file

Closes #4929

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved compatibility with older Bash versions for environment variable configuration handling.
    • Fixed issues with environment override initialization to prevent errors when configuration variables are not set.

Replace ${!env_var:-} with eval-based equivalent at 3 locations in
config-helper.sh. The ${!var:-default} syntax causes 'bad substitution'
on macOS /bin/bash 3.2.57, making config_get fail silently during
pulse-wrapper.sh startup.

Closes #4929
@gemini-code-assist
Copy link

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions github-actions bot added the bug Auto-created from TODO.md tag label Mar 15, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 15, 2026

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

Fixed Bash 3.2 compatibility issue in config-helper.sh by replacing unsupported ${!VAR:-} indirect expansion syntax with eval-based approach at three locations. Explicitly initialized env_val variable and updated comments to document compatibility rationale.

Changes

Cohort / File(s) Summary
Bash 3.2 Compatibility Fix
.agents/scripts/config-helper.sh
Replaced ${!env_var:-} with eval-based indirect expansion at lines 333, 527, and 732 to support Bash 3.2. Added explicit env_val initialization and documentation comments explaining the Bash 4.0+ incompatibility workaround.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐚 Three lines of Bash once caused a fright,
${!VAR:-} broke on macOS night—
With eval and care, we've set it right,
Now Bash 3.2 runs clean and bright! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: fixing Bash 3.2 compatibility by replacing incompatible indirect expansion syntax in config_get.
Linked Issues check ✅ Passed All objectives from issue #4929 are met: three occurrences of ${!env_var:-} replaced with eval-based Bash 3.2-compatible syntax, ShellCheck verified clean, config_get behavior preserved for both set and unset env vars.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing Bash 3.2 compatibility in config-helper.sh; no unrelated modifications or scope creep detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/t1491-bash32-config-helper
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@marcusquinn
Copy link
Owner

Duplicate PR detected: PR #4930 (branch bugfix/issue-4929-bash32-config-get, author: marcusquinn) also targets issue #4929 and was opened 9 seconds earlier.

PR #4931 (this PR) is more comprehensive — fixes 3 locations in config-helper.sh vs PR #4930's 1 location. Recommend: close PR #4930 and merge this one once CI passes.

Supervisor will check CI results next cycle and close the smaller PR if this one passes.

@github-actions
Copy link
Contributor

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 362 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Sun Mar 15 05:25:07 UTC 2026: Code review monitoring started
Sun Mar 15 05:25:08 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 362

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 362
  • VULNERABILITIES: 0

Generated on: Sun Mar 15 05:25:10 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Auto-created from TODO.md tag review-feedback-scanned Merged PR already scanned for quality feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

t1491: fix: Bash 3.2 bad substitution in config_get indirect expansion

2 participants