Quality improvements: Bun migration, code quality tooling, documentation consolidation #3
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,4 +18,4 @@ | |
| "deny": [], | ||
| "ask": [] | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,13 +5,11 @@ | |
| status.json | ||
|
|
||
| # Logs | ||
| logs/ | ||
| *.log | ||
|
|
||
| # Generated documentation | ||
| docs/generated/ | ||
|
|
||
| # OS files | ||
| .DS_Store | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,9 @@ | ||||||
| { | ||||||
| "default": true, | ||||||
| "MD013": false, | ||||||
| "MD024": false, | ||||||
|
Contributor
There was a problem hiding this comment. Remove trailing whitespace. Line 4 has trailing whitespace after the comma, which is inconsistent with line 3. 🔎 Proposed fix- "MD024": false,
+ "MD024": false,📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| "MD033": false, | ||||||
| "MD036": false, | ||||||
| "MD041": false, | ||||||
| "MD051": false | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| node_modules/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| # ShellCheck configuration for Ralph | ||
|
|
||
| # === Production Code === | ||
|
|
||
| # SC2016 - Expressions don't expand in single quotes | ||
| # (Intentional in sed patterns and echo statements for shell config) | ||
| disable=SC2016 | ||
|
|
||
| # SC2155 - Declare and assign separately to avoid masking return values | ||
| # (Consistent style choice throughout codebase - failures caught by error handling) | ||
| disable=SC2155 | ||
|
Comment on lines
+9
to
+11
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Step 1: Find and examine .shellcheckrc
find . -name ".shellcheckrc" -type f 2>/dev/null | head -20Repository: frankbria/ralph-claude-code Length of output: 86 🏁 Script executed: #!/bin/bash
# Step 2: Read the .shellcheckrc file
cat -n .shellcheckrcRepository: frankbria/ralph-claude-code Length of output: 2184 🏁 Script executed: #!/bin/bash
# Step 3: Find all shell scripts in the repository
git ls-files | grep -E '\.(sh|bash)$' | head -30Repository: frankbria/ralph-claude-code Length of output: 285 🏁 Script executed: #!/bin/bash
# Step 4: Search for local/declare with command substitution patterns
# This is the pattern SC2155 would catch: local var=$(command)
echo "=== Searching for local/declare with command substitution ==="
rg -n 'local\s+\w+\s*=\s*\$\(|declare\s+\w+\s*=\s*\$\(' --type bash --type sh
echo ""
echo "=== Checking for set -e (error handling) ==="
rg -l 'set -e|set -o errexit' --type bash --type shRepository: frankbria/ralph-claude-code Length of output: 253 🏁 Script executed: #!/bin/bash
# Step 4: Search for local/declare with command substitution patterns
# This is the pattern SC2155 would catch: local var=$(command)
echo "=== Searching for local/declare with command substitution ==="
rg -n 'local\s+\w+\s*=\s*\$\(|declare\s+\w+\s*=\s*\$\(' --type sh
echo ""
echo "=== Checking for set -e (error handling) ==="
rg -l 'set -e|set -o errexit' --type sh
echo ""
echo "=== Looking at specific script files for SC2155 patterns ==="
for file in create_files.sh install.sh lib/circuit_breaker.sh lib/response_analyzer.sh ralph_import.sh ralph_loop.sh ralph_monitor.sh setup.sh; do
if [ -f "$file" ]; then
echo "--- $file ---"
grep -n 'local.*=.*$(' "$file" | head -10
fi
doneRepository: frankbria/ralph-claude-code Length of output: 12202 🏁 Script executed: #!/bin/bash
# Step 5: Check if set -e is truly at the start of scripts with SC2155 patterns
echo "=== Checking head of ralph_loop.sh (has most SC2155 patterns) ==="
head -20 ralph_loop.sh
echo ""
echo "=== Checking head of lib/circuit_breaker.sh ==="
head -20 lib/circuit_breaker.sh
echo ""
echo "=== Checking head of lib/response_analyzer.sh ==="
head -20 lib/response_analyzer.sh
echo ""
echo "=== Checking for critical patterns: local var=$(failing_command) without error handling ==="
# Look for patterns where the command substitution could actually fail
echo "Lines with local + cat/grep/jq without explicit error handling:"
rg -n 'local\s+\w+\s*=\s*\$\((cat|grep|jq|git|curl|wget)' --type sh | grep -v '2>/dev/null' | grep -v '||' | head -20Repository: frankbria/ralph-claude-code Length of output: 4523 🏁 Script executed: #!/bin/bash
# Step 6: Check if the scripts use pipefail and examine error handling patterns
echo "=== Check for set -o pipefail ==="
rg -n 'set -o pipefail' --type sh
echo ""
echo "=== Check if specific files with SC2155 patterns have error handling ==="
echo "Examining lib/circuit_breaker.sh around the cat command on line 94:"
sed -n '90,110p' lib/circuit_breaker.sh
echo ""
echo "=== Examining lib/response_analyzer.sh around line 45 (output_content) ==="
sed -n '40,60p' lib/response_analyzer.sh
echo ""
echo "=== Examining lib/response_analyzer.sh around line 184 (jq commands) ==="
sed -n '178,200p' lib/response_analyzer.sh
echo ""
echo "=== Check what happens if cat/jq fails in these contexts ==="
echo "Looking at usage of status_data in circuit_breaker.sh:"
sed -n '94,105p' lib/circuit_breaker.shRepository: frankbria/ralph-claude-code Length of output: 3530 🏁 Script executed: #!/bin/bash
# Step 7: Verify set -e is in the library scripts too
echo "=== Check for set -e in lib/circuit_breaker.sh ==="
head -25 lib/circuit_breaker.sh | grep -n 'set -e'
echo ""
echo "=== Check for set -e in lib/response_analyzer.sh ==="
head -25 lib/response_analyzer.sh | grep -n 'set -e'
echo ""
echo "=== Count unprotected cat/grep/jq patterns ==="
echo "Patterns with NO 2>/dev/null fallback or file check preceding:"
rg -n 'local\s+\w+\s*=\s*\$\((cat|grep|jq)' --type sh | wc -l
echo ""
echo "=== Count patterns WITH protection (2>/dev/null or ||) ==="
rg -n 'local\s+\w+\s*=\s*\$\((cat|grep|jq).*\(2>/dev/null\||\|\|\)' --type sh | wc -lRepository: frankbria/ralph-claude-code Length of output: 351 🏁 Script executed: #!/bin/bash
# Step 8: Verify how these sourced libraries are called and whether set -e protects them
echo "=== Verify: Do library functions return? ==="
echo "Checking if functions in circuit_breaker.sh return or exit on error:"
grep -n 'return' lib/circuit_breaker.sh | head -10
echo ""
echo "=== Check if any functions use trap or explicit error handling ==="
grep -n 'trap\|return.*\$?\|if.*\$?' lib/circuit_breaker.sh | head -5
echo ""
echo "=== Check specific unprotected pattern: what happens if file missing ==="
echo "Example: if CB_STATE_FILE doesn't exist, cat fails. Does set -e catch it?"
echo ""
echo "Testing scenario in lib/circuit_breaker.sh line 94:"
sed -n '85,110p' lib/circuit_breaker.sh
echo ""
echo "=== Check if CB_STATE_FILE is guaranteed to exist ==="
grep -n 'CB_STATE_FILE' lib/circuit_breaker.sh | head -20Repository: frankbria/ralph-claude-code Length of output: 2259 🏁 Script executed: #!/bin/bash
# Step 9: Verify the real danger - what happens with set -e and subshell failures
echo "=== Demonstrating the SC2155 risk with set -e ==="
cat > /tmp/test_sc2155.sh << 'EOF'
#!/bin/bash
set -e
echo "Test 1: set -e with local and failed command substitution"
echo "This should demonstrate the risk..."
# This is what the code does now (with set -e)
local result=$(false) # Command fails, local succeeds
echo "After failed command: result='$result'"
# Would only exit when we use the variable
echo "Script didn't exit yet - this shows the risk!"
EOF
echo "File shows: local var=\$(command) masks failure until variable is used later"
echo ""
echo "=== Checking if functions are sourced (meaning set -e doesn't fully protect them) ==="
grep -n 'source.*circuit_breaker\|source.*response_analyzer' ralph_loop.shRepository: frankbria/ralph-claude-code Length of output: 479 🏁 Script executed: #!/bin/bash
# Step 10: Final verification - confirm actual behavior with sourced functions
echo "=== Verify error propagation in sourced functions with set -e ==="
cat > /tmp/test_source.sh << 'EOF'
#!/bin/bash
set -e
source /tmp/lib.sh
test_func
echo "This should not print"
EOF
cat > /tmp/lib.sh << 'EOF'
#!/bin/bash
test_func() {
local data=$(cat /nonexistent/file.txt)
echo "data: $data"
}
EOF
echo "Sourced function test:"
bash /tmp/test_source.sh 2>&1 | head -5 || true
echo ""
echo "=== Summary: Check comment in .shellcheckrc matches reality ==="
sed -n '9,11p' .shellcheckrcRepository: frankbria/ralph-claude-code Length of output: 478 Reconsider disabling SC2155 – the risk is real. Disabling SC2155 masks return value failures from command substitution. Testing confirms that The codebase contains 34 unprotected instances of this pattern (e.g., Refactor these patterns to declare and assign separately, or add explicit error handling (file guards, 🤖 Prompt for AI Agents |
||
|
|
||
| # SC2162 - read without -r | ||
| # (Acceptable for simple user input) | ||
| disable=SC2162 | ||
|
|
||
| # SC2012 - Use find instead of ls | ||
| # (ls is acceptable for simple time-sorted listings) | ||
| disable=SC2012 | ||
|
|
||
| # SC2035 - Use ./*glob* | ||
| # (chmod is safe in our context) | ||
| disable=SC2035 | ||
|
|
||
| # === Test Code === | ||
|
|
||
| # SC1091 - Not following sourced files | ||
| # (Test helpers are sourced dynamically, paths are correct) | ||
| disable=SC1091 | ||
|
|
||
| # SC2030 - Modification of variable is local | ||
| # SC2031 - Variable was modified in a subshell | ||
| # (Expected behavior in test subshells and setup/teardown) | ||
| disable=SC2030 | ||
| disable=SC2031 | ||
|
|
||
| # SC2034 - Variable appears unused | ||
| # (Test fixtures and mocks define variables for external use) | ||
| disable=SC2034 | ||
|
|
||
| # SC2086 - Double quote to prevent globbing and word splitting | ||
| # (Intentional in test assertions and mock functions) | ||
| disable=SC2086 | ||
|
|
||
| # SC2154 - Variable is referenced but not assigned | ||
| # (BATS framework provides variables like $status, $output, $lines) | ||
| disable=SC2154 | ||
|
|
||
| # SC2164 - Use 'cd ... || exit' or 'cd ... || return' | ||
| # (Test code uses cd within controlled teardown context) | ||
| disable=SC2164 | ||
|
Comment on lines
+49
to
+51
Contributor
There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: git ls-files | head -20Repository: frankbria/ralph-claude-code Length of output: 505 🏁 Script executed: cat -n .shellcheckrcRepository: frankbria/ralph-claude-code Length of output: 2184 🏁 Script executed: fd -type f -name "*.sh" | head -20Repository: frankbria/ralph-claude-code Length of output: 241 🏁 Script executed: fd --type f -name "*.bats"Repository: frankbria/ralph-claude-code Length of output: 301 🏁 Script executed: fd --type f -name "test_*.sh"Repository: frankbria/ralph-claude-code Length of output: 301 🏁 Script executed: find . -type d -name "test*" -o -type d -name "*test*" | head -10Repository: frankbria/ralph-claude-code Length of output: 78 🏁 Script executed: find ./tests -type fRepository: frankbria/ralph-claude-code Length of output: 321 🏁 Script executed: rg "cd\s+" ./tests -A 2 -B 2Repository: frankbria/ralph-claude-code Length of output: 3386 🏁 Script executed: rg "teardown|cleanup" ./testsRepository: frankbria/ralph-claude-code Length of output: 375 🏁 Script executed: cat -n ./tests/helpers/test_helper.bashRepository: frankbria/ralph-claude-code Length of output: 6135 🏁 Script executed: cat -n ./tests/unit/test_exit_detection.batsRepository: frankbria/ralph-claude-code Length of output: 10334 Consider narrowing focus of SC2164 disable to setup functions. The disable currently applies to all test code, but the actual risk is in setup functions where In contrast, teardown functions using 🤖 Prompt for AI Agents |
||
|
|
||
| # SC2329 - Function is never invoked | ||
| # (Mock functions are invoked indirectly by test framework) | ||
| disable=SC2329 | ||
|
|
||
| # SC2005 - Useless echo? Instead of 'echo $(cmd)', just use 'cmd' | ||
| # (Style preference in test fixtures for clarity) | ||
| disable=SC2005 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update actions/checkout to v4.
The v3 runner is deprecated and too old for current GitHub Actions. Update to v4 for continued compatibility.
🔎 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 actionlint (1.7.9)
14-14: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🤖 Prompt for AI Agents