Skip to content

chore: fix SonarCloud S7682 violations and trigger CodeRabbit audit #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcusquinn merged 1 commit into from
Jan 12, 2026
Merged

chore: fix SonarCloud S7682 violations and trigger CodeRabbit audit #63

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions .agent/scripts/fix-s131-default-cases.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ readonly YELLOW='\033[1;33m'
readonly RED='\033[0;31m'
readonly NC='\033[0m'

print_info() { echo -e "${BLUE}[INFO]${NC} $1"; }
print_success() { echo -e "${GREEN}[SUCCESS]${NC} $1"; }
print_warning() { echo -e "${YELLOW}[WARNING]${NC} $1"; }
print_error() { echo -e "${RED}[ERROR]${NC} $1" >&2; }
print_info() { echo -e "${BLUE}[INFO]${NC} $1"; return 0; }
print_success() { echo -e "${GREEN}[SUCCESS]${NC} $1"; return 0; }
print_warning() { echo -e "${YELLOW}[WARNING]${NC} $1"; return 0; }
print_error() { echo -e "${RED}[ERROR]${NC} $1" >&2; return 0; }

DRY_RUN=false
TARGET_FILE=""
Expand Down Expand Up @@ -105,6 +105,7 @@ find_missing_defaults() {
}
}
' "$file"
return 0
}

# Add default case before esac
Expand Down
9 changes: 9 additions & 0 deletions .agent/scripts/generate-skills.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,22 @@ done

log_info() {
echo -e "${BLUE}$1${NC}"
return 0
}

log_success() {
echo -e "${GREEN}✓${NC} $1"
return 0
}

log_warning() {
echo -e "${YELLOW}⚠${NC} $1"
return 0
}

log_error() {
echo -e "${RED}✗${NC} $1"
return 0
}

# Extract frontmatter field from markdown file
Expand All @@ -92,6 +96,7 @@ extract_frontmatter_field() {
exit
}
' "$file"
return 0
}

