Fix empty-string plan mismatch between Ruby and Node (#2452) by AbanoubGhadban · Pull Request #2566 · shakacode/react_on_rails

AbanoubGhadban · 2026-03-08T15:12:30Z

Summary

Align Node checkPlan with Ruby check_plan so plan: "" yields :invalid in both runtimes
Use plan == null instead of !plan in Node — only null/undefined count as absent (backwards compat), while "" falls through to the VALID_PLANS check
Add Ruby spec for plan: "" to document existing behavior

Test plan

Node licenseValidator.test.ts passes (empty string now expects 'invalid')
Ruby license_validator_spec.rb passes (60 examples, 0 failures, including new empty-string test)
null and undefined plan still return 'valid' (backwards compat preserved)

Closes #2452

🤖 Generated with Claude Code

Summary by CodeRabbit

Bug Fixes
- Updated license validation to consistently treat empty string plans as invalid.
- Enhanced null and undefined handling in plan validation to ensure more predictable licensing status determination.

In Ruby "" is truthy so `check_plan` correctly rejects it as invalid, but in Node "" is falsy so `checkPlan` incorrectly treated it as absent (valid). Use `plan == null` instead of `!plan` so only null/undefined are treated as absent, aligning Node behavior with Ruby. Closes #2452 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

coderabbitai · 2026-03-08T15:12:46Z

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d26dd7ef-6db1-44cc-8e4e-ea9b620c9328

📥 Commits

Reviewing files that changed from the base of the PR and between 76018a9 and b7ea002.

📒 Files selected for processing (3)

packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts
packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts
react_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rb

Walkthrough

This PR aligns Node.js plan validation behavior with Ruby's by treating empty string plans as invalid. Changes modify the falsy check in Node's validation logic to an explicit null check, ensuring empty strings are validated against the allowed plans list rather than treated as absent.

Changes

Cohort / File(s)	Summary
License Validator Logic `packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts`	Changed falsy check from `if (!plan)` to `if (plan == null)`, ensuring empty strings reach validation logic and are properly rejected as invalid instead of treated as absent.
Node Test Updates `packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts`	Updated test case expecting empty string plan to be invalid, with descriptive name change aligning with Ruby behavior semantics.
Ruby Test Coverage `react_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rb`	Added two new test contexts verifying that JWT payloads with empty string plans (`plan: ""`) are treated as invalid, extending test coverage for plan field validation across both runtimes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A rabbit hops through Ruby and Node,
Fixing mismatches along the code road,
Empty strings no longer pass by,
Both runtimes now unified and spry! ✨

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch 2452-ruby-node-empty-string-plan-mismatch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

greptile-apps · 2026-03-08T15:14:21Z

Greptile Summary

This PR fixes a cross-runtime behaviour mismatch where plan: "" was accepted as valid by the Node license validator but rejected by its Ruby counterpart. The fix is a minimal two-line change replacing !plan with plan == null in licenseValidator.ts, ensuring that only null and undefined plan values trigger the backwards-compatibility short-circuit, while an empty string now correctly falls through to the VALID_PLANS inclusion check and is rejected as invalid — matching Ruby's existing behaviour.

Key changes:

checkPlan: !plan → plan == null so "" reaches VALID_PLANS.includes("") and returns 'invalid'
determineLicensePlan: same guard updated for consistency
Node test description and expectation updated ('valid' → 'invalid' for plan: "")
Ruby spec gains a new example documenting the existing plan: "" → :invalid behaviour

Confidence Score: 5/5

This PR is safe to merge — the change is minimal, well-tested, and aligns two runtimes without breaking documented backwards-compat guarantees.
Both changed lines are targeted single-operator replacements (!plan → plan == null). Backwards-compat cases (null/undefined/absent plan → 'valid') are explicitly tested and unchanged. The only behavioural shift — "" plan now returns 'invalid' — is the desired fix and has matching test coverage in both Node and Ruby. No unrelated logic is touched.
No files require special attention.

Important Files Changed

Filename	Overview
packages/react-on-rails-pro-node-renderer/src/shared/licenseValidator.ts	Two `!plan` → `plan == null` changes: one in `checkPlan` (so `""` falls through to VALID_PLANS check → `'invalid'`) and one in `determineLicensePlan` (same outcome, `""` fails VALID_PLANS → `undefined`). Both are logically correct and the backwards-compat case (absent/`null`/`undefined` plan → `'valid'`) is preserved.
packages/react-on-rails-pro-node-renderer/tests/licenseValidator.test.ts	Test description and expected value updated to reflect the new behaviour: `plan: ""` now expects `'invalid'`. Existing backwards-compat cases (`null` plan → `'valid'`, absent plan → `'valid'`) are unchanged and still tested.
react_on_rails_pro/spec/react_on_rails_pro/license_validator_spec.rb	New context added to document that Ruby already returns `:invalid` for `plan: ""`. No logic changes; purely a spec gap that was missing before. A symmetrical spec for `plan: nil` (explicit null JWT claim) is absent but not a regression introduced here.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[checkPlan / check_plan called] --> B{plan == null\nor plan is nil?}
    B -- "yes (null / undefined / nil / absent)" --> C["return 'valid'\n(backwards compat)"]
    B -- "no (any other value\nincluding empty string)" --> D{VALID_PLANS\nincludes plan?}
    D -- yes --> E["return 'valid'"]
    D -- no --> F["return 'invalid'\n⬅ empty string now lands here"]

    style F fill:#f96,color:#000
    style C fill:#6c6,color:#000
    style E fill:#6c6,color:#000

_{Last reviewed commit: b7ea002}

claude · 2026-03-08T15:14:40Z

Code Review

This is a well-targeted fix for the Ruby/Node truthiness mismatch on empty-string plan values.

What's correct

Root cause correctly identified: Ruby treats "" as truthy, so return :valid unless plan in check_plan only short-circuits for nil/false — empty string falls through to the VALID_PLANS.include? check and correctly returns :invalid. Node's !plan treated "" as falsy, incorrectly returning 'valid'. The plan == null fix is the right idiom to match Ruby's nil-only bypass.

determineLicensePlan is also correct: After the fix, "" == null is false, but !VALID_PLANS.includes("") is still true, so the function still returns undefined for empty string. The effective return value doesn't change for determineLicensePlan, but the code path now consistently mirrors checkPlan's logic.

Backwards compatibility preserved: null/undefined plans (JWT fields absent or explicitly null) still return 'valid', maintaining compatibility with old paid licenses that predate the plan field.

Minor observations

Intentional inconsistency in loadLicenseString (not a bug): loadLicenseString still uses envLicense || undefined, treating a whitespace-only env var as "missing" rather than "invalid". This is intentional and documented in the comment above it, but creates a subtle asymmetry: an empty/blank REACT_ON_RAILS_PRO_LICENSE env var reports 'missing' while a valid JWT with plan: "" now reports 'invalid'. Both behaviors are correct for their respective contexts — just worth being aware of.

Breaking behavior change for plan: "": Any license JWT with plan: "" will now be rejected as invalid in Node (it was already rejected as invalid in Ruby). This is the correct behavior, but a CHANGELOG entry documenting the alignment would help operators who might debug an unexpected license status change after upgrading.

Ruby spec is additive documentation: The new spec in license_validator_spec.rb is a good addition — it explicitly documents that Ruby's behavior was already correct and pins it against future regressions.

Summary

Clean, minimal fix — the two changed lines in licenseValidator.ts directly address the mismatch described in #2452, the tests are updated appropriately, and the Ruby spec locks in the existing behavior. The only suggestion is a CHANGELOG entry given this is an observable behavior change.

claude · 2026-03-08T15:15:14Z

 function checkPlan(decodedData: LicenseData): LicenseStatus {
  const { plan } = decodedData;
-  if (!plan) {
+  if (plan == null) {


Good fix. The key insight is the Ruby/JS truthiness difference:

Ruby: unless plan only short-circuits for nil/false — "" is truthy and falls through to the VALID_PLANS.include? check → :invalid

JS before: !plan short-circuited for "" (falsy) → 'valid' ❌

JS after: plan == null only catches null/undefined → "" falls through → 'invalid' ✓

plan == null (loose equality) is the idiomatic JS equivalent of Ruby's unless plan.nil?.

claude · 2026-03-08T15:15:32Z

+      end
+
+      it "returns :invalid (empty string is not a valid plan)" do
+        expect(described_class.license_status).to eq(:invalid)


Good addition. This spec pins Ruby's existing (correct) behavior and guards against a future refactor accidentally introducing the same !plan-style falsy check in Ruby. Worth noting in the spec description that this is already-correct behavior being documented, not a behavior change on the Ruby side.

github-actions · 2026-03-08T15:15:59Z

size-limit report 📦

Path	Size
react-on-rails/client bundled (gzip)	62.58 KB (0%)
react-on-rails/client bundled (gzip) (time)	62.58 KB (0%)
react-on-rails/client bundled (brotli)	53.7 KB (0%)
react-on-rails/client bundled (brotli) (time)	53.7 KB (0%)
react-on-rails-pro/client bundled (gzip)	63.59 KB (0%)
react-on-rails-pro/client bundled (gzip) (time)	63.59 KB (0%)
react-on-rails-pro/client bundled (brotli)	54.66 KB (-0.02% 🔽)
react-on-rails-pro/client bundled (brotli) (time)	54.67 KB (0%)
registerServerComponent/client bundled (gzip)	127.36 KB (0%)
registerServerComponent/client bundled (gzip) (time)	127.36 KB (0%)
registerServerComponent/client bundled (brotli)	61.55 KB (0%)
registerServerComponent/client bundled (brotli) (time)	61.55 KB (0%)
wrapServerComponentRenderer/client bundled (gzip)	121.94 KB (0%)
wrapServerComponentRenderer/client bundled (gzip) (time)	121.94 KB (0%)
wrapServerComponentRenderer/client bundled (brotli)	56.65 KB (0%)
wrapServerComponentRenderer/client bundled (brotli) (time)	56.65 KB (0%)

Add entries for user-visible changes since v16.4.0.rc.6: - #2539: env-var-driven ports in Procfile templates - #2417: rspack generator config path fix - #2419: precompile hook load-based execution fix - #2577: create-react-on-rails-app validation improvements - #2416: StreamResponse status fallback fix (Pro) - #2566: empty-string license plan mismatch fix (Pro) - Updated #2561 entry to include #2568 contributor credit Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

## Summary - Add changelog entries for 6 user-visible PRs merged since v16.4.0.rc.6 that were missing from `[Unreleased]` - Update existing #2561 entry to include #2568 contributor credit ### New entries added | Section | PR | Description | |---|---|---| | Added | #2539 | Environment-variable-driven ports in Procfile templates | | Fixed | #2417 | Rspack generator config path fix | | Fixed | #2419 | Precompile hook load-based execution fix | | Fixed | #2577 | `create-react-on-rails-app` validation improvements | | Pro Fixed | #2416 | StreamResponse status fallback fix | | Pro Fixed | #2566 | Empty-string license plan mismatch fix | ### Skipped PRs (not user-visible) Docs (#2406, #2414, #2479, #2494, #2518, #2537, #2544), CI/internal (#2533, #2547, #2555, #2557, #2558, #2564), dependabot (#2387, #2541), dev dependencies (#2559, #2569, #2573). ## Test plan - [ ] Verify changelog formatting matches existing entries - [ ] Verify all user-visible PRs since v16.4.0.rc.6 are covered 🤖 Generated with [Claude Code](https://claude.com/claude-code)  --- > [!NOTE] > **Low Risk** > Documentation-only changelog updates with no runtime or build behavior changes. > > **Overview** > Updates `CHANGELOG.md`’s **[Unreleased]** section to include previously missing user-facing entries: Procfile templates now support env-driven ports, several generator/`bin/dev` precompile-hook and rspack-path fixes are documented, and `create-react-on-rails-app` validation improvements are noted. > > Also adds two Pro fix entries (StreamResponse status fallback and license plan empty-string validation) and updates the existing `bin/dev` precompile-hook entry to credit an additional PR/contributor. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit e75d2b5. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>  Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

AbanoubGhadban merged commit 61b1f49 into master Mar 8, 2026
26 of 27 checks passed

AbanoubGhadban deleted the 2452-ruby-node-empty-string-plan-mismatch branch March 8, 2026 15:12

claude Bot reviewed Mar 8, 2026

View reviewed changes

justin808 mentioned this pull request Mar 8, 2026

Release 16.4.0 Triage: PRs, Issues, and Prioritization #2560

Closed

justin808 mentioned this pull request Mar 9, 2026

Add missing changelog entries for unreleased PRs #2578

Merged

2 tasks

This was referenced Mar 17, 2026

Improve create/install validation flows and migration docs #2650

Merged

Docs: Add RSC landing, ExecJS limitations, debugging, and benchmarks pages #2785

Merged

Raise docs version floor to 16.4.0 in install/demo guidance #2610

Merged

This was referenced Apr 18, 2026

docs: address RSC pitfalls review follow-ups (#3155) #3156

Merged

refactor: unify renderer cache staging with mode: :copy / :symlink #3167

Merged

claude Bot mentioned this pull request Apr 22, 2026

Fix Bencher reporting permanently broken on pushes to main #3148

Merged

This was referenced May 4, 2026

feat: add REACT_ON_RAILS_BASE_PORT for concurrent worktree port management #3142

Merged

feat: rolling_deploy_adapter for seeding previous bundle hashes #3173

Merged

fix: scope RSC client reference discovery #3219

Merged

This was referenced May 25, 2026

ci: consolidate Ruby setup and Bundler caching #3354

Closed

Refuse releases on red main and surface main CI status #3407

Merged

Fix Pro RSC manifests for Rspack builds #3385

Closed

This was referenced Jun 2, 2026

Unify benchmark CI suites behind a matrix workflow #3528

Merged

[codex] Add Ruby 4 CI support #3529

Merged

Navigation Menu

Uh oh!

Uh oh!

Fix empty-string plan mismatch between Ruby and Node (#2452) - #2566

Fix empty-string plan mismatch between Ruby and Node (#2452)#2566
AbanoubGhadban merged 1 commit into
masterfrom
2452-ruby-node-empty-string-plan-mismatch

AbanoubGhadban commented Mar 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 8, 2026 •

edited

Loading

Review failed

Uh oh!

Uh oh!

greptile-apps Bot commented Mar 8, 2026

Uh oh!

claude Bot commented Mar 8, 2026

Uh oh!

claude Bot Mar 8, 2026

Uh oh!

claude Bot Mar 8, 2026

Uh oh!

github-actions Bot commented Mar 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

AbanoubGhadban commented Mar 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Estimated code review effort

Poem

Uh oh!

Uh oh!

greptile-apps Bot commented Mar 8, 2026

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

claude Bot commented Mar 8, 2026

Code Review

What's correct

Minor observations

Summary

Uh oh!

claude Bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

claude Bot Mar 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Mar 8, 2026

size-limit report 📦

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

AbanoubGhadban commented Mar 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 8, 2026 •

edited

Loading