fix: guard ((var++)) with || true to prevent silent exit under set -e#548
fix: guard ((var++)) with || true to prevent silent exit under set -e#548alex-solovyev merged 1 commit intomainfrom
Conversation
In bash, ((var++)) when var=0 evaluates the post-increment expression to 0, which (( )) treats as falsy (exit code 1). With set -e (errexit), this silently aborts the script. This caused setup.sh --non-interactive to abort at cleanup_deprecated_mcps() before reaching deploy_aidevops_agents(), leaving deployed agents at a stale version. Fixed 33 instances in setup.sh and 10 additional helper scripts that use set -e with the same pattern. Closes #t152
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
WalkthroughMultiple shell scripts across the project's automation framework add Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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: Sun Feb 8 02:27:01 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.agents/scripts/linter-manager.sh (1)
374-402:⚠️ Potential issue | 🟠 MajorGuard all unguarded arithmetic expressions in
install_detected_linters()andinstall_all_linters()— fourteen instances of((total_failures += $?))lack protection and will silently abort when any linter installation succeeds (exit 0) while total_failures remains 0, triggeringset -e.🐛 Proposed fix — guard these the same way
- ((total_failures += $?)) + ((total_failures += $?)) || trueApply to all instances at lines 378, 382, 386, 390, 394, 398, 402 (in
install_detected_linters()) and 427, 431, 435, 439, 443, 447, 451 (ininstall_all_linters())..agents/scripts/codacy-cli-chunked.sh (1)
504-506:⚠️ Potential issue | 🟡 MinorPre-existing:
maindereferences$2unconditionally underset -u.
local _arg2="$2"will abort when the script is invoked with fewer than 2 arguments (e.g.,./codacy-cli-chunked.sh help). This isn't introduced by this PR, but it's in the same function and will cause the exact class of silent-exit bug you're fixing.Suggested fix
main() { - local _arg2="$2" + local _arg2="${2:-}" local command="${1:-help}".agents/scripts/toon-helper.sh (1)
372-377:⚠️ Potential issue | 🟡 MinorPre-existing:
maindereferences positional params unconditionally underset -u.Lines 374–377 assign
$2through$5without defaults. Calling./toon-helper.sh encode foo.json(3 args) will abort onlocal arg4="$4". Same class of bug this PR is addressing.Suggested fix
main() { local command="${1:-help}" - local arg2="$2" - local arg3="$3" - local arg4="$4" - local arg5="$5" + local arg2="${2:-}" + local arg3="${3:-}" + local arg4="${4:-}" + local arg5="${5:-}"
🧹 Nitpick comments (2)
.agents/scripts/anti-detect-helper.sh (1)
1099-1099: Redundant double|| true— one is sufficient.The line now reads:
[[ -d "$dir" ]] && [[ "$(basename "$dir")" != "default" ]] && ((profile_count++)) || true || trueThe second
|| trueis a no-op since the first already guarantees exit code 0. Looks like the PR appended|| truewithout noticing one was already present.🧹 Remove the redundant guard
- [[ -d "$dir" ]] && [[ "$(basename "$dir")" != "default" ]] && ((profile_count++)) || true || true + [[ -d "$dir" ]] && [[ "$(basename "$dir")" != "default" ]] && ((profile_count++)) || true.agents/scripts/quality-cli-manager.sh (1)
92-163: Consider extracting the repetitive install/count pattern.Each CLI block in
install_clis,init_clis, andanalyze_with_clisfollows the identical structure: conditional check → execute → increment success on success → always increment total. This is repeated ~18 times across three functions. A small helper (e.g.,try_cli_command) could reduce ~6 lines per CLI to ~1 and eliminate the risk of missing a|| trueguard in the future.Not blocking — the current code is correct and readable. Just flagging the duplication since maintaining zero tech debt is the goal.
|
Reopening: task was falsely marked complete in TODO.md, triggering auto-close via issue-sync. Reverted in PR #616. |
2 participants
Summary
((var++))arithmetic expressions that silently abort scripts underset -ewhen the variable is 0((0))returns exit code 1, andset -e(errexit) treats this as a fatal errorsetup.sh --non-interactiveto abort atcleanup_deprecated_mcps()before reachingdeploy_aidevops_agents(), leaving deployed agents at a stale versionChanges
((var++))→((var++)) || trueset -e:anti-detect-helper.sh,codacy-cli-chunked.sh,crawl4ai-examples.shlinter-manager.sh,localhost-helper.sh,markdown-formatter.shquality-cli-manager.sh,setup-linters-wizard.sh,setup-local-api-keys.shtoon-helper.shTesting
bash -nsyntax check passes on all 11 modified filessetup.sh --non-interactivenow runs to completion (previously silently aborted)Root Cause
Closes t152
Summary by CodeRabbit