# Extract description from file - tries frontmatter first, then first heading
Expand Down Expand Up @@ -127,6 +132,7 @@ extract_description() {
to_skill_name() {
local name="$1"
echo "$name" | tr '[:upper:]' '[:lower:]' | sed 's/[^a-z0-9-]/-/g' | sed 's/--*/-/g' | sed 's/^-\|-$//g'
return 0
}

# Capitalize first letter (portable)
Expand All @@ -137,6 +143,7 @@ capitalize() {
first=$(echo "$str" | cut -c1 | tr '[:lower:]' '[:upper:]')
rest=$(echo "$str" | cut -c2-)
echo "${first}${rest}"
return 0
}

# Generate SKILL.md content for a folder with parent .md
Expand Down Expand Up @@ -187,6 +194,7 @@ generate_folder_skill() {
echo "## Subskills"
echo "$subskills"
fi
return 0
}

# Generate SKILL.md content for a leaf .md file
Expand Down Expand Up @@ -215,6 +223,7 @@ generate_leaf_skill() {
echo "# ${title}"
echo ""
echo "See [${filename}.md](${relative_path}) for full instructions."
return 0
}

# =============================================================================
Expand Down
17 changes: 13 additions & 4 deletions .agent/scripts/gsc-add-user-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ YELLOW='\033[1;33m'
BLUE='\033[0;34m'
NC='\033[0m'

log_info() { echo -e "${BLUE}[INFO]${NC} $1"; }
log_success() { echo -e "${GREEN}[SUCCESS]${NC} $1"; }
log_warn() { echo -e "${YELLOW}[WARN]${NC} $1"; }
log_error() { echo -e "${RED}[ERROR]${NC} $1"; }
log_info() { echo -e "${BLUE}[INFO]${NC} $1"; return 0; }
log_success() { echo -e "${GREEN}[SUCCESS]${NC} $1"; return 0; }
log_warn() { echo -e "${YELLOW}[WARN]${NC} $1"; return 0; }
log_error() { echo -e "${RED}[ERROR]${NC} $1"; return 0; }
Comment on lines +17 to +20
Copy link

@gemini-code-assist gemini-code-assist bot Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There are two issues with these logging functions:

  1. The arguments should be double-quoted to prevent word splitting and globbing issues if the messages contain spaces or special characters (ShellCheck SC2086).
  2. The log_error function should print to standard error (>&2) to follow standard practice for error messages. This allows callers to separate normal output from error output.

Here's a suggested fix for both issues:

Suggested change
log_info() { echo -e "${BLUE}[INFO]${NC} $1"; return 0; }
log_success() { echo -e "${GREEN}[SUCCESS]${NC} $1"; return 0; }
log_warn() { echo -e "${YELLOW}[WARN]${NC} $1"; return 0; }
log_error() { echo -e "${RED}[ERROR]${NC} $1"; return 0; }
log_info() { echo -e "${BLUE}[INFO]${NC} $1"; return 0; }
log_success() { echo -e "${GREEN}[SUCCESS]${NC} $1"; return 0; }
log_warn() { echo -e "${YELLOW}[WARN]${NC} $1"; return 0; }
log_error() { echo -e "${RED}[ERROR]${NC} $1" >&2; return 0; }


show_help() {
cat << 'HELP'
Expand Down Expand Up @@ -45,6 +45,7 @@ Requirements:
- Chrome browser with logged-in Google session
- User must have Owner access to GSC properties
HELP
return 0
}

get_chrome_profile_path() {
Expand All @@ -63,6 +64,7 @@ get_chrome_profile_path() {
exit 1
;;
esac
return 0
}

ensure_playwright() {
Expand All @@ -76,6 +78,7 @@ ensure_playwright() {
log_info "Installing Playwright..."
npm install playwright
fi
return 0
}

create_add_script() {
Expand Down Expand Up @@ -181,6 +184,7 @@ async function main() {

main().catch(console.error);
SCRIPT
return 0
}

create_list_script() {
Expand Down Expand Up @@ -215,11 +219,13 @@ async function main() {

main().catch(console.error);
SCRIPT
return 0
}

run_script() {
cd "${WORK_DIR}" || exit
node "${GSC_SCRIPT}"
return 0
}

cmd_add() {
Expand Down Expand Up @@ -266,6 +272,7 @@ cmd_add() {
[[ -n "$excludes" ]] && log_info "Excluding: ${excludes}"

run_script
return 0
}

cmd_list() {
Expand All @@ -274,6 +281,7 @@ cmd_list() {

log_info "Listing all GSC properties..."
run_script
return 0
}

cmd_check() {
Expand All @@ -287,6 +295,7 @@ cmd_check() {
# Use the GSC MCP to check access
log_info "Checking GSC API access for ${service_account}..."
log_info "Run: opencode mcp call google-search-console list-sites"
return 0
}

# Main
Expand Down
6 changes: 6 additions & 0 deletions .agent/scripts/memory-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ extract_ids_from_json() {
# Basic fallback - extract id values
echo "$input" | grep -o '"id":"[^"]*"' | sed 's/"id":"//g; s/"//g'
fi
return 0
}

#######################################
Expand Down Expand Up @@ -120,6 +121,7 @@ CREATE TABLE IF NOT EXISTS learning_access (
EOF
log_success "Database initialized"
fi
return 0
}

#######################################
Expand All @@ -128,6 +130,7 @@ EOF
generate_id() {
# Use timestamp + random for uniqueness
echo "mem_$(date +%Y%m%d%H%M%S)_$(head -c 4 /dev/urandom | xxd -p)"
return 0
}

#######################################
Expand Down Expand Up @@ -389,6 +392,7 @@ FROM learnings
GROUP BY 1
ORDER BY 1;
EOF
return 0
}

#######################################
Expand Down Expand Up @@ -435,6 +439,7 @@ EOF
local db_size
db_size=$(du -h "$MEMORY_DB" | cut -f1)
log_info "Database size: $db_size"
return 0
}

#######################################
Expand Down Expand Up @@ -615,6 +620,7 @@ EXAMPLES:
# Clean up old unused entries
memory-helper.sh prune --older-than-days 60 --dry-run
EOF
return 0
}

#######################################
Expand Down
12 changes: 12 additions & 0 deletions .agent/scripts/ralph-upstream-check.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,24 @@ verbose=false

log_info() {
echo -e "${BLUE}[ralph-upstream]${NC} $1"
return 0
}

log_success() {
echo -e "${GREEN}[ralph-upstream]${NC} $1"
return 0
}

log_warning() {
echo -e "${YELLOW}[ralph-upstream]${NC} $1"
return 0
}

log_verbose() {
if [[ "$verbose" == "true" ]]; then
echo -e "${BLUE}[ralph-upstream]${NC} $1"
fi
return 0
}

# =============================================================================
Expand All @@ -63,6 +67,7 @@ log_verbose() {

ensure_cache_dir() {
mkdir -p "$CACHE_DIR"
return 0
}

is_cache_valid() {
Expand All @@ -86,12 +91,14 @@ read_cache() {
if [[ -f "$CACHE_FILE" ]]; then
cat "$CACHE_FILE"
fi
return 0
}

write_cache() {
local data="$1"
ensure_cache_dir
echo "$data" > "$CACHE_FILE"
return 0
}

# =============================================================================
Expand All @@ -115,13 +122,15 @@ get_file_sha() {
local filename="$2"

echo "$json" | jq -r ".[] | select(.name == \"$filename\") | .sha" 2>/dev/null || echo ""
return 0
}

fetch_file_content() {
local filename="$1"
local url="https://raw.githubusercontent.com/${UPSTREAM_REPO}/main/${UPSTREAM_PATH}/${filename}"

curl -sf "$url" 2>/dev/null || echo ""
return 0
}

# =============================================================================
Expand All @@ -132,6 +141,7 @@ extract_version() {
local content="$1"
# Try to extract version from plugin.json or package.json
echo "$content" | jq -r '.version // empty' 2>/dev/null || echo "unknown"
return 0
}

compare_implementations() {
Expand Down Expand Up @@ -189,6 +199,7 @@ compare_implementations() {
fi

echo "$changes_found"
return 0
}

# =============================================================================
Expand Down Expand Up @@ -283,6 +294,7 @@ DESCRIPTION:
UPSTREAM:
https://github.com/anthropics/claude-code/tree/main/plugins/ralph-wiggum
EOF
return 0
}

# =============================================================================
Expand Down
6 changes: 6 additions & 0 deletions .agent/scripts/session-review-helper.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,19 +41,22 @@ find_project_root() {
# Get current branch
get_branch() {
git branch --show-current 2>/dev/null || echo "not-a-git-repo"
return 0
}

# Check if on protected branch
is_protected_branch() {
local branch
branch=$(get_branch)
[[ "$branch" == "main" || "$branch" == "master" ]]
return 0
Copy link

@augmentcode augmentcode bot Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unconditional return 0 makes is_protected_branch always succeed, which breaks the workflow checks that branch on it (e.g., if is_protected_branch; then ...). Consider preserving the exit status of the [[ ... ]] test instead of forcing success.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

}
Comment on lines 47 to 53
Copy link
Contributor

@coderabbitai coderabbitai bot Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: return 0 breaks boolean function semantics.

The is_protected_branch function is used as a boolean predicate throughout the script (lines 136, 226, 297, 312). Adding return 0 after the [[ ]] comparison causes the function to always return success, making all callers think the branch is always protected.

For example:

  • Line 136: if is_protected_branch; then issues+="on-protected-branch," will always add the issue
  • Line 297: $(is_protected_branch && echo "true" || echo "false") will always echo "true"
🐛 Proposed fix - remove the return 0 from this predicate function 
 is_protected_branch() {
     local branch
     branch=$(get_branch)
     [[ "$branch" == "main" || "$branch" == "master" ]]
-    return 0
 }

Alternatively, if SonarCloud requires explicit returns, use the comparison's exit status:

 is_protected_branch() {
     local branch
     branch=$(get_branch)
-    [[ "$branch" == "main" || "$branch" == "master" ]]
-    return 0
+    if [[ "$branch" == "main" || "$branch" == "master" ]]; then
+        return 0
+    else
+        return 1
+    fi
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Check if on protected branch
is_protected_branch() {
local branch
branch=$(get_branch)
[[ "$branch" == "main" || "$branch" == "master" ]]
return 0
}
# Check if on protected branch
is_protected_branch() {
local branch
branch=$(get_branch)
[[ "$branch" == "main" || "$branch" == "master" ]]
}
🤖 Prompt for AI Agents 
In @.agent/scripts/session-review-helper.sh around lines 47 - 53, The
is_protected_branch predicate currently forces a success by unconditionally
using "return 0", so callers always treat the branch as protected; remove that
explicit "return 0" so the function returns the exit status of the [[ "$branch"
== "main" || "$branch" == "master" ]] test (or replace it with an explicit
"return $?" of that test) and keep the rest of the function (local branch;
branch=$(get_branch)) unchanged so callers like is_protected_branch used at
lines where it's evaluated behave correctly.


# Get recent commits
get_recent_commits() {
local count="${1:-10}"
git log --oneline -"$count" 2>/dev/null || echo "No commits"
return 0
}

# Get uncommitted changes count
Expand All @@ -62,6 +65,7 @@ get_uncommitted_changes() {
staged=$(git diff --cached --name-only 2>/dev/null | wc -l | tr -d '[:space:]')
unstaged=$(git diff --name-only 2>/dev/null | wc -l | tr -d '[:space:]')
echo "staged:${staged:-0},unstaged:${unstaged:-0}"
return 0
}

# Get TODO.md status
Expand Down Expand Up @@ -95,6 +99,7 @@ get_ralph_status() {
else
echo "active:false"
fi
return 0
}

# Get open PRs
Expand All @@ -110,6 +115,7 @@ get_pr_status() {
else
echo "gh-not-installed"
fi
return 0
}

# Check workflow adherence
Expand Down
Loading
Loading