Improve release script OTP handling and retry logic

[justin808]

Summary

• Add retry logic for gem publication with OTP failures
• Add 5-second delay between gem publications to prevent OTP code reuse
• Improve error messages with actionable guidance
• Create ReleaseHelpers module to reduce code nesting

Problem

The release script was failing when publishing Ruby gems because OTP codes were being reused or expiring between NPM and RubyGems publications. The script would fail with "Your OTP code is incorrect" after successfully publishing NPM packages.

Solution

1. Retry logic: Added up to 3 automatic retries for each gem publication with clear error messages
2. OTP separation: Added explicit 5-second delay between gem publications to prevent code reuse
3. User guidance: Added clear prompts reminding users to generate fresh OTP codes for each publication
4. Code quality: Created ReleaseHelpers module to reduce block nesting and improve maintainability

Changes

• Created ReleaseHelpers.publish_gem_with_retry helper method
• Added retry logic for both react_on_rails and react_on_rails_pro gems
• Added 5-second delay between gem publications
• Improved error messages with actionable guidance
• Maintained RuboCop compliance (no block nesting violations)

Benefits

• More robust release process that handles transient OTP failures
• Clear user guidance on generating fresh OTP codes
• Automatic retry on failure reduces manual intervention
• Better separation between OTP-requiring operations

Test Plan

• RuboCop passes
• Pre-commit hooks pass
• Test next release to verify OTP handling improvements

🤖 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 21 minutes and 35 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 7dfd90b and db8d715.

📒 Files selected for processing (1)

