Repository navigation
feat: add DEPLOYMENT_MODE=tenant support - #795
Merged
Merged
Conversation
Add tenant deployment mode for managed team instances where billing is handled at the control plane level, not in-app. All users in a tenant instance get business-tier features. Changes: - Add isTenantMode() and isBillingEnabled() to deployment-mode utils - Update isCloud() to exclude tenant mode (both server and client) - Block billing/OAuth routes in middleware for tenant mode - Return business-tier storage limits for tenant mode users - Exclude Stripe/Google CSP directives in tenant mode - Update billing-visibility to use isBillingEnabled() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds tenant-mode detection and a billing-enabled gate; replaces on-prem-only checks with billing-aware logic across middleware, security headers, subscription/usage, and billing-visibility; expands cloud-only route blocking and aligns tenant with on-prem behavior and limits. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Edge as "Edge Middleware"
participant Mode as "Deployment Mode Service"
participant Handler as "Route Handler / API"
Client->>Edge: HTTP request (path)
Edge->>Mode: query isBillingEnabled / isTenantMode
Mode-->>Edge: billingEnabled? / tenant?
alt billing disabled AND path in CLOUD_ONLY_ROUTE_PREFIXES
Edge-->>Client: 404 (blocked cloud-only route)
else forward
Edge->>Handler: forward request
Handler-->>Client: 200 / handler response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
apps/web/src/lib/billing/billing-visibility.ts (1)
10-14: Update JSDoc to reflect tenant mode behavior.The comment states billing returns false for "on-prem deployments", but with the switch to
isBillingEnabled(), billing is now also hidden for tenant deployments. The documentation should be updated for accuracy.📝 Proposed documentation fix
/** * Determines whether billing UI should be shown. - * Returns false for iOS Capacitor apps and on-prem deployments. + * Returns false for iOS Capacitor apps, on-prem, and tenant deployments. * * Use this for non-React contexts. For React components, use `useBillingVisibility` hook. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/billing/billing-visibility.ts` around lines 10 - 14, Update the JSDoc for billing-visibility to accurately describe current behavior: note that billing UI is hidden for iOS Capacitor apps and for both tenant and on-prem deployments now that the logic uses isBillingEnabled(); reference the isBillingEnabled() check and keep the guidance to use useBillingVisibility for React components so the comment matches the implementation.packages/lib/src/__tests__/deployment-mode.test.ts (1)
64-70: Test coverage forisOnPremis incomplete.The
isOnPremtest suite only has one test case (tenant mode), while other functions have 3-4 cases covering variousDEPLOYMENT_MODEvalues. Consider adding tests forcloud,onprem, and unset modes for consistency.✅ Proposed additional test cases
describe('isOnPrem', () => { it('given DEPLOYMENT_MODE=tenant, should return false', async () => { vi.stubEnv('DEPLOYMENT_MODE', 'tenant'); const { isOnPrem } = await loadModule(); expect(isOnPrem()).toBe(false); }); + + it('given DEPLOYMENT_MODE=onprem, should return true', async () => { + vi.stubEnv('DEPLOYMENT_MODE', 'onprem'); + const { isOnPrem } = await loadModule(); + expect(isOnPrem()).toBe(true); + }); + + it('given DEPLOYMENT_MODE=cloud, should return false', async () => { + vi.stubEnv('DEPLOYMENT_MODE', 'cloud'); + const { isOnPrem } = await loadModule(); + expect(isOnPrem()).toBe(false); + }); + + it('given DEPLOYMENT_MODE is unset, should return false', async () => { + vi.stubEnv('DEPLOYMENT_MODE', ''); + const { isOnPrem } = await loadModule(); + expect(isOnPrem()).toBe(false); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lib/src/__tests__/deployment-mode.test.ts` around lines 64 - 70, The isOnPrem test suite only covers DEPLOYMENT_MODE='tenant'; add tests to cover the other modes by stubbing environment and asserting expected returns: add tests that set vi.stubEnv('DEPLOYMENT_MODE', 'cloud') expect isOnPrem() false, vi.stubEnv('DEPLOYMENT_MODE', 'onprem') expect isOnPrem() true, and a test with no DEPLOYMENT_MODE (clear stub or unset) to assert the default behavior; use the same pattern as the existing test (await loadModule() to import isOnPrem) so the module reads the stubbed env each time.packages/lib/src/deployment-mode.ts (1)
1-9: Update module JSDoc to document tenant mode.The module documentation only describes
DEPLOYMENT_MODE=onprembehavior, but this PR introducesDEPLOYMENT_MODE=tenant. Consider updating the JSDoc to explain tenant mode semantics for future maintainers.📝 Proposed documentation update
/** * Deployment mode detection utilities (server-side). * * When DEPLOYMENT_MODE=onprem, the application disables cloud-only features * (Stripe billing, OAuth, self-registration) and surfaces password auth, * local AI providers, and admin-managed user accounts. * + * When DEPLOYMENT_MODE=tenant, the application behaves similarly to on-prem + * (billing disabled, business-tier storage limits) but may have different + * authentication or infrastructure configurations. + * * All changes are inert unless the env var is explicitly set. */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/lib/src/deployment-mode.ts` around lines 1 - 9, Update the module JSDoc at the top of deployment-mode.ts to document the new DEPLOYMENT_MODE=tenant option: add a concise description of tenant mode semantics (how it differs from onprem and cloud), e.g., that tenant mode enables multi-tenant behavior, keeps cloud-only features disabled for individual tenants while allowing centralized management and tenant-scoped integrations, and list any behavioral effects (auth, billing, provisioning) so future maintainers see both onprem and tenant modes alongside the existing onprem description.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/lib/deployment-mode.ts`:
- Around line 12-22: The useBillingVisibility hook currently only checks
isOnPrem() and thus shows billing in tenant deployments; update
useBillingVisibility (the function using useCapacitor) to also call
isTenantMode() and treat tenant like on-prem by returning { showBilling: false,
hideBilling: true, isReady: true } when isOnPrem() || isTenantMode() is true,
and import isTenantMode from "@/lib/deployment-mode".
---
Nitpick comments:
In `@apps/web/src/lib/billing/billing-visibility.ts`:
- Around line 10-14: Update the JSDoc for billing-visibility to accurately
describe current behavior: note that billing UI is hidden for iOS Capacitor apps
and for both tenant and on-prem deployments now that the logic uses
isBillingEnabled(); reference the isBillingEnabled() check and keep the guidance
to use useBillingVisibility for React components so the comment matches the
implementation.
In `@packages/lib/src/__tests__/deployment-mode.test.ts`:
- Around line 64-70: The isOnPrem test suite only covers
DEPLOYMENT_MODE='tenant'; add tests to cover the other modes by stubbing
environment and asserting expected returns: add tests that set
vi.stubEnv('DEPLOYMENT_MODE', 'cloud') expect isOnPrem() false,
vi.stubEnv('DEPLOYMENT_MODE', 'onprem') expect isOnPrem() true, and a test with
no DEPLOYMENT_MODE (clear stub or unset) to assert the default behavior; use the
same pattern as the existing test (await loadModule() to import isOnPrem) so the
module reads the stubbed env each time.
In `@packages/lib/src/deployment-mode.ts`:
- Around line 1-9: Update the module JSDoc at the top of deployment-mode.ts to
document the new DEPLOYMENT_MODE=tenant option: add a concise description of
tenant mode semantics (how it differs from onprem and cloud), e.g., that tenant
mode enables multi-tenant behavior, keeps cloud-only features disabled for
individual tenants while allowing centralized management and tenant-scoped
integrations, and list any behavioral effects (auth, billing, provisioning) so
future maintainers see both onprem and tenant modes alongside the existing
onprem description.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f8ce5f39-f549-487b-9c8c-63106e038424
📒 Files selected for processing (9)
apps/web/middleware.tsapps/web/src/lib/billing/billing-visibility.tsapps/web/src/lib/deployment-mode.tsapps/web/src/middleware/security-headers.tspackages/lib/src/__tests__/deployment-mode.test.tspackages/lib/src/deployment-mode.tspackages/lib/src/index.tspackages/lib/src/services/__tests__/subscription-utils.test.tspackages/lib/src/services/subscription-utils.ts
- Update useBillingVisibility hook to use isBillingEnabled() instead of isOnPrem(), fixing tenant mode showing billing UI incorrectly - Update JSDoc comments to document tenant mode behavior - Add comprehensive isOnPrem test cases (onprem/cloud/tenant/unset) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
Author
Addressed all CodeRabbit review feedback in e852e38Actionable (major):
Nitpicks (all addressed):
|
usage-service.ts and rate-limit-middleware.ts only checked isOnPrem(), missing tenant mode. Tenant users would get free-tier rate limits (50 standard/0 pro calls/day) instead of unlimited. Change isOnPrem() to !isBillingEnabled() so both onprem and tenant modes bypass billing rate limits, matching the storage override pattern. Epic plan requirement: "Given DEPLOYMENT_MODE=tenant, AI model limits should return business-tier allowances" Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/src/lib/subscription/rate-limit-middleware.ts (1)
92-96: Update stale JSDoc to match billing-enabled behavior.Line 93 says only on-prem bypasses subscription gating, but Line 96 bypasses whenever billing is disabled (on-prem and tenant). Please align the comment with the actual guard.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/subscription/rate-limit-middleware.ts` around lines 92 - 96, The JSDoc above requiresProSubscription is stale: it claims only on-prem bypasses subscription gating but the function returns false whenever isBillingEnabled() is false (covering on-prem and non-billing tenants). Update the comment to state that subscription gating is bypassed when billing is disabled (i.e., both on-prem and tenants without billing), and mention the guard uses isBillingEnabled() to short-circuit; reference the function requiresProSubscription and the isBillingEnabled() check so readers know where the behavior is enforced.apps/web/src/lib/subscription/usage-service.ts (1)
14-18: Refresh “on-prem” comments to “billing-disabled” semantics.These comments still describe unlimited usage as on-prem only, but Line 18 now applies to all billing-disabled modes (including tenant). Updating wording will keep this self-documenting.
Also applies to: 81-82, 145-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/src/lib/subscription/usage-service.ts` around lines 14 - 18, Update the misleading "on-prem" comment wording to reflect "billing-disabled" semantics: change the comment above getUsageLimits (and the other occurrences around lines with similar comments) so it says it returns -1 for billing-disabled modes (i.e., when isBillingEnabled() is false) rather than referencing “on-prem”; locate the comments near the getUsageLimits function and the same comment blocks referenced by the reviewer and replace “on-prem” with “billing-disabled” (or similar phrasing like “billing-disabled deployments/modes, including tenant”) to accurately describe the behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/lib/subscription/__tests__/rate-limit-middleware.test.ts`:
- Around line 20-25: The test name/assertion mismatch: rename the existing case
to reflect that a non-pro model on cloud should NOT require a pro subscription,
and keep vi.stubEnv('DEPLOYMENT_MODE', 'cloud') and the call to
requiresProSubscription('pagespace', undefined, 'free') expecting false; then
add a new test that stubs cloud mode and passes a pro-tier model (use the same
symbol logic as getPageSpaceModelTier by supplying a model string that maps to
pro) to requiresProSubscription (e.g., requiresProSubscription('pagespace',
'<pro-model-string>', 'free')) and assert true to validate the pro-model path.
Ensure tests reference requiresProSubscription and rely on getPageSpaceModelTier
behavior.
---
Nitpick comments:
In `@apps/web/src/lib/subscription/rate-limit-middleware.ts`:
- Around line 92-96: The JSDoc above requiresProSubscription is stale: it claims
only on-prem bypasses subscription gating but the function returns false
whenever isBillingEnabled() is false (covering on-prem and non-billing tenants).
Update the comment to state that subscription gating is bypassed when billing is
disabled (i.e., both on-prem and tenants without billing), and mention the guard
uses isBillingEnabled() to short-circuit; reference the function
requiresProSubscription and the isBillingEnabled() check so readers know where
the behavior is enforced.
In `@apps/web/src/lib/subscription/usage-service.ts`:
- Around line 14-18: Update the misleading "on-prem" comment wording to reflect
"billing-disabled" semantics: change the comment above getUsageLimits (and the
other occurrences around lines with similar comments) so it says it returns -1
for billing-disabled modes (i.e., when isBillingEnabled() is false) rather than
referencing “on-prem”; locate the comments near the getUsageLimits function and
the same comment blocks referenced by the reviewer and replace “on-prem” with
“billing-disabled” (or similar phrasing like “billing-disabled
deployments/modes, including tenant”) to accurately describe the behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 16bb678e-4e63-46c6-9019-b9e74b781292
📒 Files selected for processing (8)
apps/web/src/hooks/useBillingVisibility.tsapps/web/src/lib/billing/billing-visibility.tsapps/web/src/lib/subscription/__tests__/rate-limit-middleware.test.tsapps/web/src/lib/subscription/__tests__/usage-service.test.tsapps/web/src/lib/subscription/rate-limit-middleware.tsapps/web/src/lib/subscription/usage-service.tspackages/lib/src/__tests__/deployment-mode.test.tspackages/lib/src/deployment-mode.ts
…omments Rename misleading test, add missing pro-model assertion, and replace "on-prem" comments with "billing-disabled" to accurately cover both on-prem and tenant deployment modes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
isTenantMode()andisBillingEnabled()to deployment mode utilities (server + client)isCloud()to exclude tenant mode — tenant is neither cloud nor onpremisBillingEnabled()instead ofisOnPrem()Context
Tenant mode is for managed team instances where billing is handled externally by the control plane. All users get max-tier (business) features. This is ~20 lines of logic across 7 files.
Files Changed
packages/lib/src/deployment-mode.tsisTenantMode(),isBillingEnabled(), updateisCloud()packages/lib/src/index.tsapps/web/src/lib/deployment-mode.tsapps/web/middleware.tsapps/web/src/middleware/security-headers.tspackages/lib/src/services/subscription-utils.tsapps/web/src/lib/billing/billing-visibility.tsisBillingEnabled()Test Plan
packages/lib/src/__tests__/deployment-mode.test.tscovering all mode combinationspackages/lib/src/services/__tests__/subscription-utils.test.tsfor tenant business-tier overrideDEPLOYMENT_MODE=tenantblocks/api/stripe/routes (manual/E2E)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
UI/Behavior
Security