refactor(quality): replace blanket ShellCheck disables with targeted per-file disables (t135.2) #492
Conversation
…per-file disables (t135.2) Replaced the 23-rule blanket '# shellcheck disable=...' line in 95 scripts with targeted disables containing only the rules each file actually needs. - 4 scripts now have zero ShellCheck disables (completely clean) - 91 scripts have targeted disables (avg 3-5 rules vs 23 before) - Zero ShellCheck violations across all 95 modified files - All existing tests pass (53 state machine + 56 batch quality) This makes it clear which rules each script actually triggers, making future fixes easier to identify and prioritize.
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
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. WalkthroughThis PR systematically reduces ShellCheck suppression directives across approximately 100 shell scripts in the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
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 |
The 'wait $pid' call returns the exit code of the background process. Under set -euo pipefail (added in t135.1), a non-zero exit from codacy-cli.sh kills the script before reaching the status check. Use 'wait $pid || status=$?' to capture the exit code safely.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| @@ -1,5 +1,5 @@ | |||
| #!/usr/bin/env bash | |||
| # shellcheck disable=SC2034,SC2155,SC2317,SC2329,SC2016,SC2181,SC1091,SC2154,SC2015,SC2086,SC2129,SC2030,SC2031,SC2119,SC2120,SC2001,SC2162,SC2088,SC2089,SC2090,SC2029,SC2006,SC2153 | |||
| # shellcheck disable=SC1091,SC2129,SC2317 | |||
There was a problem hiding this comment.
SC2317 (unreachable code) is masking real bugs — don't suppress it, fix the root cause.
SC2317 fires because check_status and list_apps have misplaced return 0 statements that make subsequent code unreachable. Suppressing this warning hides functional bugs rather than silencing a false positive. See comments below on lines 175–188 and 150–153.
🤖 Prompt for AI Agents
In @.agents/scripts/cloudron-helper.sh at line 2, The shellcheck suppression is
hiding real unreachable-code bugs: the functions check_status and list_apps
contain misplaced early "return 0" statements that make later logic unreachable
and trigger SC2317; remove or relocate those premature returns so the full
function logic executes and place a single, final return at the intended
function exit, then remove SC2317 from the "# shellcheck disable=..." list;
update check_status and list_apps to return consistent exit codes only at the
end and run shellcheck to verify SC2317 no longer appears.
| @@ -1,5 +1,5 @@ | |||
| #!/usr/bin/env bash | |||
| # shellcheck disable=SC2034,SC2155,SC2317,SC2329,SC2016,SC2181,SC1091,SC2154,SC2015,SC2086,SC2129,SC2030,SC2031,SC2119,SC2120,SC2001,SC2162,SC2088,SC2089,SC2090,SC2029,SC2006,SC2153 | |||
| # shellcheck disable=SC1091,SC2034,SC2154,SC2155,SC2162,SC2181,SC2317 | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, check if the file exists and read the relevant sections
if [ -f ".agents/scripts/git-platforms-helper.sh" ]; then
echo "=== Lines 1-10 of git-platforms-helper.sh ==="
head -10 ".agents/scripts/git-platforms-helper.sh" | cat -n
echo ""
echo "=== File size ==="
wc -l ".agents/scripts/git-platforms-helper.sh"
else
echo "File not found"
fiRepository: marcusquinn/aidevops
Length of output: 582
🏁 Script executed:
#!/bin/bash
# Search for SC2181 violations - checking $? instead of direct command evaluation
echo "=== SC2181 pattern: checking \$? in conditionals ==="
rg '\$\?' .agents/scripts/git-platforms-helper.sh -n | head -20
echo ""
echo "=== SC2317 pattern: code after return ==="
rg -A2 'return [0-9]' .agents/scripts/git-platforms-helper.sh -n | grep -E '^\s+[a-z]|return' | head -30Repository: marcusquinn/aidevops
Length of output: 827
🏁 Script executed:
#!/bin/bash
# Check for the disable directive on line 2
echo "=== Line 2 content (the disable directive) ==="
sed -n '2p' .agents/scripts/git-platforms-helper.sh
echo ""
echo "=== Count disabled rules ==="
sed -n '2p' .agents/scripts/git-platforms-helper.sh | grep -oP 'SC\d+' | wc -lRepository: marcusquinn/aidevops
Length of output: 212
🏁 Script executed:
#!/bin/bash
# Look for duplicate constants mentioned in review
echo "=== Checking for duplicate HELP_SHOW_MESSAGE ==="
rg 'HELP_SHOW_MESSAGE=' .agents/scripts/git-platforms-helper.sh -n
echo ""
echo "=== Checking for duplicate USAGE_COMMAND_OPTIONS ==="
rg 'USAGE_COMMAND_OPTIONS=' .agents/scripts/git-platforms-helper.sh -nRepository: marcusquinn/aidevops
Length of output: 392
🏁 Script executed:
#!/bin/bash
# Check main() function structure around lines 519-540
echo "=== Lines 519-540 (main function area) ==="
sed -n '519,540p' .agents/scripts/git-platforms-helper.sh | cat -nRepository: marcusquinn/aidevops
Length of output: 1048
Remove ShellCheck disables and fix the underlying issues instead of suppressing them—the script has critical execution bugs.
The targeted disable approach violates the zero-violations requirement. Verification confirms these suppressions mask real, fixable bugs:
- SC2181 (6 violations): Lines 194, 214, 245, 265, 295, 406 use
if [[ $? -eq 0 ]]—fix with direct command evaluation (if command; then) - Duplicate constants: Lines 14 and 22 define
HELP_SHOW_MESSAGEidentically; lines 15 and 23 defineUSAGE_COMMAND_OPTIONSidentically—remove duplicates - Broken main() function (lines 520–540): Contains triple-duplicated variable assignments followed by contradictory overwrites. Lines 521–523 self-reference undefined variables (
local account_name="$account_name"), causing empty/undefined assignments. Lines 533–536 then overwrite with different undefined variables. This pattern prevents the script from functioning correctly.
These issues are not ShellCheck false positives—they are runtime correctness bugs that must be fixed to meet the zero-technical-debt standard.
🤖 Prompt for AI Agents
In @.agents/scripts/git-platforms-helper.sh at line 2, Remove the ShellCheck
disables by fixing the root issues: replace all occurrences of post-command exit
checks like "if [[ $? -eq 0 ]]" with direct command tests (e.g., "if <command>;
then") to satisfy SC2181; remove the duplicate constant definitions for
HELP_SHOW_MESSAGE and USAGE_COMMAND_OPTIONS so each is defined only once; and
correct the broken main() function by declaring and assigning its local
variables exactly once from the intended sources (e.g., function parameters,
parsed options, or explicit env vars) instead of self-referencing undefined
names (remove lines like local account_name="$account_name" and the later
contradictory overwrites), ensuring variables are populated in a single, clear
assignment flow inside main().
| @@ -1,5 +1,5 @@ | |||
| #!/usr/bin/env bash | |||
| # shellcheck disable=SC2034,SC2155,SC2317,SC2329,SC2016,SC2181,SC1091,SC2154,SC2015,SC2086,SC2129,SC2030,SC2031,SC2119,SC2120,SC2001,SC2162,SC2088,SC2089,SC2090,SC2029,SC2006,SC2153 | |||
| # shellcheck disable=SC1091,SC2129,SC2153,SC2317 | |||
There was a problem hiding this comment.
SC2153 is suppressing a real bug — review before keeping this disable.
SC2153 warns about possible misspellings (e.g., SCRIPT_DIR vs script_dir). On line 289, a local script_dir (lowercase) is defined, but line 290 references $SCRIPT_DIR (uppercase), which is never assigned in this script. The SC2153 suppress is hiding the exact bug it was designed to catch.
SC1091, SC2129, and SC2317 are justified.
🤖 Prompt for AI Agents
In @.agents/scripts/setup-local-api-keys.sh at line 2, The SC2153 suppression is
hiding a real bug: the script defines a local variable script_dir but later
references $SCRIPT_DIR which is never set; update the references so the same
identifier is used consistently (either rename the definition to SCRIPT_DIR or
change all occurrences of $SCRIPT_DIR to $script_dir) and then remove the SC2153
disable; verify references in the block around the script_dir/SCRIPT_DIR
assignment and any functions that use it (look for script_dir and SCRIPT_DIR
symbols) so the variable is correctly assigned before use.
| @@ -1,5 +1,5 @@ | |||
| #!/usr/bin/env bash | |||
| # shellcheck disable=SC2034,SC2155,SC2317,SC2329,SC2016,SC2181,SC1091,SC2154,SC2015,SC2086,SC2129,SC2030,SC2031,SC2119,SC2120,SC2001,SC2162,SC2088,SC2089,SC2090,SC2029,SC2006,SC2153 | |||
| # shellcheck disable=SC1091,SC2016,SC2154 | |||
There was a problem hiding this comment.
SC2154 suppression is masking a real bug on Line 135.
The narrowed disable list is a clear improvement. However, SC2154 (variable referenced but not assigned) is included here specifically because $_arg2 on Line 135 is never defined in this script — that's a genuine bug, not a false positive. Rather than suppressing SC2154 to paper over it, fix the reference and drop SC2154 from the list entirely.
Proposed fix
-# shellcheck disable=SC1091,SC2016,SC2154
+# shellcheck disable=SC1091,SC2016(after fixing $_arg2 → $2 on Line 135)
As per coding guidelines, "Use ShellCheck for all scripts with zero violations required." Suppressing SC2154 here hides a real undefined-variable defect rather than achieving genuine zero-violation compliance. Based on learnings: "Use ShellCheck for all scripts with zero violations required."
🤖 Prompt for AI Agents
In @.agents/scripts/sonarcloud-autofix.sh at line 2, The suppress-all for SC2154
is hiding a real undefined-variable bug: replace the incorrect variable
reference $_arg2 with $2 in the script (the occurrence currently used on Line
135) and remove SC2154 from the ShellCheck disable comment (the "# shellcheck
disable=SC1091,SC2016,SC2154" line) so the script no longer suppresses the
undefined-variable warning; keep the other disables (SC1091, SC2016) as-is if
still needed.
🔍 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: Sat Feb 7 23:52:50 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
1 participant
Summary
# shellcheck disable=...line in 95 scripts with targeted disables containing only the rules each file actually needsWhy
The blanket disable suppressed all warnings, making it impossible to identify real issues. Targeted disables make it clear which rules each script actually triggers, enabling incremental fixes.
Testing
bash -nsyntax check: 95/95 passImplements t135.2 from the Codebase Quality Hardening plan.
Summary by CodeRabbit