• rakelib/release.rake (2 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/fix-release-otp-timing

---

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 - PR #2045

Summary

This PR improves the reliability of the gem release process by adding retry logic for OTP failures and timing separation between gem publications. Overall, this is a solid improvement that addresses a real pain point in the release workflow.

✅ Strengths

1. Clear Problem Statement: The PR effectively addresses a concrete issue with OTP code reuse/expiration during gem releases
2. Good Modularization: Extracting ReleaseHelpers.publish_gem_with_retry reduces code duplication and improves maintainability
3. User Experience: Clear, actionable error messages guide users through retry scenarios
4. RuboCop Compliance: The refactoring successfully avoids block nesting violations

🔍 Code Quality Observations

1. Exception Handling Scope (Minor Issue)

Location: rakelib/release.rake:18-32

The current implementation catches StandardError, which is quite broad. Consider being more specific:

rescue Gem::CommandException, IOError => e
  # Handle gem-specific and network errors

Reasoning: StandardError will catch a wide range of errors (including things like NoMethodError, TypeError, etc.) that shouldn't trigger a retry. Gem release failures are typically:

• Gem::CommandException for gem-specific errors (including OTP failures)
• IOError or network-related errors

Impact: Low - unlikely to cause issues in practice, but more precise exception handling is a best practice.

2. OTP Timing Assumptions

Location: rakelib/release.rake:29-35

The 5-second delay between gem publications (line 270 in the full file) is a good start, but the retry delay is only 3 seconds. Consider:

# Current:
sleep 3  # Before retry

# Suggested:
sleep 5  # Match the inter-publication delay

Reasoning: OTP codes typically have 30-second windows. A 3-second delay might be too short if the issue is an expiring code rather than a network glitch. Consistency with the 5-second inter-publication delay would be clearer.

3. Module Function Declaration

Location: rakelib/release.rake:48

Using module_function after method definition works, but declaring it before is more conventional:

module ReleaseHelpers
  include ReactOnRails::TaskHelpers
  module_function

  def publish_gem_with_retry(dir, gem_name, max_retries: 3)
    # ...
  end
end

Impact: None functionally, just style preference.

4. Hardcoded Retry Count

Location: rakelib/release.rake:21

The max_retries: 3 default is reasonable, but consider making it configurable via environment variable for edge cases:

def publish_gem_with_retry(dir, gem_name, max_retries: ENV.fetch('GEM_RELEASE_MAX_RETRIES', '3').to_i)

Impact: Low priority - current default is fine for most cases.

🧪 Testing Considerations

Missing Test Coverage: The release rake task has no automated tests. While this is understandable given its nature (interacts with external services), consider:

1. Unit tests for the helper module: Mock sh_in_dir to verify retry logic
2. Integration tests with dry-run mode: Verify the flow without actually publishing

Example test structure:

# spec/tasks/release_rake_spec.rb
RSpec.describe 'release rake task' do
  describe ReleaseHelpers do
    describe '.publish_gem_with_retry' do
      it 'retries on failure' do
        # Mock sh_in_dir to fail twice, then succeed
      end
      
      it 'raises after max retries' do
        # Mock sh_in_dir to always fail
      end
    end
  end
end

Priority: Medium - This would prevent regressions in the retry logic.

🔒 Security Considerations

✅ No security concerns identified. The code:

• Doesn't expose OTP codes
• Uses safe shell execution via existing sh_in_dir helper
• Doesn't introduce new dependencies

⚡ Performance Considerations

The added delays (5 seconds between publications, 3 seconds between retries) are intentional and appropriate. Total worst-case delay:

• First gem: 0s + (2 retries × 3s) = ~6s
• Delay: 5s
• Second gem: 0s + (2 retries × 3s) = ~6s
• Total worst case: ~17 seconds

This is acceptable for a release process that's not time-critical.

📝 Documentation & Changelog

Good: The PR body is comprehensive and includes a test plan.

Suggestion: Since this improves the developer experience for maintainers (not end users), no CHANGELOG.md entry is needed per the project's guidelines:

Do NOT add entries for: linting, formatting, refactoring, tests, or documentation fixes

However, consider adding a comment in the code explaining the 5-second delay rationale:

# Add delay before next OTP operation to ensure:
# 1. Previous OTP code is fully consumed by RubyGems
# 2. User has time to generate a fresh OTP code
# OTP codes typically expire after 30 seconds
puts "\n⏳ Waiting 5 seconds before next publication to ensure OTP separation..."
sleep 5

🎯 Recommendations

Priority: High

• ✅ Approve and merge as-is - the improvements outweigh the minor issues

Priority: Medium (Follow-up PR or future improvement)

1. Narrow exception handling to gem-specific errors
2. Increase retry delay from 3s to 5s for consistency
3. Add unit tests for the retry logic

Priority: Low

1. Make retry count configurable via ENV var
2. Add inline comment explaining OTP timing rationale

✨ Final Verdict

APPROVED ✅

This PR successfully addresses a real pain point in the release process with a clean, maintainable solution. The retry logic and timing improvements will make releases more reliable. The minor suggestions above are nice-to-haves but not blockers.

Great work on improving the developer experience! 🚀

---

Review generated with attention to the React on Rails CLAUDE.md guidelines

[claude]

Code Review: Improve release script OTP handling and retry logic

Summary

This PR adds retry logic and timing improvements to handle OTP failures during gem publication. The changes are well-structured and address a real pain point in the release process.

---

✅ Strengths

1. Excellent Code Organization: Creating the ReleaseHelpers module is a good architectural decision that reduces nesting and improves maintainability.

2. Clear User Guidance: The error messages and prompts provide actionable guidance (e.g., "Generate a FRESH OTP code").

3. Appropriate Retry Strategy: Using 3 retries with a 3-second delay is reasonable for OTP-related failures.

4. Good Separation of Concerns: The 5-second delay between gem publications helps prevent OTP code reuse issues.

---

🔍 Potential Issues & Suggestions

1. Broad Exception Handling (Medium Priority)

Issue: Line 32 catches StandardError, which is very broad and could mask unexpected errors.

rescue StandardError => e

Recommendation: Consider catching more specific exceptions related to gem publication failures. The gem-release gem typically raises specific errors. You might want to:

• Only retry on OTP-specific failures (if identifiable)
• Let other errors (network, permissions, etc.) fail fast
• Add logging to distinguish between error types

Example:

rescue StandardError => e
  retry_count += 1
  # Log the actual error for debugging
  puts "Error: #{e.class} - #{e.message}"
  
  if retry_count < max_retries && otp_related_error?(e)
    # retry logic
  else
    raise e
  end
end

2. Retry Sleep Timing Inconsistency (Low Priority)

Issue: There's a 3-second delay between retries (line 40) but a 5-second delay between gem publications (line 271).

Recommendation: Consider if the retry delay should match the inter-publication delay for consistency, or document why they differ.

3. User Experience: Prompt Before Retry (Low Priority)

Issue: The script automatically retries without user confirmation. If an OTP fails, the user might need a moment to prepare a fresh code.

Recommendation: Consider prompting the user before retrying:

puts "\nPress ENTER to retry with a fresh OTP code, or Ctrl+C to cancel..."
$stdin.gets

This gives users time to generate a new OTP code rather than rushing them.

4. Testing Considerations

Observation: The PR mentions testing will happen during the next release.

Recommendation: Consider adding:

• A dry-run test that mocks the gem release command
• Unit tests for the ReleaseHelpers module
• Documentation of manual testing procedure

---

🛡️ Security Considerations

✅ No security issues identified. The code handles OTP codes appropriately without logging sensitive information.

---

⚡ Performance Considerations

✅ No significant performance concerns. The delays (3s, 5s) are appropriate for OTP operations and won't meaningfully impact release time.

---

🧪 Test Coverage

Gap: No automated tests for the new ReleaseHelpers module.

Recommendation: Add specs in spec/ directory:

# spec/rake/release_helpers_spec.rb
RSpec.describe ReleaseHelpers do
  describe '.publish_gem_with_retry' do
    # Test successful publication
    # Test retry on failure
    # Test max retries exceeded
  end
end

---

📝 Code Style & Best Practices

✅ Follows Ruby conventions: Good use of module_function, clear variable names, appropriate use of symbols.

✅ Consistent with codebase: Matches the existing style in release.rake and uses the TaskHelpers pattern.

⚠️ Minor: Line 39 says "Generating a FRESH OTP code" but the script doesn't actually generate it - the user does. Consider: "Please generate a FRESH OTP code and have it ready. Retrying in 3 seconds..."

---

🎯 Overall Assessment

Rating: ✅ Approve with minor suggestions

This is a solid improvement to the release process that addresses a real pain point. The code is well-structured and maintainable. The suggestions above are mostly about error handling specificity and user experience polish - none are blockers.

Recommended next steps:

1. Consider more specific exception handling (optional but recommended)
2. Add user prompt before retries (optional but improves UX)
3. Add automated tests when feasible
4. Test during next actual release as planned

Great work on improving the developer experience! 🎉

---

Review performed by Claude Code following React on Rails conventions from CLAUDE.md

[claude]

Code Review

I've reviewed PR #2045 and have the following feedback:

✅ Strengths

1. Well-motivated change: The retry logic addresses a real pain point with OTP expiration/reuse during gem publication
2. Good user guidance: Clear messaging about generating fresh OTP codes helps prevent user confusion
3. RuboCop compliance: Creating the ReleaseHelpers module reduces block nesting and maintains style compliance
4. Consistent approach: Both gem publications now use the same retry mechanism

🐛 Potential Issues

1. Exception Handling Verification Needed

begin
  sh_in_dir(dir, "gem release")
  success = true
rescue StandardError => e
  # ...
end

Analysis: The sh_in_dir method uses Rake's sh command. In Ruby, RuntimeError inherits from StandardError, so catching StandardError should work. However, worth verifying during testing.

Recommendation: Test manually during next release, or add a comment confirming this catches Rake failures.

2. Missing Return Value

The method doesn't explicitly return a value. While success is set, callers can't programmatically verify publication succeeded.

Suggestion: Add explicit return at the end: success

3. Retry Delay May Be Too Short

• Current retry delay: 3 seconds
• Time needed: see error, open authenticator app, generate new OTP, enter it

Recommendation: Consider increasing to 5 seconds to give users adequate time.

🎯 Code Quality Observations

1. ✅ Module structure: module_function :publish_gem_with_retry is correctly used
2. ✅ Separation of concerns: 5-second inter-publication delay is at the right abstraction level
3. ✅ User experience: Emoji usage improves readability
4. ✅ TaskHelpers inclusion: Properly included for sh_in_dir access

🧪 Testing Concerns

Observation: No tests exist for the release script. The retry logic involves exception handling, conditional branches, timing, and external commands.

Recommendation: At minimum, manual testing during the next release to verify:

• Retry triggers on OTP failure
• Success on subsequent retry
• Failure after max retries

🔒 Security & Performance

• ✅ No security issues. OTP codes remain securely handled through interactive user input
• ✅ Acceptable overhead: 5-second delay + worst case ~18 seconds on retry failures

📋 Suggestions

1. Add explanatory comment for the 5-second delay
2. Consider increasing retry delay from 3 to 5 seconds for better UX
3. Add brief method documentation
4. Consistent messaging: Emphasize "FRESH code" in both OTP prompts

✅ Verdict

APPROVE with minor suggestions

The PR effectively solves a real problem. The implementation is sound and follows repository conventions.

Why approve:

• ✅ Addresses genuine pain point
• ✅ Clean, maintainable code
• ✅ RuboCop compliant
• ✅ Good user guidance

Suggested follow-ups (non-blocking):

1. Increase retry delay to 5 seconds
2. Add explicit return value
3. Verify during next actual release

The code is ready to merge. 🚀