Repository navigation
Restructure purchasable tiers: Standard ($5) and Pro ($20), persistent-service counts - #1242
Conversation
…re-22b0 Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR replaces the partner tier with Standard, adds independent Standard and Pro Stripe billing, migrates stored plan values, introduces bounded and persistent package-service modes, and enforces finite persistent-service limits through UserMeter. ChangesEntitlements, billing, and service limits
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AccountBillingPage
participant accountBilling
participant billingConfig
participant Stripe
AccountBillingPage->>accountBilling: Submit selected standard or pro plan
accountBilling->>billingConfig: Resolve plan price ID
billingConfig->>Stripe: Create checkout session
Stripe-->>AccountBillingPage: Return checkout session
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
|
🔎 Preview deployed: https://kody-pr-1242.kody-a99.workers.dev Worker: Mocks:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (1)
packages/worker/src/test-support/user-meter.ts (1)
363-375: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate mode inputs in the in-memory UserMeter.
When
input.modeis an unsupported string, this test double storespersistentat Line 374. Production UserMeter rejects that input. At Line 491, an unsupported filter returns zero instead of rejecting the request.Tests can pass for calls that fail in production. Reject unsupported non-
undefinedmodes in both RPC methods. Keep thepersistentdefault only when callers omitmode.Proposed fix
+ function normalizePackageServiceMode( + mode: string | undefined, + ): UserMeterPackageServiceMode { + if (mode === undefined) return 'persistent' + if (mode === 'bounded' || mode === 'persistent') return mode + throw new Error(`Invalid package service mode: ${JSON.stringify(mode)}`) + } + async upsertPackageServiceState(input: { packageId: string serviceName: string status: string mode?: string @@ - const mode: UserMeterPackageServiceMode = - input.mode === 'bounded' ? 'bounded' : 'persistent' + const mode = normalizePackageServiceMode(input.mode) @@ async countRunningPackageServices( @@ ) { + const mode = + input.mode === undefined + ? undefined + : normalizePackageServiceMode(input.mode) const now = input.now ? new Date(input.now) : new Date() @@ - if (input.mode && row.mode !== input.mode) continue + if (mode && row.mode !== mode) continueAlso applies to: 476-491
🤖 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/worker/src/test-support/user-meter.ts` around lines 363 - 375, Update the in-memory UserMeter RPC handlers around the `input.mode` normalization and the filter handling near the second method to reject any unsupported non-undefined mode, matching production behavior. Preserve `persistent` as the default only when `mode` is omitted, and ensure invalid mode filters throw instead of returning zero results.
🧹 Nitpick comments (3)
packages/worker/client/routes/account-billing.tsx (1)
509-509: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider an auto-fitting track for three cards.
Three fixed columns compress each card to about one third of the width at tablet sizes.
minmax(0, 1fr)prevents overflow, so this is readability only. An auto-fitting track wraps cards before they become too narrow, without adding a breakpoint.♻️ Proposed change
- gridTemplateColumns: 'repeat(3, minmax(0, 1fr))', + gridTemplateColumns: 'repeat(auto-fit, minmax(16rem, 1fr))',🤖 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/worker/client/routes/account-billing.tsx` at line 509, Update the gridTemplateColumns configuration for the three-card layout to use an auto-fitting responsive track that wraps cards when available width becomes too narrow, while preserving the existing minimum-width and flexible-column behavior.packages/worker/src/entitlements/plan-tier-migration.node.test.ts (1)
33-71: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the AUTOINCREMENT high-water mark.
The migration contains dedicated logic to preserve
sqlite_sequenceforusers(Lines 196-212 ofpackages/worker/migrations/0002-restructure-plan-tiers.sql). No test asserts it. Delete the highest-id user before running the migration, then assert that a new insert does not reuse that id.💚 Proposed test addition
test('tier migration preserves the users autoincrement high-water mark', () => { const db = new DatabaseSync(':memory:') db.exec(baselineSql) insertUser(db, { id: 10, plan: 'pro', stripePlan: null }) insertUser(db, { id: 42, plan: 'free', stripePlan: null }) db.prepare(`DELETE FROM users WHERE id = 42`).run() db.exec(tierMigrationSql) db.prepare( `INSERT INTO users (username, email, password_hash, stable_user_id) VALUES ('fresh', 'fresh@example.com', 'hash', 'stable-fresh')`, ).run() expect( db.prepare(`SELECT MAX(id) AS id FROM users`).get()?.id, ).toBe(43) })🤖 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/worker/src/entitlements/plan-tier-migration.node.test.ts` around lines 33 - 71, Add a test alongside the existing tier migration tests that inserts a user with the highest ID, deletes it before executing tierMigrationSql, then inserts a new user afterward and asserts the generated ID is one greater than the deleted high-water mark. Use the existing baselineSql and insertUser helpers, and keep the assertion focused on users AUTOINCREMENT behavior.packages/worker/src/billing/billing-config.node.test.ts (1)
128-167: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case where one subscription holds both price ids.
planFromSubscriptionshort-circuits on the Pro price and defers the Standard match throughmatchedStandardPrice. No test covers a subscription that contains both prices, and item order decides whether the flag path or the short-circuit runs first.💚 Proposed test addition
expect( resolveSubscriptionPlan( [ subscription({ status: 'active', priceIds: ['price_standard', 'price_pro'], }), ], env, ).stripePlan, ).toBe('pro')🤖 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/worker/src/billing/billing-config.node.test.ts` around lines 128 - 167, Add a test in the resolveSubscriptionPlan cases for a single active subscription whose priceIds contain both price_standard and price_pro, asserting the returned stripePlan is pro. This should cover planFromSubscription behavior regardless of the price ID order.
🤖 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.
Inline comments:
In `@docs/contributing/setup-manifest.md`:
- Around line 362-366: Update the GitHub Actions deployment section to document
both STRIPE_STANDARD_PRICE_ID and STRIPE_PRO_PRICE_ID as Worker deployment
secrets, and remove the outdated instruction describing STRIPE_PRO_PRICE_ID as a
committed Wrangler variable. Keep the documented secret names and plan mappings
consistent with the entries above.
In `@packages/worker/client/routes/account-billing.tsx`:
- Around line 246-249: Update startCheckout and its related completion/error
cleanup paths to track the pending plan in checkoutPendingPlan rather than the
shared checkoutPending boolean, setting it to the selected plan and resetting it
to null wherever checkoutPending is cleared. In the paid-card rendering logic,
show “Starting checkout…” only when that card’s plan matches
checkoutPendingPlan; leave other cards’ labels unchanged.
---
Outside diff comments:
In `@packages/worker/src/test-support/user-meter.ts`:
- Around line 363-375: Update the in-memory UserMeter RPC handlers around the
`input.mode` normalization and the filter handling near the second method to
reject any unsupported non-undefined mode, matching production behavior.
Preserve `persistent` as the default only when `mode` is omitted, and ensure
invalid mode filters throw instead of returning zero results.
---
Nitpick comments:
In `@packages/worker/client/routes/account-billing.tsx`:
- Line 509: Update the gridTemplateColumns configuration for the three-card
layout to use an auto-fitting responsive track that wraps cards when available
width becomes too narrow, while preserving the existing minimum-width and
flexible-column behavior.
In `@packages/worker/src/billing/billing-config.node.test.ts`:
- Around line 128-167: Add a test in the resolveSubscriptionPlan cases for a
single active subscription whose priceIds contain both price_standard and
price_pro, asserting the returned stripePlan is pro. This should cover
planFromSubscription behavior regardless of the price ID order.
In `@packages/worker/src/entitlements/plan-tier-migration.node.test.ts`:
- Around line 33-71: Add a test alongside the existing tier migration tests that
inserts a user with the highest ID, deletes it before executing
tierMigrationSql, then inserts a new user afterward and asserts the generated ID
is one greater than the deleted high-water mark. Use the existing baselineSql
and insertUser helpers, and keep the assertion focused on users AUTOINCREMENT
behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 74f638cc-ce6f-4970-bf6e-5f32f2d40bf7
📒 Files selected for processing (36)
docs/contributing/architecture/entitlements.mddocs/contributing/architecture/index.mddocs/contributing/environment-variables.mddocs/contributing/setup-manifest.mddocs/use/email-primitives.mddocs/use/index.mddocs/use/workflows.mdpackages/worker/.env.examplepackages/worker/client/routes/account-billing.tsxpackages/worker/client/routes/admin-insights.tsxpackages/worker/client/routes/pricing.tsxpackages/worker/migrations/0002-restructure-plan-tiers.sqlpackages/worker/src/admin/user-usage-data.node.test.tspackages/worker/src/app/account-billing-data.node.test.tspackages/worker/src/app/account-billing-data.tspackages/worker/src/app/handlers/account-billing.tspackages/worker/src/app/loader-data.tspackages/worker/src/billing/billing-config.node.test.tspackages/worker/src/billing/billing-config.tspackages/worker/src/blog/posts/zero-inference-calls.mdpackages/worker/src/entitlements/entitlements.node.test.tspackages/worker/src/entitlements/plan-tier-migration.node.test.tspackages/worker/src/entitlements/plans.tspackages/worker/src/entitlements/service.tspackages/worker/src/entitlements/usage-snapshot.node.test.tspackages/worker/src/entitlements/user-meter-do.tspackages/worker/src/entitlements/user-meter.workers.test.tspackages/worker/src/env-schema.tspackages/worker/src/mcp/capabilities/services/service-start.node.test.tspackages/worker/src/mcp/capabilities/services/service-start.tspackages/worker/src/og/pages.tspackages/worker/src/package-runtime/package-service.tspackages/worker/src/test-support/user-meter.tspackages/worker/worker-configuration.d.tspackages/worker/wrangler.jsonctools/migration-ledger.json
| - `STRIPE_STANDARD_PRICE_ID` (optional Worker secret; Stripe Price id mapped to | ||
| the $5/month `standard` plan and used for authenticated Checkout Sessions.) | ||
| - `STRIPE_PRO_PRICE_ID` (optional Worker secret; Stripe Price id mapped to the | ||
| $20/month `pro` plan and used for authenticated Checkout Sessions.) Each price | ||
| id is independent; an unset value only disables checkout for that tier. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Keep the Stripe deployment contract consistent.
These entries define both price IDs as Worker secrets. The GitHub Actions section below still says STRIPE_PRO_PRICE_ID is a committed Wrangler variable. Update that section to document both STRIPE_STANDARD_PRICE_ID and STRIPE_PRO_PRICE_ID as deployment secrets and remove the old variable instruction. Otherwise, a production deploy can omit Standard checkout configuration or keep the old Pro mapping.
🤖 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 `@docs/contributing/setup-manifest.md` around lines 362 - 366, Update the
GitHub Actions deployment section to document both STRIPE_STANDARD_PRICE_ID and
STRIPE_PRO_PRICE_ID as Worker deployment secrets, and remove the outdated
instruction describing STRIPE_PRO_PRICE_ID as a committed Wrangler variable.
Keep the documented secret names and plan mappings consistent with the entries
above.
…eckout pending Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5af94fb. Configure here.
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
Co-authored-by: Kent C. Dodds <me+github@kentcdodds.com>
What
Approved product restructure of the plan tiers:
pro→standard(same $5/month price and mostly unchanged limits),partner→pro(new $20/month purchasable tier).freeand operator-onlymaxremain. Rank order: free < standard < pro < max.executorTimeoutMs: null), so the concurrent count is the cost/margin control. Enforcement atservice_startcounts running persistent-mode services.0002-restructure-plan-tiers.sql: fail-closed assertions on unknown stored values, single-pass CASE rename (cannot double-apply) acrossusers.plan,users.stripe_plan, andinvites.plan, CHECK-constraint rebuild for the new vocabulary, FK-referencing tables snapshotted/restored, AUTOINCREMENT high-water mark preserved, post-migration assertion that nopartnervalues remain.STRIPE_STANDARD_PRICE_ID(the existing $5 price) resolves tostandard; newSTRIPE_PRO_PRICE_IDresolves toproat $20/month. Legacy subscription metadata (kody_planwritten before the rename) mapspartner→proandpro→standard; price-id resolution is preferred. Billing degrades gracefully when either env var is unset. Billing UI shows three cards: Free $0, Standard $5/mo, Pro $20/mo.Deployment steps (operator)
STRIPE_PRO_PRICE_IDto the new price id and move the old $5 price id toSTRIPE_STANDARD_PRICE_IDin deployment secrets.Testing
npm run validatefully green in the implementing environment (1,859 unit/Workers tests + 7 e2e) and re-validated by the conductor after merging latest main (which brought in the usage-visibility and admin-fleet-visibility PRs; one semantic conflict fixed — a new test referenced the retiredpartnerplan name).System recap — extends existing primitives (high care: users-table migration)
Mode: recap · Base:
main· Head:63e4c748Classification: extends — changes the plan registry vocabulary and limit contract, rebuilds
users/invitesCHECK constraints, and extends billing price resolution. No new primitives.Primitives touched
entitlementsbillingd1-app-dbusersandinviteswith new plan CHECKsapp-uipackage-servicesservice_startenforces persistent running countSystem map
The plan registry rename flows from the entitlements module through the users-table migration into billing price resolution and the billing UI; service_start picks up the persistent count.
Legend: green = composes (wiring only) · amber = extended by this PR · red = new primitive · gray = context (unchanged, included only when an edge crosses it).
Conductor report
Implemented by the tier-restructure track; PR opened and shepherded by the conductor after the track's environment could not create PRs. Conductor verified all limit values against the approved table, reviewed the migration, merged latest main, and re-ran validate. Status: awaiting CI + CodeRabbit, then squash-merge. Post-deploy: conductor migrates existing manual
maxgrants tostandard(keeping the operator and platform accounts).Summary by CodeRabbit
New Features
Bug Fixes
Documentation