Skip to content

t4929: Fix Bash 3.2 bad substitution in config_get#4930

Closed
marcusquinn wants to merge 1 commit intomainfrom
bugfix/issue-4929-bash32-config-get
Closed

t4929: Fix Bash 3.2 bad substitution in config_get#4930
marcusquinn wants to merge 1 commit intomainfrom
bugfix/issue-4929-bash32-config-get

Conversation

@marcusquinn
Copy link
Owner

@marcusquinn marcusquinn commented Mar 15, 2026

Summary

  • replace Bash 4-style indirect expansion (${!name:-}) in config_get with a Bash 3.2-safe check and read path
  • preserve env override precedence while avoiding bad substitution when pulse-wrapper.sh sources config-helper.sh on macOS /bin/bash
  • validated with shellcheck and a /bin/bash source repro

Closes #4929

Verification

  • shellcheck .agents/scripts/config-helper.sh
  • /bin/bash -lc 'source .agents/scripts/pulse-wrapper.sh'
  • /bin/bash -lc 'source .agents/scripts/config-helper.sh; AIDEVOPS_MAX_WORKERS_CAP=11; config_get orchestration.max_workers_cap 8'

Summary by CodeRabbit

  • Bug Fixes
    • Improved environment variable override handling to properly distinguish between unset and set (empty) variables, ensuring more reliable configuration management.

@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

This change fixes Bash 3.2 compatibility in the config helper by replacing incompatible indirect expansion syntax ${!env_var:-} with a safe parameter expansion check pattern that detects whether an environment variable is explicitly set before reading its value.

Changes

Cohort / File(s) Summary
Bash 3.2 Compatibility Fix
.agents/scripts/config-helper.sh
Modified config_get function to replace Bash 4.0+ indirect expansion syntax with Bash 3.2-compatible checks using ${!env_var+x} parameter expansion. Now properly distinguishes between unset and set (potentially empty) environment variables without triggering "bad substitution" errors.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

From macOS 3.2 comes a gentle plea,
"Expand my variables, safely!" 🐚
Parameter checks with finesse align,
Bash compatibility—a perfect design,
Pulse keeps dancing, workers aligned,
Zero technical debt, perfectly refined. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main fix: replacing Bash 4 syntax with Bash 3.2-compatible code in config_get function.
Linked Issues check ✅ Passed The PR successfully addresses all linked issue objectives: replaces ${!env_var:-} with Bash 3.2-safe logic at all three locations, preserves env-var override precedence, and validates ShellCheck compatibility.
Out of Scope Changes check ✅ Passed Changes are strictly confined to config-helper.sh with focused updates to the config_get function's environment variable handling, directly addressing the Bash 3.2 compatibility issue.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ 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/issue-4929-bash32-config-get
📝 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.

@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:00 UTC 2026: Code review monitoring started
Sun Mar 15 05:25:01 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:03 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@sonarqubecloud
Copy link

@marcusquinn
Copy link
Owner Author

Closing in favour of PR #4931 (branch bugfix/t1491-bash32-config-helper, author: alex-solovyev) which is more comprehensive — fixes 3 locations in config-helper.sh vs this PR's 1 location. Both PRs have green CI. Supervisor merging #4931.

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

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

1 participant