feat: Add automatic license renewal for gem and Node Renderer#2254
feat: Add automatic license renewal for gem and Node Renderer#2254
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces automatic license renewal for React on Rails Pro by adding persistent token caching, API-based license fetching with retry logic, intelligent refresh scheduling based on expiry dates, and async validation integration across both TypeScript and Ruby implementations with comprehensive test coverage. Changes
Sequence Diagram(s)sequenceDiagram
participant App as App/Renderer
participant Validator as LicenseValidator
participant Refresher as LicenseRefreshChecker
participant Fetcher as LicenseFetcher
participant Cache as LicenseCache
participant API as License API
participant Env as ENV/Config
App->>Validator: getValidatedLicenseData()
activate Validator
Validator->>Refresher: maybeRefreshLicense()
activate Refresher
Refresher->>Refresher: shouldCheckForRefresh()?
alt Refresh Needed
Refresher->>Fetcher: fetchLicense()
activate Fetcher
Fetcher->>Env: Get license_key
Fetcher->>API: GET /api/license<br/>(5s timeout, retries)
alt Success (200)
API-->>Fetcher: {token, expires_at}
Fetcher-->>Refresher: LicenseResponse
else Failure
API-->>Fetcher: Error/Timeout
Fetcher-->>Refresher: null
end
deactivate Fetcher
alt Token Received
Refresher->>Cache: writeCache({token, expires_at})
activate Cache
Cache->>Cache: Compute license_key_hash
Cache->>Env: Write to disk (0600 perms)
deactivate Cache
end
end
Refresher-->>Validator: (void)
deactivate Refresher
rect rgb(200, 220, 255)
Note over Validator: Load & Validate License
Validator->>Cache: getCachedToken()<br/>(if auto-refresh enabled)
alt Cache Hit
Cache-->>Validator: token
else Cache Miss
Validator->>Env: Read ENV/Config file
Env-->>Validator: token
end
end
Validator->>Validator: decode & validate JWT
Validator->>Cache: writeCache() for seeding
Validator-->>App: LicenseData
deactivate Validator
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Code Review: Automatic License Renewal FeatureThanks for this comprehensive implementation! This is a well-thought-out feature with solid test coverage. Here's my detailed review: ✅ Strengths1. Excellent Architecture
2. Strong Test Coverage
3. Security Considerations
4. Good User Experience
🔍 Issues & RecommendationsCRITICAL: Security - License Key StorageRuby gem ( "Authorization" => "Bearer #{license_key}"Potential Issue: The license_key is used directly in HTTP headers. If logging middleware captures request headers, the permanent license key could be logged. Recommendation:
HIGH: Cache File Location - Ephemeral StorageRuby ( def cache_dir
Rails.root.join("tmp")
endIssue: On cloud platforms (Heroku, AWS Elastic Beanstalk, etc.), the Impact:
Recommendation: # Use a persistent location in production
def cache_dir
if Rails.env.production? && ENV['REACT_ON_RAILS_PRO_LICENSE_CACHE_DIR']
Pathname.new(ENV['REACT_ON_RAILS_PRO_LICENSE_CACHE_DIR'])
else
Rails.root.join("tmp")
end
endOr document clearly that users should configure persistent storage for the cache in production. MEDIUM: Race Condition in Cache SeedingRuby ( def seed_cache_if_needed(license_data)
return unless ReactOnRailsPro.configuration.auto_refresh_enabled?
return if LicenseCache.token.present? # Cache already exists
token = load_token_from_env_or_file
# ... write cache
endIssue: In multi-process deployments (Puma, Passenger), multiple workers could simultaneously check Impact:
Recommendation: # Add file locking
File.open(cache_path, File::RDWR|File::CREAT, 0600) do |file|
file.flock(File::LOCK_EX)
# Check again after acquiring lock
return if LicenseCache.token.present?
# ... write cache
endMEDIUM: Error Handling - Missing ValidationRuby ( Rails.logger.debug { "[ReactOnRailsPro] License fetched successfully" }
JSON.parse(response.body.to_s)Issue: No validation that the response contains required fields ( Impact: If API response format changes, invalid data could be cached. Recommendation: data = JSON.parse(response.body.to_s)
return nil unless data.is_a?(Hash) && data["token"].present? && data["expires_at"].present?
dataMEDIUM: TypeScript - Async Timing IssueTypeScript ( The refresh check in Question: How does the async refresh integrate with startup validation? If the app starts while a refresh is in-progress, does it use stale cache? Recommendation:
LOW: Code Quality - Magic NumbersRuby ( days_until_expiry = ((expires_at - Time.now) / 1.day).to_iNote: Uses DAILY_CHECK_THRESHOLD_DAYS = 7
WEEKLY_CHECK_THRESHOLD_DAYS = 30
if days_until_expiry <= DAILY_CHECK_THRESHOLD_DAYS
# ...Improves readability and makes thresholds configurable in the future. LOW: Documentation - Missing API ContractThe PR description mentions "Requires licensing app PR with /api/license endpoint" but: Missing:
Recommendation: Add API contract documentation so users understand the endpoint behavior. 🧪 Testing ObservationsGood Coverage:
Missing Tests:
📝 Documentation FeedbackLICENSE_SETUP.md is excellent! Clear distinction between license_key and license_token. Suggestions:
🎯 Summary
Overall: This is solid work with good architecture and test coverage. The critical and high issues are configuration/documentation problems rather than code bugs, so they're easy to address. Recommendation: ✅ Approve with changes - address the cache storage location and add documentation about logging risks before merging. 🚀 Pre-Merge ChecklistBefore merging, verify:
Great work on this feature! 🎉 |
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts (1)
11-21: Consider consolidating mock setup.The auto-refresh modules are mocked twice: once at the top level (lines 11-21) and again in
beforeEach(lines 83-93). While this is necessary due tojest.resetModules(), consider extracting the mock definitions to a helper function to reduce duplication and ensure consistency.🔎 Proposed refactor to reduce duplication
+function setupAutoRefreshMocks() { + jest.doMock('../src/shared/licenseFetcher', () => ({ + isAutoRefreshEnabled: jest.fn().mockReturnValue(false), + })); + jest.doMock('../src/shared/licenseCache', () => ({ + getCachedToken: jest.fn().mockReturnValue(null), + })); + jest.doMock('../src/shared/licenseRefreshChecker', () => ({ + maybeRefreshLicense: jest.fn().mockResolvedValue(undefined), + seedCacheIfNeeded: jest.fn(), + })); +} + // Mock auto-refresh modules to prevent side effects -jest.mock('../src/shared/licenseFetcher', () => ({ - isAutoRefreshEnabled: jest.fn().mockReturnValue(false), -})); -jest.mock('../src/shared/licenseCache', () => ({ - getCachedToken: jest.fn().mockReturnValue(null), -})); -jest.mock('../src/shared/licenseRefreshChecker', () => ({ - maybeRefreshLicense: jest.fn().mockResolvedValue(undefined), - seedCacheIfNeeded: jest.fn(), -})); +setupAutoRefreshMocks(); // ... in beforeEach ... // Re-mock auto-refresh modules after resetModules - jest.doMock('../src/shared/licenseFetcher', () => ({ - isAutoRefreshEnabled: jest.fn().mockReturnValue(false), - })); - jest.doMock('../src/shared/licenseCache', () => ({ - getCachedToken: jest.fn().mockReturnValue(null), - })); - jest.doMock('../src/shared/licenseRefreshChecker', () => ({ - maybeRefreshLicense: jest.fn().mockResolvedValue(undefined), - seedCacheIfNeeded: jest.fn(), - })); + setupAutoRefreshMocks();Also applies to: 83-93
react_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb (1)
28-29: Non-200 responses silently return nil.The implementation returns
nilfor all non-200 status codes (401, 403, 404, 500, etc.) without distinguishing between client errors (4xx) and server errors (5xx). While this provides a graceful fallback, it may make debugging difficult.Consider logging the status code for non-200 responses to aid troubleshooting.
🔎 Proposed enhancement for debugging
response = HTTPX .plugin(:retries, max_retries: MAX_RETRIES, retry_after: RETRY_DELAY_SECONDS) .with(timeout: { request_timeout: REQUEST_TIMEOUT_SECONDS }) .with(headers: { "Authorization" => "Bearer #{license_key}", "User-Agent" => "ReactOnRailsPro-Gem" }) .get("#{api_url}/api/license") return nil if response.is_a?(HTTPX::ErrorResponse) - return nil unless response.status == 200 + unless response.status == 200 + Rails.logger.warn { "[ReactOnRailsPro] License fetch failed: HTTP #{response.status}" } + return nil + end Rails.logger.debug { "[ReactOnRailsPro] License fetched successfully" } JSON.parse(response.body.to_s)packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts (1)
89-120: Cache-first loading may mask ENV/file changes.When auto-refresh is enabled, the cache takes precedence over ENV and config file (lines 91-96). This means if a user updates
REACT_ON_RAILS_PRO_LICENSEor the config file, the cached token will still be used until it expires or the cache is cleared.Consider documenting this behavior, or provide a way to force cache invalidation (e.g., deleting the cache file) for users who need to switch licenses immediately.
react_on_rails_pro/lib/react_on_rails_pro/license_cache.rb (1)
35-36: Consider atomic file write with secure permissions.There's a small race window between
File.writeandFile.chmodwhere the file could be readable by others (depending on umask). For a security-sensitive license file, consider using atomic write with initial permissions.🔎 Proposed fix for atomic write with secure permissions
- File.write(cache_path, JSON.pretty_generate(cache_data)) - File.chmod(0o600, cache_path) + require "tempfile" + Tempfile.create(["license_cache", ".tmp"], cache_dir, mode: 0o600) do |temp| + temp.write(JSON.pretty_generate(cache_data)) + temp.close + FileUtils.mv(temp.path, cache_path) + endreact_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
65-68: Track the TODO for production URL update.There's a TODO comment indicating the licensing app URL may need updating. Ensure this is addressed before the final release.
Do you want me to open an issue to track updating the license API URL once the licensing app is deployed to production?
packages/react-on-rails-pro-node-renderer/src/shared/licenseCache.ts (2)
150-161: Consider caching the result to avoid redundant file reads.
getFetchedAt()andgetExpiresAt()each callreadCache()independently. If called together (which seems likely), this results in multiple file reads. Consider either exposingreadCache()for callers to use directly, or caching the result.Additionally, the empty
catchblock on lines 158-159 silently swallows date parsing errors. While returningnullis reasonable, logging might help debugging.
128-131: Consider atomic write with secure permissions.Similar to the Ruby implementation, there's a small race window between
writeFileSyncandchmodSync. For improved security, consider writing to a temp file first and then renaming.🔎 Proposed fix for atomic write
+import * as os from 'os'; ... - fs.writeFileSync(cachePath, JSON.stringify(cacheData, null, 2)); - - // Set file permissions to 0600 (owner read/write only) - fs.chmodSync(cachePath, 0o600); + // Write atomically with secure permissions + const tempPath = path.join(cacheDir, `.license_cache_${process.pid}.tmp`); + fs.writeFileSync(tempPath, JSON.stringify(cacheData, null, 2), { mode: 0o600 }); + fs.renameSync(tempPath, cachePath);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
packages/react-on-rails-pro-node-renderer/package.jsonpackages/react-on-rails-pro-node-renderer/src/ReactOnRailsProNodeRenderer.tspackages/react-on-rails-pro-node-renderer/src/master.tspackages/react-on-rails-pro-node-renderer/src/shared/licenseCache.tspackages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.tspackages/react-on-rails-pro-node-renderer/src/shared/licenseRefreshChecker.tspackages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.tspackages/react-on-rails-pro-node-renderer/tests/licenseCache.test.tspackages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.tspackages/react-on-rails-pro-node-renderer/tests/licenseRefreshChecker.test.tspackages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.tsreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/README.mdreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
🧰 Additional context used
📓 Path-based instructions (2)
{package.json,webpack.config.js,packages/*/package.json,react_on_rails_pro/package.json}
📄 CodeRabbit inference engine (CLAUDE.md)
When resolving merge conflicts in build configuration files, verify file paths are correct by running
grep -r 'old/path' .and test affected scripts likepnpm run prepackbefore continuing the merge
Files:
packages/react-on-rails-pro-node-renderer/package.json
**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rb: Runbundle exec rubocopand fix ALL violations before every commit/push
Runbundle exec rubocop(MANDATORY) before every commit to ensure zero offenses
Files:
react_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for user-visible changes in the Pro-only react_on_rails_pro gem and npm packages
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes in the open-source react_on_rails gem and npm package (features, bug fixes, breaking changes, deprecations, performance improvements)
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to react_on_rails/spec/dummy/e2e/playwright/e2e/**/*.spec.js : Use Playwright E2E tests in `react_on_rails/spec/dummy/e2e/playwright/` for React component integration testing. Tests automatically start Rails server on port 5017 before running
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for user-visible changes in the Pro-only react_on_rails_pro gem and npm packages
Applied to files:
react_on_rails_pro/docs/installation.mdpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbpackages/react-on-rails-pro-node-renderer/src/master.tspackages/react-on-rails-pro-node-renderer/src/ReactOnRailsProNodeRenderer.tsreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbpackages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.tsreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbpackages/react-on-rails-pro-node-renderer/src/shared/licenseCache.tsreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/README.mdreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes in the open-source react_on_rails gem and npm package (features, bug fixes, breaking changes, deprecations, performance improvements)
Applied to files:
react_on_rails_pro/docs/installation.mdpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/README.mdreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb
📚 Learning: 2025-10-23T17:22:01.074Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Applied to files:
react_on_rails_pro/docs/installation.mdreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbpackages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.tsreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/README.mdreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Applied to files:
react_on_rails_pro/docs/installation.mdreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/README.mdreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
react_on_rails_pro/docs/installation.mdreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/README.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
react_on_rails_pro/docs/installation.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to sig/react_on_rails/**/*.rbs : RBS signatures in `sig/react_on_rails/` should be added for new Ruby files in `lib/react_on_rails/`, included in Steepfile, validated with `bundle exec rake rbs:validate`, and type-checked with `bundle exec rake rbs:steep`
Applied to files:
react_on_rails_pro/docs/installation.mdreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/README.mdreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rb
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
react_on_rails_pro/docs/installation.mdreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/README.mdreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to lib/react_on_rails/engine.rb : In Rails Engine code, DO NOT use `rake_tasks` block to explicitly load rake files from `lib/tasks/` - Rails::Engine automatically loads them. Only use `rake_tasks` block if tasks are in non-standard location, need programmatic generation, or need to pass context
Applied to files:
react_on_rails_pro/docs/installation.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro.rb
📚 Learning: 2024-12-12T13:07:09.929Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Applied to files:
packages/react-on-rails-pro-node-renderer/package.json
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to react_on_rails/spec/dummy/e2e/playwright/e2e/**/*.spec.js : Use Playwright E2E tests in `react_on_rails/spec/dummy/e2e/playwright/` for React component integration testing. Tests automatically start Rails server on port 5017 before running
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.tsreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbpackages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.tsreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbpackages/react-on-rails-pro-node-renderer/tests/licenseRefreshChecker.test.tsreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb
📚 Learning: 2025-02-13T14:29:49.267Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: In IDE configuration, exclude these directories to prevent slowdowns: /coverage, /tmp, /gen-examples, /packages/react-on-rails/lib, /node_modules, /react_on_rails/spec/dummy/node_modules, /react_on_rails/spec/dummy/tmp, /react_on_rails/spec/dummy/app/assets/webpack, /react_on_rails/spec/dummy/log, /react_on_rails/spec/dummy/e2e/playwright-report, /react_on_rails/spec/dummy/test-results
Applied to files:
react_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
react_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/README.md
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to react_on_rails/spec/dummy/e2e/playwright/app_commands/**/*.rb : Create custom Rails app commands in `e2e/playwright/app_commands/` directory with .rb files for reusable helpers in Playwright tests
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to react_on_rails/spec/dummy/e2e/playwright/e2e/**/*.spec.js : Use `app('clean')`, `appEval(code)`, `appFactories(options)`, and `appScenario(name)` helpers from cypress-on-rails in Playwright tests for Rails integration
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbpackages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.tsreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Install Playwright browsers with `cd react_on_rails/spec/dummy && pnpm playwright install --with-deps` before running E2E tests
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Shakapacker configuration should be tested by creating debug scripts in dummy app root (debug-webpack-rules.js, debug-webpack-with-config.js) to inspect webpack rules before committing configuration changes
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
📚 Learning: 2025-02-13T16:50:26.861Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Applied to files:
react_on_rails_pro/LICENSE_SETUP.md
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in the `rsc_payload_react_component` helper method.
Applied to files:
react_on_rails_pro/LICENSE_SETUP.md
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
react_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro.rb
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
Applied to files:
react_on_rails_pro/docs/configuration.mdreact_on_rails_pro/lib/react_on_rails_pro.rb
📚 Learning: 2024-06-27T05:22:46.156Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-06-27T05:22:46.156Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
before do
original_value = ENV["VARIABLE_NAME"]
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
end
after do
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
ENV["VARIABLE_NAME"] = original_value
end
it "tests something" do
# Your test code here
end
end
```
This practice ensures test isolation and reliability.
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb
🧬 Code graph analysis (7)
packages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.ts (2)
react_on_rails_pro/spec/dummy/client/node-renderer.js (1)
process(5-5)react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/RouterApp.server.jsx (1)
controller(10-10)
packages/react-on-rails-pro-node-renderer/src/shared/licenseRefreshChecker.ts (2)
packages/react-on-rails-pro-node-renderer/src/shared/licenseCache.ts (4)
getFetchedAt(150-161)getExpiresAt(167-178)writeCache(105-136)getCachedToken(141-144)packages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.ts (2)
isAutoRefreshEnabled(51-60)fetchLicense(66-131)
packages/react-on-rails-pro-node-renderer/src/master.ts (2)
packages/react-on-rails-pro-node-renderer/src/shared/configBuilder.ts (1)
Config(25-89)packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts (1)
getValidatedLicenseData(204-247)
react_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/license_cache.rb (6)
cache_dir(75-77)write(27-39)cache_path(79-81)token(41-43)fetched_at(45-50)expires_at(52-57)
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (5)
configure(4-7)configuration(9-38)license_api_url(217-222)license_key(202-207)auto_refresh_enabled?(234-236)
react_on_rails_pro/lib/react_on_rails_pro/license_validator.rb (3)
react_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rb (2)
seed_cache_if_needed(26-39)maybe_refresh_license(13-21)react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (2)
configuration(9-38)auto_refresh_enabled?(234-236)react_on_rails_pro/lib/react_on_rails_pro/license_cache.rb (1)
token(41-43)
react_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (3)
configuration(9-38)auto_refresh_enabled?(234-236)license_api_url(217-222)
🪛 Gitleaks (8.30.0)
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts
[high] 62-62: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 187-187: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
react_on_rails_pro/LICENSE_SETUP.md
[high] 102-102: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: dummy-app-integration-tests (3.4, 22, latest)
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: rspec-package-tests (3.4, latest)
- GitHub Check: build
- GitHub Check: examples (3.4, latest)
- GitHub Check: precompile-check
- GitHub Check: pro-lint-js-and-ruby
- GitHub Check: build-dummy-app-webpack-test-bundles
- GitHub Check: check-bundle-size
- GitHub Check: markdown-link-check
- GitHub Check: claude-review
🔇 Additional comments (31)
react_on_rails_pro/lib/react_on_rails_pro.rb (1)
13-15: LGTM! Clean module loading sequence.The three new license management modules are loaded in the correct order before the validator, ensuring all dependencies are available when needed.
packages/react-on-rails-pro-node-renderer/tests/licenseRefreshChecker.test.ts (1)
1-222: LGTM! Comprehensive test coverage for license refresh logic.The test suite thoroughly exercises all three exported functions with appropriate mocking and edge case coverage. The beforeEach setup properly isolates tests, and console suppression keeps output clean.
packages/react-on-rails-pro-node-renderer/src/ReactOnRailsProNodeRenderer.ts (1)
45-45: Verify awaiting behavior is documented.The addition of
awaitmakes the master path consistent with the worker path and is necessary for async license validation. SincereactOnRailsProNodeRendererwas alreadyasync, callers should have been awaiting it.Please confirm that this change is documented in CHANGELOG_PRO.md, as it could affect callers who weren't properly awaiting the function (though they should have been).
Based on learnings, user-visible changes to the react_on_rails_pro npm package should be documented in CHANGELOG_PRO.md.
react_on_rails_pro/docs/installation.md (1)
50-51: LGTM! Clear guidance for automatic license renewal.The documentation appropriately directs paid subscribers to use the license key environment variable and links to detailed setup instructions.
react_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rb (1)
45-50: LGTM! Helpful configuration examples for development.The commented examples clearly document the auto-refresh options available for local testing, providing a useful reference without affecting default behavior.
react_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb (1)
24-27: LGTM! Standard WebMock setup for HTTP testing.The WebMock integration with HTTPX adapter is properly configured to support testing of the license fetching functionality across all specs.
react_on_rails_pro/docs/configuration.md (1)
165-186: LGTM! Clear and comprehensive auto-refresh configuration documentation.The new LICENSE AUTO-REFRESH CONFIGURATION section thoroughly documents all three options with appropriate defaults, environment variable names, and usage guidance. The link to LICENSE_SETUP.md provides users with detailed setup instructions.
packages/react-on-rails-pro-node-renderer/package.json (1)
23-23: Ensure async-retry addition is documented in react_on_rails_pro/CHANGELOG.md.The
async-retrydependency version ^1.3.3 is confirmed as the latest stable version and is appropriately added for the license fetch retry logic. However, this change is not documented in the changelog. Update react_on_rails_pro/CHANGELOG.md to include this new dependency addition under the appropriate release section.Also applies to: 38-38
⛔ Skipped due to learnings
Learnt from: CR Repo: shakacode/react_on_rails PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T18:57:59.314Z Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes in the open-source react_on_rails gem and npm package (features, bug fixes, breaking changes, deprecations, performance improvements)Learnt from: CR Repo: shakacode/react_on_rails PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T18:57:59.314Z Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for user-visible changes in the Pro-only react_on_rails_pro gem and npm packagesLearnt from: CR Repo: shakacode/react_on_rails PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T18:57:59.314Z Learning: Applies to /{CHANGELOG.md,CHANGELOG_PRO.md} : Use format `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)` in changelog entries (no hash in PR number)Learnt from: AbanoubGhadban Repo: shakacode/react_on_rails PR: 1875 File: lib/react_on_rails/utils.rb:112-124 Timestamp: 2025-10-23T17:22:01.074Z Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.Learnt from: alexeyr-ci Repo: shakacode/react_on_rails PR: 1687 File: spec/dummy/package.json:0-0 Timestamp: 2025-01-23T18:20:45.824Z Learning: When adding or updating dependencies in spec/dummy/package.json, maintain version consistency with other package.json files in the codebase to avoid potential version conflicts.Learnt from: Romex91 Repo: shakacode/react_on_rails PR: 1697 File: package-scripts.yml:28-28 Timestamp: 2025-02-12T16:38:06.537Z Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.Learnt from: alexeyr-ci Repo: shakacode/react_on_rails PR: 1644 File: node_package/src/ReactOnRailsRSC.ts:87-87 Timestamp: 2024-12-12T13:07:09.929Z Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.Learnt from: AbanoubGhadban Repo: shakacode/react_on_rails PR: 1781 File: node_package/src/ClientSideRenderer.ts:82-95 Timestamp: 2025-09-15T21:24:48.207Z Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.Learnt from: CR Repo: shakacode/react_on_rails PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-19T18:57:59.314Z Learning: Applies to sig/react_on_rails/**/*.rbs : RBS signatures in `sig/react_on_rails/` should be added for new Ruby files in `lib/react_on_rails/`, included in Steepfile, validated with `bundle exec rake rbs:validate`, and type-checked with `bundle exec rake rbs:steep`react_on_rails_pro/README.md (1)
297-298: LGTM! Clear guidance for paid subscribers.The documentation clearly directs paid subscribers to use the
LICENSE_KEYenvironment variable for automatic renewal, with an appropriate reference to detailed setup instructions.react_on_rails_pro/lib/react_on_rails_pro/license_validator.rb (2)
29-31: LGTM! Cache seeding correctly positioned.The cache seeding logic is appropriately placed after successful validation, ensuring only valid license data is cached. The comments clearly explain the purpose.
159-174: LGTM! License loading priority correctly updated.The license loading flow correctly implements the auto-refresh feature:
- Pre-load refresh attempt ensures fresh tokens
- Cache-first retrieval when auto-refresh is enabled
- Proper fallback chain: cache → ENV → config file
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts (1)
1-206: LGTM! Comprehensive test coverage.The test suite thoroughly covers the licenseFetcher functionality:
- Auto-refresh enabling/disabling scenarios
- API request handling with proper headers (Authorization, User-Agent)
- Error handling (401, network errors, JSON parsing)
- Retry logic with fake timers
- Custom API URL support
The test data (JWT tokens, license keys) are clearly test values, not real secrets.
Note: The Gitleaks warnings for lines 62 and 187 are false positives—these are test JWT tokens and mock API keys used exclusively in test scenarios.
react_on_rails_pro/LICENSE_SETUP.md (2)
79-142: LGTM! Excellent documentation for automatic license renewal.The automatic license renewal section is comprehensive and user-friendly:
- Clear distinction between License Token and License Key
- Well-organized setup instructions
- Helpful frequency table for refresh checks
- Appropriate guidance for air-gapped environments
- Privacy considerations transparently documented
350-360: LGTM! Clear FAQ entries for dual-token model.The FAQ entries effectively clarify:
- Differences between LICENSE and LICENSE_KEY
- When to use each method
- Behavior when both are set
- Data transmission transparency
Note: The Gitleaks warning for line 102 is a false positive—"lic_abc123xyz..." is clearly a placeholder example in documentation, not a real license key.
packages/react-on-rails-pro-node-renderer/tests/licenseCache.test.ts (1)
1-254: LGTM! Thorough test coverage for license cache.The test suite comprehensively validates the cache functionality:
- Read/write operations with license key validation
- Security features (0600 permissions, license_key_hash verification)
- Error handling (missing files, JSON parse errors, write failures)
- All public API methods (getCachedToken, getFetchedAt, getExpiresAt)
- Proper mocking and cleanup for filesystem operations
The tests ensure data integrity and security of the cached license data.
react_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rb (1)
258-601: LGTM! Comprehensive auto-refresh test coverage.The test suite thoroughly validates the license auto-refresh feature:
- Token source priority (cache → ENV → config) with proper gating
- Refresh timing logic with boundary tests (30, 8, 7, 3 days)
- Seeding behavior for first-boot scenarios
- Integration with LicenseCache, LicenseFetcher, and LicenseRefreshChecker
- Proper handling of fetch success/failure scenarios
The comment at lines 346-352 explaining the 60-day choice to avoid timing races demonstrates thoughtful test design.
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb (1)
301-427: LGTM! Complete configuration test coverage.The test suite thoroughly validates the license auto-refresh configuration:
- Default values (auto_refresh_license defaults to true)
- Configuration mutability
- ENV variable precedence (LICENSE_KEY, LICENSE_API_URL)
- Helper method logic (auto_refresh_enabled?)
- Edge cases (empty ENV, nil values)
- Proper ENV cleanup for test isolation
packages/react-on-rails-pro-node-renderer/src/master.ts (1)
14-18: The async conversion ofmasterRunis correctly implemented with properawaitofgetValidatedLicenseData(). The only call site inReactOnRailsProNodeRenderer.tsline 45 is already properly awaiting the function withawait master.default(config). No further changes needed.react_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rb (1)
1-185: LGTM! Comprehensive test coverage.The test suite thoroughly covers all scenarios for
LicenseFetcher.fetch:
- Auto-refresh disabled/enabled states
- Various HTTP response codes (200, 401, 404, 500)
- Network errors and timeouts
- Invalid JSON handling
- ENV variable precedence
Test isolation is properly maintained with appropriate setup/teardown.
react_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rb (1)
1-75: LGTM! Well-structured refresh checker.The refresh timing logic is clean and correctly implements the documented schedule:
- ≤7 days: daily checks
- ≤30 days: weekly checks
30 days: no refresh
Cache seeding handles the first-boot scenario properly, and token loading follows the correct precedence (ENV → config file).
packages/react-on-rails-pro-node-renderer/src/shared/licenseRefreshChecker.ts (1)
1-133: LGTM! TypeScript implementation matches Ruby behavior.The TypeScript refresh checker correctly mirrors the Ruby implementation:
- Identical refresh schedule logic
- Proper async/await for fetch operations
- Consistent cache seeding behavior
- Correct time conversions (Unix timestamp to Date)
react_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rb (1)
1-283: LGTM! Comprehensive cache test coverage.The test suite thoroughly covers all aspects of the license cache:
- Read validation with hash verification
- Write operations with proper permissions
- Environment variable precedence
- Timestamp parsing and error handling
- Edge cases (missing files, invalid JSON, mismatched hashes)
FakeFS usage is correct with proper activation/deactivation.
packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts (1)
108-297: LGTM! Comprehensive async validator tests.The test suite has been properly migrated to the async API:
- All calls to
getValidatedLicenseData(),isEvaluation()useawait- Covers all critical scenarios (valid, expired, invalid, missing licenses)
- Tests caching behavior and reset functionality
- Proper test isolation with RSA key generation
packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts (1)
204-247: LGTM! Async validator implementation is solid.The migration to async API is well-executed:
- Cache-first loading with auto-refresh integration
- Proper cache seeding on first validation
- Consistent error handling
- All exported functions correctly updated to async
The integration of
maybeRefreshLicense()andseedCacheIfNeeded()completes the auto-renewal workflow.react_on_rails_pro/lib/react_on_rails_pro/license_cache.rb (2)
15-25: LGTM!The
readmethod properly handles edge cases with appropriate error handling. The defensive rescue forErrno::ENOENTalongside the existence check is good practice against TOCTOU race conditions.
68-73: LGTM!The hash truncation to 16 hex characters provides sufficient entropy for cache validation while keeping the stored data compact. The implementation correctly handles the nil case.
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (2)
199-222: LGTM!The environment variable precedence pattern is well-implemented. Using
.present?correctly handles both nil and empty string cases, and the fallback toDEFAULT_LICENSE_API_URLprovides sensible defaults.
229-236: LGTM!The
auto_refresh_enabled?method correctly gates auto-refresh on both the feature flag and the presence of a license key, preventing unnecessary API calls when credentials are missing.packages/react-on-rails-pro-node-renderer/src/shared/licenseCache.ts (3)
48-54: Verify cache directory alignment with Ruby implementation.This uses
process.cwd()to determine the cache directory, while Ruby usesRails.root. If the Node Renderer is started from a different working directory than the Rails root, they may use different cache files.Is it intentional that Ruby and Node maintain separate caches, or should they share the same cache file? If sharing is intended, consider making the cache path configurable or using a more explicit path resolution.
80-99: LGTM!The
readCachefunction has appropriate error handling that fails gracefully. The validation flow correctly checks for file existence, parses JSON, and validates the license key hash before returning data.
15-20: LGTM!The
CacheDatainterface clearly defines the cache structure, matching the Ruby implementation. This ensures consistency between the two implementations.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
react_on_rails_pro/LICENSE_SETUP.md (1)
1-1: Fix broken external link in documentation.The
https://licenses.shakacode.comlink referenced on line 96 is currently inaccessible (HTTP 502 Bad Gateway). This link directs users to the "React on Rails Pro dashboard" to retrieve their license key. Update or replace this URL with a working alternative, or provide an alternative method for users to access their license management dashboard.The other external links (
https://shakacode.com/react-on-rails-proandhttps://shakacode.com) are accessible and working correctly.react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
100-144: RBS signature file is out of sync and must be updated to match the initialize method.The initialize method in
react_on_rails_pro/lib/react_on_rails_pro/configuration.rbnow includes four new parameters (concurrent_component_streaming_buffer_size,auto_refresh_license,license_api_url,license_key), but the corresponding RBS signature file atreact_on_rails_pro/sig/react_on_rails_pro/configuration.rbsis missing:
- The four new parameters in the
initializemethod signature (lines 52-78)- The corresponding
attr_accessordeclarations for these attributes (lines 26-50)- The four new
DEFAULT_*constants (lines 3-24)Update the RBS file to include all four parameters and their attribute declarations, then validate with
bundle exec rake rbs:validateandbundle exec rake rbs:steep.
🧹 Nitpick comments (7)
react_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb (1)
28-35: Consider handling JSON parse errors explicitly.If the API returns a 200 status but with malformed or empty JSON,
JSON.parse(response.body.to_s)will raiseJSON::ParserError. While this is caught by theStandardErrorrescue, it might be worth logging a more specific message for this case to aid debugging.🔎 Proposed enhancement for clearer error handling
return nil if response.is_a?(HTTPX::ErrorResponse) return nil unless response.status == 200 Rails.logger.debug { "[ReactOnRailsPro] License fetched successfully" } - JSON.parse(response.body.to_s) + begin + JSON.parse(response.body.to_s) + rescue JSON::ParserError => e + Rails.logger.warn { "[ReactOnRailsPro] License fetch received invalid JSON: #{e.message}" } + nil + end rescue StandardError => e Rails.logger.warn { "[ReactOnRailsPro] License fetch failed: #{e.message}" } nilpackages/react-on-rails-pro-node-renderer/src/shared/licenseRefreshChecker.ts (1)
85-103: Consider adding logging when config file is found but has issues.The function silently ignores errors when reading the config file. While this is graceful fallback behavior, a debug-level log might help users troubleshoot configuration issues.
🔎 Optional: Add debug logging
// Try config file try { const configPath = path.join(process.cwd(), 'config', 'react_on_rails_pro_license.key'); if (fs.existsSync(configPath)) { return fs.readFileSync(configPath, 'utf8').trim(); } - } catch { + } catch (error) { // Ignore errors reading config file + console.debug?.(`[React on Rails Pro] Could not read license file: ${error}`); }packages/react-on-rails-pro-node-renderer/src/shared/licenseCache.ts (1)
105-136: Solid cache write implementation with security considerations.The function:
- Creates directory recursively if needed
- Validates license key is configured before writing
- Sets restrictive file permissions (0600)
- Handles errors gracefully with warning logs
One minor note:
fs.writeFileSyncfollowed byfs.chmodSynchas a brief window where the file exists with default permissions. Consider usingfs.writeFileSyncwithmodeoption, though this is a very minor concern for a cache file.🔎 Optional: Atomic permission setting
- fs.writeFileSync(cachePath, JSON.stringify(cacheData, null, 2)); - - // Set file permissions to 0600 (owner read/write only) - fs.chmodSync(cachePath, 0o600); + fs.writeFileSync(cachePath, JSON.stringify(cacheData, null, 2), { mode: 0o600 });packages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.ts (1)
95-99: Clarify the bail+throw pattern for future maintainers.The
throw new UnauthorizedError()afterbail()is needed becausebail()doesn't returnneverin TypeScript. Consider adding a comment or using a type assertion.🔎 Optional: Clarify the pattern
if (response.status === 401 || response.status === 403) { // Don't retry auth errors - bail immediately bail(new UnauthorizedError()); - throw new UnauthorizedError(); // TypeScript needs this + // TypeScript requires a throw here because bail() doesn't return `never`. + // The throw is unreachable at runtime since bail() aborts the retry loop. + throw new UnauthorizedError(); }react_on_rails_pro/lib/react_on_rails_pro/license_cache.rb (2)
35-36: Non-atomic file write and permission setting could leave file temporarily exposed.There's a brief window between
File.writeandFile.chmodwhere the file has default permissions (typically 0644). For sensitive license data, consider using a more secure approach.🔎 Suggested atomic write approach
- File.write(cache_path, JSON.pretty_generate(cache_data)) - File.chmod(0o600, cache_path) + # Write to temp file with correct permissions, then atomic rename + temp_path = cache_path.sub_ext(".tmp") + File.open(temp_path, File::WRONLY | File::CREAT | File::TRUNC, 0o600) do |f| + f.write(JSON.pretty_generate(cache_data)) + end + File.rename(temp_path, cache_path)
45-56: Consider usingTime.iso8601for stricter parsing.Since timestamps are stored in ISO8601 format (line 32), using
Time.iso8601instead ofTime.parsewould provide stricter validation and clearer intent.Time.parseis more permissive and could accept malformed dates.🔎 Suggested change
def fetched_at timestamp = read&.dig("fetched_at") - Time.parse(timestamp) if timestamp - rescue ArgumentError + Time.iso8601(timestamp) if timestamp + rescue ArgumentError, TypeError nil end def expires_at timestamp = read&.dig("expires_at") - Time.parse(timestamp) if timestamp - rescue ArgumentError + Time.iso8601(timestamp) if timestamp + rescue ArgumentError, TypeError nil endreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rb (1)
68-69: Consider rescuing file read errors.While
config_path.exist?reduces risk, there's still a race condition where the file could be deleted or become unreadable between the check and read. Also, encoding issues or permission problems could cause exceptions.🔎 Suggested defensive approach
config_path = Rails.root.join("config", "react_on_rails_pro_license.key") - return File.read(config_path).strip if config_path.exist? + return File.read(config_path).strip if config_path.exist? && config_path.readable? + rescue Errno::ENOENT, Errno::EACCES + nil
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (27)
packages/react-on-rails-pro-node-renderer/package.jsonpackages/react-on-rails-pro-node-renderer/src/ReactOnRailsProNodeRenderer.tspackages/react-on-rails-pro-node-renderer/src/master.tspackages/react-on-rails-pro-node-renderer/src/shared/licenseCache.tspackages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.tspackages/react-on-rails-pro-node-renderer/src/shared/licenseRefreshChecker.tspackages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.tspackages/react-on-rails-pro-node-renderer/tests/licenseCache.test.tspackages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.tspackages/react-on-rails-pro-node-renderer/tests/licenseRefreshChecker.test.tspackages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.tsreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/README.mdreact_on_rails_pro/docs/configuration.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rb
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rb: Runbundle exec rubocopand fix ALL violations before every commit/push
Runbundle exec rubocop(MANDATORY) before every commit to ensure zero offenses
Files:
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rb
{package.json,webpack.config.js,packages/*/package.json,react_on_rails_pro/package.json}
📄 CodeRabbit inference engine (CLAUDE.md)
When resolving merge conflicts in build configuration files, verify file paths are correct by running
grep -r 'old/path' .and test affected scripts likepnpm run prepackbefore continuing the merge
Files:
packages/react-on-rails-pro-node-renderer/package.json
🧠 Learnings (25)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for user-visible changes in the Pro-only react_on_rails_pro gem and npm packages
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes in the open-source react_on_rails gem and npm package (features, bug fixes, breaking changes, deprecations, performance improvements)
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for user-visible changes in the Pro-only react_on_rails_pro gem and npm packages
Applied to files:
packages/react-on-rails-pro-node-renderer/src/master.tsreact_on_rails_pro/README.mdreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/LICENSE_SETUP.mdpackages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.tsreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbpackages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.tsreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbpackages/react-on-rails-pro-node-renderer/src/shared/licenseCache.tsreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/docs/configuration.mdpackages/react-on-rails-pro-node-renderer/src/ReactOnRailsProNodeRenderer.ts
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG.md : Update `/CHANGELOG.md` for user-visible changes in the open-source react_on_rails gem and npm package (features, bug fixes, breaking changes, deprecations, performance improvements)
Applied to files:
react_on_rails_pro/README.mdreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rbpackages/react-on-rails-pro-node-renderer/src/shared/licenseCache.tsreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-10-23T17:22:01.074Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Applied to files:
react_on_rails_pro/README.mdreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbpackages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.tsreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbpackages/react-on-rails-pro-node-renderer/src/shared/licenseCache.tsreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /{CHANGELOG.md,CHANGELOG_PRO.md} : Use format `[PR 1818](https://github.com/shakacode/react_on_rails/pull/1818) by [username](https://github.com/username)` in changelog entries (no hash in PR number)
Applied to files:
react_on_rails_pro/README.md
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Applied to files:
react_on_rails_pro/README.mdreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to sig/react_on_rails/**/*.rbs : RBS signatures in `sig/react_on_rails/` should be added for new Ruby files in `lib/react_on_rails/`, included in Steepfile, validated with `bundle exec rake rbs:validate`, and type-checked with `bundle exec rake rbs:steep`
Applied to files:
react_on_rails_pro/README.mdreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/lib/react_on_rails_pro.rb
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
react_on_rails_pro/README.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro.rb
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
react_on_rails_pro/README.mdreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
react_on_rails_pro/README.mdreact_on_rails_pro/lib/react_on_rails_pro/license_cache.rbpackages/react-on-rails-pro-node-renderer/package.jsonreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
react_on_rails_pro/README.mdreact_on_rails_pro/docs/installation.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to react_on_rails/spec/dummy/e2e/playwright/e2e/**/*.spec.js : Use Playwright E2E tests in `react_on_rails/spec/dummy/e2e/playwright/` for React component integration testing. Tests automatically start Rails server on port 5017 before running
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbpackages/react-on-rails-pro-node-renderer/tests/licenseRefreshChecker.test.tsreact_on_rails_pro/LICENSE_SETUP.mdpackages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.tsreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbpackages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.tsreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rbreact_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-02-13T14:29:49.267Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
📚 Learning: 2024-06-27T05:22:46.156Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-06-27T05:22:46.156Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
before do
original_value = ENV["VARIABLE_NAME"]
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
end
after do
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
ENV["VARIABLE_NAME"] = original_value
end
it "tests something" do
# Your test code here
end
end
```
This practice ensures test isolation and reliability.
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
📚 Learning: 2025-02-13T16:50:26.861Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Applied to files:
react_on_rails_pro/LICENSE_SETUP.md
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation is handled in deeper level calls of the React on Rails Pro codebase, so it doesn't need to be validated again in the `rsc_payload_react_component` helper method.
Applied to files:
react_on_rails_pro/LICENSE_SETUP.md
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to lib/react_on_rails/engine.rb : In Rails Engine code, DO NOT use `rake_tasks` block to explicitly load rake files from `lib/tasks/` - Rails::Engine automatically loads them. Only use `rake_tasks` block if tasks are in non-standard location, need programmatic generation, or need to pass context
Applied to files:
react_on_rails_pro/docs/installation.mdreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/lib/react_on_rails_pro.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: In IDE configuration, exclude these directories to prevent slowdowns: /coverage, /tmp, /gen-examples, /packages/react-on-rails/lib, /node_modules, /react_on_rails/spec/dummy/node_modules, /react_on_rails/spec/dummy/tmp, /react_on_rails/spec/dummy/app/assets/webpack, /react_on_rails/spec/dummy/log, /react_on_rails/spec/dummy/e2e/playwright-report, /react_on_rails/spec/dummy/test-results
Applied to files:
react_on_rails_pro/lib/react_on_rails_pro/license_cache.rbreact_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Install Playwright browsers with `cd react_on_rails/spec/dummy && pnpm playwright install --with-deps` before running E2E tests
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to react_on_rails/spec/dummy/e2e/playwright/e2e/**/*.spec.js : Use `app('clean')`, `appEval(code)`, `appFactories(options)`, and `appScenario(name)` helpers from cypress-on-rails in Playwright tests for Rails integration
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.tsreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to react_on_rails/spec/dummy/e2e/playwright/app_commands/**/*.rb : Create custom Rails app commands in `e2e/playwright/app_commands/` directory with .rb files for reusable helpers in Playwright tests
Applied to files:
react_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rbreact_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Shakapacker configuration should be tested by creating debug scripts in dummy app root (debug-webpack-rules.js, debug-webpack-with-config.js) to inspect webpack rules before committing configuration changes
Applied to files:
react_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
react_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.md
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
Applied to files:
react_on_rails_pro/lib/react_on_rails_pro.rbreact_on_rails_pro/docs/configuration.md
🧬 Code graph analysis (10)
packages/react-on-rails-pro-node-renderer/src/master.ts (2)
packages/react-on-rails-pro-node-renderer/src/shared/configBuilder.ts (1)
Config(25-89)packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts (1)
getValidatedLicenseData(204-247)
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (5)
configure(4-7)configuration(9-38)license_api_url(217-222)license_key(202-207)auto_refresh_enabled?(234-236)
packages/react-on-rails-pro-node-renderer/src/shared/licenseRefreshChecker.ts (2)
packages/react-on-rails-pro-node-renderer/src/shared/licenseCache.ts (4)
getFetchedAt(150-161)getExpiresAt(167-178)writeCache(105-136)getCachedToken(141-144)packages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.ts (2)
isAutoRefreshEnabled(51-60)fetchLicense(66-131)
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts (1)
packages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.ts (2)
isAutoRefreshEnabled(51-60)fetchLicense(66-131)
react_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rb (2)
react_on_rails_pro/lib/react_on_rails_pro/license_validator.rb (2)
validated_license_data!(6-200)validated_license_data!(15-44)react_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rb (3)
should_check_for_refresh?(41-54)maybe_refresh_license(13-21)seed_cache_if_needed(26-39)
react_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rb (2)
react_on_rails_pro/lib/react_on_rails_pro/license_cache.rb (6)
cache_dir(75-77)write(27-39)cache_path(79-81)token(41-43)fetched_at(45-50)expires_at(52-57)react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (1)
configure(4-7)
react_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rb (2)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (2)
configuration(9-38)auto_refresh_enabled?(234-236)react_on_rails_pro/lib/react_on_rails_pro/license_cache.rb (4)
write(27-39)token(41-43)expires_at(52-57)fetched_at(45-50)
react_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb (1)
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (3)
configuration(9-38)auto_refresh_enabled?(234-236)license_api_url(217-222)
packages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.ts (2)
react_on_rails_pro/spec/dummy/client/node-renderer.js (1)
process(5-5)react_on_rails_pro/spec/dummy/client/app/ror-auto-load-components/RouterApp.server.jsx (1)
controller(10-10)
react_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rb (2)
react_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb (1)
api_url(44-46)react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (2)
configure(4-7)license_api_url(217-222)
🪛 GitHub Actions: Check Markdown Links
react_on_rails_pro/LICENSE_SETUP.md
[error] 1-1: Network error: Connection failed. Check network connectivity and firewall settings
🪛 Gitleaks (8.30.0)
packages/react-on-rails-pro-node-renderer/tests/licenseCache.test.ts
[high] 50-50: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: rspec-dummy-app-node-renderer
- GitHub Check: dummy-app-node-renderer-e2e-tests
🔇 Additional comments (51)
packages/react-on-rails-pro-node-renderer/package.json (1)
23-23: Verify package version validity and security.The
async-retryversion (^1.3.3) is current and has no known security vulnerabilities. Version 1.3.3 is the latest available release (published 2021-08-17), is not deprecated, and is licensed under MIT. The corresponding type definitions (@types/async-retry@1.4.9) are compatible and properly included.react_on_rails_pro/spec/react_on_rails_pro/spec_helper.rb (1)
24-26: LGTM! WebMock configuration is appropriate.The addition of WebMock with its HTTPX adapter enables proper HTTP request stubbing for the new license-fetching tests, which is essential for testing the automatic license renewal feature without making real HTTP calls.
react_on_rails_pro/lib/react_on_rails_pro.rb (1)
13-15: LGTM! Module requires are appropriately ordered.The new license-related modules are correctly placed between
license_public_keyandlicense_validator, following a logical dependency order.react_on_rails_pro/docs/installation.md (1)
50-51: LGTM! Clear guidance for paid subscribers.The note appropriately directs paid subscribers to use the dedicated license key environment variable for automatic renewal, with a reference to detailed setup documentation.
packages/react-on-rails-pro-node-renderer/src/master.ts (1)
14-17: LGTM! Async transformation is appropriate.Converting
masterRunto async and awaitinggetValidatedLicenseData()is necessary to support the automatic license refresh flow, which includes HTTP requests and cache operations. The validation correctly blocks startup to ensure a valid license before forking workers.react_on_rails_pro/spec/dummy/config/initializers/react_on_rails_pro.rb (1)
45-50: LGTM! Helpful configuration example.The commented configuration block provides clear guidance for developers testing the license auto-refresh feature locally with a licensing app, without affecting the existing test suite.
packages/react-on-rails-pro-node-renderer/src/ReactOnRailsProNodeRenderer.ts (1)
45-45: LGTM! Consistent async handling.Awaiting
master.default(config)is consistent with the async transformation of the master initialization process and ensures license validation completes before the renderer is considered ready.react_on_rails_pro/README.md (1)
297-298: LGTM! Consistent documentation guidance.The note is consistent with the similar guidance in installation.md and appropriately directs paid subscribers to use the dedicated license key environment variable for automatic renewal.
react_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rb (4)
301-320: LGTM! Proper test setup with ENV cleanup.The test suite correctly includes an
afterhook to clean up environment variables, following best practices for test isolation. Theauto_refresh_licensetests appropriately cover defaults and mutability.
322-338: LGTM! Complete coverage for license_api_url configuration.The tests appropriately verify both the default value and customization of the
license_api_urlconfiguration option.
340-379: LGTM! Comprehensive license_key precedence testing.The tests thoroughly cover the license key sourcing logic, including environment variable precedence and edge cases like empty ENV values, matching the implementation in
configuration.rb.
381-427: LGTM! Thorough auto_refresh_enabled? predicate testing.The tests comprehensively verify the
auto_refresh_enabled?logic, covering all combinations ofauto_refresh_licenseandlicense_keystates, including edge cases like empty strings and environment-based configuration.react_on_rails_pro/docs/configuration.md (1)
165-186: LGTM! Clear documentation for auto-refresh configuration.The new LICENSE AUTO-REFRESH CONFIGURATION section is well-structured and provides clear guidance on the three new configuration options. The cross-reference to LICENSE_SETUP.md for detailed documentation is appropriate.
react_on_rails_pro/spec/react_on_rails_pro/license_fetcher_spec.rb (1)
1-185: LGTM! Comprehensive test coverage for license fetcher.The test suite provides thorough coverage of the LicenseFetcher module:
- Configuration-driven behavior (auto_refresh on/off, license_key presence)
- Successful API fetch with proper Authorization header
- All major error scenarios (401, 404, 500, timeout, network errors, invalid JSON)
- ENV variable override support
- Proper test isolation with WebMock
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts (1)
1-206: LGTM! Thorough test coverage for Node license fetcher.The test suite provides comprehensive coverage:
- isAutoRefreshEnabled logic with all configuration combinations and case-insensitive values
- fetchLicense success and error paths
- Retry logic verification using fake timers
- User-Agent header inclusion
- Custom API URL support
- Proper mocking and cleanup
react_on_rails_pro/lib/react_on_rails_pro/license_validator.rb (2)
29-31: LGTM! Cache seeding on first boot.The call to
LicenseRefreshChecker.seed_cache_if_neededafter successful validation ensures the cache is populated on first boot, enabling subsequent auto-refresh checks.
158-180: LGTM! Cache-first license loading with auto-refresh.The updated
load_license_stringmethod correctly implements:
- Pre-refresh check via
maybe_refresh_license- Cache-first retrieval when auto-refresh is enabled
- Fallback to ENV and config file sources
- Clear priority documentation in comments
react_on_rails_pro/LICENSE_SETUP.md (2)
79-142: LGTM! Comprehensive auto-refresh documentation.The new "Automatic License Renewal" section provides clear guidance:
- Helpful comparison table (LICENSE_KEY vs LICENSE_TOKEN)
- Straightforward setup instructions
- Clear explanation of refresh schedule thresholds
- Privacy assurances about data transmission
- Instructions for disabling auto-refresh in air-gapped environments
350-360: LGTM! Helpful FAQ additions.The new FAQ entries clearly distinguish LICENSE from LICENSE_KEY and guide users on when to use each option.
react_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rb (4)
258-323: LGTM! Token source priority tests verify cache-first behavior.The tests correctly validate the priority order:
- Cached token (when auto-refresh enabled)
- ENV variable (fallback)
- Cache seeding when cache is empty
325-471: LGTM! Comprehensive boundary testing for refresh thresholds.The
should_check_for_refresh?tests thoroughly cover all critical boundaries:
30 days (no checks)
- 30 days boundary (weekly checks)
- 8 days boundary (still weekly)
- 7 days boundary (daily checks)
- 3 days (daily checks)
- nil fetched_at (triggers fetch)
The comment on lines 346-349 explaining the timing race with 31 days demonstrates thoughtful test design.
473-538: LGTM! maybe_refresh_license behavior properly tested.Tests verify:
- No fetch when auto_refresh disabled or should_check_for_refresh? returns false
- Cache write on successful fetch
- No cache write on fetch failure
540-601: LGTM! seed_cache_if_needed covers all scenarios.Tests validate seeding logic across:
- auto_refresh disabled (no write)
- Cache already has token (no write)
- Cache empty with ENV token (write with token and expiry)
- No token available (no write)
packages/react-on-rails-pro-node-renderer/tests/licenseCache.test.ts (1)
1-254: LGTM! Comprehensive cache test coverage.The test suite thoroughly validates:
- readCache: non-existent files, license_key_hash validation, valid cache, JSON errors
- writeCache: directory creation, correct data structure, file permissions (0600), error handling
- Accessor functions: getCachedToken, getFetchedAt, getExpiresAt
The static analysis warnings on lines 50 and 70 are false positives—these are test JWT token examples, not real secrets.
react_on_rails_pro/spec/react_on_rails_pro/license_cache_spec.rb (1)
1-283: LGTM! Thorough cache implementation tests.The test suite provides comprehensive coverage:
- read: all edge cases including hash validation, ENV override, invalid data
- write: data merging, hash/timestamp addition, permissions, directory creation, error handling
- Accessors (token, fetched_at, expires_at): valid and invalid scenarios
The use of FakeFS for filesystem isolation and time freezing for deterministic timestamps demonstrates solid testing practices.
react_on_rails_pro/lib/react_on_rails_pro/license_fetcher.rb (1)
16-36: Implementation looks solid overall.The fetch method correctly:
- Guards with
auto_refresh_enabled?check- Uses HTTPX retry plugin appropriately
- Includes timeout and proper headers
- Handles error responses gracefully
- Returns nil on failures for graceful fallback
packages/react-on-rails-pro-node-renderer/tests/licenseRefreshChecker.test.ts (3)
1-45: Comprehensive test setup with proper mock isolation.The test file correctly:
- Mocks dependencies before importing the module under test
- Resets environment variables and mocks in
beforeEach- Restores all mocks in
afterEach- Suppresses console output during tests
47-109: Thorough coverage of refresh timing logic.The
shouldCheckForRefreshtests appropriately cover:
- No expiry data case
- Far expiry (>30 days) - no refresh needed
- 30-day window with weekly check logic
- 7-day window with daily check logic
- Never-fetched edge case
131-150: Verify the expected cache write payload matches the implementation.The test expects
writeCacheto be called with onlytokenandexpires_at, but looking atlicenseCache.tsline 104-135,writeCachealso addsfetched_atandlicense_key_hashinternally. Ensure the test's expectation aligns with the actual function signature.packages/react-on-rails-pro-node-renderer/src/shared/licenseRefreshChecker.ts (3)
36-55: Consider edge case when exactly on 7-day or 30-day boundary.When
daysUntilExpiryequals exactly 7 or 30, the<=conditions work correctly. However,Math.floortruncates, so a license expiring in 7.9 days would show as 7 days. This seems intentional but worth noting for clarity.The logic is correct for the documented behavior.
61-79: Clean async orchestration with proper guards.The function:
- Checks
isAutoRefreshEnabled()first- Then checks
shouldCheckForRefresh()to avoid unnecessary API calls- Only writes to cache on successful fetch
- Returns void rather than success/failure, relying on cache state
112-133: Seed function correctly handles first-boot scenario.The function properly:
- Guards on auto-refresh being enabled
- Avoids overwriting existing cache
- Seeds from ENV or file
- Converts Unix timestamp to ISO string for cache storage
packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts (4)
5-7: Clean integration of auto-refresh modules.The new imports properly bring in the required functionality for cache-first loading and refresh orchestration.
88-96: Cache-first loading when auto-refresh is enabled.Good pattern: when auto-refresh is enabled, prefer the cached (potentially refreshed) token over the static ENV/file token. This ensures users benefit from renewed licenses.
204-227: Async flow integrates refresh before validation.The sequence is correct:
- Return cached data if already validated
- Try auto-refresh if enabled and near expiry
- Load and decode license
- Validate license data
- Cache results
- Seed cache for future boots
One observation:
seedCacheIfNeededis called even if the token came from cache (since validation passed). This is harmless but could be optimized to skip ifisAutoRefreshEnabled() && getCachedToken()was already true inloadLicenseString.
255-273: Async wrappers for public API.The
isEvaluationandgetGraceDaysRemainingfunctions correctly awaitgetValidatedLicenseData()before accessing cached data.packages/react-on-rails-pro-node-renderer/src/shared/licenseCache.ts (2)
33-35: Truncated hash provides reasonable security.Using the first 16 characters of SHA-256 (64 bits) provides sufficient collision resistance for cache invalidation purposes while keeping storage compact.
150-178: Accessors handle errors defensively.Both
getFetchedAtandgetExpiresAtproperly handle:
- Missing cache data
- Invalid date strings (via try-catch)
The pattern of returning
nullon any error is appropriate for optional cache data.packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts (3)
11-21: Proper mock isolation for auto-refresh modules.The mocks correctly disable auto-refresh behavior during tests:
isAutoRefreshEnabledreturnsfalsegetCachedTokenreturnsnullmaybeRefreshLicenseresolves immediatelyseedCacheIfNeededis a no-opThis ensures tests focus on license validation logic without refresh side effects.
83-94: Re-mocking afterresetModulesis necessary.Since
jest.resetModules()clears the module cache, the mocks need to be re-established usingjest.doMock. This is the correct pattern for dynamic module mocking.
109-124: Test correctly updated for async API.The test properly:
- Uses
asyncfunction declaration- Awaits
getValidatedLicenseData()- Validates the returned data structure
packages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.ts (2)
51-60: Auto-refresh enabled logic is correct.The function properly:
- Requires a license key to be set
- Defaults to enabled unless explicitly set to 'false'
- Uses case-insensitive comparison
80-118: Robust retry implementation with proper timeout handling.The implementation correctly:
- Uses AbortController with setTimeout for request timeout
- Clears timeout in finally block to prevent leaks
- Bails immediately on auth errors (no point retrying)
- Retries on transient failures (non-200 status)
- Logs retry attempts with error context
react_on_rails_pro/lib/react_on_rails_pro/configuration.rb (2)
65-68: Default values align with PR requirements.
DEFAULT_AUTO_REFRESH_LICENSE = trueenables auto-refresh by defaultDEFAULT_LICENSE_API_URLpoints to the licensing service- TODO comment appropriately notes pending deployment
199-236: Configuration accessors with proper ENV override pattern.The methods correctly:
- Check environment variables first (consistent with 12-factor app principles)
- Fall back to configured values
- Provide default for API URL
- Combine both conditions in
auto_refresh_enabled?Environment variable names (
REACT_ON_RAILS_PRO_LICENSE_KEY,REACT_ON_RAILS_PRO_LICENSE_API_URL) match the TypeScript implementation inlicenseFetcher.tsandlicenseCache.ts.react_on_rails_pro/lib/react_on_rails_pro/license_cache.rb (3)
15-25: LGTM on the read method structure.The method correctly handles cache file absence, JSON parsing errors, and validates the cached data against the current license key before returning. Good defensive programming with the rescue clause.
68-73: LGTM on hash truncation for cache validation.Using the first 16 hex characters (64 bits) of SHA256 is sufficient for detecting license key changes. This isn't used for cryptographic security, just cache invalidation.
75-81: LGTM on cache path configuration.Using
Rails.root.join("tmp")is appropriate for cache files. The Pathname-based approach is clean and handles path construction correctly.react_on_rails_pro/lib/react_on_rails_pro/license_refresh_checker.rb (3)
13-21: LGTM on refresh orchestration.The method has clean guard clauses and properly delegates to
LicenseFetcherandLicenseCache. The graceful fallback behavior (returning early on nil response) aligns with the PR objectives.
41-54: LGTM on refresh timing logic.The implementation correctly follows the documented refresh schedule. The approach of returning
falsewhenexpires_atis nil is a safe default.
56-61: LGTM on cooldown enforcement.Returning
truewhenfetched_atis nil correctly allows the first fetch attempt. The time comparison is straightforward.
Code Review: Automatic License Renewal FeatureGreat work on this comprehensive feature! The implementation is well-designed and thoroughly tested (118 new tests!). However, I've identified 2 critical race conditions that need to be addressed before merge, plus several medium-priority improvements. Critical Issues (Must Fix)1. Race Condition: Non-Atomic Cache File WritesSeverity: HIGH Problem: Multiple processes can simultaneously write to cache file, causing corrupted JSON. Current Code: File.write(cache_path, JSON.pretty_generate(cache_data))
File.chmod(0o600, cache_path) # NOT atomicFix: Use atomic rename pattern:
Add test for concurrent writes to verify no corruption. 2. Race Condition: Duplicate API Fetch on StartupSeverity: MEDIUM Problem: During deployment, multiple processes trigger simultaneous API fetches (10+ workers × 2 retries = 20-30 requests). Fix: Add file-based lock using File.flock(LOCK_NB) to deduplicate API calls. Important Issues (Should Fix)3. Security DocumentationAdd best practices to LICENSE_SETUP.md:
4. Blocking Network Call on Boot
5. ObservabilityAdd structured logging for refresh events, expiry dates, and success/failure status. Nice-to-Have Improvements
Strengths
SummaryMust fix before merge:
Should fix: Security docs, performance, observability Once the critical race conditions are addressed, this will be a robust feature. Great work! 🎉 |
Comprehensive Code ReviewI've thoroughly reviewed all code changes in this PR. Overall, this is high-quality code with excellent test coverage and thoughtful architecture. Here are my findings: 🔴 Critical Issues - Must Fix Before Merge1. Negative Days Until Expiry BugLocation: Issue: If the license is already expired, days_until_expiry = ((expires_at - Time.now) / 1.day).to_i
# If license expired 1 day ago: days_until_expiry = -1
# This matches: days_until_expiry <= 7 ✅ (triggers daily refresh)This could trigger daily refresh attempts for expired licenses indefinitely. Fix: Add explicit check in both implementations: return false if days_until_expiry < 0 # Don't refresh if already expired2. File Permission Race ConditionLocation: Issue: Between File.write(cache_path, JSON.pretty_generate(cache_data))
File.chmod(0o600, cache_path) # File is publicly readable until this lineFix: Use atomic write with proper permissions: # Ruby
File.write(cache_path, JSON.pretty_generate(cache_data), perm: 0o600)
# TypeScript
fs.writeFileSync(cachePath, JSON.stringify(cacheData, null, 2), { mode: 0o600 });🟡 High Priority - Should Fix Soon3. Thread Safety: File Cache Race ConditionLocation: All cache operations Issue: In multi-threaded Rails (Puma) or multi-process Node Renderer, multiple processes might:
Fix: Add file locking: def write(data)
File.open(cache_path, File::RDWR | File::CREAT, 0o600) do |f|
f.flock(File::LOCK_EX)
f.write(JSON.pretty_generate(cache_data))
f.flush
end
endFor TypeScript, consider using the 4. Blocking Startup on Network DelayLocation: Issue: License validation blocks Node Renderer startup: await getValidatedLicenseData(); // Blocks for up to 5-15 seconds if network is slowRecommendation: Make refresh non-blocking: // Validate with cached/ENV license first (fast)
const licenseData = await getValidatedLicenseData();
// Trigger async refresh in background (don't block startup)
maybeRefreshLicense().catch(err => log.warn('Background refresh failed:', err));🟢 Minor Improvements5. TypeScript: Unreachable CodeLocation: bail(new UnauthorizedError());
throw new UnauthorizedError(); // TypeScript needs this - but bail() already throwsIf 6. Ruby: Explicit HTTP Status HandlingLocation: Consider matching TypeScript's explicit 401/403 handling: case response.status
when 401, 403
Rails.logger.error { "[ReactOnRailsPro] License fetch unauthorized" }
return nil
when 200
# success
else
Rails.logger.warn { "[ReactOnRailsPro] License fetch failed: HTTP #{response.status}" }
return nil
end7. Security: Add HTTPS ValidationLocation: Configuration Add validation to prevent accidental HTTP usage in production: if (!url.startsWith('https://') && process.env.NODE_ENV === 'production') {
throw new Error('License API URL must use HTTPS in production');
}✅ StrengthsExcellent work on:
Test coverage is particularly impressive:
📋 Missing Test Cases (Optional)Consider adding:
Overall AssessmentThis is production-ready code with the critical fixes applied. The architecture is solid, error handling is thorough, and test coverage is excellent. Main concerns are edge cases around thread safety and expired licenses. Recommended merge strategy:
Great work on this feature! 🎉 |
Code Review: Automatic License RenewalGreat work on this feature! The implementation is well-structured with comprehensive test coverage. Critical Issues1. HTTPX Thread Safety 2. Missing clearTimeout in TypeScript 3. TODO Comment Security Concerns1. Timing Attack - Cache validation uses direct string comparison. Use OpenSSL.fixed_length_secure_compare instead. 2. Weak Hash - Only 16 chars of SHA256 used (64 bits). Use at least 32 characters (128 bits). Bugs1. Race Condition - maybe_refresh_license called before seed_cache_if_needed on first boot. Cache is empty so refresh timing cannot be determined. 2. File I/O - Each cache accessor reads entire JSON file. Add memoization. RecommendationsMust fix: HTTPX thread-safety, clearTimeout, TODO comment Overall: Excellent design and test coverage. Address critical issues before merge! |
Code Review: Automatic License Renewal FeatureI've reviewed PR #2254 and overall this is a well-implemented feature with excellent test coverage. Here's my detailed feedback: Strengths1. Excellent Test Coverage
2. Strong Security Practices
3. Good Architecture
4. Documentation
Issues and RecommendationsHIGH PRIORITY1. Race Condition in Cache File OperationsLocation: licenseCache.ts:126 and license_cache.rb:82 Issue: The cache read/write operations are not atomic. Multiple Node processes could read cache simultaneously, both trigger refresh, race to write new token, and potentially corrupt the cache file. Recommendation: Use atomic write-rename pattern. Write to a temp file first, then atomically rename it. This is the standard pattern for safe file updates. 2. Missing Cache Directory PermissionsLocation: licenseCache.ts:130 Issue: If mkdirSync fails silently, chmodSync might fail on the cache file. Recommendation: Set directory mode during creation: fs.mkdirSync(cacheDir, { recursive: true, mode: 0o700 }) 3. Async/Await ConsistencyLocation: master.ts:14-17 Issue: While you correctly made masterRun async and await getValidatedLicenseData(), the function signature change could break existing code that doesn't await the result. Recommendation: Check if any code calls masterRun() without awaiting. Document as breaking change if needed. MEDIUM PRIORITY4. HTTP Retry Logic DifferencesRuby version uses HTTPX with fixed 1s delay between retries. TypeScript version uses async-retry with exponential backoff. Issue: Inconsistent retry behavior between implementations. Recommendation: Align retry strategies. Exponential backoff is better for production systems. 5. Cache Timestamp Parsing Edge CaseLocation: licenseCache.ts:153-158 and license_cache.rb:53-58 Issue: If cache contains invalid ISO8601 date, parsing fails silently and returns null. This could mask data corruption. Recommendation: Log warnings for invalid timestamps to help debug cache corruption issues. 6. Environment Variable Type SafetyLocation: licenseFetcher.ts:57 Issue: autoRefreshSetting?.toLowerCase() !== 'false' means any typo (fasle, False, FALSE) will enable auto-refresh. Recommendation: Also check for '0' as falsy value for better robustness. LOW PRIORITY7. HTTPX Dependency DocumentationLocation: license_fetcher.rb:3 Issue: The PR adds httpx as a dependency but doesn't mention it in the PR description. Recommendation: Verify httpx is in react_on_rails_pro.gemspec and document the new dependency. 8. Test Mock CleanupLocation: licenseCache.test.ts:32, licenseFetcher.test.ts:14 Issue: console.warn is mocked but output isn't verified in all tests where warnings are expected. Recommendation: Add assertions for warning messages where applicable. Security Review
Performance ConsiderationsPositive:
Potential concern:
Testing RecommendationsGreat coverage overall! A few additional test cases to consider:
Documentation ReviewLICENSE_SETUP.md is excellent! Clear, comprehensive, addresses common questions. Minor suggestions:
Pre-Merge ChecklistBefore merging, please verify:
Overall AssessmentQuality: 5/5 Recommendation: Approve with minor fixes The implementation is solid and well-tested. The race condition issue should be addressed before production deployment, but it's not a blocker for merge if you're comfortable with the risk (unlikely to manifest in typical deployments). Excellent work on the parallel implementation (Ruby + TypeScript), comprehensive testing, and thorough documentation! |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts (1)
56-202: Strong test coverage with minor gaps.The test suite is well-structured and covers most critical paths including retry logic, error handling, and header verification. The use of fake timers for retry tests is appropriate.
However, consider adding tests for:
- 403 Forbidden status: The implementation treats 403 the same as 401 (no retry), but only 401 is tested.
- Request timeout: The implementation uses
AbortControllerwith a timeout, but there's no explicit test for timeout scenarios.📝 Suggested additional test cases
Add a test for 403 status:
it('returns null on 403 forbidden', async () => { process.env.REACT_ON_RAILS_PRO_LICENSE_KEY = 'lic_invalid'; fetchSpy.mockResolvedValueOnce({ status: 403, }); const result = await fetchLicense(); expect(result).toBeNull(); expect(fetchSpy).toHaveBeenCalledTimes(1); // No retry });Add a test for timeout:
it('returns null on request timeout after retries', async () => { jest.useFakeTimers(); process.env.REACT_ON_RAILS_PRO_LICENSE_KEY = 'lic_test123'; fetchSpy.mockImplementation(() => new Promise((_, reject) => setTimeout(() => reject(new Error('Aborted')), 15000) ) ); const resultPromise = fetchLicense(); await jest.runAllTimersAsync(); const result = await resultPromise; expect(result).toBeNull(); expect(fetchSpy).toHaveBeenCalledTimes(3); });
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.lychee.tomlpackages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to /CHANGELOG_PRO.md : Update `/CHANGELOG_PRO.md` for user-visible changes in the Pro-only react_on_rails_pro gem and npm packages
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: In IDE configuration, exclude these directories to prevent slowdowns: /coverage, /tmp, /gen-examples, /packages/react-on-rails/lib, /node_modules, /react_on_rails/spec/dummy/node_modules, /react_on_rails/spec/dummy/tmp, /react_on_rails/spec/dummy/app/assets/webpack, /react_on_rails/spec/dummy/log, /react_on_rails/spec/dummy/e2e/playwright-report, /react_on_rails/spec/dummy/test-results
Applied to files:
.lychee.toml
📚 Learning: 2025-12-19T18:57:59.314Z
Learnt from: CR
Repo: shakacode/react_on_rails PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-19T18:57:59.314Z
Learning: Applies to react_on_rails/spec/dummy/e2e/playwright/e2e/**/*.spec.js : Use Playwright E2E tests in `react_on_rails/spec/dummy/e2e/playwright/` for React component integration testing. Tests automatically start Rails server on port 5017 before running
Applied to files:
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts
🧬 Code graph analysis (1)
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts (2)
react_on_rails_pro/spec/dummy/client/node-renderer.js (1)
process(5-5)packages/react-on-rails-pro-node-renderer/src/shared/licenseFetcher.ts (2)
isAutoRefreshEnabled(51-60)fetchLicense(66-131)
🪛 Gitleaks (8.30.0)
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts
[high] 67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 186-186: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (7)
.lychee.toml (4)
1-13: LGTM! Well-configured retry and timeout settings.The retry logic (4 retries with 15s wait) and 20s timeout are appropriate for handling transient network failures in CI while avoiding excessive delays. The accepted status codes (200 and 308) are standard.
64-65: Track the TODO for post-deployment cleanup.The exclusion for
licenses.shakacode.comis appropriate while deployment is pending. Ensure this TODO is tracked (e.g., in a follow-up issue) so the exclusion is removed once the licensing app is live.Can you confirm:
- Is there a tracking issue for removing this exclusion after deployment?
- What is the expected deployment timeline for licenses.shakacode.com?
107-113: LGTM! Sensible path exclusions.The excluded paths are well-justified: placeholder links in planning docs, dependencies, test fixtures, and legacy TOC structure. The comment explaining why
SUMMARY.mdis retained is particularly helpful.
115-120: LGTM! Appropriate caching and fragment settings.Disabling fragment checking avoids flaky anchor validation, and 1-day cache expiry provides a good balance between freshness and CI performance.
packages/react-on-rails-pro-node-renderer/tests/licenseFetcher.test.ts (3)
1-25: LGTM: Solid test setup with proper timer cleanup.The test setup properly addresses async-retry timer leaks mentioned in the PR objectives. The combination of
jest.clearAllTimers()andjest.useRealTimers()inafterEachprevents "Cannot log after tests are done" errors, and the explanatory comment with the Jest issue reference is helpful for future maintainers.
27-54: LGTM: Comprehensive coverage of auto-refresh logic.The test cases thoroughly cover all branches of
isAutoRefreshEnabled(), including edge cases like case-insensitive handling and default behavior.
67-67: Static analysis false positive: mock test data, not real secrets.Gitleaks flagged these JWT tokens as potential secrets, but they're clearly mock test data (note the
...truncation). This is a false positive and safe to ignore.Also applies to: 186-186
Add configuration options for automatic license token refresh: - auto_refresh_license: enable/disable auto-refresh (default: true) - license_api_url: API endpoint URL (placeholder, TODO for production) - license_key: permanent key for API auth (ENV takes precedence) - auto_refresh_enabled?: helper to check if properly configured Follows existing pattern from load_license_string for ENV precedence. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
HTTP client to fetch license tokens from the licensing API: - 5 second request timeout - 2 retries with 1 second delay on transient failures - Returns parsed JSON on success, nil on failure - Falls back silently to cached token if fetch fails 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Caches license tokens to tmp/react_on_rails_pro_license.cache: - Persists across app restarts to reduce API calls - Validates cached token belongs to current license_key via hash - Sets 0600 file permissions for security - Stores token, expires_at, plan, fetched_at for refresh logic 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Modified load_license_string to support automatic license renewal: - Calls maybe_refresh_license before loading token - Cache takes priority when auto-refresh is enabled - Falls back to ENV → config file if no cache Refresh timing (based on cached expires_at): - >30 days until expiry: no checks - 8-30 days: weekly checks - ≤7 days: daily checks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Prevents fetching on every boot during the final week. Now consistent: daily checks (1 day threshold) when ≤7 days, weekly checks (7 day threshold) when 8-30 days. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Adds 64 new tests for the auto-refresh license feature: - LicenseFetcher (13 tests): HTTP responses, timeouts, network errors, ENV priority - LicenseCache (22 tests): file operations, hash validation, time parsing - Configuration (14 tests): license_key, auto_refresh_enabled? conditions - LicenseValidator (15 tests): token priority, timing thresholds, refresh orchestration Uses WebMock with HTTPX adapter for HTTP stubbing and FakeFS for file system tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove separate webmock_helper.rb file and add the requires directly to spec_helper.rb, matching the pattern used in spec/dummy/spec/. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The original implementation had a gap: should_check_for_refresh? checked LicenseCache.expires_at, but the cache was only populated via API refresh. Since refresh required the cache to exist (to know the expiry), refresh never triggered - a chicken-and-egg problem. Fix: After successful JWT validation, if auto-refresh is enabled and cache is empty, seed the cache with the current token's data (token + expiry). This allows should_check_for_refresh? to work on subsequent boots. Flow: 1. First boot: JWT from ENV → validate → seed cache 2. Subsequent boots: cache exists → check expiry → refresh if needed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Extract auto-refresh timing logic from LicenseValidator into a dedicated LicenseRefreshChecker module. This reduces LicenseValidator from 264 to 201 lines and removes the need for rubocop:disable directive. - Create LicenseRefreshChecker with: maybe_refresh_license, seed_cache_if_needed, should_check_for_refresh?, last_fetch_older_than?, load_token_from_env_or_file - LicenseValidator now calls the module directly at call sites - Update tests to call LicenseRefreshChecker directly instead of testing private methods via described_class.send() 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add LICENSE_SETUP.md section explaining auto-refresh feature: - License Key vs License Token comparison table - Setup instructions - How refresh timing works (weekly at 30d, daily at 7d) - How to disable for air-gapped environments - Privacy section - New FAQ entries - Add configuration.md section with config options: - license_key - auto_refresh_license - license_api_url - Add notes in README.md and installation.md pointing paid subscribers to auto-refresh documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Implements Phase 4 of automatic license renewal for the Node Renderer. When configured with REACT_ON_RAILS_PRO_LICENSE_KEY, the Node Renderer will automatically fetch fresh license tokens when approaching expiry. New modules: - licenseFetcher.ts: HTTP client for license API (5s timeout, 2 retries) - licenseCache.ts: File-based token caching with license_key validation - licenseRefreshChecker.ts: Timing logic (weekly at 30 days, daily at 7 days) Changes: - licenseValidator.ts: Now async, integrates refresh before loading token - master.ts: Awaits async license validation - ReactOnRailsProNodeRenderer.ts: Awaits async master.default() ENV vars (shared with Rails via .env): - REACT_ON_RAILS_PRO_LICENSE_KEY: Permanent key for API auth - REACT_ON_RAILS_PRO_AUTO_REFRESH_LICENSE: Enable/disable (default: true) - REACT_ON_RAILS_PRO_LICENSE_API_URL: API endpoint (default: production) Tests: 51 tests covering all new functionality 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Both Ruby gem and Node Renderer now send identifying User-Agent headers: - Ruby gem: ReactOnRailsPro-Gem - Node Renderer: ReactOnRailsPro-NodeRenderer Helps with server-side logging and debugging license issues. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Use jest.useFakeTimers() to control async-retry's internal setTimeout calls, preventing timer leakage between tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add User-Agent header verification test - Add invalid JSON response handling test - Add write failure handling test for cache 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- configuration.rb: Add ENV support for license_api_url (REACT_ON_RAILS_PRO_LICENSE_API_URL) to match license_key behavior. This enables consistent configuration via environment variables for both Ruby gem and Node Renderer. - licenseFetcher.ts: Fix TypeScript errors in async-retry callback: - Add generic type parameter to retry<LicenseResponse>() - Properly handle unknown error type in onRetry callback - dummy app: Add commented license config example for local testing 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy .lychee.toml from master and add exclusion for licenses.shakacode.com which doesn't exist yet (licensing app deployment pending). TODO: Remove the exclusion once the licensing app is deployed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add jest.clearAllTimers() and jest.useRealTimers() to afterEach to prevent "Cannot log after tests are done" errors caused by async-retry's internal setTimeout callbacks escaping the fake timer boundary. The issue occurred because async-retry schedules retries via setTimeout, and when jest.useRealTimers() was called, pending microtasks could fire after the test completed. See: jestjs/jest#10487 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
ihabadham
force-pushed
the
ihabadham/pro/automatic-license-renewal
branch
from
4079e0e to
7593128
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| def token | ||
| read&.dig("token") | ||
| end | ||
|
|
||
| def fetched_at | ||
| timestamp = read&.dig("fetched_at") | ||
| Time.parse(timestamp) if timestamp | ||
| rescue ArgumentError | ||
| nil | ||
| end | ||
|
|
||
| def expires_at | ||
| timestamp = read&.dig("expires_at") | ||
| Time.parse(timestamp) if timestamp | ||
| rescue ArgumentError | ||
| nil | ||
| end |
There was a problem hiding this comment.
Performance: each accessor independently reads and parses the cache file
token, fetched_at, and expires_at each call read (file read + JSON parse). Callers like should_check_for_refresh? invoke all three, resulting in 3 disk reads per check.
Consider returning the full parsed struct from read and letting callers destructure, or adding a lightweight request-scoped memoisation (e.g. a @cache_data ivar with a short TTL or cleared on write).
| return unless license_string | ||
|
|
||
| decoded_data = decode_license(license_string) | ||
| return unless decoded_data |
There was a problem hiding this comment.
Bug: Failed validation state is never memoized
When the license is invalid or missing, the early return unless guards exit without ever assigning @validated_license_data. This means defined?(@validated_license_data) will still be false on the next call, causing repeated mutex acquisitions, disk reads, and (once refresh is triggered) potential HTTP calls on every subsequent invocation.
Use a sentinel to cache the nil result:
INVALID_LICENSE = :invalid_license_sentinel
def validated_license_data!
return nil if @validated_license_data == INVALID_LICENSE
return @validated_license_data if defined?(@validated_license_data)
LICENSE_MUTEX.synchronize do
return nil if @validated_license_data == INVALID_LICENSE
return @validated_license_data if defined?(@validated_license_data)
license_string = load_license_string
decoded_data = license_string && decode_license(license_string)
unless decoded_data &&
check_plan(decoded_data) == :valid &&
check_organization(decoded_data) == :valid &&
check_expiration(decoded_data) == :valid
@validated_license_data = INVALID_LICENSE
return nil
end
LicenseRefreshChecker.seed_cache_if_needed(decoded_data)
@validated_license_data = decoded_data
end
end
| .get("#{api_url}/api/license") | ||
|
|
||
| return nil if response.is_a?(HTTPX::ErrorResponse) | ||
| return nil unless response.status == 200 |
There was a problem hiding this comment.
Correctness: return nil unless response.status == 200 silently swallows 401/403 after no retries
The retry_on lambda correctly limits retries to 5xx responses — so auth errors are not retried (good). However, the current code treats a 401/403 the same as a 500: both return nil with just a generic log message. When license_key is wrong or revoked, the caller gets no signal that the key itself is the problem, and the next boot will silently try again.
Consider logging a distinct warning for 4xx responses so operators can distinguish a transient network failure from a bad key:
if response.status == 401 || response.status == 403
Rails.logger.warn { "[ReactOnRailsPro] License fetch failed: unauthorized (status #{response.status}). Check your license_key." }
return nil
end
return nil unless response.status == 200
| :rsc_payload_generation_url_path, :rsc_bundle_js_file, :react_client_manifest_file, | ||
| :react_server_client_manifest_file | ||
| :react_server_client_manifest_file, | ||
| :auto_refresh_license |
There was a problem hiding this comment.
Inconsistent accessor pattern: attr_accessor reader is immediately overridden
:auto_refresh_license in attr_accessor generates both a reader (auto_refresh_license) and a writer (auto_refresh_license=). However, a custom def auto_refresh_license is defined later (line ~232), silently overriding the generated reader. This is confusing — it looks like a simple attribute but has ENV-precedence logic.
The pattern used for license_key and license_api_url (custom getter + attr_writer) is cleaner and should be used here too:
# Remove :auto_refresh_license from the attr_accessor list above, and add:
attr_writer :auto_refresh_license
| end | ||
|
|
||
| def fetched_at | ||
| timestamp = read&.dig("fetched_at") |
There was a problem hiding this comment.
Performance: read parses the disk file on every call
token, fetched_at, and expires_at each call read independently, producing up to 3 file reads and 3 JSON parses per maybe_refresh_license check. Consider an in-process cache with a short TTL (e.g. 1 minute) or restructure the callers to call read once and pass the result around.
def token
read&.dig("token")
end
def fetched_at
timestamp = read&.dig("fetched_at") # separate disk read!
...
end
| # frozen_string_literal: true | ||
|
|
||
| require "httpx" | ||
| require "json" |
There was a problem hiding this comment.
Missing gemspec dependency
require "httpx" will raise LoadError at runtime if httpx is not declared in the gemspec. Please verify that httpx is listed as a runtime dependency in react_on_rails_pro.gemspec. If it's not already there, add:
spec.add_dependency "httpx", ">= 1.0"
| import { isAutoRefreshEnabled, fetchLicense } from './licenseFetcher.js'; | ||
| import { getCachedToken, getFetchedAt, getExpiresAt, writeCache } from './licenseCache.js'; | ||
|
|
||
| const ONE_DAY_MS = 24 * 60 * 60 * 1000; |
There was a problem hiding this comment.
Module-level mutable state: refreshInFlight survives across test runs
refreshInFlight is a module-level variable that is never reset via the module's public API. In Jest (which caches modules between tests unless jest.resetModules() is called), a test that triggers a refresh and leaves it in-flight can contaminate the next test. This is already visible in the test suite where maybeRefreshLicense calls coalesce — but if a test ends while the promise is still pending, the next import sees the stale refreshInFlight.
Consider exporting a resetRefreshState() for use in afterEach, similar to how reset() is exported from licenseValidator.ts.
Review SummaryGood overall structure — the feature is well-decomposed into focused modules (fetcher / cache / refresh-checker) with symmetric Ruby and TypeScript implementations, solid test coverage, and clear documentation. A few issues worth addressing before merge: Bugs1.
2. Double-checked locking in When the license is missing or fails any check, the early Performance3. Multiple disk reads per check — Ruby (license_cache.rb)
4. Multiple disk reads per check — Node (licenseCache.ts) Same issue: 5. The refresh check runs synchronously every time Minor6. A simple 7. 401/403 from license API logs as generic failure (license_fetcher.rb, line 36) Operators can't distinguish a bad key from a transient network error without a distinct warning message for 4xx responses. ObservationThe |
| function getCacheDir(): string { | ||
| return path.join(process.cwd(), 'tmp'); | ||
| } | ||
|
|
There was a problem hiding this comment.
Hardcoded tmp/ relative to process.cwd()
The cache is always written to <cwd>/tmp/. This directory:
- May not exist in non-Rails environments (e.g. standalone Node.js deployments).
- Is not guaranteed to be writable (some container images use read-only root filesystems).
- Has no gitignore entry added by this PR — the cache file containing a valid JWT token may accidentally be committed.
Consider making the cache path configurable via REACT_ON_RAILS_PRO_LICENSE_CACHE_DIR and documenting that tmp/react_on_rails_pro_license.cache should be in .gitignore.
|
Review comment - please see full review details |
|
placeholder body two |
Review: feat/automatic-license-renewalGood feature concept and solid test coverage. A few issues need addressing before merge, ranging from a potential performance regression to a thread-safety bug. Critical Issues Blocking HTTP call while holding LICENSE_MUTEX (Ruby) validated_license_data! never caches the invalid-license case High Priority Issues Race condition in LicenseCache.write (Ruby) Missing httpx gemspec dependency Medium Priority Issues attr_accessor :auto_refresh_license + custom getter is confusing LicenseCache reads disk 3 times per refresh check Module-level refreshInFlight leaks between Jest test runs Low Priority Issues Cache file directory hardcoded to tmp/ validated_license_data! naming |
| # Loads license string from environment variable | ||
| # @return [String, nil] License string or nil if not found | ||
| def load_license_string | ||
| LicenseRefreshChecker.maybe_refresh_license if ReactOnRailsPro.configuration.auto_refresh_enabled? |
There was a problem hiding this comment.
Blocking HTTP inside LICENSE_MUTEX
load_license_string is called from inside LICENSE_MUTEX.synchronize blocks (e.g. determine_license_status, determine_license_plan, etc.). If should_check_for_refresh? returns true on the first startup call, this makes a synchronous HTTP request (up to REQUEST_TIMEOUT_SECONDS × (MAX_RETRIES + 1) = 15 s) while holding the mutex — blocking every other thread that needs a license check until it completes.
The Node renderer handles this correctly by calling warmLicenseValidationState before the server starts accepting traffic. Consider the same for the Ruby gem, e.g. in a Rails after_initialize callback or Railtie initializer:
# In a Railtie or config/initializers/react_on_rails_pro.rb
ActiveSupport.on_load(:after_initialize) do
LicenseRefreshChecker.maybe_refresh_license if ReactOnRailsPro.configuration.auto_refresh_enabled?
endWith that in place, maybe_refresh_license can be removed from load_license_string (or guarded to avoid running inside the mutex).
| end | ||
|
|
||
| private | ||
|
|
There was a problem hiding this comment.
retry_on lambda won't retry network-level failures
response.respond_to?(:status) is false for HTTPX::ErrorResponse objects (what HTTPX returns for DNS failures, connection refused, timeouts, etc.), so the lambda evaluates to false and those errors won't be retried.
If HTTPX's :retries plugin internally retries ErrorResponse objects regardless of retry_on, this is a non-issue — but that should be verified. Otherwise, transient network errors (e.g. a blip in DNS resolution to licenses.shakacode.com) will surface as a hard failure rather than being retried, which undermines the "graceful fallback" story.
A safer lambda:
retry_on: lambda { |response|
response.is_a?(HTTPX::ErrorResponse) ||
(response.respond_to?(:status) && response.status >= 500)
}
|
|
||
| FileUtils.mv(temp_path, cache_path) | ||
| rescue StandardError => e | ||
| FileUtils.rm_f(temp_path) if defined?(temp_path) && temp_path |
There was a problem hiding this comment.
defined?(temp_path) is misleading
In Ruby all local variables are in scope for the entire method body (the parser registers them at parse time), so defined?(temp_path) always returns "local-variable" regardless of whether the assignment actually executed at runtime. The real guard here is just the truthiness of temp_path itself, which is nil if the assignment line was never reached.
| FileUtils.rm_f(temp_path) if defined?(temp_path) && temp_path | |
| FileUtils.rm_f(temp_path) if temp_path |
| return false; | ||
| } | ||
|
|
||
| const daysUntilExpiry = Math.floor((expiresAt.getTime() - Date.now()) / ONE_DAY_MS); |
There was a problem hiding this comment.
Math.floor truncation causes boundary inconsistency with Ruby
Ruby compares the exact float (e.g. time_until_expiry <= 7.days), so a license with 7 days and 23 hours remaining falls in the weekly-check bucket.
Here, Math.floor((expiresAt.getTime() - Date.now()) / ONE_DAY_MS) truncates that same 7.96-day remainder to 7, placing it in the daily-check bucket.
The Node renderer therefore starts daily checks up to ~23 h earlier than the Ruby gem, which is a subtle but surprising inconsistency. Consider keeping the raw milliseconds:
| const daysUntilExpiry = Math.floor((expiresAt.getTime() - Date.now()) / ONE_DAY_MS); | |
| const msUntilExpiry = expiresAt.getTime() - Date.now(); |
and using msUntilExpiry <= SEVEN_DAYS_MS / msUntilExpiry <= 30 * ONE_DAY_MS in the comparisons below, matching the Ruby float semantics exactly.
| * Uses tmp/ relative to current working directory. | ||
| */ | ||
| function getCacheDir(): string { | ||
| return path.join(process.cwd(), 'tmp'); |
There was a problem hiding this comment.
Cache directory relies on process.cwd() matching Rails.root
The Ruby gem writes the cache to Rails.root.join("tmp"). The Node renderer writes to path.join(process.cwd(), 'tmp'). These are the same only when the Node renderer is started from the Rails root, which is the typical case but not guaranteed (e.g. cd /tmp && node /app/renderer/server.js).
If they diverge, the Ruby gem and Node renderer maintain separate cache files, so a token refreshed on the Ruby side won't be available to the Node renderer on the next restart (and vice versa). Consider making the cache path configurable via an environment variable (e.g. REACT_ON_RAILS_PRO_CACHE_DIR) so both sides can be pointed at the same location explicitly.
Review: Auto License RenewalGood overall design — the feature is well-scoped, the Ruby and Node implementations are structurally symmetric, tests are thorough, and the graceful fallback on network failure is the right call. A few issues to address before merging: Blocking HTTP request inside LICENSE_MUTEX (see inline comment on
Cache directory coupling (see inline on Ruby uses Minor issues:
|
| // AbortRetryError means we're stopping retries intentionally, don't log | ||
| if (error instanceof AbortRetryError) { | ||
| return; |
There was a problem hiding this comment.
Dead code — this guard can never be reached. withRetry re-throws AbortRetryError immediately (line 53-54) before calling onFailedAttempt, so the callback is never invoked with an AbortRetryError. Safe to remove the check to avoid confusion.
| // AbortRetryError means we're stopping retries intentionally, don't log | |
| if (error instanceof AbortRetryError) { | |
| return; | |
| console.warn( |
| LicenseRefreshChecker.maybe_refresh_license if ReactOnRailsPro.configuration.auto_refresh_enabled? | ||
|
|
There was a problem hiding this comment.
Blocking network call inside LICENSE_MUTEX. maybe_refresh_license can call LicenseFetcher.fetch, which has a 5 s timeout × 3 attempts = up to 15 s. While that's in flight the mutex is held, blocking every concurrent thread that calls license_status, license_expiration, license_plan, etc.
On the very first cold boot this is fine (cache is empty → should_check_for_refresh? returns false immediately). But after the cache is seeded on a previous run, the next restart may hit a stale fetched_at that triggers a real HTTP call inside the lock.
Consider either:
- Running the refresh asynchronously (fire-and-forget background thread) and falling back to the cached token in the meantime, or
- Moving the
maybe_refresh_licensecall outside the mutex so the blocking work happens before the lock is acquired.
| function getCacheDir(): string { | ||
| return path.join(process.cwd(), 'tmp'); | ||
| } |
There was a problem hiding this comment.
The Node renderer is a separate process from Rails and its working directory may not be the Rails application root. Depending on how the renderer is launched, process.cwd() could resolve to the renderer's own package directory, leaving the cache file outside the Rails tmp/ folder that the Ruby gem reads from.
The Ruby gem resolves the cache path as Rails.root.join("tmp", ...). If the paths diverge, the token refreshed by the Node renderer will never be seen by the gem (and vice versa), making the cache effectively write-only from one side.
Consider making the cache directory configurable (e.g. REACT_ON_RAILS_PRO_CACHE_DIR env var) so operators can point both sides at the same path, and add a note to the documentation about this requirement.
| # Seeds the cache on first boot so that refresh logic works on subsequent boots. | ||
| # The cache stores the token's expiry, which should_check_for_refresh? needs to determine | ||
| # when to trigger a refresh. | ||
| def seed_cache_if_needed(license_data) |
There was a problem hiding this comment.
The license_refresh_checker_spec.rb file is missing — no unit tests exist for this class despite it containing the core refresh scheduling logic (should_check_for_refresh?, seed_cache_if_needed, load_token_from_env_or_file). The other two new Ruby files (LicenseFetcher, LicenseCache) both have specs; this one should too.
| ) | ||
| end | ||
|
|
||
| def should_check_for_refresh? |
There was a problem hiding this comment.
time_until_expiry can be negative when the license has already expired (past expires_at). In that case:
time_until_expiry <= 7.daysis true → checkslast_fetch_older_than?(1.day)→ likely triggers a refresh of an already-expired license.
That may be intentional (try to get a renewed token even after expiry), but the condition doesn't distinguish between "expiring soon" and "already expired". If the API rejects expired licenses, this will retry indefinitely on every daily check. Worth adding an explicit guard or at least a comment documenting the intent.
| }) | ||
| .get("#{api_url}/api/license") | ||
|
|
||
| return nil if response.is_a?(HTTPX::ErrorResponse) |
There was a problem hiding this comment.
The retry lambda only fires on 5xx responses. A 429 Too Many Requests won't be retried, which is correct for avoiding hammering a rate-limited API — but it also won't back off and retry when it should. If the API starts returning 429 during high-frequency restarts, the gem will silently fall back to the cached token without logging the rate-limit specifically.
Minor, but worth adding 429 to the non-retried list with an explicit log message so operators can diagnose the issue.
|
Code Review: Automatic License Renewal Overall the implementation is well-structured: atomic temp-file writes for the cache, a key-hash guard to avoid serving another tenant token, matching schedules across Ruby and Node, and good test coverage for LicenseCache and LicenseFetcher. A few issues worth addressing before merge. MUST-FIX
SHOULD-FIX
MINOR
DESIGN NOTE (no action required) The daily/weekly refresh schedule fires only when |
|
It's a re-evaluation because we changed the licensing strategy to be much more permissive. |
2 participants
Summary
This PR adds automatic license renewal support for both the Ruby gem and Node Renderer. When a user's license is approaching expiry, the client automatically fetches the renewed license from the licensing API, eliminating the need for manual token updates after subscription renewals.
Key Features
LicenseFetcher,LicenseCache, andLicenseRefreshCheckermoduleslicenseFetcher.ts,licenseCache.ts,licenseRefreshChecker.ts)license_key,license_api_url, andauto_refresh_licensesettingsRefresh Schedule
Configuration
Ruby Gem (via initializer or ENV):
Node Renderer (via ENV):
REACT_ON_RAILS_PRO_LICENSE_KEY=lic_... REACT_ON_RAILS_PRO_LICENSE_API_URL=https://licenses.shakacode.com REACT_ON_RAILS_PRO_AUTO_REFRESH_LICENSE=true # defaultTest Plan
Related
/api/licenseendpoint🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
Configuration
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.