fix: resolve SonarCloud S1481 and S1066 issues#201
Conversation
- Remove unused local variables (S1481): - cloudron-package-helper.sh: Remove unused 'app' parameter - session-time-helper.sh: Prefix unused IFS vars with underscore - add-skill-helper.sh: Prefix unused 'description' with underscore - worktree-sessions.sh: Prefix unused 'worktree_path' with underscore - opencode-github-setup-helper.sh: Remove unused vars - beads-sync-helper.sh: Prefix unused 'force' with underscore - site-crawler-helper.sh: Prefix unused vars with underscore - fix-s131-default-cases.sh: Remove unused 'offset' and 'total_fixed' - keyword-research-helper.sh: Remove unused array declarations - dspyground-helper.sh: Remove duplicate variable assignments - pagespeed-helper.sh: Remove duplicate variable assignments - Merge collapsible if statements (S1066): - linters-local.sh: Combine file existence and size checks - mail-helper.sh: Combine type check with helper/payload checks - secretlint-helper.sh: Combine grep and npm list checks - localhost-helper.sh: Combine nc availability and port check - worktree-sessions.sh: Combine branch checks - quality-loop-helper.sh: Combine review trigger conditions - opencode-github-setup-helper.sh: Combine file and grep checks - site-crawler-helper.sh: Combine python availability checks - eeat-score-helper.sh: Combine config file and jq checks - add-skill-helper.sh: Restructure openskills check
|
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. Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis pull request applies a systematic refactoring across 15+ shell scripts in the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes The pull request applies repetitive refactoring patterns across many files (which reduces individual file review burden), but spans 15+ distinct scripts requiring consistency verification. Most changes are safe simplifications, but the 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. 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 |
Summary of ChangesHello @marcusquinn, 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 significantly enhances the codebase's quality by systematically resolving two common SonarCloud issues: S1481 (unused local variables) and S1066 (collapsible if statements). The changes streamline conditional logic and eliminate redundant variable declarations, making the scripts cleaner, easier to read, and more maintainable. These improvements are purely refactoring-based, ensuring no functional impact while boosting adherence to coding standards. Highlights
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
|
🔍 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 Jan 25 03:29:52 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Code Review
This pull request addresses SonarCloud issues by removing unused variables and collapsing nested if statements across multiple shell scripts. The changes are generally well-executed and improve code quality. Most unused variables are correctly removed or prefixed with an underscore, and many if statements are simplified effectively. I have one point of feedback on the refactoring in add-skill-helper.sh, where the attempt to collapse a more complex if structure has introduced a minor regression in logging behavior and a small inefficiency. My comment provides details on this for your consideration.
.agent/scripts/add-skill-helper.sh
Outdated
| if command -v openskills &>/dev/null \ | ||
| && openskills install "$owner/$repo${subpath:+/$subpath}" --yes --universal 2>/dev/null; then | ||
| log_info "Using openskills to fetch skill..." | ||
| if openskills install "$owner/$repo${subpath:+/$subpath}" --yes --universal 2>/dev/null; then | ||
| log_success "Skill installed via openskills" | ||
| # openskills handles everything, just register it | ||
| local skill_name="${custom_name:-$(basename "${subpath:-$repo}")}" | ||
| skill_name=$(to_kebab_case "$skill_name") | ||
| # openskills installs to ~/.config/opencode/skills/<name>/SKILL.md | ||
| # Register with -skill suffix for consistency with direct imports | ||
| register_skill "$skill_name" "https://github.com/$owner/$repo" ".agent/skills/${skill_name}-skill.md" "skill-md" "" "openskills" "Installed via openskills CLI" | ||
| return 0 | ||
| fi | ||
| log_success "Skill installed via openskills" | ||
| # openskills handles everything, just register it | ||
| local skill_name="${custom_name:-$(basename "${subpath:-$repo}")}" | ||
| skill_name=$(to_kebab_case "$skill_name") | ||
| # openskills installs to ~/.config/opencode/skills/<name>/SKILL.md | ||
| # Register with -skill suffix for consistency with direct imports | ||
| register_skill "$skill_name" "https://github.com/$owner/$repo" ".agent/skills/${skill_name}-skill.md" "skill-md" "" "openskills" "Installed via openskills CLI" | ||
| return 0 | ||
| elif command -v openskills &>/dev/null; then | ||
| log_warning "openskills failed, falling back to direct fetch" | ||
| fi |
There was a problem hiding this comment.
While this refactoring resolves the SonarCloud S1066 issue, it introduces a couple of drawbacks:
- Misplaced Log Message: The
log_info "Using openskills to fetch skill..."is now only executed upon successful installation. It's better practice to inform the user of an action before it's attempted, which was the original behavior. - Redundant Check: The
elif command -v openskills &>/dev/nullperforms a redundant check, as this was already evaluated in the initialifcondition.
A clearer approach would be to use a nested if-else structure. This would preserve the logging order and avoid the redundant check. The original code structure was not a simple collapsible if due to the follow-on logic, so this might be a case where the linter's suggestion leads to less clear code.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @.agent/scripts/session-time-helper.sh:
- Around line 249-251: The JSON output references an undefined variable $afk
because the read now assigns _afk; update the JSON-emission code to use $_afk
(or change the IFS read to produce afk instead of _afk) so the variable
referenced by JSON mode is bound under set -u; search for usages of $afk near
the JSON output generation and replace them with $_afk (or rename _afk→afk in
the read assignment) to fix the mismatch.
🤖 Augment PR SummarySummary: This PR updates multiple Changes:
Technical Notes: The refactors are intended to be behavior-preserving and improve static-analysis/readability; ShellCheck compliance is expected to remain unchanged. 🤖 Was this summary useful? React with 👍 or 👎 |
- session-time-helper.sh: Keep 'afk' variable name since it's used in JSON output - add-skill-helper.sh: Restore original nested if structure to preserve logging order (log before action) and avoid redundant command check
|
🔍 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 Jan 25 03:35:09 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |



Summary
Changes
S1481 - Unused Variables
S1066 - Collapsible If Statements
Files Modified
Testing
Summary by CodeRabbit
Refactor
Chores
✏️ Tip: You can customize this high-level summary in your review settings.