Improve reliability of CI debugging scripts

[justin808]

Summary

This PR addresses the remaining reliability improvements from #1975 for the CI debugging scripts.

Changes made:

• ✅ Add bounds check for array access in bin/ci-run-failed-specs

  • Ensures the UNIQUE_SPECS array is not empty before accessing index 0
  • Prevents potential errors with set -u (unset variable checking)

• ✅ Document Ruby version requirement in bin/ci-switch-config

  • Clarifies that script/convert runs with the current Ruby installation (not the target version)
  • Documents Ruby 2.6+ compatibility requirement

Already fixed in previous commits:

• ✅ Removed eval from both scripts (uses case statement and array execution instead)
• ✅ Added dependency checks (gh, jq, bundle)
• ✅ Improved git restore error handling

These changes improve script reliability without changing functionality. All security concerns from the original issue have been addressed.

Test plan

• Verified bash syntax with bash -n
• All linting passes (bundle exec rubocop, yarn run eslint, yarn start format.listDifferent)
• Pre-commit and pre-push hooks pass
• Scripts maintain existing functionality

Closes #1975

🤖 Generated with Claude Code

[coderabbitai]

Warning

Rate limit exceeded

@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 14 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 39267e8 and 6b9de44.

📒 Files selected for processing (2)

• bin/ci-run-failed-specs (1 hunks)
• bin/ci-switch-config (1 hunks)

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch justin808/ci-scripts-improvements

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Code Review: CI Script Reliability Improvements

Summary

This PR addresses the remaining reliability improvements from issue #1975. The changes are focused, well-tested, and improve script robustness without altering functionality.

---

✅ Code Quality & Best Practices

Excellent improvements:

1. Array bounds check (bin/ci-run-failed-specs:142-149)

   • ✅ Properly checks array length before accessing index 0
   • ✅ Prevents potential errors with set -u (unset variable checking)
   • ✅ Nested conditionals are logically structured and easy to read

2. Documentation improvements (bin/ci-switch-config:257-261)

   • ✅ Clarifies Ruby version requirement (Ruby 2.6+)
   • ✅ Explains why script/convert runs with current Ruby (version manager hasn't reloaded yet)
   • ✅ Helpful for developers debugging version-related issues

Code style:

• Both scripts follow consistent bash best practices
• Proper use of set -euo pipefail for error handling
• Clear variable naming and logical flow

---

🔒 Security Assessment

Addressed from #1975:

• ✅ No use of eval with user input (already fixed in previous commits)
• ✅ Array execution instead of string evaluation
• ✅ Proper quoting of variables throughout

Current changes:

• ✅ No new security concerns introduced
• ✅ The bounds check actually prevents a potential crash scenario
• ✅ All user inputs are properly handled with array execution

Recommendation: No security concerns with this PR.

---

🐛 Potential Bugs/Issues

None identified. The changes are defensive improvements:

1. The array bounds check prevents crashes when UNIQUE_SPECS is empty (though line 121 already exits if empty, this adds defense in depth)
2. The Ruby version documentation prevents user confusion, not a bug fix

Edge cases handled:

• Empty array access: ✅ Protected
• Missing dependencies: ✅ Already checked earlier in scripts
• Git restore failures: ✅ Already handled with conditional in line 402

---

⚡ Performance Considerations

Impact: None. These are documentation and safety check additions.

• The array length check is O(1)
• No new external commands or I/O operations
• Scripts maintain their original performance characteristics

---

🧪 Test Coverage

From PR description:

• ✅ Bash syntax validated with bash -n
• ✅ All linting passes (RuboCop, ESLint, Prettier)
• ✅ Pre-commit and pre-push hooks pass
• ✅ Scripts maintain existing functionality

Suggested additional testing (optional):

• Manually test bin/ci-run-failed-specs with empty input to verify graceful handling
• Test bin/ci-switch-config with Ruby 2.5 vs 2.6+ to verify documentation accuracy

---

📋 Adherence to CLAUDE.md

Compliance check:

• ✅ Files end with newline characters
• ✅ No manual formatting (bash scripts don't require Prettier/RuboCop)
• ✅ Changes are user-visible improvements (reliability)
• ✅ PR properly references issue CI debugging tools: Security and reliability improvements #1975

---

💡 Suggestions (Optional Enhancements)

These are not blockers, just ideas for future improvements:

1. ShellCheck integration: Consider adding ShellCheck to CI for bash scripts

   shellcheck bin/ci-run-failed-specs bin/ci-switch-config

2. Error message enhancement: When bounds check prevents array access, could log why (though current silent handling is reasonable)

3. Version detection: Could detect current Ruby version and warn if < 2.6 before running script/convert

---

✨ Final Verdict

Recommendation: ✅ APPROVE

This PR successfully addresses the remaining items from #1975:

• Adds defensive programming (bounds check)
• Improves documentation (Ruby version requirement)
• No security, performance, or correctness concerns
• Well-tested and follows project conventions

The changes are minimal, focused, and improve script reliability. Great work on the systematic approach to hardening these developer tools!

---

Minor note: Both changed files properly handle the edge cases they address. The git restore error handling on line 402 of bin/ci-switch-config is already adequate with the current warning message.

🤖 Review generated by Claude Code