Allow Pro license tokens from Rails and Node configuration - #4552
Conversation
|
+ci-status |
CI StatusHead SHA: Only the required gate is active unless hosted CI is requested. |
|
+ci-run-hosted |
github-actions
Bot
added
the
ready-for-hosted-ci
Hosted CI RequestedTriggered 9 workflow(s) for View progress in the Actions tab. |
![chatgpt-codex-connector[bot]](https://avatars.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4ee31c7c8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Greptile SummaryThis PR adds configurable Pro license-token sources for Rails and the Node renderer. The main changes are:
Confidence Score: 4/5The Node license metadata path needs a cache fix before merging.
packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
RailsConfig[config.license_token] --> RailsValidator[Rails license validator]
RailsEnv[REACT_ON_RAILS_PRO_LICENSE] --> RailsValidator
NodeConfig[licenseToken option] --> BuildConfig[buildConfig]
NodeEnv[REACT_ON_RAILS_PRO_LICENSE] --> BuildConfig
BuildConfig --> Startup[Node renderer startup]
Startup --> LogLicense[logLicenseStatus]
LogLicense --> NodeValidator[getLicenseStatus]
NodeValidator --> Cache[process-wide license caches]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
RailsConfig[config.license_token] --> RailsValidator[Rails license validator]
RailsEnv[REACT_ON_RAILS_PRO_LICENSE] --> RailsValidator
NodeConfig[licenseToken option] --> BuildConfig[buildConfig]
NodeEnv[REACT_ON_RAILS_PRO_LICENSE] --> BuildConfig
BuildConfig --> Startup[Node renderer startup]
Startup --> LogLicense[logLicenseStatus]
LogLicense --> NodeValidator[getLicenseStatus]
NodeValidator --> Cache[process-wide license caches]
Reviews (1): Last reviewed commit: "Record configurable Pro license secret s..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.github.com/in/867647?s=60&v=4){kind=small}
size-limit report 📦
|
WalkthroughAdds ChangesLicense token configuration
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Review: Configurable Pro license tokens (Rails
|
Review: Allow Pro license tokens from Rails and Node configurationOverviewThis PR adds Code quality
Potential issues (non-blocking / minor)
Security
Test coverage
PerformanceNo performance concerns — this is startup-time config resolution and logging only; no hot-path changes. SummarySolid, well-tested, low-risk feature addition with thorough documentation updates. No blocking issues found; the two notes above are forward-looking observations rather than defects in this PR. |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/react-on-rails-pro-node-renderer/tests/configBuilder.test.ts (1)
105-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a positive-path test for
envValuesUsed().REACT_ON_RAILS_PRO_LICENSE.Coverage tests the negative case (ENV usage suppressed when overridden, 128-134) but not the positive case: that when the ENV token is actually the effective value,
envValuesUsed().REACT_ON_RAILS_PRO_LICENSEreports'<MASKED>'. Since this diagnostic is the security-relevant signal that a secret was sourced from ENV, a regression here (e.g. an inverted condition) wouldn't be caught by existing tests.✅ Suggested additional test
+ it('reports the license ENV as used (masked) when no configured token overrides it', () => { + process.env.REACT_ON_RAILS_PRO_LICENSE = 'env-license-token'; + + const envValues = envValuesUsedForRenderedConfig({}); + + expect(envValues.REACT_ON_RAILS_PRO_LICENSE).toBe('<MASKED>'); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-on-rails-pro-node-renderer/tests/configBuilder.test.ts` around lines 105 - 134, Add a positive-path test in configBuilder.test.ts for envValuesUsed() covering the effective ENV license case: when REACT_ON_RAILS_PRO_LICENSE supplies the actual token and no configured licenseToken overrides it, assert envValuesUsed().REACT_ON_RAILS_PRO_LICENSE is '<MASKED>'. Use the existing helpers buildConfig and envValuesUsedForRenderedConfig to locate the behavior, and keep the current negative override test as-is.packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts (1)
27-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding parallel token-precedence tests for
getLicenseOrganizationandgetLicensePlan.The new tests cover
getLicenseStatuswith an explicit token, precedence, and fallback, butgetLicenseOrganizationandgetLicensePlanreceived signature updates without corresponding test coverage for the explicit-token path. While the underlyingloadLicenseStringlogic is shared, parallel tests would guard against future divergence.The
validPayloadobject is also duplicated across the three new test cases (lines 92–97, 106–111, 121–126). Extracting it to a shared constant orbeforeEach/factory would reduce repetition.Also applies to: 91-135
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts` around lines 27 - 29, Add parallel token-precedence coverage in licenseValidator.test.ts for getLicenseOrganization and getLicensePlan, mirroring the existing getLicenseStatus explicit-token, precedence, and fallback cases so the shared loadLicenseString behavior stays protected. Update the test suite around the getLicenseStatus/getLicenseOrganization/getLicensePlan signatures to assert the explicit token path for each helper, and refactor the repeated validPayload setup into a shared constant, factory, or beforeEach to remove duplication across those test cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/react-on-rails-pro-node-renderer/tests/configBuilder.test.ts`:
- Around line 105-134: Add a positive-path test in configBuilder.test.ts for
envValuesUsed() covering the effective ENV license case: when
REACT_ON_RAILS_PRO_LICENSE supplies the actual token and no configured
licenseToken overrides it, assert envValuesUsed().REACT_ON_RAILS_PRO_LICENSE is
'<MASKED>'. Use the existing helpers buildConfig and
envValuesUsedForRenderedConfig to locate the behavior, and keep the current
negative override test as-is.
In `@packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts`:
- Around line 27-29: Add parallel token-precedence coverage in
licenseValidator.test.ts for getLicenseOrganization and getLicensePlan,
mirroring the existing getLicenseStatus explicit-token, precedence, and fallback
cases so the shared loadLicenseString behavior stays protected. Update the test
suite around the getLicenseStatus/getLicenseOrganization/getLicensePlan
signatures to assert the explicit token path for each helper, and refactor the
repeated validPayload setup into a shared constant, factory, or beforeEach to
remove duplication across those test cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7ff6801-0cba-4d2b-9df6-bf8dd2c74d76
📒 Files selected for processing (34)
CHANGELOG.mddocs/oss/api-reference/generator-details.mddocs/oss/building-features/node-renderer/js-configuration.mddocs/oss/configuration/configuration-pro.mddocs/pro/deployment/review-app-security.mddocs/pro/installation.mddocs/pro/license-ci-integration.mddocs/pro/troubleshooting.mddocs/pro/updating.mddocs/pro/upgrading-to-pro.mdllms-full-pro.txtllms-full.txtpackages/react-on-rails-pro-node-renderer/README.mdpackages/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/configBuilder.tspackages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.tspackages/react-on-rails-pro-node-renderer/src/shared/logLicenseStatus.tspackages/react-on-rails-pro-node-renderer/tests/configBuilder.test.tspackages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.tspackages/react-on-rails-pro-node-renderer/tests/logLicenseStatus.test.tspackages/react-on-rails-pro-node-renderer/tests/masterStartupFailure.test.tspackages/react-on-rails-pro-node-renderer/tests/nodeRendererStartupChecks.test.tsreact_on_rails_pro/CLAUDE.mdreact_on_rails_pro/LICENSE_SETUP.mdreact_on_rails_pro/README.mdreact_on_rails_pro/lib/react_on_rails_pro/configuration.rbreact_on_rails_pro/lib/react_on_rails_pro/license_task_formatter.rbreact_on_rails_pro/lib/react_on_rails_pro/license_validator.rbreact_on_rails_pro/sig/react_on_rails_pro/configuration.rbsreact_on_rails_pro/spec/react_on_rails_pro/configuration_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/engine_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_task_formatter_spec.rbreact_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rb
|
Addressed the CodeRabbit coverage suggestions in a264e0f: added the positive effective-ENV diagnostic assertion ( |
Address-review summaryScan scope: full PR history; no prior summary. Mattered
Optional
Skipped
Follow-up issue: None. Next default scan starts after this comment. Say |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a264e0fb2d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Review Summary This is a well-scoped, thoroughly-tested feature that adds Rails ( Correctness
Security
Minor / optional nits (non-blocking)
Test coverage No blocking issues found — approving from a code-review standpoint. |
Address-review summaryScan scope: activity after latest summary at 2026-07-09T20:58:47Z (default cutoff). Mattered
Optional
Skipped
Follow-up issue: None. Validation: focused pre/post-fix Ruby probe; targeted Pro RuboCop; RBS validation; repository Prettier check; Pro license-header check; Pre-push review gate: manual self-review plus explicit Ruby/Node parity review; Next default scan starts after this comment. Say |
![claude[bot]](https://avatars.github.com/in/1236702?s=60&v=4){kind=small}
Review: Allow Pro license tokens from Rails and Node configurationReviewed the diff (34 files, +760/-131) — this is a well-scoped, well-tested change that adds Overview
Code quality / correctness
Risks / things to double check
Nice work — this is a solid, low-risk enablement change with strong test coverage on both runtimes. |
Address-review summaryScan scope: activity after latest summary at 2026-07-09T21:14:53Z (default cutoff). Mattered
Optional
Skipped
Follow-up issue: None. Validation: focused pre/post-fix Ruby probe; targeted Pro RuboCop; RBS validation; Pro license-header check; Next default scan starts after this comment. Say |
|
Review: Allow Pro license tokens from Rails and Node configuration Overview Correctness
Security
Test coverage
Minor observations (non-blocking)
Overall this is a solid, well-tested change with good attention to the security-sensitive masking requirement and to a real pre-existing cache-staleness bug. No blocking correctness or security issues found. |
|
@ihabadham my agent it trying to finish this one up. |
|
@ihabadham we need to decide if this one goes into release/17.0.0 rather than main |
Batch handoff — ROR A 07-09 17:33Immediate maintainer attention
FYI / decisions made
QA Evidenceqa-evidence v1
Merge ledgerpriority-finding-dispositions v1
Confidence note: high for the merged PR. Coordination dashboard URL is Next action: no PR action remains. Continue the repository’s normal post-merge |
|
@ihabadham @AbanoubGhadban this is on the release branch. It can be coming out very quickly after the first release. |
2 participants
Why
React on Rails Pro license validation only consumed
REACT_ON_RAILS_PRO_LICENSE, even though Rails applications commonly keep secrets in encrypted credentials and the renderer password already supports application configuration. That forced Rails deployments to expose the license through the process environment and left the separate Node renderer's configuration boundary unclear.What changed
ReactOnRailsPro.configuration.license_token, with explicit nonblank Rails configuration taking precedence overREACT_ON_RAILS_PRO_LICENSElicenseTokenoption with the same precedence and blank-value fallbackREACT_ON_RAILS_PRO_LICENSEas the backward-compatible default and generator pathRails configuration intentionally does not propagate to a separately running Node service. Each process must receive the same token through its own configuration or environment.
Validation
pnpm run lintpnpm start format.listDifferentpnpm run type-checkpnpm run build(cd react_on_rails_pro && BUNDLE_GEMFILE=../Gemfile bundle exec rubocop --ignore-parent-exclusion)(cd react_on_rails && bundle exec rake rbs:validate)node script/generate-llms-full.mjs --checkscript/check-docs-sidebarscript/check-pro-license-headersgit diff --checkLocal test suites were intentionally left to CI under the repository's React on Rails PR policy. An automated local review pass was stopped when it attempted to start those prohibited suites; no local test result is claimed.
Review churn
Demo coordination
No flagship demo change is needed: the existing environment-variable setup remains supported and generated defaults are unchanged. This adds optional secret-source configuration without changing the recommended generated deployment path.
Summary by CodeRabbit
config.license_tokensupport for Rails apps (including credentials/config) with precedence overREACT_ON_RAILS_PRO_LICENSE.licenseTokenoption with env fallback and masked diagnostics.Codex Decision Log
release/17.0.0rather thanmain?maintarget.Review Coverage Note
1325f12e7d921699f09b8c52270b4a964f814273.claude-reviewand CodeRabbit completed successfully.codex reviewcould not start because the installed CLI is too old for its configured model; the Claude CLI report-only fallback is not authenticated. Independent implementation review and required runtime QA were completed instead.