Feature/razorpay integration #17
Conversation
- Implement core Razorpay service for customer and subscription management - Add webhook handler for subscription lifecycle events (activated, cancelled, paused, etc.) - Create billing API routes for checkout, cancel, pause, resume, and verify - Support PRO and TEAM subscription tiers with monthly/annual billing
- Add Razorpay environment variables to config - Update Prisma schema with razorpayCustomerId, razorpaySubscriptionId, razorpayCurrentPeriodEnd - Add Razorpay keys to .env.example documentation
- Create new billing dashboard page at /dashboard/billing - Implement Razorpay Standard Checkout integration - Update pricing constants with INR tiers (PRO: 99/588, TEAM: 499/2988) - Add tier badge component and subscription management UI
- Implement feature gate service for extension - Add server-side feature gate utilities - Define feature flags in shared constants - Support tier-based feature access (FREE, PRO, TEAM)
- Add razorpay dependency to server package.json - Register billing routes in Fastify server - Add Fastify raw body type declaration for webhooks - Update extension services index to export feature gating - Add tier badge variant for subscription tiers
- Export tier constants from shared package - Update home page with billing link - Update privacy policy page
- Add .env.local to gitignore for local environment variables - Ensure sensitive Razorpay credentials are not committed
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughWalkthroughAdds Razorpay-backed billing (server client, routes, webhooks, DB fields), a shared feature-gating model, extension feature-gating service and commands, web auth/billing UI and client API, per-app .env examples, and related minor packaging and UI updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as Browser/User
participant Web as Web App
participant Server as Billing API
participant Razorpay as Razorpay
participant DB as Database
User->>Web: Click "Upgrade" (tier, interval)
Web->>Server: POST /billing/checkout
Server->>DB: Get or create customer
Server->>Razorpay: Create subscription & order
Server->>DB: Persist subscriptionId/orderId
Server->>Web: Return { orderId, keyId }
Web->>Web: Load Razorpay checkout script
User->>Razorpay: Complete payment
Razorpay->>Web: Payment success callback
Web->>Server: POST /billing/verify (paymentId, subscriptionId, signature)
Server->>Razorpay: Verify signature / fetch subscription
Server->>DB: Update user tier & period end
Server->>Web: Return verification status
Web->>User: Show success/redirect
sequenceDiagram
participant Razorpay as Razorpay
participant Server as Webhook Handler
participant DB as Database
participant Log as Logger
Razorpay->>Server: POST /billing/webhooks (raw body + signature)
Server->>Server: verifyWebhookSignature(rawBody, signature)
alt signature invalid
Server->>Log: log invalid signature
Server-->>Razorpay: 400
else valid
Server->>Server: parse event and route
alt subscription event
Server->>DB: update subscription state & period end
Server->>Log: log subscription change
else payment/invoice event
Server->>Log: log payment/invoice details
end
Server-->>Razorpay: 200 ack
end
sequenceDiagram
participant User as Extension User
participant Extension as VS Code Extension
participant FeatureGate as FeatureGatingService
participant Auth as AuthService
participant Browser as Web Browser
User->>Extension: Activate "Enable Ghost Mode"
Extension->>FeatureGate: promptUpgrade('ghostMode')
FeatureGate->>Auth: getCurrentTier()
Auth-->>FeatureGate: returns tier
FeatureGate->>Extension: allow or deny
alt denied
Extension->>User: show upgrade prompt & open billing URL
else allowed
Extension->>Extension: toggle privacyMode setting
Extension->>User: show confirmation
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
🚥 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)
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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/package.json (1)
1-56: CI failure: Lockfile is out of date and must be regenerated.The pipeline is failing because the lockfile doesn't reflect the dependency changes. Run
pnpm installto regeneratepnpm-lock.yamland commit it.
🤖 Fix all issues with AI agents
In `@apps/extension/src/extension.ts`:
- Around line 801-816: The getWebAppUrl implementation is duplicated between
extension.ts and FeatureGatingService; remove the private getWebAppUrl method
from the Extension class and reuse a single implementation by exposing
FeatureGatingService.getWebAppUrl() as a public method, then update
handleOpenBilling to call this.featureGatingService.getWebAppUrl() (and keep
building billingUrl and opening it as before); ensure FeatureGatingService's
method is public (change visibility from private/protected to public if
necessary) and delete the redundant getWebAppUrl in extension.ts.
In `@apps/extension/src/services/featureGatingService.ts`:
- Line 133: The private field `disposables: vscode.Disposable[]` is never
populated so `dispose()` is effectively a no-op; either remove the unused
`disposables` field and the `dispose()` method if no cleanup is required, or add
actual disposables where event listeners or subscriptions are created (push them
into the `disposables` array) and then keep `dispose()` to iterate and dispose
them; update both the declaration (`disposables`) and the corresponding
`dispose()` implementation (also referenced around lines 229-233 in this file)
so they are consistent with whether cleanup is needed.
- Around line 14-129: The file duplicates types and feature data already defined
in packages/shared/src/features.ts (Feature, SubscriptionTier, FREE_FEATURES,
PRO_ADDITIONAL, TEAM_ADDITIONAL, SUBSCRIPTION_FEATURES, FEATURE_DESCRIPTIONS),
which risks drift; replace these local definitions by importing the shared
exports (Feature, SubscriptionTier, SUBSCRIPTION_FEATURES and any helpers like
getRequiredTier if present) from the shared package and remove the duplicated
constants (FREE_FEATURES, PRO_ADDITIONAL, TEAM_ADDITIONAL, FEATURE_DESCRIPTIONS)
here; update any local usages (e.g., functions referencing FEATURE_TIER_MAP or
getRequiredTier) to use the shared helper or derive tier info from
SUBSCRIPTION_FEATURES to ensure a single source of truth.
In `@apps/server/package.json`:
- Line 33: The dotenv dependency was bumped to v17.2.3 which changes the default
for config({ quiet }) from true to false; update your dotenv initialization so
it explicitly silences runtime info by calling dotenv.config({ quiet: true })
(or otherwise handle/log the new informational output), locating the existing
dotenv import and its config invocation and replacing it with the explicit
config({ quiet: true }) call.
In `@apps/server/src/lib/featureGate.ts`:
- Around line 197-201: The local const tierHierarchy inside the function
duplicates the module-level TIER_HIERARCHY; remove the inline declaration and
import TIER_HIERARCHY from the shared features module, then replace uses of
tierHierarchy with TIER_HIERARCHY (ensuring the SubscriptionTier typing aligns,
e.g., keep any explicit Record type if needed or rely on the imported constant's
type).
- Around line 13-136: This file duplicates types, constants and helpers from the
shared package; remove the local declarations and import them instead: delete
the local definitions of Feature, SubscriptionTier, FREE_FEATURES,
PRO_ADDITIONAL, TEAM_ADDITIONAL, SUBSCRIPTION_FEATURES, FEATURE_TIER_MAP,
FEATURE_DESCRIPTIONS and the functions hasFeatureAccess and getRequiredTier,
then add imports for those symbols from the shared package (the module that
contains the originals, e.g. packages/shared/src/features) and re-export or use
them directly so this file no longer maintains its own copies.
- Around line 148-161: The code unsafely asserts request.user and user.tier
which can throw or produce invalid tiers; update the middleware returned by the
feature gate to defensively validate request.user exists and has a string userId
before using it (check request.user and typeof request.user.userId === 'string',
throw AuthorizationError if not), then validate the DB result: ensure
db.user.findUnique returned a user and that user.tier is one of the allowed
SubscriptionTier values (e.g., whitelist/enum check) before assigning userTier;
if validation fails, throw AuthorizationError or a clear error so access control
cannot proceed with invalid data.
In `@apps/server/src/routes/billing.ts`:
- Around line 149-155: The verify endpoint's DB update (in the code that calls
db.user.update) sets tier and razorpayCurrentPeriodEnd but omits saving
razorpaySubscriptionId; modify the update payload to persist
razorpaySubscriptionId (set it to the subscription id returned by Razorpay,
e.g., subscriptionDetails.id) so subsequent logic like
handleSubscriptionActivated can look up the user by subscription id.
- Around line 161-180: The webhook route at '/webhooks' relies on
request.rawBody for Razorpay signature verification but the `@fastify/raw-body`
plugin is not registered, so request.rawBody will be undefined; fix this by
registering the `@fastify/raw-body` plugin in your server bootstrap (where Fastify
instance is created, e.g., in server.ts) with appropriate limits so route-level
config { rawBody: true } works, or alternatively add a preHandler hook on the
'/webhooks' route (similar to the approach used in routes/slack.ts) that
captures and attaches the raw request body to request.rawBody before signature
verification.
In `@apps/server/src/services/razorpay-webhooks.ts`:
- Around line 97-108: The update uses db.user.update(... where: {
razorpaySubscriptionId: subscriptionId }) which can miss a user when the webhook
arrives before the client sets razorpaySubscriptionId; use the userId extracted
from subscription.notes instead (userId variable) to locate the record or
perform an upsert by userId/razorpaySubscriptionId to ensure the row is updated.
Modify the db.user.update call to use where: { id: userId } (or an upsert that
sets razorpaySubscriptionId and updates tier and razorpayCurrentPeriodEnd) so
the subscription webhook cannot silently match zero rows.
In `@apps/server/src/services/razorpay.ts`:
- Around line 407-421: The current verifyPaymentSignature function compares hex
strings with === which is vulnerable to timing attacks; update
verifyPaymentSignature to compute the expected signature as a Buffer (e.g.,
Buffer.from(expectedSignature, 'hex')) and convert the incoming
razorpaySignature to a Buffer (Buffer.from(razorpaySignature, 'hex')), ensure
both buffers are the same length and then use crypto.timingSafeEqual to compare
them; apply the same change to the other signature-checking function referenced
around lines 423-432 so all signature comparisons use timingSafeEqual and
length-checked Buffers.
- Around line 22-23: The local SubscriptionTier type only includes 'PRO' and
'TEAM' while the shared definition in packages/shared/src/features.ts includes
'FREE' as well; either import the shared type (e.g., the exported type from
features.ts) and replace the local SubscriptionTier with that to keep types
consistent, or explicitly document and rename the local type to something like
BillingTier/ PaidSubscriptionTier (used by BillingInterval) and add a comment
stating it intentionally excludes 'FREE' because it's not billable; update any
references to SubscriptionTier in this file (e.g., functions or interfaces
interacting with Razorpay) to use the imported/shared type or the newly named
documented type.
- Around line 224-228: The order creation is using
subscription.notes.total_amount which may be missing and defaults to 0,
producing invalid Razorpay orders; update the logic around
clientCasted.orders.create to compute the amount from the subscription's
plan/pricing configuration (e.g., look up the plan by subscription.planId or
similar canonical field) or, if an order is not required for this flow, skip
calling clientCasted.orders.create entirely; ensure you validate the computed
amount is a positive integer before calling clientCasted.orders.create and throw
or return a clear error if no valid amount can be determined.
- Around line 123-143: The current use of clientCasted.customers.all() scans all
customers at once which will not scale and doesn't support email filtering;
change find-or-create logic in the function handling user.email to page through
customers using Razorpay's count/skip (fetch batches up to 100 with increasing
skip) until you find a matching email or exhaust results, replacing the single
customers.all() call with a loop that calls clientCasted.customers.all({ count,
skip }), preserves the existing behavior of updating db.user.update({ where: {
id: user.id }, data: { razorpayCustomerId: ... } }) and logging via
logger.info/logger.warn, and ensure the catch still handles request errors and
breaks the loop on fatal errors; alternatively consider switching to the
Contacts API if you decide to query by email directly.
- Line 215: The subscription payload sets total_count as strings; change
total_count to numeric values so Razorpay receives integers (e.g., total_count:
billingInterval === 'annual' ? 5 : 12) in the object where total_count is
defined (look for the total_count field in the subscription creation payload in
apps/server/src/services/razorpay.ts and the surrounding use of
billingInterval).
In `@apps/web/src/app/dashboard/billing/page.tsx`:
- Around line 152-157: Replace blocking alert() calls in the catch blocks and
other places with non-blocking toast notifications: import and use your app's
toast utility (e.g., sonner or react-hot-toast) instead of alert in the catch
inside the Checkout flow where you currently console.error('Checkout failed:',
error) and alert('Failed to initialize checkout. Please try again.'), and
similarly replace the alert usages around lines 167-169, 196, and 202; call the
toast function with a concise error message and optionally include error
details, then keep the existing setLoading(null) behavior in the finally blocks
so loading state still clears.
- Around line 135-137: The catch block that currently redirects to
"/dashboard/billing?canceled=true" conflates verification failures with user
cancellations; update the catch handling around the /verify flow (the try/catch
that sets window.location.href) to redirect to a distinct query (e.g.,
?verification_error=true or ?verification_failed=true) instead of ?canceled=true
and include the caught error details in a support/telemetry log (use the
existing logging/monitoring utility or console.error) so the team can
investigate failed verification after payment.
- Around line 66-68: The useEffect references fetchBillingStatus which is
recreated on every render; to satisfy React rules and the linter, either wrap
fetchBillingStatus in useCallback (exporting the existing function as const
fetchBillingStatus = useCallback(..., [dependencies]) and then include
fetchBillingStatus in the useEffect dependency array) or move the fetch logic
directly inside the useEffect so no external function is referenced; update the
useEffect to either include fetchBillingStatus in its dependency array (if you
use useCallback) or to call the inline fetch function defined within the effect.
- Around line 352-354: The monthly price calculation is wrong: replace the
hardcoded 50% discount logic used where PRICING_TIERS is iterated (the const
price = isAnnual ? Math.round(tier.price * 0.5) : tier.price) with a computation
that derives the displayed monthly rate from the configured annual amount (use
tier.annualPrice divided by 12 when isAnnual is true) so the Team and other
tiers reflect the actual annual plan configuration; keep the existing rounding
and the isCurrentPlan check that uses tier.id and billingStatus.tier.
In `@apps/web/src/app/privacy/page.tsx`:
- Line 246: Update the branding text in the JSX fragment that currently renders
<strong className="text-foreground">RazorPay</strong> to use the correct
provider name "Razorpay" (lowercase 'p') so it reads <strong
className="text-foreground">Razorpay</strong>; locate the string in the privacy
page component where the payment provider is listed and replace the text only
(no other changes).
In `@apps/web/src/lib/constants.ts`:
- Around line 141-148: The Team tier pricing is inconsistent: the price constant
(price: 499) does not match the inline comment and annualPrice which state
"₹249/month billed annually (₹2988/year)"; verify the intended monthly rate and
then update the constants accordingly—either set price to 249 and keep
annualPrice at 2988, or change the comment and annualPrice to reflect a
₹499/month rate (and adjust annualPrice to 499*12) — modify the price,
annualPrice and the inline comment near the Team object (symbols: price,
annualPrice, priceNote, description, razorpayPlanIds) to keep values and
comments consistent.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (20)
.env.example.gitignoreapps/extension/src/extension.tsapps/extension/src/services/featureGatingService.tsapps/extension/src/services/index.tsapps/server/package.jsonapps/server/prisma/schema.prismaapps/server/src/config.tsapps/server/src/lib/featureGate.tsapps/server/src/routes/billing.tsapps/server/src/server.tsapps/server/src/services/razorpay-webhooks.tsapps/server/src/services/razorpay.tsapps/server/src/types/fastify.d.tsapps/web/src/app/dashboard/billing/page.tsxapps/web/src/app/privacy/page.tsxapps/web/src/components/ui/badge.tsxapps/web/src/lib/constants.tspackages/shared/src/features.tspackages/shared/src/index.ts
💤 Files with no reviewable changes (1)
- apps/web/src/components/ui/badge.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
apps/server/src/server.ts (1)
apps/server/src/routes/billing.ts (1)
billingRoutes(37-271)
apps/server/src/services/razorpay.ts (5)
packages/shared/src/features.ts (1)
SubscriptionTier(39-39)apps/server/src/config.ts (1)
env(121-121)apps/server/src/lib/errors.ts (2)
InternalError(218-226)ValidationError(78-97)apps/server/src/services/db.ts (1)
getDb(20-33)apps/server/src/lib/logger.ts (1)
logger(68-68)
apps/extension/src/services/featureGatingService.ts (2)
packages/shared/src/features.ts (4)
Feature(11-34)SubscriptionTier(39-39)SUBSCRIPTION_FEATURES(86-90)FEATURE_DESCRIPTIONS(188-212)apps/server/src/lib/logger.ts (1)
logger(68-68)
apps/server/src/services/razorpay-webhooks.ts (3)
apps/server/src/services/razorpay.ts (1)
verifyWebhookSignature(423-432)apps/server/src/lib/logger.ts (1)
logger(68-68)apps/server/src/services/db.ts (1)
getDb(20-33)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 46-46: [UnorderedKey] The RAZORPAY_PRO_ANNUAL_PLAN_ID key should go before the RAZORPAY_PRO_MONTHLY_PLAN_ID key
(UnorderedKey)
[warning] 48-48: [UnorderedKey] The RAZORPAY_TEAM_ANNUAL_PLAN_ID key should go before the RAZORPAY_TEAM_MONTHLY_PLAN_ID key
(UnorderedKey)
🪛 GitHub Actions: CI
apps/server/package.json
[error] 1-1: Command failed: pnpm install --frozen-lockfile. Lockfile is out of date: 1 new dependency (razorpay@^2.9.6) added, and 3 dependencies mismatched (dotenv: lockfile ^16.5.0 vs manifest ^17.2.3; zod: lockfile ^3.24.0 vs manifest ^3.25.76; esbuild: lockfile ^0.24.2 vs manifest ^0.27.2). Run 'pnpm install --no-frozen-lockfile' or update the lockfile to resolve.
🔇 Additional comments (24)
.gitignore (1)
50-52: LGTM! The.gitignoreadditions follow standard practices.The entries for
rules/,docs/, andopencode.jsoncappropriately exclude generated or project-specific artifacts. Since these directories and files do not currently exist in the repository and there's no documentation generation tooling configured, thedocs/exclusion poses no risk to version-controlling important manual documentation.apps/server/src/types/fastify.d.ts (1)
13-15: LGTM!The
rawBodyaugmentation onFastifyRequestis correctly implemented for webhook signature verification. This is a standard pattern for payment provider integrations that require raw body access..env.example (1)
38-54: LGTM!The Razorpay environment configuration is well-structured with clear documentation. The separation between backend secrets (
RAZORPAY_KEY_SECRET,RAZORPAY_WEBHOOK_SECRET) and the frontend-safe public key (NEXT_PUBLIC_RAZORPAY_KEY_ID) follows best practices.apps/server/prisma/schema.prisma (2)
24-26: LGTM on field definitions.The Razorpay billing fields are correctly typed as optional with appropriate
@uniqueconstraints to ensure 1:1 mapping between users and Razorpay entities.
23-27: Consider adding an index onrazorpaySubscriptionId.Razorpay webhooks frequently reference
subscription_idfor event lookups. While you have an index onrazorpayCustomerId, adding one onrazorpaySubscriptionIdwould improve query performance when handling subscription-related webhook events (e.g.,subscription.charged,subscription.cancelled).Suggested addition
@@index([username]) @@index([githubId]) @@index([razorpayCustomerId]) + @@index([razorpaySubscriptionId])Likely an incorrect or invalid review comment.
apps/server/package.json (1)
39-39: No action needed. Therazorpaypackage is already at the latest stable version (2.9.6).packages/shared/src/features.ts (1)
1-212: Well-designed feature gating module.This is a clean, well-documented implementation with:
- Clear type definitions and separation of concerns
- Proper tier inheritance (PRO includes FREE, TEAM includes PRO+FREE)
- Useful utility functions for access checks and upgrade paths
- Human-readable descriptions for UI
The design provides a single source of truth for feature access control across server and client.
packages/shared/src/index.ts (1)
4-4: LGTM!The re-export follows the existing pattern and correctly exposes the new feature gating utilities from the shared package.
apps/extension/src/services/index.ts (1)
5-5: LGTM!The new
FeatureGatingServiceexport follows the established module re-export pattern.apps/server/src/server.ts (1)
28-28: LGTM!The billing routes are correctly imported and registered at the top-level
/billingprefix, consistent with other webhook-related routes like/authand/webhooks. This makes sense for payment webhooks that need a stable, non-versioned endpoint.Also applies to: 206-206
apps/server/src/routes/billing.ts (2)
26-35: LGTM!The Zod validation schemas are well-defined with appropriate enum constraints for tier and billing interval.
195-270: LGTM!The
/statusand/subscriptionendpoints provide appropriate separation—lightweight status check vs. detailed subscription data. Both are properly authenticated and handle missing users consistently.apps/server/src/services/razorpay-webhooks.ts (3)
12-22: LGTM!The webhook payload interface is appropriately typed for Razorpay events with optional nested entities.
113-176: LGTM!The cancellation and completion handlers correctly look up users by
razorpaySubscriptionIdand reset to FREE tier. This works because these events only fire for existing subscriptions where the ID is already stored.
255-279: Invoice paid handler performs a read but doesn't update subscription state.
handleInvoicePaidlooks up the user by subscription ID but only logs. If this is for renewal tracking, consider updatingrazorpayCurrentPeriodEndto reflect the new billing period.Is this intentional logging-only behavior, or should subscription renewal update the period end date?
apps/web/src/app/dashboard/billing/page.tsx (2)
521-533: LGTM!The Suspense wrapper correctly handles
useSearchParamswhich requires a Suspense boundary in Next.js App Router client components.
36-49: LGTM!The dynamic script loader correctly checks for existing Razorpay and handles async loading with proper error rejection.
apps/extension/src/extension.ts (3)
14-14: LGTM!The
FeatureGatingServiceis properly imported, instantiated with the correct dependencies, and registered in the disposables collection for lifecycle management.Also applies to: 52-53, 83-88, 101-102
207-214: LGTM!New commands for ghost mode and billing are properly registered following the existing command registration pattern.
782-799: LGTM!The ghost mode handler correctly gates the feature access before enabling. The flow properly:
- Checks feature access via
promptUpgrade- Returns early if access denied
- Toggles privacy mode and notifies the user
- Sends status update to sync with server
apps/extension/src/services/featureGatingService.ts (2)
146-156: LGTM!The
hasAccessmethod correctly handles the unauthenticated case and defensively defaults toFREEtier when the user's tier is not set.
183-206: LGTM!The
promptUpgrademethod provides good UX by:
- Short-circuiting if user already has access
- Showing a clear warning with the required tier
- Offering an actionable "Upgrade Now" button
- Logging the upgrade action for analytics
apps/server/src/services/razorpay.ts (2)
48-59: LGTM!The
isRazorpayEnabledfunction properly validates all required environment variables before enabling billing functionality.
61-75: LGTM!Lazy initialization of the Razorpay client is a good pattern. The singleton ensures the SDK is loaded only when needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| "@slack/types": "^2.16.0", | ||
| "@slack/web-api": "^7.12.0", | ||
| "dotenv": "^16.5.0", | ||
| "dotenv": "^17.2.3", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
dotenv npm package v17 breaking changes release notes
💡 Result:
Key breaking change in dotenv v17 (from the changelog):
- Default for config({ quiet }) changed to false, so dotenv now prints an informational runtime log by default (file and keys count). If you want to suppress it, call config({ quiet: true }). [1][2]
Context / related notes:
- The changelog documents v17.0.0 as the release that flips quiet to false and points to earlier notes about comment/value parsing changes introduced in earlier major releases. Review the full changelog for details. [1][2]
Sources:
[1] dotenv changelog (v17.0.0). https://unpkg.com/dotenv@17.0.0/files/CHANGELOG.md.
[2] dotenv npm package / v17 release notes (package page & CHANGELOG excerpts). https://www.npmjs.com/package/dotenv/v/17.0.1.
Verify dotenv v17 breaking change: quiet default changed to false.
The upgrade from ^16.5.0 to ^17.2.3 includes a breaking change: the default for config({ quiet }) changed from true to false. This means dotenv will now print informational runtime logs by default (file path and keys count). If your code depends on silent behavior, add config({ quiet: true }) or handle the new log output accordingly.
🤖 Prompt for AI Agents
In `@apps/server/package.json` at line 33, The dotenv dependency was bumped to
v17.2.3 which changes the default for config({ quiet }) from true to false;
update your dotenv initialization so it explicitly silences runtime info by
calling dotenv.config({ quiet: true }) (or otherwise handle/log the new
informational output), locating the existing dotenv import and its config
invocation and replacing it with the explicit config({ quiet: true }) call.
| /* Razorpay Billing (Required for billing features) */ | ||
| RAZORPAY_KEY_ID: z.string().trim().min(1, 'RAZORPAY_KEY_ID is required'), | ||
| RAZORPAY_KEY_SECRET: z.string().trim().min(1, 'RAZORPAY_KEY_SECRET is required'), | ||
| RAZORPAY_WEBHOOK_SECRET: z.string().trim().min(1, 'RAZORPAY_WEBHOOK_SECRET is required'), | ||
| RAZORPAY_PRO_MONTHLY_PLAN_ID: z | ||
| .string() | ||
| .trim() | ||
| .min(1, 'RAZORPAY_PRO_MONTHLY_PLAN_ID is required'), | ||
| RAZORPAY_PRO_ANNUAL_PLAN_ID: z | ||
| .string() | ||
| .trim() | ||
| .min(1, 'RAZORPAY_PRO_ANNUAL_PLAN_ID is required'), | ||
| RAZORPAY_TEAM_MONTHLY_PLAN_ID: z | ||
| .string() | ||
| .trim() | ||
| .min(1, 'RAZORPAY_TEAM_MONTHLY_PLAN_ID is required'), | ||
| RAZORPAY_TEAM_ANNUAL_PLAN_ID: z | ||
| .string() | ||
| .trim() | ||
| .min(1, 'RAZORPAY_TEAM_ANNUAL_PLAN_ID is required'), | ||
| WEB_APP_URL: z.string().url().default('http://localhost:3000'), |
There was a problem hiding this comment.
Required Razorpay variables may break existing deployments.
Unlike Slack integration (optional), Razorpay variables are unconditionally required. This means all existing deployments without Razorpay credentials will fail to start. Consider either:
- Making them optional like Slack (with a
superRefinefor all-or-nothing validation) - Adding a feature flag to conditionally require billing
Also, RAZORPAY_WEBHOOK_SECRET uses min(1) validation, but GITHUB_WEBHOOK_SECRET enforces min(32). Webhook secrets should be cryptographically strong to prevent signature forgery.
♻️ Suggested change for webhook secret validation
- RAZORPAY_WEBHOOK_SECRET: z.string().trim().min(1, 'RAZORPAY_WEBHOOK_SECRET is required'),
+ RAZORPAY_WEBHOOK_SECRET: z.string().trim().min(32, 'RAZORPAY_WEBHOOK_SECRET must be at least 32 characters'),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| /* Razorpay Billing (Required for billing features) */ | |
| RAZORPAY_KEY_ID: z.string().trim().min(1, 'RAZORPAY_KEY_ID is required'), | |
| RAZORPAY_KEY_SECRET: z.string().trim().min(1, 'RAZORPAY_KEY_SECRET is required'), | |
| RAZORPAY_WEBHOOK_SECRET: z.string().trim().min(1, 'RAZORPAY_WEBHOOK_SECRET is required'), | |
| RAZORPAY_PRO_MONTHLY_PLAN_ID: z | |
| .string() | |
| .trim() | |
| .min(1, 'RAZORPAY_PRO_MONTHLY_PLAN_ID is required'), | |
| RAZORPAY_PRO_ANNUAL_PLAN_ID: z | |
| .string() | |
| .trim() | |
| .min(1, 'RAZORPAY_PRO_ANNUAL_PLAN_ID is required'), | |
| RAZORPAY_TEAM_MONTHLY_PLAN_ID: z | |
| .string() | |
| .trim() | |
| .min(1, 'RAZORPAY_TEAM_MONTHLY_PLAN_ID is required'), | |
| RAZORPAY_TEAM_ANNUAL_PLAN_ID: z | |
| .string() | |
| .trim() | |
| .min(1, 'RAZORPAY_TEAM_ANNUAL_PLAN_ID is required'), | |
| WEB_APP_URL: z.string().url().default('http://localhost:3000'), | |
| /* Razorpay Billing (Required for billing features) */ | |
| RAZORPAY_KEY_ID: z.string().trim().min(1, 'RAZORPAY_KEY_ID is required'), | |
| RAZORPAY_KEY_SECRET: z.string().trim().min(1, 'RAZORPAY_KEY_SECRET is required'), | |
| RAZORPAY_WEBHOOK_SECRET: z.string().trim().min(32, 'RAZORPAY_WEBHOOK_SECRET must be at least 32 characters'), | |
| RAZORPAY_PRO_MONTHLY_PLAN_ID: z | |
| .string() | |
| .trim() | |
| .min(1, 'RAZORPAY_PRO_MONTHLY_PLAN_ID is required'), | |
| RAZORPAY_PRO_ANNUAL_PLAN_ID: z | |
| .string() | |
| .trim() | |
| .min(1, 'RAZORPAY_PRO_ANNUAL_PLAN_ID is required'), | |
| RAZORPAY_TEAM_MONTHLY_PLAN_ID: z | |
| .string() | |
| .trim() | |
| .min(1, 'RAZORPAY_TEAM_MONTHLY_PLAN_ID is required'), | |
| RAZORPAY_TEAM_ANNUAL_PLAN_ID: z | |
| .string() | |
| .trim() | |
| .min(1, 'RAZORPAY_TEAM_ANNUAL_PLAN_ID is required'), | |
| WEB_APP_URL: z.string().url().default('http://localhost:3000'), |
| price: 99, | ||
| annualPrice: 588, // ₹49/month billed annually (₹588/year) | ||
| description: 'For serious developers', | ||
| razorpayPlanIds: { | ||
| monthly: process.env.NEXT_PUBLIC_RAZORPAY_PRO_MONTHLY_PLAN_ID || 'plan_pro_monthly', | ||
| annual: process.env.NEXT_PUBLIC_RAZORPAY_PRO_ANNUAL_PLAN_ID || 'plan_pro_annual', | ||
| }, |
There was a problem hiding this comment.
Pricing comment inconsistency and placeholder plan IDs.
-
The comment states "₹49/month billed annually" but the monthly
priceis ₹99, not ₹49. The annualPrice of ₹588 suggests ~₹49/month, so either the monthly price or the comment is incorrect. -
The fallback plan IDs (
'plan_pro_monthly', etc.) are generic placeholders that won't work with Razorpay. If these environment variables aren't set, subscription creation will fail silently or with confusing errors.
Consider using empty strings or undefined as fallbacks to fail fast, or validate at build time.
- Add AuthContext for user session management and auth state - Create API client with auth header injection for billing endpoints - Update dashboard to show user content when signed in - Update header with Sign In/Sign Out button and user dropdown menu - Add AuthProvider to app layout for global auth state - Update billing page with authenticated API calls and redirect
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In `@apps/web/src/app/dashboard/billing/page.tsx`:
- Around line 156-158: The catch block that currently calls
console.error('Failed to check subscription status') discards the error object;
change it to capture the thrown error (e.g., catch (err) or catch (error)) and
include that error when logging so you get the stack/message (for example,
console.error('Failed to check subscription status', err)); update the catch in
the subscription-status check where console.error is used so the real error
details are preserved for debugging.
In `@apps/web/src/app/dashboard/page.tsx`:
- Around line 11-16: You’re mixing a Server Component export (export const
metadata) with Client Component hooks (useAuth used in SignedOutView,
SignedInView, and DashboardPage), which Next.js App Router forbids; fix by
splitting the file: keep page.tsx as the Server Component that exports metadata
and simply imports/renders a new Client Component (e.g., DashboardContent), then
move all hook-using logic (useAuth, SignedOutView, SignedInView, and
DashboardPage’s UI) into a new dashboard-content.tsx with a 'use client'
directive at the top; ensure page.tsx only imports and returns <DashboardContent
/> and that dashboard-content.tsx contains the original hook calls and component
implementations.
- Around line 36-42: The GitHub icon SVG (the <svg> with className "w-5 h-5
mr-2" in page.tsx) is decorative and should be hidden from assistive tech; add
aria-hidden="true" and focusable="false" to that <svg> element so it isn't
announced or focusable while keeping the visible text ("Continue with GitHub")
for screen readers.
In `@apps/web/src/components/layout/header.tsx`:
- Around line 99-106: Replace the semantically incorrect Settings icon used in
the Billing Link with the CreditCard icon: in the JSX for the Link component
that navigates to "/dashboard/billing" (the element using Settings and calling
setIsOpen onClick), swap the <Settings className="w-4 h-4" /> node for
<CreditCard className="w-4 h-4" /> and ensure CreditCard is imported (e.g., from
lucide-react) at the top of the module so the component compiles.
- Around line 108-119: The Sign Out button lacks an explicit type which can
cause it to act as a submit button in forms; update the <button> element inside
the header component (the one that calls setIsOpen(false) and signOut()) to
include type="button" so it won’t trigger form submissions — locate the button
that renders <LogOut /> and the signOut call and add the type attribute.
- Around line 59-76: The button that toggles the menu in the header lacks an
explicit type, which can cause accidental form submissions; update the button
element in header.tsx (the one using onClick={() => setIsOpen(!isOpen)} and
rendering user.avatarUrl / <User /> fallback) to include type="button" so it
never acts as a submit button when nested in a form.
- Around line 260-285: The Dashboard button currently calls signIn() even when
isAuthenticated is true; update the authenticated branch's onClick handler in
the header component (the Button rendered when isAuthenticated is true) to close
the mobile menu with setIsMobileMenuOpen(false) and navigate to "/dashboard"
instead of calling signIn(); use the existing navigation utility (e.g.,
next/router useRouter().push or the app's router helper) to perform the
navigation so authenticated users are directed to the Dashboard.
In `@apps/web/src/lib/auth/auth-context.tsx`:
- Line 1: This file uses React hooks (createContext, useContext, useState,
useEffect, useCallback) and must be a Client Component: add the `"use client"`
directive as the very first line of the file before any imports so the auth
context code (including createContext, AuthProvider, useAuth or any exported
hooks) is treated as client-side and the build will succeed.
- Around line 61-68: The signIn function unconditionally redirects to /dashboard
if an auth_token exists without validating it; update signIn to check the
authenticated user state (e.g., the user variable in auth-context) or call
refreshUser() to validate/refresh the token before redirecting; specifically,
modify signIn to await refreshUser() or check user !== null (or
user.isAuthenticated) and only set window.location.href = '/dashboard' when
validation succeeds, otherwise fall back to initiating the GitHub OAuth redirect
to `${API_URL}/auth/github?redirect_uri=${window.location.origin}/dashboard`.
In `@apps/web/src/lib/auth/index.ts`:
- Around line 1-3: Re-export the User interface from auth-context so consumers
can import it from this module; update index.ts to add a type-only export of
User (e.g., export type { User } from './auth-context') alongside the existing
exports (AuthProvider, useAuth, authApi, ProtectedRoute) so callers can
reference the User type without importing auth-context directly.
In `@apps/web/src/lib/auth/protected-route.tsx`:
- Around line 30-42: The SVG used in protected-route.tsx is missing accessible
labeling for screen readers; update the <svg> element (the lock icon) to include
an accessible title and ARIA attributes by adding a <title> element with a
unique id (e.g., "protectedRouteIconTitle"), set aria-labelledby to that id and
role="img" on the same <svg>, or alternatively provide an aria-label if you
prefer a single attribute; ensure the chosen id is unique to avoid collisions
and keep the visual rendering unchanged.
♻️ Duplicate comments (3)
apps/web/src/app/dashboard/billing/page.tsx (3)
74-78:fetchBillingStatusmissing from effect dependencies.The effect references
fetchBillingStatuswhich is redefined on each render. This was flagged in a previous review. Define the function inside the effect or wrap it inuseCallback.
119-121: Verification failure still redirects to?canceled=true.As flagged in a previous review, redirecting to
?canceled=truewhen payment verification fails is misleading—the payment may have succeeded but verification failed. Use a distinct error state.
344-346: Annual pricing calculation still incorrect.As flagged previously,
tier.price * 0.5doesn't reflect actual annual pricing fromPRICING_TIERS. Usetier.annualPrice / 12to display the correct monthly equivalent.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
apps/web/src/app/dashboard/billing/page.tsxapps/web/src/app/dashboard/page.tsxapps/web/src/app/layout.tsxapps/web/src/components/layout/header.tsxapps/web/src/lib/auth/api.tsapps/web/src/lib/auth/auth-context.tsxapps/web/src/lib/auth/index.tsapps/web/src/lib/auth/protected-route.tsx
🧰 Additional context used
🧬 Code graph analysis (4)
apps/web/src/app/dashboard/page.tsx (2)
apps/web/src/lib/auth/auth-context.tsx (1)
useAuth(104-110)apps/web/src/components/layout/container.tsx (1)
Container(18-29)
apps/web/src/components/layout/header.tsx (1)
apps/web/src/lib/auth/auth-context.tsx (2)
useAuth(104-110)User(3-10)
apps/web/src/lib/auth/protected-route.tsx (2)
apps/web/src/lib/auth/index.ts (2)
ProtectedRoute(3-3)useAuth(1-1)apps/web/src/lib/auth/auth-context.tsx (1)
useAuth(104-110)
apps/web/src/lib/auth/api.ts (2)
apps/web/src/lib/auth/index.ts (1)
authApi(2-2)apps/web/src/lib/auth/auth-context.tsx (1)
User(3-10)
🪛 Biome (2.1.2)
apps/web/src/app/dashboard/page.tsx
[error] 36-36: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
apps/web/src/components/layout/header.tsx
[error] 59-62: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 109-115: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/web/src/lib/auth/protected-route.tsx
[error] 30-35: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
🪛 GitHub Actions: CI
apps/web/src/lib/auth/auth-context.tsx
[error] 1-1: Turbopack build failed: You're importing a component that needs createContext. This React Hook only works in a Client Component. To fix, mark the file with the "use client" directive. Command failed: 'pnpm run build' for apps/web
[error] 1-1: Turbopack build failed: You're importing a component that needs useContext. This React Hook only works in a Client Component. To fix, mark the file with the "use client" directive. Command failed: 'pnpm run build' for apps/web
[error] 1-1: Turbopack build failed: You're importing a component that needs useState/useEffect/useCallback. These React Hooks only work in a Client Component. To fix, mark the file with the "use client" directive. Command failed: 'pnpm run build' for apps/web
🔇 Additional comments (12)
apps/web/src/lib/auth/auth-context.tsx (3)
70-86: LGTM!The
signOutfunction properly handles cleanup in thefinallyblock regardless of API call success, ensuring the user is always logged out client-side.
88-110: LGTM!The context provider and
useAuthhook are well-structured with proper error handling for usage outside the provider.
29-55:isLoadingremainstruewhen no token exists on initial load.When
refreshUserreturns early at line 34 due to missing token,setIsLoading(false)in thefinallyblock is never reached becausereturnexits before trying/catching. This leavesisLoadingstuck attrue, causing infinite loading states.🐛 Proposed fix
const refreshUser = useCallback(async () => { try { const token = localStorage.getItem('auth_token'); if (!token) { setUser(null); + setIsLoading(false); return; }Likely an incorrect or invalid review comment.
apps/web/src/lib/auth/api.ts (3)
1-1: Potential issue: Importing from a client-only module.
auth-context.tsxrequires the"use client"directive (once added). Importing types from it is fine, but ensure this file isn't accidentally included in server-side bundles. Thetypeimport is correct and safe.
5-8: LGTM!Good defensive check for server-side rendering with
typeof window !== 'undefined'before accessinglocalStorage.
28-96: LGTM!The
authApiobject provides a clean, typed interface for all billing-related API calls. Method signatures align with expected backend endpoints and return types are well-defined.apps/web/src/app/layout.tsx (2)
8-8: LGTM!Import path is clean using the barrel export from
@/lib/auth.
97-108: LGTM!Correctly wrapping the app content with
AuthProviderinsideThemeProvider. Client Components can be rendered as children in Server Components, and the context will be available to all nested client components.apps/web/src/lib/auth/protected-route.tsx (2)
1-4: LGTM!Correctly marked as a client component with
"use client"directive.
55-56: LGTM!Simple and correct rendering of children when authenticated.
apps/web/src/app/dashboard/billing/page.tsx (2)
517-529: Good use of Suspense boundary foruseSearchParams.Correctly wrapping the component that uses
useSearchParamsin a Suspense boundary, as required by Next.js App Router for client-side search params access.
68-72: Review comment is incorrect - no infinite redirect loop is possible.The
signIn()function redirects to/dashboard(not back to/billing), so after OAuth completes, users land on/dashboard, not/billing. On subsequent visits to the billing page,isAuthenticatedwill be true and the effect won't trigger. There is no redirect loop.However, there is a UX issue: users visiting
/dashboard/billingdirectly will be redirected to/dashboardupon authentication, requiring them to navigate back manually. Consider preserving the return URL or allowing direct access to the billing page after sign-in by passing it as a redirect parameter.Likely an incorrect or invalid review comment.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
apps/web/src/app/dashboard/page.tsx
Outdated
| import { useAuth } from '@/lib/auth'; | ||
|
|
||
| export const metadata: Metadata = { | ||
| title: 'Dashboard', | ||
| description: 'DevRadar Dashboard - Manage your account and view your coding activity.', | ||
| }; |
There was a problem hiding this comment.
Critical: Cannot use hooks alongside metadata export in the same file.
This file exports metadata (Server Component feature) while also using useAuth() hooks in SignedOutView, SignedInView, and DashboardPage. In Next.js App Router, you cannot mix Server Component exports with Client Component hooks in the same file.
This will cause a runtime error.
Recommended approach
Split the file:
- Keep
page.tsxas a Server Component with themetadataexport - Create a separate
dashboard-content.tsxClient Component with'use client'directive containing the hook-based logic - Import and render the client component from
page.tsx
// page.tsx (Server Component)
import type { Metadata } from 'next';
import { DashboardContent } from './dashboard-content';
export const metadata: Metadata = {
title: 'Dashboard',
description: 'DevRadar Dashboard - Manage your account and view your coding activity.',
};
export default function DashboardPage() {
return <DashboardContent />;
}// dashboard-content.tsx (Client Component)
'use client';
import { useAuth } from '@/lib/auth';
// ... rest of the component code🤖 Prompt for AI Agents
In `@apps/web/src/app/dashboard/page.tsx` around lines 11 - 16, You’re mixing a
Server Component export (export const metadata) with Client Component hooks
(useAuth used in SignedOutView, SignedInView, and DashboardPage), which Next.js
App Router forbids; fix by splitting the file: keep page.tsx as the Server
Component that exports metadata and simply imports/renders a new Client
Component (e.g., DashboardContent), then move all hook-using logic (useAuth,
SignedOutView, SignedInView, and DashboardPage’s UI) into a new
dashboard-content.tsx with a 'use client' directive at the top; ensure page.tsx
only imports and returns <DashboardContent /> and that dashboard-content.tsx
contains the original hook calls and component implementations.
| <svg className="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 24 24"> | ||
| <path | ||
| fillRule="evenodd" | ||
| d="M12 2C6.477 2 2 6.477 2 12c0 4.42 2.87 8.17 6.84 9.5.5.08.66-.23.66-.5v-1.69c-2.77.6-3.36-1.34-3.36-1.34-.46-1.16-1.11-1.47-1.11-1.47-.91-.62.07-.6.07-.6 1 .07 1.53 1.03 1.53 1.03.87 1.52 2.34 1.07 2.91.83.09-.65.35-1.09.63-1.34-2.22-.25-4.55-1.11-4.55-4.92 0-1.11.38-2 1.03-2.71-.1-.25-.45-1.29.1-2.64 0 0 .84-.27 2.75 1.02.79-.22 1.65-.33 2.5-.33.85 0 1.71.11 2.5.33 1.91-1.29 2.75-1.02 2.75-1.02.55 1.35.2 2.39.1 2.64.65.71 1.03 1.6 1.03 2.71 0 3.82-2.34 4.66-4.57 4.91.36.31.69.92.69 1.85V21c0 .27.16.59.67.5C19.14 20.16 22 16.42 22 12A10 10 0 0012 2z" | ||
| clipRule="evenodd" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
Add accessibility attributes to SVG.
The GitHub icon SVG lacks accessible labeling. Since it's paired with text ("Continue with GitHub"), it's decorative and should have aria-hidden="true".
Proposed fix
- <svg className="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 24 24">
+ <svg className="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 24 24" aria-hidden="true">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <svg className="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 24 24"> | |
| <path | |
| fillRule="evenodd" | |
| d="M12 2C6.477 2 2 6.477 2 12c0 4.42 2.87 8.17 6.84 9.5.5.08.66-.23.66-.5v-1.69c-2.77.6-3.36-1.34-3.36-1.34-.46-1.16-1.11-1.47-1.11-1.47-.91-.62.07-.6.07-.6 1 .07 1.53 1.03 1.53 1.03.87 1.52 2.34 1.07 2.91.83.09-.65.35-1.09.63-1.34-2.22-.25-4.55-1.11-4.55-4.92 0-1.11.38-2 1.03-2.71-.1-.25-.45-1.29.1-2.64 0 0 .84-.27 2.75 1.02.79-.22 1.65-.33 2.5-.33.85 0 1.71.11 2.5.33 1.91-1.29 2.75-1.02 2.75-1.02.55 1.35.2 2.39.1 2.64.65.71 1.03 1.6 1.03 2.71 0 3.82-2.34 4.66-4.57 4.91.36.31.69.92.69 1.85V21c0 .27.16.59.67.5C19.14 20.16 22 16.42 22 12A10 10 0 0012 2z" | |
| clipRule="evenodd" | |
| /> | |
| </svg> | |
| <svg className="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 24 24" aria-hidden="true"> | |
| <path | |
| fillRule="evenodd" | |
| d="M12 2C6.477 2 2 6.477 2 12c0 4.42 2.87 8.17 6.84 9.5.5.08.66-.23.66-.5v-1.69c-2.77.6-3.36-1.34-3.36-1.34-.46-1.16-1.11-1.47-1.11-1.47-.91-.62.07-.6.07-.6 1 .07 1.53 1.03 1.53 1.03.87 1.52 2.34 1.07 2.91.83.09-.65.35-1.09.63-1.34-2.22-.25-4.55-1.11-4.55-4.92 0-1.11.38-2 1.03-2.71-.1-.25-.45-1.29.1-2.64 0 0 .84-.27 2.75 1.02.79-.22 1.65-.33 2.5-.33.85 0 1.71.11 2.5.33 1.91-1.29 2.75-1.02 2.75-1.02.55 1.35.2 2.39.1 2.64.65.71 1.03 1.6 1.03 2.71 0 3.82-2.34 4.66-4.57 4.91.36.31.69.92.69 1.85V21c0 .27.16.59.67.5C19.14 20.16 22 16.42 22 12A10 10 0 0012 2z" | |
| clipRule="evenodd" | |
| /> | |
| </svg> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 36-36: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
🤖 Prompt for AI Agents
In `@apps/web/src/app/dashboard/page.tsx` around lines 36 - 42, The GitHub icon
SVG (the <svg> with className "w-5 h-5 mr-2" in page.tsx) is decorative and
should be hidden from assistive tech; add aria-hidden="true" and
focusable="false" to that <svg> element so it isn't announced or focusable while
keeping the visible text ("Continue with GitHub") for screen readers.
| <button | ||
| onClick={() => setIsOpen(!isOpen)} | ||
| className="flex items-center gap-2 p-1 rounded-full hover:bg-muted transition-colors" | ||
| > | ||
| <span className="relative z-10 flex items-center">{children}</span> | ||
| <div className="absolute inset-0 -translate-x-full group-hover:animate-[shine_1s_ease-in-out_infinite] bg-linear-to-r from-transparent via-white/20 to-transparent" /> | ||
| </Link> | ||
| </Button> | ||
| </motion.div> | ||
| {user.avatarUrl ? ( | ||
| <Image | ||
| src={user.avatarUrl} | ||
| alt={user.displayName || user.username} | ||
| width={32} | ||
| height={32} | ||
| className="rounded-full" | ||
| /> | ||
| ) : ( | ||
| <div className="w-8 h-8 rounded-full bg-muted flex items-center justify-center"> | ||
| <User className="w-4 h-4" /> | ||
| </div> | ||
| )} | ||
| </button> |
There was a problem hiding this comment.
Add explicit type="button" to prevent form submission issues.
The button lacks an explicit type attribute. Per the static analysis hint, this can cause unintended form submissions if this component is ever placed inside a form.
Proposed fix
<button
+ type="button"
onClick={() => setIsOpen(!isOpen)}
className="flex items-center gap-2 p-1 rounded-full hover:bg-muted transition-colors"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <button | |
| onClick={() => setIsOpen(!isOpen)} | |
| className="flex items-center gap-2 p-1 rounded-full hover:bg-muted transition-colors" | |
| > | |
| <span className="relative z-10 flex items-center">{children}</span> | |
| <div className="absolute inset-0 -translate-x-full group-hover:animate-[shine_1s_ease-in-out_infinite] bg-linear-to-r from-transparent via-white/20 to-transparent" /> | |
| </Link> | |
| </Button> | |
| </motion.div> | |
| {user.avatarUrl ? ( | |
| <Image | |
| src={user.avatarUrl} | |
| alt={user.displayName || user.username} | |
| width={32} | |
| height={32} | |
| className="rounded-full" | |
| /> | |
| ) : ( | |
| <div className="w-8 h-8 rounded-full bg-muted flex items-center justify-center"> | |
| <User className="w-4 h-4" /> | |
| </div> | |
| )} | |
| </button> | |
| <button | |
| type="button" | |
| onClick={() => setIsOpen(!isOpen)} | |
| className="flex items-center gap-2 p-1 rounded-full hover:bg-muted transition-colors" | |
| > | |
| {user.avatarUrl ? ( | |
| <Image | |
| src={user.avatarUrl} | |
| alt={user.displayName || user.username} | |
| width={32} | |
| height={32} | |
| className="rounded-full" | |
| /> | |
| ) : ( | |
| <div className="w-8 h-8 rounded-full bg-muted flex items-center justify-center"> | |
| <User className="w-4 h-4" /> | |
| </div> | |
| )} | |
| </button> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 59-62: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🤖 Prompt for AI Agents
In `@apps/web/src/components/layout/header.tsx` around lines 59 - 76, The button
that toggles the menu in the header lacks an explicit type, which can cause
accidental form submissions; update the button element in header.tsx (the one
using onClick={() => setIsOpen(!isOpen)} and rendering user.avatarUrl / <User />
fallback) to include type="button" so it never acts as a submit button when
nested in a form.
| <Link | ||
| href="/dashboard/billing" | ||
| className="flex items-center gap-2 px-4 py-2 text-sm hover:bg-muted transition-colors" | ||
| onClick={() => setIsOpen(false)} | ||
| > | ||
| <Settings className="w-4 h-4" /> | ||
| Billing | ||
| </Link> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using CreditCard icon for Billing link.
The Settings icon is semantically misleading for a billing/payment link. CreditCard (already imported in the billing page) would better convey the purpose.
🤖 Prompt for AI Agents
In `@apps/web/src/components/layout/header.tsx` around lines 99 - 106, Replace the
semantically incorrect Settings icon used in the Billing Link with the
CreditCard icon: in the JSX for the Link component that navigates to
"/dashboard/billing" (the element using Settings and calling setIsOpen onClick),
swap the <Settings className="w-4 h-4" /> node for <CreditCard className="w-4
h-4" /> and ensure CreditCard is imported (e.g., from lucide-react) at the top
of the module so the component compiles.
| const signIn = () => { | ||
| const token = localStorage.getItem('auth_token'); | ||
| if (token) { | ||
| window.location.href = '/dashboard'; | ||
| } else { | ||
| window.location.href = `${API_URL}/auth/github?redirect_uri=${window.location.origin}/dashboard`; | ||
| } | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
signIn redirects even when already authenticated.
If a token exists, the user is redirected to /dashboard without verifying token validity. Consider checking user state instead, or calling refreshUser first to validate the token.
♻️ Suggested improvement
const signIn = () => {
- const token = localStorage.getItem('auth_token');
- if (token) {
+ if (user) {
window.location.href = '/dashboard';
} else {
window.location.href = `${API_URL}/auth/github?redirect_uri=${window.location.origin}/dashboard`;
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const signIn = () => { | |
| const token = localStorage.getItem('auth_token'); | |
| if (token) { | |
| window.location.href = '/dashboard'; | |
| } else { | |
| window.location.href = `${API_URL}/auth/github?redirect_uri=${window.location.origin}/dashboard`; | |
| } | |
| }; | |
| const signIn = () => { | |
| if (user) { | |
| window.location.href = '/dashboard'; | |
| } else { | |
| window.location.href = `${API_URL}/auth/github?redirect_uri=${window.location.origin}/dashboard`; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In `@apps/web/src/lib/auth/auth-context.tsx` around lines 61 - 68, The signIn
function unconditionally redirects to /dashboard if an auth_token exists without
validating it; update signIn to check the authenticated user state (e.g., the
user variable in auth-context) or call refreshUser() to validate/refresh the
token before redirecting; specifically, modify signIn to await refreshUser() or
check user !== null (or user.isAuthenticated) and only set window.location.href
= '/dashboard' when validation succeeds, otherwise fall back to initiating the
GitHub OAuth redirect to
`${API_URL}/auth/github?redirect_uri=${window.location.origin}/dashboard`.
| export { AuthProvider, useAuth } from './auth-context'; | ||
| export { authApi } from './api'; | ||
| export { ProtectedRoute } from './protected-route'; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider exporting the User type.
The User interface from auth-context.tsx may be needed by consumers of this module. Consider re-exporting it for convenience.
♻️ Suggested improvement
-export { AuthProvider, useAuth } from './auth-context';
+export { AuthProvider, useAuth, type User } from './auth-context';
export { authApi } from './api';
export { ProtectedRoute } from './protected-route';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export { AuthProvider, useAuth } from './auth-context'; | |
| export { authApi } from './api'; | |
| export { ProtectedRoute } from './protected-route'; | |
| export { AuthProvider, useAuth, type User } from './auth-context'; | |
| export { authApi } from './api'; | |
| export { ProtectedRoute } from './protected-route'; |
🤖 Prompt for AI Agents
In `@apps/web/src/lib/auth/index.ts` around lines 1 - 3, Re-export the User
interface from auth-context so consumers can import it from this module; update
index.ts to add a type-only export of User (e.g., export type { User } from
'./auth-context') alongside the existing exports (AuthProvider, useAuth,
authApi, ProtectedRoute) so callers can reference the User type without
importing auth-context directly.
| if (isLoading) { | ||
| return ( | ||
| fallback || ( | ||
| <div className="min-h-screen flex items-center justify-center"> | ||
| <div className="animate-spin rounded-full h-8 w-8 border-b-2 border-primary" /> | ||
| </div> | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider separate fallbacks for loading vs unauthenticated states.
The same fallback prop is used for both loading and unauthenticated states, but these are semantically different. Consider adding a loadingFallback prop for clarity.
| <svg | ||
| className="w-10 h-10 text-muted-foreground" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| viewBox="0 0 24 24" | ||
| > | ||
| <path | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| strokeWidth={2} | ||
| d="M12 15v2m-6 4h12a2 2 0 002-2v-6a2 2 0 00-2-2H6a2 2 0 00-2 2v6a2 2 0 002 2zm10-10V7a4 4 0 00-8 0v4h8z" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
Add accessible label to SVG for screen readers.
The SVG icon lacks alternative text, which is an accessibility issue flagged by static analysis.
🔧 Proposed fix
<svg
className="w-10 h-10 text-muted-foreground"
fill="none"
stroke="currentColor"
viewBox="0 0 24 24"
+ role="img"
+ aria-label="Lock icon"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <svg | |
| className="w-10 h-10 text-muted-foreground" | |
| fill="none" | |
| stroke="currentColor" | |
| viewBox="0 0 24 24" | |
| > | |
| <path | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| strokeWidth={2} | |
| d="M12 15v2m-6 4h12a2 2 0 002-2v-6a2 2 0 00-2-2H6a2 2 0 00-2 2v6a2 2 0 002 2zm10-10V7a4 4 0 00-8 0v4h8z" | |
| /> | |
| </svg> | |
| <svg | |
| className="w-10 h-10 text-muted-foreground" | |
| fill="none" | |
| stroke="currentColor" | |
| viewBox="0 0 24 24" | |
| role="img" | |
| aria-label="Lock icon" | |
| > | |
| <path | |
| strokeLinecap="round" | |
| strokeLinejoin="round" | |
| strokeWidth={2} | |
| d="M12 15v2m-6 4h12a2 2 0 002-2v-6a2 2 0 00-2-2H6a2 2 0 00-2 2v6a2 2 0 002 2zm10-10V7a4 4 0 00-8 0v4h8z" | |
| /> | |
| </svg> |
🧰 Tools
🪛 Biome (2.1.2)
[error] 30-35: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
🤖 Prompt for AI Agents
In `@apps/web/src/lib/auth/protected-route.tsx` around lines 30 - 42, The SVG used
in protected-route.tsx is missing accessible labeling for screen readers; update
the <svg> element (the lock icon) to include an accessible title and ARIA
attributes by adding a <title> element with a unique id (e.g.,
"protectedRouteIconTitle"), set aria-labelledby to that id and role="img" on the
same <svg>, or alternatively provide an aria-label if you prefer a single
attribute; ensure the chosen id is unique to avoid collisions and keep the
visual rendering unchanged.
- Add MagneticButton component with hover animation - Update UserMenu to use magnetic button for Sign In - Maintains user dropdown when authenticated
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/web/src/lib/auth/auth-context.tsx`:
- Around line 52-56: The catch block currently swallows errors (catch {
setUser(null); }) which hides failures; update it to capture the thrown error
(e) and log it before clearing user so failures are observable — e.g., in the
same async function/context that calls setUser and setIsLoading (refer to
setUser and setIsLoading in auth-context.tsx) call your logger or console.error
with a descriptive message and the error object, then setUser(null) and finally
setIsLoading(false) in the finally block.
♻️ Duplicate comments (1)
apps/web/src/lib/auth/auth-context.tsx (1)
63-70: Token validity not checked before redirect.This redirects to
/dashboardif any token exists, even if expired or invalid. The user would land on the dashboard and experience auth failures. Consider checking theuserstate (which is validated) instead of raw token presence.♻️ Suggested improvement
const signIn = () => { - const token = localStorage.getItem('auth_token'); - if (token) { + if (user) { window.location.href = '/dashboard'; } else { window.location.href = `${API_URL}/auth/github?redirect_uri=${window.location.origin}/dashboard`; } };
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/lib/auth/auth-context.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
apps/web/src/lib/auth/auth-context.tsx (1)
apps/web/src/lib/auth/index.ts (2)
AuthProvider(1-1)useAuth(1-1)
🔇 Additional comments (4)
apps/web/src/lib/auth/auth-context.tsx (4)
1-3: LGTM! Build issue resolved.The
'use client'directive is correctly placed at the top of the file, resolving the previous build failures related to React hooks requiring Client Component context.
5-21: LGTM!Type definitions are well-structured. The
Userinterface correctly includes thetierfield for the new billing integration, andAuthContextTypeproperly defines the context shape.
72-88: LGTM!The
finallyblock correctly ensures cleanup (token removal, state reset, redirect) occurs regardless of whether the logout API call succeeds or fails.
90-112: LGTM!The
AuthProvidercorrectly memoizes the context value implicitly through stable references, anduseAuthfollows the standard pattern with a helpful error message when used outside the provider.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| } catch { | ||
| setUser(null); | ||
| } finally { | ||
| setIsLoading(false); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add error logging for debugging and observability.
The catch block silently swallows errors, making it difficult to diagnose authentication failures in production. Network errors, JSON parsing failures, or unexpected API responses will be hidden.
♻️ Suggested improvement
- } catch {
+ } catch (error) {
+ console.error('Failed to refresh user:', error);
setUser(null);
} finally {🤖 Prompt for AI Agents
In `@apps/web/src/lib/auth/auth-context.tsx` around lines 52 - 56, The catch block
currently swallows errors (catch { setUser(null); }) which hides failures;
update it to capture the thrown error (e) and log it before clearing user so
failures are observable — e.g., in the same async function/context that calls
setUser and setIsLoading (refer to setUser and setIsLoading in auth-context.tsx)
call your logger or console.error with a descriptive message and the error
object, then setUser(null) and finally setIsLoading(false) in the finally block.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/server/src/integrations/slack.ts (1)
151-156: Incorrect fallback for Slack OAuth redirect URI.When
API_BASE_URLis not set, the redirect URI falls back toWEB_APP_URL. However,/slack/callbackis an API route handled by the server (seeapps/server/src/routes/slack.ts), not the web frontend. This will cause Slack OAuth to fail because Slack will redirect users to the web app which doesn't handle this route.The fallback should construct a URL pointing to the API server, not the web app.
🐛 Proposed fix
function getSlackRedirectUri(): string { if (env.API_BASE_URL) { return `${env.API_BASE_URL}/slack/callback`; } - return `${env.WEB_APP_URL}/slack/callback`; + // Fallback to localhost API server in development + const port = env.PORT || 3001; + return `http://localhost:${port}/slack/callback`; }Alternatively, require
API_BASE_URLto be set when Slack integration is enabled, or derive the server URL from another environment variable.
🤖 Fix all issues with AI agents
In `@apps/extension/.env.example`:
- Around line 19-26: The env var name NEXT_PUBLIC_WEB_APP_URL is misleading for
an extension (Next.js browser prefix); rename it to a generic name (e.g.,
WEB_APP_URL or DEVRADAR_WEB_URL) in apps/extension/.env.example and update all
code references that read process.env.NEXT_PUBLIC_WEB_APP_URL (search for
NEXT_PUBLIC_WEB_APP_URL, NEXT_PUBLIC_WEB_APP_URL=, and usages in extension code)
to the new symbol so runtime lookups continue to work, and adjust any
README/docs or deployment config that mention the old name.
In `@apps/extension/src/extension.ts`:
- Around line 778-795: The handler handleEnableGhostMode currently gates on
featureGatingService.promptUpgrade('ghostMode') but toggles
configManager.get/update('privacyMode'), causing a semantic mismatch; create and
use a distinct config key (e.g., 'ghostMode') instead of 'privacyMode' in this
handler: replace configManager.get('privacyMode') and update('privacyMode', ...)
with configManager.get('ghostMode') and update('ghostMode', !currentGhost),
update the user message to reference ghost mode, and audit any other code paths
that read 'privacyMode' to ensure PRO-only ghost behavior is implemented via the
new 'ghostMode' flag (or, if you choose the refactor route, centralize
privacy/ghost behavior behind a feature-aware accessor that returns the
effective mode based on tier and config).
In `@apps/extension/src/services/featureGatingService.ts`:
- Around line 219-221: The getWebAppUrl method uses the || operator which treats
an empty NEXT_PUBLIC_WEB_APP_URL as falsy; replace the fallback logic in
getWebAppUrl to use the nullish coalescing operator (??) so it only falls back
to 'http://localhost:3000' when NEXT_PUBLIC_WEB_APP_URL is null or undefined,
leaving empty strings intact; update the expression in getWebAppUrl accordingly
while preserving the return type string.
In `@apps/server/src/server.ts`:
- Line 206: The Razorpay webhook route needs to bypass the global IP-based rate
limiter to avoid dropped webhook events; update the '/webhooks' POST route in
the billingRoutes (the app.post('/webhooks', ...) handler in billing.ts) to
include a route config that disables rate limiting and preserves raw body (e.g.,
add config: { rawBody: true, rateLimit: { max: 0 } } to that route), or
alternatively apply a dedicated higher-rate/whitelist config for that endpoint
so external Razorpay requests are not subject to the global 100 req/min IP
limit.
In `@apps/web/.env.example`:
- Around line 10-17: The example env uses the same default port for
NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WEB_APP_URL which is confusing for local
development; update NEXT_PUBLIC_API_URL to a different default port (e.g.,
http://localhost:4000 or http://localhost:8000) or add a clarifying comment
above both vars explaining that the API server typically runs on a different
port than the web app and show example values (e.g., API on :4000, web on
:3000), referencing the NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WEB_APP_URL entries
so reviewers can find and update them.
In `@apps/web/.gitignore`:
- Around line 33-34: Update the git ignore rules so local secret override files
are protected: keep ignoring ".env" but add patterns to ignore ".env.local" and
all ".env.*.local" variants while explicitly allowing ".env.example" to be
committed; modify the existing ".env" entry to include these new ignore patterns
and add an allow rule for ".env.example" so example envs remain in the repo.
In `@apps/web/src/lib/auth/api.ts`:
- Around line 56-68: The getSubscription return type declares subscription.tier
as a plain string but should use the same literal union used elsewhere (e.g.,
the User type's tier union); update the typing for getSubscription (or
import/reuse the shared Subscription/User tier type) so subscription.tier is the
exact union of allowed string literals rather than string, by adjusting the
Promise return type for the getSubscription function to reference the shared
Subscription/User tier union (or the shared Subscription interface) so types
remain consistent across the codebase.
- Around line 33-40: getBillingStatus currently returns tier: string which
conflicts with User.tier ('FREE' | 'PRO' | 'TEAM'); change the return type of
getBillingStatus to use a shared Tier literal union (e.g., export type Tier =
'FREE' | 'PRO' | 'TEAM') and update the Promise return type so tier: Tier;
extract the Tier into a shared types module (or export it from auth-context.tsx)
and import it into apps/web/src/lib/auth/api.ts, keeping the rest of the shape
and the api('/billing/status') call unchanged.
♻️ Duplicate comments (7)
apps/web/src/lib/auth/auth-context.tsx (2)
52-56: Previous review comment still applies: Add error logging for debugging.The catch block silently swallows errors, making it difficult to diagnose authentication failures in production.
63-70: Previous review comment still applies:signInchecks token existence without validating.The function redirects to
/dashboardif a token exists without verifying token validity. Consider checkinguserstate or callingrefreshUserfirst.apps/web/src/app/dashboard/page.tsx (1)
31-40: Previous review comment still applies: Add accessibility attribute to SVG.The GitHub icon SVG lacks accessible labeling. Since it's paired with text ("Continue with GitHub"), it's decorative and should have
aria-hidden="true".🐛 Proposed fix
- <Button size="lg" onClick={signIn} className="mb-8"> - <svg className="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 24 24"> + <Button size="lg" onClick={signIn} className="mb-8"> + <svg className="w-5 h-5 mr-2" fill="currentColor" viewBox="0 0 24 24" aria-hidden="true">apps/web/src/lib/auth/api.ts (1)
25-25: Handle non-JSON or empty responses.The final
response.json()call will throw if the server returns a successful non-JSON response (e.g., 204 No Content). Consider adding a guard.♻️ Suggested improvement
- return response.json(); + const text = await response.text(); + return text ? JSON.parse(text) : ({} as T);apps/extension/src/services/featureGatingService.ts (2)
13-128: Significant code duplication with shared package.The
Featuretype,SubscriptionTier, feature arrays,SUBSCRIPTION_FEATURES,FEATURE_TIER_MAP, andFEATURE_DESCRIPTIONSare all duplicated frompackages/shared/src/features.ts. Import from the shared package instead.
132-132: Unused disposables array.The
disposablesarray is initialized but never populated, makingdispose()a no-op. Either remove the pattern or populate it when event listeners are added.apps/extension/src/extension.ts (1)
797-808: DuplicategetWebAppUrlimplementation and lint error.This helper duplicates the implementation in
FeatureGatingService. Additionally, line 807 causes a pipeline failure due to using||instead of??.🔧 Proposed fix for lint error and duplication
Fix the lint error first:
private getWebAppUrl(): string { - return process.env.NEXT_PUBLIC_WEB_APP_URL || 'http://localhost:3000'; + return process.env.NEXT_PUBLIC_WEB_APP_URL ?? 'http://localhost:3000'; }Then consider exposing
getWebAppUrl()as public inFeatureGatingServiceand removing this duplicate method entirely.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
.env.exampleapps/extension/.env.exampleapps/extension/src/extension.tsapps/extension/src/services/featureGatingService.tsapps/server/.env.exampleapps/server/src/integrations/slack.tsapps/server/src/routes/slack.tsapps/server/src/server.tsapps/web/.env.exampleapps/web/.gitignoreapps/web/src/app/dashboard/layout.tsxapps/web/src/app/dashboard/page.tsxapps/web/src/lib/auth/api.tsapps/web/src/lib/auth/auth-context.tsx
🧰 Additional context used
🧬 Code graph analysis (7)
apps/server/src/integrations/slack.ts (1)
apps/server/src/config.ts (1)
env(121-121)
apps/web/src/lib/auth/auth-context.tsx (1)
apps/web/src/lib/auth/index.ts (2)
AuthProvider(1-1)useAuth(1-1)
apps/server/src/routes/slack.ts (1)
apps/server/src/config.ts (1)
env(121-121)
apps/extension/src/services/featureGatingService.ts (1)
packages/shared/src/features.ts (4)
Feature(11-34)SubscriptionTier(39-39)SUBSCRIPTION_FEATURES(86-90)FEATURE_DESCRIPTIONS(188-212)
apps/extension/src/extension.ts (2)
apps/extension/src/services/featureGatingService.ts (2)
FeatureGatingService(131-228)hasAccess(144-154)apps/extension/src/services/index.ts (1)
FeatureGatingService(5-5)
apps/web/src/lib/auth/api.ts (2)
apps/web/src/lib/auth/index.ts (1)
authApi(2-2)apps/web/src/lib/auth/auth-context.tsx (1)
User(5-12)
apps/server/src/server.ts (2)
apps/server/src/config.ts (2)
isDevelopment(125-125)env(121-121)apps/server/src/routes/billing.ts (1)
billingRoutes(37-271)
🪛 Biome (2.1.2)
apps/web/src/app/dashboard/page.tsx
[error] 32-32: Alternative text title element cannot be empty
For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.
(lint/a11y/noSvgWithoutTitle)
🪛 dotenv-linter (4.0.0)
apps/server/.env.example
[warning] 15-15: [UnorderedKey] The HOST key should go before the PORT key
(UnorderedKey)
[warning] 27-27: [UnorderedKey] The GITHUB_CALLBACK_URL key should go before the GITHUB_CLIENT_ID key
(UnorderedKey)
[warning] 35-35: [UnorderedKey] The JWT_EXPIRES_IN key should go before the JWT_SECRET key
(UnorderedKey)
[warning] 61-61: [UnorderedKey] The RAZORPAY_PRO_ANNUAL_PLAN_ID key should go before the RAZORPAY_PRO_MONTHLY_PLAN_ID key
(UnorderedKey)
[warning] 63-63: [UnorderedKey] The RAZORPAY_TEAM_ANNUAL_PLAN_ID key should go before the RAZORPAY_TEAM_MONTHLY_PLAN_ID key
(UnorderedKey)
🪛 GitHub Actions: CI
apps/extension/src/extension.ts
[error] 807-807: eslint: Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator (@typescript-eslint/prefer-nullish-coalescing)
🪛 GitHub Check: Build and Test
apps/extension/src/services/featureGatingService.ts
[failure] 220-220:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator
apps/extension/src/extension.ts
[failure] 807-807:
Prefer using nullish coalescing operator (??) instead of a logical or (||), as it is a safer operator
🔇 Additional comments (16)
apps/server/src/routes/slack.ts (1)
188-191: LGTM!Using
env.WEB_APP_URLfor the post-OAuth redirect to the dashboard is correct. This redirects users to the web frontend after the API server completes the OAuth exchange.apps/web/src/app/dashboard/layout.tsx (1)
1-10: LGTM!Clean and minimal dashboard layout following Next.js App Router conventions. The metadata export provides appropriate SEO defaults for the dashboard section.
.env.example (1)
1-15: Good documentation structure for per-app environment organization.The guidance for per-app environment configuration is clear and helpful for developers working with this monorepo. The deployment notes for Vercel and Koyeb provide useful context for production setup.
apps/server/.env.example (1)
1-74: Comprehensive server environment configuration.The file provides good structure with logical groupings by feature (database, auth, billing, etc.) and helpful comments for generating secure secrets. The static analysis warnings about key ordering can be safely ignored since logical grouping by feature is more readable and maintainable than alphabetical ordering.
apps/web/src/lib/auth/auth-context.tsx (1)
1-112: Auth context implementation is well-structured.The overall implementation follows React patterns correctly:
- Proper use of
'use client'directiveuseCallbackfor memoization ofrefreshUser- Clean separation of concerns with typed context
- Appropriate error boundary with
useAuthhook validationapps/web/src/app/dashboard/page.tsx (1)
1-218: Dashboard page refactoring looks good.The conversion to a client component with conditional rendering based on auth state is well-implemented:
- Clean separation into
SignedOutViewandSignedInViewcomponents- Proper loading state handling with spinner
- Good use of the
useAuthhook for state management- Appropriate tier-based conditional rendering in the subscription card
Note: The
metadataexport was removed to allow client-side hooks. Consider adding metadata back via a parent layout if SEO for this page is important.apps/web/src/lib/auth/api.ts (2)
1-8: LGTM!The SSR guard for
localStorageaccess and the Bearer token pattern are correctly implemented. The environment variable fallback forAPI_URLis appropriate for development.
70-96: LGTM!The subscription management methods (
cancel,pause,resume) andverifyPaymentare well-structured with appropriate HTTP methods and payload typing.apps/extension/src/services/featureGatingService.ts (2)
144-154: LGTM!Good defensive programming with null checks and defaulting to 'FREE' tier when user data is unavailable.
181-204: LGTM!The upgrade prompt flow is well-structured: checks access first, shows descriptive warning with tier requirement, and properly handles user action to open the billing page.
apps/extension/src/extension.ts (4)
52-53: LGTM!Service member declaration follows the established pattern in this class.
83-84: LGTM!Service instantiation is correctly placed after its dependencies (
authService,logger) are initialized.
97-98: LGTM!Service properly added to disposables for lifecycle management.
203-210: LGTM!New commands follow the established registration pattern.
apps/server/src/server.ts (2)
28-28: LGTM!Import follows the established pattern for route modules.
57-61: CORS configuration validation is already properly handled.The
WEB_APP_URLvalidation in config.ts (line 74) usesz.string().url().default('http://localhost:3000'), which rejects empty strings and malformed URLs—addressing the concern about validation edge cases.Regarding breaking changes: if other origins (mobile apps, staging environments, additional subdomains) were previously allowed, confirm they're no longer needed or add them to the configuration.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| # Web Application URL (for links from extension to web app) | ||
| # Update this to your Vercel deployment URL in production | ||
| NEXT_PUBLIC_WEB_APP_URL=http://localhost:3000 | ||
|
|
||
| # ----------------------------------------------------------------------------- | ||
| # PRODUCTION VALUES (update these when deploying) | ||
| # ----------------------------------------------------------------------------- | ||
| # NEXT_PUBLIC_WEB_APP_URL=https://devradar.io |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider renaming NEXT_PUBLIC_WEB_APP_URL for clarity.
The NEXT_PUBLIC_ prefix is a Next.js convention for exposing environment variables to the browser. In a VS Code extension context, this naming is misleading. Consider using a more generic name like WEB_APP_URL or DEVRADAR_WEB_URL.
♻️ Suggested rename
# Web Application URL (for links from extension to web app)
# Update this to your Vercel deployment URL in production
-NEXT_PUBLIC_WEB_APP_URL=http://localhost:3000
+WEB_APP_URL=http://localhost:3000
# -----------------------------------------------------------------------------
# PRODUCTION VALUES (update these when deploying)
# -----------------------------------------------------------------------------
-# NEXT_PUBLIC_WEB_APP_URL=https://devradar.io
+# WEB_APP_URL=https://devradar.io📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Web Application URL (for links from extension to web app) | |
| # Update this to your Vercel deployment URL in production | |
| NEXT_PUBLIC_WEB_APP_URL=http://localhost:3000 | |
| # ----------------------------------------------------------------------------- | |
| # PRODUCTION VALUES (update these when deploying) | |
| # ----------------------------------------------------------------------------- | |
| # NEXT_PUBLIC_WEB_APP_URL=https://devradar.io | |
| # Web Application URL (for links from extension to web app) | |
| # Update this to your Vercel deployment URL in production | |
| WEB_APP_URL=http://localhost:3000 | |
| # ----------------------------------------------------------------------------- | |
| # PRODUCTION VALUES (update these when deploying) | |
| # ----------------------------------------------------------------------------- | |
| # WEB_APP_URL=https://devradar.io |
🤖 Prompt for AI Agents
In `@apps/extension/.env.example` around lines 19 - 26, The env var name
NEXT_PUBLIC_WEB_APP_URL is misleading for an extension (Next.js browser prefix);
rename it to a generic name (e.g., WEB_APP_URL or DEVRADAR_WEB_URL) in
apps/extension/.env.example and update all code references that read
process.env.NEXT_PUBLIC_WEB_APP_URL (search for NEXT_PUBLIC_WEB_APP_URL,
NEXT_PUBLIC_WEB_APP_URL=, and usages in extension code) to the new symbol so
runtime lookups continue to work, and adjust any README/docs or deployment
config that mention the old name.
| private async handleEnableGhostMode(): Promise<void> { | ||
| // Check if user has access to ghost mode feature | ||
| const hasAccess = await this.featureGatingService.promptUpgrade('ghostMode'); | ||
| if (!hasAccess) { | ||
| return; | ||
| } | ||
|
|
||
| // Toggle ghost mode | ||
| const currentMode = this.configManager.get('privacyMode'); | ||
| await this.configManager.update('privacyMode', !currentMode); | ||
|
|
||
| const message = !currentMode | ||
| ? 'DevRadar: Ghost mode enabled - you are now invisible to others' | ||
| : 'DevRadar: Ghost mode disabled - your activity is now visible'; | ||
|
|
||
| void vscode.window.showInformationMessage(message); | ||
| this.activityTracker.sendStatusUpdate(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n "privacyMode|ghostMode" --type=ts -C2 apps/extension/Repository: senutpal/devRadar
Length of output: 5655
Use a separate config key for ghostMode or refactor the feature architecture.
The handleEnableGhostMode() handler gates access on the ghostMode feature (PRO tier: "Go completely invisible") but toggles the privacyMode config setting (FREE tier: "Hide activity details"). This creates a semantic mismatch:
- Both FREE and PRO users toggle the same
privacyModeconfig ghostModeis only checked for feature gating, never for actual behavior- The two features have different descriptions and tier levels but share a single config key
Either create a separate ghostMode config key or refactor so PRO users enable a distinct behavior.
🤖 Prompt for AI Agents
In `@apps/extension/src/extension.ts` around lines 778 - 795, The handler
handleEnableGhostMode currently gates on
featureGatingService.promptUpgrade('ghostMode') but toggles
configManager.get/update('privacyMode'), causing a semantic mismatch; create and
use a distinct config key (e.g., 'ghostMode') instead of 'privacyMode' in this
handler: replace configManager.get('privacyMode') and update('privacyMode', ...)
with configManager.get('ghostMode') and update('ghostMode', !currentGhost),
update the user message to reference ghost mode, and audit any other code paths
that read 'privacyMode' to ensure PRO-only ghost behavior is implemented via the
new 'ghostMode' flag (or, if you choose the refactor route, centralize
privacy/ghost behavior behind a feature-aware accessor that returns the
effective mode based on tier and config).
| NEXT_PUBLIC_API_URL=http://localhost:3000 | ||
|
|
||
| # Razorpay Configuration | ||
| # Get your key ID from https://dashboard.razorpay.com/#/app/keys | ||
| NEXT_PUBLIC_RAZORPAY_KEY_ID=your_razorpay_key_id | ||
|
|
||
| # Web App URL (for links and redirects) | ||
| NEXT_PUBLIC_WEB_APP_URL=http://localhost:3000 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify port differences between API server and web app for local development.
Both NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WEB_APP_URL default to localhost:3000, which could cause confusion. Typically, the API server and web app run on different ports locally.
Consider updating the defaults or adding a clarifying comment:
♻️ Suggested improvement
# API Server URL
-NEXT_PUBLIC_API_URL=http://localhost:3000
+# Note: Server runs on port 3000, web app typically runs on port 3001 (Next.js default when 3000 is taken)
+NEXT_PUBLIC_API_URL=http://localhost:3000
# Razorpay Configuration
# Get your key ID from https://dashboard.razorpay.com/#/app/keys
NEXT_PUBLIC_RAZORPAY_KEY_ID=your_razorpay_key_id
# Web App URL (for links and redirects)
-NEXT_PUBLIC_WEB_APP_URL=http://localhost:3000
+NEXT_PUBLIC_WEB_APP_URL=http://localhost:3001🤖 Prompt for AI Agents
In `@apps/web/.env.example` around lines 10 - 17, The example env uses the same
default port for NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WEB_APP_URL which is
confusing for local development; update NEXT_PUBLIC_API_URL to a different
default port (e.g., http://localhost:4000 or http://localhost:8000) or add a
clarifying comment above both vars explaining that the API server typically runs
on a different port than the web app and show example values (e.g., API on
:4000, web on :3000), referencing the NEXT_PUBLIC_API_URL and
NEXT_PUBLIC_WEB_APP_URL entries so reviewers can find and update them.
| # env files (can opt-in for committing if needed) | ||
| .env* | ||
| .env |
There was a problem hiding this comment.
Risk of accidentally committing secrets in .env.local or .env.*.local files.
Narrowing the ignore from .env* to just .env means files like .env.local, .env.development.local, and .env.production.local (which Next.js uses for local overrides with secrets) are no longer ignored.
Consider a more targeted approach that allows .env.example while still protecting local secret files:
🔒 Proposed fix
# env files (can opt-in for committing if needed)
-.env
+.env
+.env.local
+.env*.localThis ignores the main .env and any .local variants while allowing .env.example to be committed.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # env files (can opt-in for committing if needed) | |
| .env* | |
| .env | |
| # env files (can opt-in for committing if needed) | |
| .env | |
| .env.local | |
| .env*.local |
🤖 Prompt for AI Agents
In `@apps/web/.gitignore` around lines 33 - 34, Update the git ignore rules so
local secret override files are protected: keep ignoring ".env" but add patterns
to ignore ".env.local" and all ".env.*.local" variants while explicitly allowing
".env.example" to be committed; modify the existing ".env" entry to include
these new ignore patterns and add an allow rule for ".env.example" so example
envs remain in the repo.
| async getBillingStatus(): Promise<{ | ||
| tier: string; | ||
| hasSubscription: boolean; | ||
| currentPeriodEnd: string | null; | ||
| billingEnabled: boolean; | ||
| }> { | ||
| return api('/billing/status'); | ||
| }, |
There was a problem hiding this comment.
Type inconsistency: tier should use literal union type.
The tier field returns string, but User.tier from auth-context.tsx is typed as 'FREE' | 'PRO' | 'TEAM'. This weakens type safety when consuming the billing status alongside user data.
🔧 Suggested fix
async getBillingStatus(): Promise<{
- tier: string;
+ tier: 'FREE' | 'PRO' | 'TEAM';
hasSubscription: boolean;
currentPeriodEnd: string | null;
billingEnabled: boolean;
}> {Consider extracting a shared Tier type to auth-context.tsx or a shared types file to avoid duplication.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async getBillingStatus(): Promise<{ | |
| tier: string; | |
| hasSubscription: boolean; | |
| currentPeriodEnd: string | null; | |
| billingEnabled: boolean; | |
| }> { | |
| return api('/billing/status'); | |
| }, | |
| async getBillingStatus(): Promise<{ | |
| tier: 'FREE' | 'PRO' | 'TEAM'; | |
| hasSubscription: boolean; | |
| currentPeriodEnd: string | null; | |
| billingEnabled: boolean; | |
| }> { | |
| return api('/billing/status'); | |
| }, |
🤖 Prompt for AI Agents
In `@apps/web/src/lib/auth/api.ts` around lines 33 - 40, getBillingStatus
currently returns tier: string which conflicts with User.tier ('FREE' | 'PRO' |
'TEAM'); change the return type of getBillingStatus to use a shared Tier literal
union (e.g., export type Tier = 'FREE' | 'PRO' | 'TEAM') and update the Promise
return type so tier: Tier; extract the Tier into a shared types module (or
export it from auth-context.tsx) and import it into
apps/web/src/lib/auth/api.ts, keeping the rest of the shape and the
api('/billing/status') call unchanged.
| async getSubscription(): Promise<{ | ||
| hasSubscription: boolean; | ||
| subscription: { | ||
| id: string; | ||
| status: string; | ||
| tier: string; | ||
| currentPeriodStart: string; | ||
| currentPeriodEnd: string; | ||
| endAt: string | null; | ||
| } | null; | ||
| }> { | ||
| return api('/billing/subscription'); | ||
| }, |
There was a problem hiding this comment.
Same type inconsistency: subscription.tier should use literal union.
Apply the same fix here for consistency with the User type.
🔧 Suggested fix
async getSubscription(): Promise<{
hasSubscription: boolean;
subscription: {
id: string;
status: string;
- tier: string;
+ tier: 'FREE' | 'PRO' | 'TEAM';
currentPeriodStart: string;
currentPeriodEnd: string;
endAt: string | null;
} | null;
}> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async getSubscription(): Promise<{ | |
| hasSubscription: boolean; | |
| subscription: { | |
| id: string; | |
| status: string; | |
| tier: string; | |
| currentPeriodStart: string; | |
| currentPeriodEnd: string; | |
| endAt: string | null; | |
| } | null; | |
| }> { | |
| return api('/billing/subscription'); | |
| }, | |
| async getSubscription(): Promise<{ | |
| hasSubscription: boolean; | |
| subscription: { | |
| id: string; | |
| status: string; | |
| tier: 'FREE' | 'PRO' | 'TEAM'; | |
| currentPeriodStart: string; | |
| currentPeriodEnd: string; | |
| endAt: string | null; | |
| } | null; | |
| }> { | |
| return api('/billing/subscription'); | |
| }, |
🤖 Prompt for AI Agents
In `@apps/web/src/lib/auth/api.ts` around lines 56 - 68, The getSubscription
return type declares subscription.tier as a plain string but should use the same
literal union used elsewhere (e.g., the User type's tier union); update the
typing for getSubscription (or import/reuse the shared Subscription/User tier
type) so subscription.tier is the exact union of allowed string literals rather
than string, by adjusting the Promise return type for the getSubscription
function to reference the shared Subscription/User tier union (or the shared
Subscription interface) so types remain consistent across the codebase.
…Equal - Replace string comparison with timing-safe comparison for payment signature verification - Apply same fix to webhook signature verification - Fix total_count type from string to number (5 for annual, 12 for monthly)
…rify - Add rate limit of 20 requests/minute to /verify endpoint - Save razorpaySubscriptionId in database during payment verification
- Fix handleSubscriptionActivated to use userId from notes for DB update - This provides more reliable user lookup than razorpaySubscriptionId
- Remove duplicate types and import from @devradar/shared - Add defensive validation for request.user and user.tier
- Remove duplicate getWebAppUrl method from Extension class - Make getWebAppUrl public in FeatureGatingService - Import types and constants from @devradar/shared package - Remove unused disposables field
- Align price with annualPrice comment (₹249/month) - Previously showed ₹499 which was inconsistent
- Replace alert() calls with toast notifications using sonner - Wrap fetchBillingStatus in useCallback for proper memoization - Fix monthly price calculation using annualPrice/12 instead of hardcoded 50% - Add Toaster component to root layout - Fix verification error redirect to use ?verification_failed=true
- Register fastify-raw-body plugin for raw body access in webhooks - Required for Razorpay webhook signature verification
- Update dotenv initialization to use config({ quiet: true })
- Reduces console noise during Prisma CLI operations
- Add sonner package for modern toast notifications - Replaces browser alert() with polished UI notifications
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@apps/extension/src/services/featureGatingService.ts`:
- Around line 133-135: getWebAppUrl currently reads NEXT_PUBLIC_WEB_APP_URL from
process.env which doesn't exist at runtime in the VS Code extension; update it
to read the value from the extension config instead. Add a webAppUrl property to
DevRadarConfig in configManager (mirroring serverUrl), inject ConfigManager (or
the existing configManager instance) into FeatureGatingService (constructor or
DI), and change getWebAppUrl() to return configManager.getConfig().webAppUrl
(with a sensible default if missing). Update any callers/tests to use the
injected configManager where necessary.
In `@apps/server/src/routes/billing.ts`:
- Around line 170-174: The /webhooks route is currently inheriting the global
rate limit; update the app.post('/webhooks', { config: { rawBody: true } }, ...)
call to explicitly disable rate limiting for this route (e.g., add
config.rateLimit = false or the equivalent route-level option your rate-limit
plugin expects) so Razorpay webhook requests are exempt from the global 100
req/min IP-based limit; ensure you modify the app.post call that defines the
'/webhooks' handler to include the new config property.
In `@apps/server/src/services/razorpay-webhooks.ts`:
- Around line 256-280: handleInvoicePaid currently logs paid invoices but
doesn't extend the user's subscription period; update it to fetch Razorpay
subscription details and set the user's razorpayCurrentPeriodEnd from the
subscription's current_period_end when an invoice with subscription_id is paid.
In handleInvoicePaid, after locating the user via db.user.findFirst (by
razorpaySubscriptionId), call Razorpay's subscription retrieval (using the
subscription_id) to get the current_period_end (or period_end equivalent),
convert it to the DB timestamp format, and update the user record via
db.user.update to set razorpayCurrentPeriodEnd; ensure you handle missing fields
and errors gracefully and only update when a valid period end is returned.
In `@apps/web/src/app/dashboard/billing/page.tsx`:
- Around line 125-128: The prefill object in page.tsx is empty; obtain the
authenticated user's name/email from the auth context (e.g., useAuth() or the
app's user hook) and set prefill.name and prefill.email to those values (e.g.,
user?.name || '' and user?.email || ''); also ensure the hook is
imported/initialized in the component so user is available when constructing the
prefill object.
- Around line 430-444: The "Contact Sales" text branch is dead for the Team tier
because isCurrentPlan already handles the current TEAM case; update the Button
label logic to remove the unreachable !canUpgrade branch: inside the Button for
TEAM (component rendering with props like isHighlighted, loading,
handleCheckout('TEAM'), canUpgrade), replace the conditional that renders
{canUpgrade ? 'Upgrade to Team' : 'Contact Sales'} with a single deterministic
label—e.g., always render 'Upgrade to Team' when canUpgrade is true and
otherwise render nothing or a disabled label—so eliminate the unreachable
"Contact Sales" path and keep the loading/ icon and disabled logic intact.
In `@apps/web/src/app/layout.tsx`:
- Line 4: Replace the direct sonner import with the themed wrapper: change the
import of Toaster from 'sonner' to import { Toaster } from
'@/components/ui/sonner' so the app uses the custom Toaster that integrates with
next-themes and custom icons (keep existing usages of Toaster unchanged).
In `@apps/web/src/lib/constants.ts`:
- Around line 141-148: The Team tier currently sets price: 249 and annualPrice:
2988 so there is no actual annual discount while the UI shows "Save 50% with
annual"; update the Team tier pricing in apps/web/src/lib/constants.ts by either
lowering annualPrice to reflect the intended discount (e.g., set annualPrice to
a 50% discounted yearly total like 1494) or change the UI logic to suppress the
"Save 50% with annual" badge for the Team tier only; adjust the Team object (the
properties price and annualPrice and any label logic that reads them) and ensure
razorpayPlanIds remain unchanged.
♻️ Duplicate comments (10)
apps/server/package.json (1)
33-33:dotenvv17 breaking change already flagged.The breaking change regarding the
quietdefault has been noted in a previous review. Ensure the initialization code has been updated accordingly.apps/extension/src/extension.ts (1)
778-795: Semantic mismatch: ghostMode feature gates privacyMode config.This handler gates access on
'ghostMode'(PRO tier: "Go completely invisible") but toggles the'privacyMode'configuration, which is the same config toggled byhandleTogglePrivacy(available to all tiers).If ghost mode is meant to be a distinct PRO behavior beyond privacy mode, it needs its own config key and behavior. Otherwise, PRO users paying for "ghost mode" are just getting tier-gated access to the same privacy toggle FREE users already have.
🔧 Option: Create distinct ghostMode config
private async handleEnableGhostMode(): Promise<void> { const hasAccess = await this.featureGatingService.promptUpgrade('ghostMode'); if (!hasAccess) { return; } - const currentMode = this.configManager.get('privacyMode'); - await this.configManager.update('privacyMode', !currentMode); + const currentMode = this.configManager.get('ghostMode'); + await this.configManager.update('ghostMode', !currentMode); const message = !currentMode ? 'DevRadar: Ghost mode enabled - you are now invisible to others' : 'DevRadar: Ghost mode disabled - your activity is now visible';This requires adding
'ghostMode'to the extension's configuration schema inpackage.json.apps/extension/src/services/featureGatingService.ts (1)
19-43: FEATURE_TIER_MAP is still duplicated from shared package.The shared package already exports
getRequiredTier(seepackages/shared/src/features.tslines 144-146). This local constant duplicates that data and may drift out of sync.♻️ Import getRequiredTier from shared instead
import { type Feature, type SubscriptionTier, SUBSCRIPTION_FEATURES, FEATURE_DESCRIPTIONS, + getRequiredTier, } from '@devradar/shared'; import * as vscode from 'vscode'; import type { AuthService } from './authService'; import type { Logger } from '../utils/logger'; -const FEATURE_TIER_MAP: Record<Feature, SubscriptionTier> = { - presence: 'FREE', - friends: 'FREE', - // ... all 24 entries -} as const;Then update the
getRequiredTiermethod:getRequiredTier(feature: Feature): SubscriptionTier { - return FEATURE_TIER_MAP[feature]; + return getRequiredTier(feature); }apps/web/src/lib/constants.ts (1)
118-124: Pricing comment inconsistency (previously flagged).The comment states "₹49/month billed annually" but the monthly
priceis ₹99. TheannualPriceof ₹588 (₹588/12 = ₹49/month) is correct for annual billing, but the comment is misleading when displayed alongside the monthly price.apps/web/src/app/dashboard/billing/page.tsx (1)
158-159: Capture the error object for debugging (previously flagged).The catch block discards the error, making debugging harder.
Proposed fix
- } catch { - console.error('Failed to check subscription status'); + } catch (error) { + console.error('Failed to check subscription status:', error); } finally {apps/server/src/services/razorpay.ts (5)
22-23: Document the intentional exclusion of FREE tier.The local
SubscriptionTiertype differs from the shared package (packages/shared/src/features.ts) which includes'FREE' | 'PRO' | 'TEAM'. Adding a brief comment clarifies this is intentional for billing contexts where FREE is not a billable tier.-type BillingInterval = 'monthly' | 'annual'; -type SubscriptionTier = 'PRO' | 'TEAM'; +type BillingInterval = 'monthly' | 'annual'; +/** Paid subscription tiers only (excludes FREE as it's not billable) */ +type SubscriptionTier = 'PRO' | 'TEAM';
247-251: Order amount may be zero or undefined.
subscription.notes.total_amountis user-defined metadata and may not exist. Defaulting to0creates an invalid Razorpay order. Either derive the amount from the plan configuration or validate it's positive before creating the order.
315-340: Add error handling to matchcancelSubscriptionpattern.
pauseSubscriptionlacks try-catch error handling, unlikecancelSubscription. This inconsistency can lead to unhandled promise rejections if the Razorpay API call fails.🐛 Proposed fix
export async function pauseSubscription(userId: string): Promise<void> { const client = await getClient(); const clientCasted = client as { subscriptions: { pause: (id: string) => Promise<unknown>; }; }; const db = getDb(); const user = await db.user.findUnique({ where: { id: userId } }); if (!user) { throw new ValidationError('User not found'); } if (!user.razorpaySubscriptionId) { throw new ValidationError('No active subscription found'); } + try { await clientCasted.subscriptions.pause(user.razorpaySubscriptionId); logger.info( { userId, subscriptionId: user.razorpaySubscriptionId }, 'Paused Razorpay subscription' ); + } catch (error) { + logger.error( + { userId, subscriptionId: user.razorpaySubscriptionId, error }, + 'Failed to pause subscription' + ); + throw new InternalError('Failed to pause subscription'); + } }
342-366: Add error handling to matchcancelSubscriptionpattern.
resumeSubscriptionalso lacks try-catch error handling, unlikecancelSubscription. Apply the same fix pattern aspauseSubscription.🐛 Proposed fix
export async function resumeSubscription(userId: string): Promise<void> { const client = await getClient(); const clientCasted = client as { subscriptions: { resume: (id: string) => Promise<unknown>; }; }; const db = getDb(); const user = await db.user.findUnique({ where: { id: userId } }); if (!user) { throw new ValidationError('User not found'); } if (!user.razorpaySubscriptionId) { throw new ValidationError('No subscription found'); } + try { await clientCasted.subscriptions.resume(user.razorpaySubscriptionId); logger.info( { userId, subscriptionId: user.razorpaySubscriptionId }, 'Resumed Razorpay subscription' ); + } catch (error) { + logger.error( + { userId, subscriptionId: user.razorpaySubscriptionId, error }, + 'Failed to resume subscription' + ); + throw new InternalError('Failed to resume subscription'); + } }
253-258: Store subscription ID only after payment verification or add cleanup for failed subscriptions.The subscription ID is stored immediately after creation, but the subscription is in "created" state and only becomes "active" after successful payment. If payment fails, Razorpay will transition the subscription to "pending" → "halted", leaving an orphaned subscription ID in the database. This blocks the user from retrying (line 217-221 check). Consider deferring ID storage until
/verifyconfirms payment, or implement a webhook handler forsubscription.haltedto clean up failed subscription IDs.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (15)
apps/extension/src/extension.tsapps/extension/src/services/featureGatingService.tsapps/server/package.jsonapps/server/prisma.config.tsapps/server/src/lib/featureGate.tsapps/server/src/routes/billing.tsapps/server/src/server.tsapps/server/src/services/razorpay-webhooks.tsapps/server/src/services/razorpay.tsapps/web/package.jsonapps/web/src/app/dashboard/billing/page.tsxapps/web/src/app/layout.tsxapps/web/src/app/privacy/page.tsxapps/web/src/components/ui/sonner.tsxapps/web/src/lib/constants.ts
🧰 Additional context used
🧬 Code graph analysis (7)
apps/web/src/app/layout.tsx (4)
apps/web/src/lib/auth/auth-context.tsx (1)
AuthProvider(27-104)apps/web/src/lib/auth/index.ts (1)
AuthProvider(1-1)apps/web/src/components/ui/sonner.tsx (1)
Toaster(40-40)apps/web/src/components/layout/header.tsx (1)
Header(203-372)
apps/server/src/routes/billing.ts (5)
apps/server/src/services/razorpay.ts (7)
isRazorpayEnabled(48-59)createSubscription(193-270)cancelSubscription(272-313)pauseSubscription(315-340)resumeSubscription(342-366)verifyPaymentSignature(430-451)getSubscriptionDetails(368-395)apps/server/src/lib/errors.ts (2)
InternalError(218-226)ValidationError(78-97)apps/server/src/services/db.ts (1)
getDb(20-33)apps/server/src/services/razorpay-webhooks.ts (1)
handleWebhook(24-80)apps/server/src/lib/logger.ts (1)
logger(68-68)
apps/server/src/lib/featureGate.ts (4)
packages/shared/src/features.ts (6)
Feature(11-34)SubscriptionTier(39-39)hasFeatureAccess(136-138)getRequiredTier(145-147)FEATURE_DESCRIPTIONS(188-212)isTierAtLeast(181-183)apps/server/src/lib/errors.ts (1)
AuthorizationError(128-136)apps/server/src/services/db.ts (1)
getDb(20-33)apps/extension/src/services/featureGatingService.ts (1)
getRequiredTier(74-76)
apps/web/src/app/dashboard/billing/page.tsx (6)
apps/web/src/lib/auth/auth-context.tsx (1)
useAuth(106-112)apps/web/src/lib/auth/api.ts (1)
authApi(28-96)apps/web/src/components/ui/card.tsx (5)
Card(75-75)CardHeader(75-75)CardTitle(75-75)CardDescription(75-75)CardContent(75-75)apps/web/src/lib/utils.ts (1)
cn(9-11)apps/web/src/components/ui/badge.tsx (1)
Badge(35-35)apps/web/src/lib/constants.ts (2)
PRICING_TIERS(95-162)SITE_CONFIG(1-20)
apps/extension/src/extension.ts (2)
apps/extension/src/services/featureGatingService.ts (2)
FeatureGatingService(46-140)hasAccess(57-67)apps/extension/src/services/index.ts (1)
FeatureGatingService(5-5)
apps/server/src/services/razorpay-webhooks.ts (3)
apps/server/src/services/razorpay.ts (1)
verifyWebhookSignature(453-469)apps/server/src/lib/logger.ts (1)
logger(68-68)apps/server/src/services/db.ts (1)
getDb(20-33)
apps/server/src/services/razorpay.ts (5)
packages/shared/src/features.ts (1)
SubscriptionTier(39-39)apps/server/src/config.ts (1)
env(121-121)apps/server/src/lib/errors.ts (2)
InternalError(218-226)ValidationError(78-97)apps/server/src/services/db.ts (1)
getDb(20-33)apps/server/src/lib/logger.ts (1)
logger(68-68)
🪛 GitHub Check: CodeQL
apps/server/src/routes/billing.ts
[failure] 123-167: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
🔇 Additional comments (38)
apps/server/prisma.config.ts (2)
8-10: Correct initialization order for Prisma 7.The approach of explicitly importing dotenv and calling
dotenv.config()before importing fromprisma/configis correct for Prisma 7, which requires environment variables to be explicitly loaded before use.
8-9: No changes needed. Thequietoption is a valid configuration option for dotenv.config() and suppresses runtime logging messages as intended. The code is correct.Likely an incorrect or invalid review comment.
apps/server/package.json (3)
42-42: No action needed. Zod 3.25 is a backward-compatible minor release with no breaking changes compared to 3.24.
35-35: New Razorpay integration dependencies are appropriately added.
fastify-raw-body^5.0.0 is compatible with the project's Fastify ^5.6.2 and is correctly used for webhook signature verification. Therazorpay^2.9.6 package is the official SDK with no known security advisories.
50-50: Theesbuildupgrade from 0.24.2 to 0.27.2 is compatible with your current configuration.Your
esbuild.config.mjsuses thebuild()API withtarget: 'node22'and no binary loaders, so none of the breaking changes (CORS behavior in 0.25, Uint8Array.fromBase64 in 0.27) apply. The configuration is safe to proceed.apps/server/src/lib/featureGate.ts (2)
8-15: Good: Imports from shared package correctly.The shared types and utilities are now properly imported from
@devradar/shared, avoiding the duplication issues previously flagged.
29-69: Well-implemented defensive validation.The function properly validates
request.user,userIdtype, user existence, and tier validity with a safe fallback to'FREE'. This addresses the previous concerns about unsafe type assertions.apps/extension/src/services/featureGatingService.ts (2)
57-67: LGTM: hasAccess uses shared SUBSCRIPTION_FEATURES correctly.Good defensive handling with nullish coalescing for the tier.
137-139: LGTM: Dispose method properly documented.The empty dispose with a comment is appropriate since no resources need cleanup.
apps/extension/src/extension.ts (3)
14-14: LGTM: FeatureGatingService integration.Proper initialization, dependency injection, and lifecycle management via disposables.
Also applies to: 52-53, 83-84, 97-98
203-210: LGTM: New commands registered correctly.The command IDs follow the extension's naming convention.
797-804: LGTM: Billing page handler uses service correctly.Good refactoring - now uses
featureGatingService.getWebAppUrl()instead of duplicating the logic.apps/web/package.json (1)
31-31: LGTM!The
sonnerdependency addition aligns with the newToastercomponent introduced inapps/web/src/components/ui/sonner.tsxfor toast notifications.apps/web/src/components/ui/sonner.tsx (1)
1-40: LGTM!Clean implementation of a themed toast wrapper. The
next-themesintegration ensures consistent dark/light mode support, and the custom icons align with the project's use oflucide-react. The CSS custom properties properly reference the design system tokens.apps/web/src/app/layout.tsx (1)
98-110: LGTM!The
AuthProvidercorrectly wraps the layout components, enabling auth context access forHeaderand child pages. TheToasterplacement insideThemeProviderensures proper theme access.apps/web/src/lib/constants.ts (2)
100-100: LGTM!Adding
annualPrice: 0to the Free tier maintains consistency with the other tiers' structure.
159-159: LGTM!The CTA update from "Contact Sales" to "Upgrade to Team" aligns with the self-service billing flow being introduced.
apps/web/src/app/dashboard/billing/page.tsx (9)
1-36: LGTM!Imports and type definitions are well-organized. The
BillingStatusinterface properly captures the billing state needed for the UI.
38-51: LGTM!The Razorpay script loader correctly checks for existing instances before loading, uses async loading, and properly handles both success and error cases.
69-93: LGTM!The
fetchBillingStatusis properly wrapped inuseCallback, and both effects have correct dependency arrays. The authentication redirect logic is appropriate.
95-144: LGTM!Good implementation of the Razorpay checkout flow with proper error handling, toast notifications, and loading state management. The verification failure is now correctly distinguished from user cancellation.
165-185: LGTM!Cancel subscription flow has proper confirmation, error handling, and status refresh. The use of
confirm()is acceptable for this destructive action, though a custom modal would provide better UX consistency.
187-198: LGTM!Clean tier icon mapping with appropriate defaults.
345-354: LGTM!The pricing calculation correctly uses
tier.annualPrice / 12for annual billing, addressing the previous hardcoded discount issue.
471-512: LGTM!Well-structured FAQ section with helpful billing information. Proper HTML entity usage and support email linking.
520-532: LGTM!Proper Suspense boundary for
useSearchParamsusage in Next.js App Router, with an appropriate loading fallback.apps/web/src/app/privacy/page.tsx (1)
246-246: LGTM!The payment provider update from Stripe to Razorpay is correct and uses the proper branding (lowercase 'p').
apps/server/src/services/razorpay.ts (3)
430-451: LGTM!Timing-safe signature verification is correctly implemented using
crypto.timingSafeEqualwith proper buffer length validation.
453-468: LGTM!Webhook signature verification matches the same secure pattern with timing-safe comparison.
126-166: LGTM!Customer search now correctly implements pagination using
count/skipparameters with proper loop termination conditions.apps/server/src/services/razorpay-webhooks.ts (2)
102-109: LGTM!The race condition fix is correctly implemented - now updates by
userIdfrom notes and also setsrazorpaySubscriptionIdin the same update, ensuring the user record is found regardless of webhook timing.
24-80: LGTM!Well-structured webhook handler with proper signature verification, defensive guards for missing data, and comprehensive event routing.
apps/server/src/routes/billing.ts (3)
112-167: LGTM - Rate limiting is correctly configured.The static analysis hint is a false positive. The
/verifyendpoint has explicit rate limiting configured at lines 116-121 withmax: 20requests per minute. TherazorpaySubscriptionIdis now correctly persisted at line 161.
170-202: LGTM!Webhook endpoint correctly opts into raw body handling with
config: { rawBody: true }, which works with thefastify-raw-bodyplugin registered inserver.tswithglobal: false.
234-279: LGTM!The
/subscriptionendpoint correctly handles both cases (with/without subscription) and fetches real-time details from Razorpay.apps/server/src/server.ts (3)
93-95: LGTM!
fastify-raw-bodyis correctly registered withglobal: false, allowing routes to opt-in viaconfig: { rawBody: true }.
58-62: LGTM!CORS origin configuration correctly uses environment-based
WEB_APP_URLfor production while allowing all origins in development.
211-211: LGTM!Billing routes correctly registered at
/billingprefix.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| export function requireTier(requiredTier: SubscriptionTier): preHandlerHookHandler { | ||
| // Fastify preHandler hooks support async functions - Fastify awaits promises internally | ||
| // eslint-disable-next-line @typescript-eslint/no-misused-promises | ||
| return async (request: FastifyRequest, _reply: FastifyReply): Promise<void> => { | ||
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- runtime check needed | ||
| if (!request.user || typeof request.user !== 'object') { | ||
| throw new AuthorizationError('User not authenticated'); | ||
| } | ||
|
|
||
| const userPayload = request.user as { userId?: unknown }; | ||
| if (typeof userPayload.userId !== 'string') { | ||
| throw new AuthorizationError('Invalid user ID'); | ||
| } | ||
|
|
||
| const { userId } = userPayload; | ||
| const db = getDb(); | ||
|
|
||
| const user = await db.user.findUnique({ | ||
| where: { id: userId }, | ||
| select: { tier: true }, | ||
| }); | ||
|
|
||
| if (!user) { | ||
| throw new AuthorizationError('User not found'); | ||
| } | ||
|
|
||
| const validTiers: SubscriptionTier[] = ['FREE', 'PRO', 'TEAM']; | ||
| const userTier = validTiers.includes(user.tier as SubscriptionTier) | ||
| ? (user.tier as SubscriptionTier) | ||
| : 'FREE'; | ||
|
|
||
| if (!isTierAtLeast(userTier, requiredTier)) { | ||
| throw new AuthorizationError( | ||
| `This feature requires ${requiredTier} tier. Upgrade at /dashboard/billing` | ||
| ); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extract duplicated user tier resolution logic.
The pattern for validating the user, extracting userId, fetching from DB, and validating the tier is repeated in all three functions. Consider extracting a helper to reduce duplication.
♻️ Proposed helper extraction
async function resolveUserTier(userId: string): Promise<SubscriptionTier> {
const db = getDb();
const user = await db.user.findUnique({
where: { id: userId },
select: { tier: true },
});
if (!user) {
throw new AuthorizationError('User not found');
}
const validTiers: SubscriptionTier[] = ['FREE', 'PRO', 'TEAM'];
return validTiers.includes(user.tier as SubscriptionTier)
? (user.tier as SubscriptionTier)
: 'FREE';
}Then simplify each exported function to use this helper.
Also applies to: 125-143
| async function handleInvoicePaid( | ||
| db: ReturnType<typeof getDb>, | ||
| invoice: Record<string, unknown> | undefined | ||
| ): Promise<void> { | ||
| if (!invoice) { | ||
| logger.warn('No invoice data in webhook'); | ||
| return; | ||
| } | ||
|
|
||
| const invoiceId = invoice.id as string; | ||
| const amount = invoice.amount as number; | ||
| const subscriptionId = invoice.subscription_id as string | undefined; | ||
|
|
||
| logger.info({ invoiceId, amount, subscriptionId }, 'Invoice paid'); | ||
|
|
||
| if (subscriptionId) { | ||
| const subscription = await db.user.findFirst({ | ||
| where: { razorpaySubscriptionId: subscriptionId }, | ||
| }); | ||
|
|
||
| if (subscription) { | ||
| logger.info({ userId: subscription.id, invoiceId }, 'Invoice paid for subscription'); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider updating subscription period on invoice payment.
The invoice.paid handler logs the event but doesn't update razorpayCurrentPeriodEnd. For recurring subscriptions, each paid invoice extends the billing period. Consider fetching subscription details and updating the user's period end date to keep it in sync.
♻️ Proposed enhancement
async function handleInvoicePaid(
db: ReturnType<typeof getDb>,
invoice: Record<string, unknown> | undefined
): Promise<void> {
if (!invoice) {
logger.warn('No invoice data in webhook');
return;
}
const invoiceId = invoice.id as string;
const amount = invoice.amount as number;
const subscriptionId = invoice.subscription_id as string | undefined;
logger.info({ invoiceId, amount, subscriptionId }, 'Invoice paid');
if (subscriptionId) {
- const subscription = await db.user.findFirst({
+ const user = await db.user.findFirst({
where: { razorpaySubscriptionId: subscriptionId },
});
- if (subscription) {
- logger.info({ userId: subscription.id, invoiceId }, 'Invoice paid for subscription');
+ if (user) {
+ // Update period end based on invoice billing period if available
+ const billingEnd = invoice.billing_end as number | undefined;
+ if (billingEnd) {
+ await db.user.update({
+ where: { id: user.id },
+ data: { razorpayCurrentPeriodEnd: new Date(billingEnd * 1000) },
+ });
+ }
+ logger.info({ userId: user.id, invoiceId }, 'Invoice paid for subscription');
}
}
}🤖 Prompt for AI Agents
In `@apps/server/src/services/razorpay-webhooks.ts` around lines 256 - 280,
handleInvoicePaid currently logs paid invoices but doesn't extend the user's
subscription period; update it to fetch Razorpay subscription details and set
the user's razorpayCurrentPeriodEnd from the subscription's current_period_end
when an invoice with subscription_id is paid. In handleInvoicePaid, after
locating the user via db.user.findFirst (by razorpaySubscriptionId), call
Razorpay's subscription retrieval (using the subscription_id) to get the
current_period_end (or period_end equivalent), convert it to the DB timestamp
format, and update the user record via db.user.update to set
razorpayCurrentPeriodEnd; ensure you handle missing fields and errors gracefully
and only update when a valid period end is returned.
| prefill: { | ||
| name: '', | ||
| email: '', | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider pre-filling user details from auth context.
The prefill object is currently empty. If the authenticated user's name and email are available from useAuth(), pre-filling these fields would improve checkout UX.
Example enhancement
prefill: {
name: user?.name || '',
email: user?.email || '',
},🤖 Prompt for AI Agents
In `@apps/web/src/app/dashboard/billing/page.tsx` around lines 125 - 128, The
prefill object in page.tsx is empty; obtain the authenticated user's name/email
from the auth context (e.g., useAuth() or the app's user hook) and set
prefill.name and prefill.email to those values (e.g., user?.name || '' and
user?.email || ''); also ensure the hook is imported/initialized in the
component so user is available when constructing the prefill object.
| {isAnnual && price > 0 && ( | ||
| <p className="text-sm text-primary mt-1">Save 50% with annual</p> | ||
| )} |
There was a problem hiding this comment.
"Save 50%" message is misleading for Team tier.
This message displays for all paid tiers when isAnnual is true, but the Team tier's annual pricing (₹2988) doesn't actually provide a 50% discount from the monthly rate (₹249 × 12 = ₹2988).
Either adjust the Team tier's annualPrice in constants to provide an actual discount, or conditionally display the savings percentage based on the actual discount calculation.
Proposed fix for dynamic savings display
-{isAnnual && price > 0 && (
- <p className="text-sm text-primary mt-1">Save 50% with annual</p>
-)}
+{isAnnual && price > 0 && tier.price > 0 && (
+ (() => {
+ const savings = Math.round((1 - tier.annualPrice / (tier.price * 12)) * 100);
+ return savings > 0 ? (
+ <p className="text-sm text-primary mt-1">Save {savings}% with annual</p>
+ ) : null;
+ })()
+)}| <Button | ||
| className="w-full" | ||
| variant={isHighlighted ? 'default' : 'outline'} | ||
| onClick={() => handleCheckout('TEAM')} | ||
| disabled={loading === 'TEAM' || !canUpgrade} | ||
| > | ||
| {loading === 'TEAM' ? ( | ||
| <Loader2 className="w-4 h-4 mr-2 animate-spin" /> | ||
| ) : ( | ||
| <CreditCard className="w-4 h-4 mr-2" /> | ||
| )} | ||
| {canUpgrade ? 'Upgrade to Team' : 'Contact Sales'} | ||
| <ArrowRight className="w-4 h-4 ml-2" /> | ||
| </Button> | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
"Contact Sales" text appears unreachable for Team tier.
The canUpgrade check for the Team tier is billingStatus.tier !== 'TEAM'. However, if the user is already on TEAM, the isCurrentPlan check on line 425 renders "Current Plan" instead. This means the !canUpgrade branch (line 441 showing "Contact Sales") is unreachable.
Consider simplifying or removing the dead branch.
Simplified version
<Button
className="w-full"
variant={isHighlighted ? 'default' : 'outline'}
onClick={() => handleCheckout('TEAM')}
- disabled={loading === 'TEAM' || !canUpgrade}
+ disabled={loading === 'TEAM'}
>
{loading === 'TEAM' ? (
<Loader2 className="w-4 h-4 mr-2 animate-spin" />
) : (
<CreditCard className="w-4 h-4 mr-2" />
)}
- {canUpgrade ? 'Upgrade to Team' : 'Contact Sales'}
+ Upgrade to Team
<ArrowRight className="w-4 h-4 ml-2" />
</Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Button | |
| className="w-full" | |
| variant={isHighlighted ? 'default' : 'outline'} | |
| onClick={() => handleCheckout('TEAM')} | |
| disabled={loading === 'TEAM' || !canUpgrade} | |
| > | |
| {loading === 'TEAM' ? ( | |
| <Loader2 className="w-4 h-4 mr-2 animate-spin" /> | |
| ) : ( | |
| <CreditCard className="w-4 h-4 mr-2" /> | |
| )} | |
| {canUpgrade ? 'Upgrade to Team' : 'Contact Sales'} | |
| <ArrowRight className="w-4 h-4 ml-2" /> | |
| </Button> | |
| ) | |
| <Button | |
| className="w-full" | |
| variant={isHighlighted ? 'default' : 'outline'} | |
| onClick={() => handleCheckout('TEAM')} | |
| disabled={loading === 'TEAM'} | |
| > | |
| {loading === 'TEAM' ? ( | |
| <Loader2 className="w-4 h-4 mr-2 animate-spin" /> | |
| ) : ( | |
| <CreditCard className="w-4 h-4 mr-2" /> | |
| )} | |
| Upgrade to Team | |
| <ArrowRight className="w-4 h-4 ml-2" /> | |
| </Button> |
🤖 Prompt for AI Agents
In `@apps/web/src/app/dashboard/billing/page.tsx` around lines 430 - 444, The
"Contact Sales" text branch is dead for the Team tier because isCurrentPlan
already handles the current TEAM case; update the Button label logic to remove
the unreachable !canUpgrade branch: inside the Button for TEAM (component
rendering with props like isHighlighted, loading, handleCheckout('TEAM'),
canUpgrade), replace the conditional that renders {canUpgrade ? 'Upgrade to
Team' : 'Contact Sales'} with a single deterministic label—e.g., always render
'Upgrade to Team' when canUpgrade is true and otherwise render nothing or a
disabled label—so eliminate the unreachable "Contact Sales" path and keep the
loading/ icon and disabled logic intact.
- Add webAppUrl property to DevRadarConfig interface - Add default value (https://devradar.dev) and development override - Load webAppUrl from VS Code configuration
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@apps/extension/src/services/featureGatingService.ts`:
- Around line 20-44: FEATURE_TIER_MAP duplicates tier data from
SUBSCRIPTION_FEATURES and can drift; replace the hardcoded FEATURE_TIER_MAP with
a derived map built from SUBSCRIPTION_FEATURES (imported from `@devradar/shared`)
by iterating tiers in priority order (e.g., TIER_PRIORITY =
['FREE','PRO','TEAM']) and mapping each feature to its first/lowest-tier
occurrence so the resulting Record<Feature,SubscriptionTier> (used wherever
FEATURE_TIER_MAP is referenced) stays in sync with SUBSCRIPTION_FEATURES.
- Around line 135-137: Remove the unnecessary nullish coalescing in
getWebAppUrl: update the getWebAppUrl() method to return the value directly from
this.configManager.get('webAppUrl') since DevRadarConfig.webAppUrl is typed as
string and loadConfig() already provides a default; locate the getWebAppUrl
method in featureGatingService.ts and eliminate the '?? "http://localhost:3000"'
fallback so the method simply returns this.configManager.get('webAppUrl').
- Around line 16-18: Reorder the type imports in featureGatingService.ts to
satisfy alphabetical import order: place the type import for ConfigManager
before Logger (while keeping AuthService where it belongs), i.e., adjust the
import statements that reference AuthService, ConfigManager, and Logger so that
ConfigManager appears alphabetically before Logger to resolve the pipeline
warning.
In `@apps/server/src/routes/billing.ts`:
- Around line 265-277: The handler that returns subscription data reads
subscriptionDetails.notes.tier directly (in the object returned by reply.send)
without validating it; update the code that builds the response (the reply.send
block in this route) to validate subscriptionDetails.notes.tier against the
expected tier values (or a central enum/allowed list) and use a safe fallback
(e.g., 'unknown' or the default tier) when the value is missing or invalid so
the API never returns arbitrary/unexpected tier strings.
- Around line 221-223: Replace the incorrect ValidationError thrown for a
missing authenticated user: locate the check that throws new
ValidationError('User not found') (in the billing route where user is retrieved)
and throw a more semantically appropriate error such as NotFoundError('User not
found') or an InternalServerError/AuthenticationError if NotFoundError isn't
available in your error types; ensure you import the chosen error class and keep
the original message for context.
- Around line 38-40: Remove the redundant authenticate wrapper function that
simply calls app.authenticate; replace usages of the local const authenticate
with app.authenticate directly in the route options for the billing routes
(e.g., where const authenticate: typeof app.authenticate = async (request,
reply) => { await app.authenticate(request, reply); }; is defined) and apply the
same change to the /cancel, /pause, /resume, and /verify route handlers so they
reference app.authenticate directly.
- Around line 157-164: subscriptionDetails.notes.tier is untrusted external
input and is being cast directly in the db.user.update call; validate it against
the allowed enum before updating. In the handler around where db.user.update is
called (referencing db.user.update, subscriptionDetails.notes.tier,
razorpaySubscriptionId, razorpayCurrentPeriodEnd), check that
subscriptionDetails.notes.tier is exactly 'PRO' or 'TEAM' (e.g., with a small
helper isValidTier or a switch), and either map/normalize it or reject/log and
use a safe default (or abort the update) if it is invalid, then pass the
validated value into the update call.
♻️ Duplicate comments (2)
apps/extension/src/extension.ts (1)
782-799: Semantic mismatch betweenghostModeandprivacyModestill present.This handler gates access using the
ghostModefeature (PRO tier) but toggles theprivacyModeconfig setting. The existinghandleTogglePrivacy(line 438) also togglesprivacyModewithout any feature gate. This means:
- FREE users can toggle
privacyModeviahandleTogglePrivacy- PRO users toggle the same
privacyModeviahandleEnableGhostModeEither introduce a separate
ghostModeconfig key with distinct behavior, or clarify why both commands should share the same underlying setting.apps/web/src/lib/constants.ts (1)
115-137: LGTM on pricing, but verify fallback plan IDs won't cause silent failures.The pricing math is correct (₹588/year = ₹49/month, ~50% off ₹99 monthly). However, the fallback plan IDs (
'plan_pro_monthly','plan_pro_annual') are placeholders that Razorpay will reject. SinceNEXT_PUBLIC_*environment variables are evaluated at build time, if they're not set during the build, these fallbacks become permanent.Consider either:
- Using
undefinedas fallback to fail fast during checkout- Adding build-time validation for required billing env vars
This duplicates a concern from a previous review. Verify that the environment variables are properly configured in your deployment pipeline.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
apps/extension/src/extension.tsapps/extension/src/services/featureGatingService.tsapps/extension/src/utils/configManager.tsapps/server/src/routes/billing.tsapps/web/src/app/layout.tsxapps/web/src/lib/constants.ts
🧰 Additional context used
🪛 GitHub Actions: CI
apps/extension/src/services/featureGatingService.ts
[warning] 18-18: ESLint: import/order - '../utils/configManager type import should occur before type import of ../utils/logger'
[error] 136-136: ESLint: '@typescript-eslint/no-unnecessary-condition' - Unnecessary conditional, expected left-hand side of '??' operator to be possibly null or undefined
🪛 GitHub Check: Build and Test
apps/extension/src/services/featureGatingService.ts
[failure] 136-136:
Unnecessary conditional, expected left-hand side of ?? operator to be possibly null or undefined
[warning] 18-18:
../utils/configManager type import should occur before type import of ../utils/logger
🪛 GitHub Check: CodeQL
apps/server/src/routes/billing.ts
[failure] 123-167: Missing rate limiting
This route handler performs authorization, but is not rate-limited.
🔇 Additional comments (15)
apps/web/src/app/layout.tsx (2)
4-4: Previous feedback addressed: now using the themed Toaster wrapper.The import has been correctly updated to use the custom
Toasterfrom@/components/ui/sonner, which integrates withnext-themesfor proper theme synchronization.
98-110: LGTM! AuthProvider integration looks correct.The AuthProvider is appropriately nested inside ThemeProvider, ensuring theme context is available to authentication components. The Toaster placement within the authenticated layout region is suitable for displaying notifications throughout the app.
apps/extension/src/utils/configManager.ts (1)
14-14: LGTM! Clean addition ofwebAppUrlconfiguration.The new configuration field follows the established pattern for
serverUrlandwsUrl, with appropriate production defaults and commented development alternatives.Also applies to: 31-36, 102-102
apps/extension/src/extension.ts (3)
52-53: LGTM! Clean integration of FeatureGatingService.The service is properly initialized with required dependencies and correctly registered in the disposables array for proper lifecycle management.
Also applies to: 83-88, 101-102
207-214: LGTM! Command registrations follow established patterns.
801-808: LGTM! Clean implementation using the centralizedgetWebAppUrl()from FeatureGatingService.The refactoring addressed the previous duplicate code concern. The billing URL construction and external link opening are handled correctly.
apps/extension/src/services/featureGatingService.ts (3)
59-69: LGTM! Defensive tier handling is appropriate.The fallback to
'FREE'tier whenuser.tieris undefined protects against runtime inconsistencies.
96-119: LGTM! Clean upgrade prompt implementation.The flow correctly checks access first, shows an informative warning message, and opens the billing page when the user chooses to upgrade.
139-141: LGTM! Dispose method correctly implements the interface with no-op.The comment clarifies that no resources need cleanup, addressing the previous concern about unused disposables.
apps/server/src/routes/billing.ts (4)
42-65: LGTM!The
/checkoutendpoint properly validates Razorpay configuration, validates request body with Zod, and delegates subscription creation correctly.
67-110: LGTM!The
/cancel,/pause, and/resumeendpoints follow a consistent pattern with proper Razorpay checks and success responses.
116-121: Static analysis false positive: rate limiting is present.The CodeQL alert flagging missing rate limiting is incorrect. This route has proper rate limiting configured (
max: 20, timeWindow: '1 minute').
170-202: LGTM!The
/webhooksendpoint correctly:
- Exempts itself from rate limiting to handle Razorpay's webhook traffic
- Enables raw body access for signature verification
- Validates signature header and raw body presence
- Handles errors appropriately with logging
apps/web/src/lib/constants.ts (2)
96-114: LGTM!Free tier with
annualPrice: 0is consistent and correct.
138-161: Team tier pricing is now correct.The
annualPrice: 2999provides ~50% discount compared to monthly (499 × 12 = 5988), which matches the comment. The CTA update to "Upgrade to Team" is appropriate. Same note about fallback plan IDs applies here.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| const FEATURE_TIER_MAP: Record<Feature, SubscriptionTier> = { | ||
| presence: 'FREE', | ||
| friends: 'FREE', | ||
| globalLeaderboard: 'FREE', | ||
| friendsLeaderboard: 'FREE', | ||
| streaks: 'FREE', | ||
| achievements: 'FREE', | ||
| poke: 'FREE', | ||
| privacyMode: 'FREE', | ||
| unlimitedFriends: 'PRO', | ||
| ghostMode: 'PRO', | ||
| customStatus: 'PRO', | ||
| history30d: 'PRO', | ||
| themes: 'PRO', | ||
| customEmoji: 'PRO', | ||
| prioritySupport: 'PRO', | ||
| conflictRadar: 'TEAM', | ||
| teamCreation: 'TEAM', | ||
| teamAnalytics: 'TEAM', | ||
| slackIntegration: 'TEAM', | ||
| privateLeaderboards: 'TEAM', | ||
| adminControls: 'TEAM', | ||
| ssoSaml: 'TEAM', | ||
| dedicatedSupport: 'TEAM', | ||
| } as const; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider deriving FEATURE_TIER_MAP from SUBSCRIPTION_FEATURES to prevent drift.
This map duplicates tier information already present in SUBSCRIPTION_FEATURES. If features or tiers change in the shared package, this map could become stale.
♻️ Optional: Derive from SUBSCRIPTION_FEATURES
import {
type Feature,
type SubscriptionTier,
SUBSCRIPTION_FEATURES,
FEATURE_DESCRIPTIONS,
} from '@devradar/shared';
const TIER_PRIORITY: SubscriptionTier[] = ['FREE', 'PRO', 'TEAM'];
const FEATURE_TIER_MAP: Record<Feature, SubscriptionTier> = Object.fromEntries(
TIER_PRIORITY.flatMap((tier) =>
SUBSCRIPTION_FEATURES[tier].map((feature) => [feature, tier] as const)
).reduce((acc, [feature, tier]) => {
// Keep the lowest tier (first occurrence)
if (!acc.has(feature)) acc.set(feature, tier);
return acc;
}, new Map<Feature, SubscriptionTier>())
) as Record<Feature, SubscriptionTier>;🤖 Prompt for AI Agents
In `@apps/extension/src/services/featureGatingService.ts` around lines 20 - 44,
FEATURE_TIER_MAP duplicates tier data from SUBSCRIPTION_FEATURES and can drift;
replace the hardcoded FEATURE_TIER_MAP with a derived map built from
SUBSCRIPTION_FEATURES (imported from `@devradar/shared`) by iterating tiers in
priority order (e.g., TIER_PRIORITY = ['FREE','PRO','TEAM']) and mapping each
feature to its first/lowest-tier occurrence so the resulting
Record<Feature,SubscriptionTier> (used wherever FEATURE_TIER_MAP is referenced)
stays in sync with SUBSCRIPTION_FEATURES.
| getWebAppUrl(): string { | ||
| return this.configManager.get('webAppUrl') ?? 'http://localhost:3000'; | ||
| } |
There was a problem hiding this comment.
Remove unnecessary nullish coalescing to fix pipeline failure.
configManager.get('webAppUrl') returns string (not string | undefined) because DevRadarConfig.webAppUrl is typed as string. The fallback in loadConfig() already ensures a default value exists.
🔧 Proposed fix
getWebAppUrl(): string {
- return this.configManager.get('webAppUrl') ?? 'http://localhost:3000';
+ return this.configManager.get('webAppUrl');
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getWebAppUrl(): string { | |
| return this.configManager.get('webAppUrl') ?? 'http://localhost:3000'; | |
| } | |
| getWebAppUrl(): string { | |
| return this.configManager.get('webAppUrl'); | |
| } |
🧰 Tools
🪛 GitHub Actions: CI
[error] 136-136: ESLint: '@typescript-eslint/no-unnecessary-condition' - Unnecessary conditional, expected left-hand side of '??' operator to be possibly null or undefined
🪛 GitHub Check: Build and Test
[failure] 136-136:
Unnecessary conditional, expected left-hand side of ?? operator to be possibly null or undefined
🤖 Prompt for AI Agents
In `@apps/extension/src/services/featureGatingService.ts` around lines 135 - 137,
Remove the unnecessary nullish coalescing in getWebAppUrl: update the
getWebAppUrl() method to return the value directly from
this.configManager.get('webAppUrl') since DevRadarConfig.webAppUrl is typed as
string and loadConfig() already provides a default; locate the getWebAppUrl
method in featureGatingService.ts and eliminate the '?? "http://localhost:3000"'
fallback so the method simply returns this.configManager.get('webAppUrl').
| const authenticate: typeof app.authenticate = async (request, reply) => { | ||
| await app.authenticate(request, reply); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove redundant authenticate wrapper.
This wrapper function adds no value—it simply delegates to app.authenticate with identical signature and behavior. Use app.authenticate directly in route options.
♻️ Proposed fix
export function billingRoutes(app: FastifyInstance): void {
- const authenticate: typeof app.authenticate = async (request, reply) => {
- await app.authenticate(request, reply);
- };
-
app.post(
'/checkout',
- { onRequest: [authenticate] },
+ { onRequest: [app.authenticate] },
async (request: FastifyRequest, reply: FastifyReply) => {Apply the same change to /cancel, /pause, /resume, and /verify routes.
🤖 Prompt for AI Agents
In `@apps/server/src/routes/billing.ts` around lines 38 - 40, Remove the redundant
authenticate wrapper function that simply calls app.authenticate; replace usages
of the local const authenticate with app.authenticate directly in the route
options for the billing routes (e.g., where const authenticate: typeof
app.authenticate = async (request, reply) => { await app.authenticate(request,
reply); }; is defined) and apply the same change to the /cancel, /pause,
/resume, and /verify route handlers so they reference app.authenticate directly.
| await db.user.update({ | ||
| where: { id: userId }, | ||
| data: { | ||
| tier: subscriptionDetails.notes.tier as 'PRO' | 'TEAM', | ||
| razorpaySubscriptionId: razorpaySubscriptionId, | ||
| razorpayCurrentPeriodEnd: new Date(subscriptionDetails.current_end * 1000), | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Validate tier value from external Razorpay response.
The subscriptionDetails.notes.tier is external data that's cast directly to 'PRO' | 'TEAM' without validation. If Razorpay returns an unexpected value (e.g., due to misconfiguration or tampering), this could store invalid data.
🐛 Proposed fix
+ const tier = subscriptionDetails.notes.tier;
+ if (tier !== 'PRO' && tier !== 'TEAM') {
+ throw new ValidationError('Invalid subscription tier in payment details');
+ }
+
await db.user.update({
where: { id: userId },
data: {
- tier: subscriptionDetails.notes.tier as 'PRO' | 'TEAM',
+ tier,
razorpaySubscriptionId: razorpaySubscriptionId,
razorpayCurrentPeriodEnd: new Date(subscriptionDetails.current_end * 1000),
},
});🤖 Prompt for AI Agents
In `@apps/server/src/routes/billing.ts` around lines 157 - 164,
subscriptionDetails.notes.tier is untrusted external input and is being cast
directly in the db.user.update call; validate it against the allowed enum before
updating. In the handler around where db.user.update is called (referencing
db.user.update, subscriptionDetails.notes.tier, razorpaySubscriptionId,
razorpayCurrentPeriodEnd), check that subscriptionDetails.notes.tier is exactly
'PRO' or 'TEAM' (e.g., with a small helper isValidTier or a switch), and either
map/normalize it or reject/log and use a safe default (or abort the update) if
it is invalid, then pass the validated value into the update call.
| if (!user) { | ||
| throw new ValidationError('User not found'); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using a more appropriate error type for "User not found".
ValidationError typically indicates malformed input. For authenticated users where the user record doesn't exist (likely a data integrity issue), a different error type would be more semantically accurate. However, this scenario should be rare since authentication implies user existence.
🤖 Prompt for AI Agents
In `@apps/server/src/routes/billing.ts` around lines 221 - 223, Replace the
incorrect ValidationError thrown for a missing authenticated user: locate the
check that throws new ValidationError('User not found') (in the billing route
where user is retrieved) and throw a more semantically appropriate error such as
NotFoundError('User not found') or an InternalServerError/AuthenticationError if
NotFoundError isn't available in your error types; ensure you import the chosen
error class and keep the original message for context.
| return reply.send({ | ||
| hasSubscription: true, | ||
| subscription: { | ||
| id: subscriptionDetails.id, | ||
| status: subscriptionDetails.status, | ||
| tier: subscriptionDetails.notes.tier, | ||
| currentPeriodStart: new Date(subscriptionDetails.current_start * 1000).toISOString(), | ||
| currentPeriodEnd: new Date(subscriptionDetails.current_end * 1000).toISOString(), | ||
| endAt: subscriptionDetails.end_at | ||
| ? new Date(subscriptionDetails.end_at * 1000).toISOString() | ||
| : null, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Same tier validation concern as /verify.
The subscriptionDetails.notes.tier is returned directly without validation. While this is less critical than the DB write in /verify, consider validating for consistency or using a fallback.
🤖 Prompt for AI Agents
In `@apps/server/src/routes/billing.ts` around lines 265 - 277, The handler that
returns subscription data reads subscriptionDetails.notes.tier directly (in the
object returned by reply.send) without validating it; update the code that
builds the response (the reply.send block in this route) to validate
subscriptionDetails.notes.tier against the expected tier values (or a central
enum/allowed list) and use a safe fallback (e.g., 'unknown' or the default tier)
when the value is missing or invalid so the API never returns
arbitrary/unexpected tier strings.
- Fix import order for ConfigManager type import - Add eslint-disable for configManager.get() possibly null check
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@apps/extension/src/services/featureGatingService.ts`:
- Around line 96-119: In promptUpgrade, when building upgradeUrl use
URL-encoding for the query values: replace the raw interpolation of feature and
requiredTier with encoded values (e.g., via encodeURIComponent or
URLSearchParams) so upgradeUrl is constructed from getWebAppUrl() plus a safely
encoded query like ?upgrade=<encodedRequiredTier>&feature=<encodedFeature>;
update references to feature and requiredTier in that construction and keep the
rest of the method (openExternal, logger.info) unchanged.
- Around line 59-69: In hasAccess, guard against unexpected user.tier strings
before indexing SUBSCRIPTION_FEATURES: retrieve user via
this.authService.getUser(), normalize tierCandidate = (user.tier ?? 'FREE') as
string, check if tierCandidate is a key in SUBSCRIPTION_FEATURES (e.g.,
Object.prototype.hasOwnProperty.call(SUBSCRIPTION_FEATURES, tierCandidate)); if
not, fall back to 'FREE' (or return false), then cast to SubscriptionTier and
safely call SUBSCRIPTION_FEATURES[tier].includes(feature); update code to remove
the eslint disable and ensure the runtime check prevents
SUBSCRIPTION_FEATURES[tier] from being undefined.
♻️ Duplicate comments (1)
apps/extension/src/services/featureGatingService.ts (1)
20-44: Consider derivingFEATURE_TIER_MAPfromSUBSCRIPTION_FEATURES.This hardcoded map duplicates tier information from
SUBSCRIPTION_FEATURESand could drift if features or tiers change in the shared package.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/extension/src/services/featureGatingService.ts
🔇 Additional comments (4)
apps/extension/src/services/featureGatingService.ts (4)
8-18: LGTM - Imports properly structured.Good use of the shared package for feature definitions instead of local duplication. Import order issue from previous review has been addressed.
46-52: LGTM - Clean dependency injection.Proper use of
private readonlyfor injected dependencies.
76-78: LGTM.Simple lookup with clear intent.
126-142: LGTM - Utility methods and cleanup.The
eslint-disablecomment ongetWebAppUrldocuments the intentional defensive fallback. The no-opdisposeis appropriate given no subscriptions are registered.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| hasAccess(feature: Feature): boolean { | ||
| const user = this.authService.getUser(); | ||
| if (!user) { | ||
| return false; | ||
| } | ||
|
|
||
| // Defensive: ensure tier is valid, default to FREE if not | ||
| // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- runtime value might differ from type | ||
| const tier = (user.tier ?? 'FREE') as SubscriptionTier; | ||
| return SUBSCRIPTION_FEATURES[tier].includes(feature); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Add validation for unexpected tier values.
The defensive ?? handles null/undefined, but if user.tier is an unexpected string (e.g., from API version mismatch), SUBSCRIPTION_FEATURES[tier] would be undefined and .includes() would throw a TypeError.
♻️ Suggested defensive approach
hasAccess(feature: Feature): boolean {
const user = this.authService.getUser();
if (!user) {
return false;
}
- // Defensive: ensure tier is valid, default to FREE if not
- // eslint-disable-next-line `@typescript-eslint/no-unnecessary-condition` -- runtime value might differ from type
- const tier = (user.tier ?? 'FREE') as SubscriptionTier;
- return SUBSCRIPTION_FEATURES[tier].includes(feature);
+ // Defensive: ensure tier is valid and exists in SUBSCRIPTION_FEATURES
+ const tier = user.tier as SubscriptionTier | undefined;
+ const features = SUBSCRIPTION_FEATURES[tier ?? 'FREE'];
+ return features?.includes(feature) ?? false;
}🤖 Prompt for AI Agents
In `@apps/extension/src/services/featureGatingService.ts` around lines 59 - 69, In
hasAccess, guard against unexpected user.tier strings before indexing
SUBSCRIPTION_FEATURES: retrieve user via this.authService.getUser(), normalize
tierCandidate = (user.tier ?? 'FREE') as string, check if tierCandidate is a key
in SUBSCRIPTION_FEATURES (e.g.,
Object.prototype.hasOwnProperty.call(SUBSCRIPTION_FEATURES, tierCandidate)); if
not, fall back to 'FREE' (or return false), then cast to SubscriptionTier and
safely call SUBSCRIPTION_FEATURES[tier].includes(feature); update code to remove
the eslint disable and ensure the runtime check prevents
SUBSCRIPTION_FEATURES[tier] from being undefined.
| async promptUpgrade(feature: Feature): Promise<boolean> { | ||
| if (this.hasAccess(feature)) { | ||
| return true; | ||
| } | ||
|
|
||
| const requiredTier = this.getRequiredTier(feature); | ||
| const featureDescription = FEATURE_DESCRIPTIONS[feature]; | ||
|
|
||
| const action = await vscode.window.showWarningMessage( | ||
| `DevRadar: "${featureDescription}" requires ${requiredTier} tier.`, | ||
| 'Upgrade Now', | ||
| 'Maybe Later' | ||
| ); | ||
|
|
||
| if (action === 'Upgrade Now') { | ||
| const webAppUrl = this.getWebAppUrl(); | ||
| const upgradeUrl = `${webAppUrl}/dashboard/billing?upgrade=${requiredTier}&feature=${feature}`; | ||
|
|
||
| await vscode.env.openExternal(vscode.Uri.parse(upgradeUrl)); | ||
| this.logger.info('Opened upgrade page', { feature, requiredTier }); | ||
| } | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider URL-encoding query parameters.
While current Feature and SubscriptionTier values are safe alphanumeric strings, URL-encoding the parameters is best practice and guards against future feature names with special characters.
♻️ Optional fix
if (action === 'Upgrade Now') {
const webAppUrl = this.getWebAppUrl();
- const upgradeUrl = `${webAppUrl}/dashboard/billing?upgrade=${requiredTier}&feature=${feature}`;
+ const upgradeUrl = `${webAppUrl}/dashboard/billing?upgrade=${encodeURIComponent(requiredTier)}&feature=${encodeURIComponent(feature)}`;
await vscode.env.openExternal(vscode.Uri.parse(upgradeUrl));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async promptUpgrade(feature: Feature): Promise<boolean> { | |
| if (this.hasAccess(feature)) { | |
| return true; | |
| } | |
| const requiredTier = this.getRequiredTier(feature); | |
| const featureDescription = FEATURE_DESCRIPTIONS[feature]; | |
| const action = await vscode.window.showWarningMessage( | |
| `DevRadar: "${featureDescription}" requires ${requiredTier} tier.`, | |
| 'Upgrade Now', | |
| 'Maybe Later' | |
| ); | |
| if (action === 'Upgrade Now') { | |
| const webAppUrl = this.getWebAppUrl(); | |
| const upgradeUrl = `${webAppUrl}/dashboard/billing?upgrade=${requiredTier}&feature=${feature}`; | |
| await vscode.env.openExternal(vscode.Uri.parse(upgradeUrl)); | |
| this.logger.info('Opened upgrade page', { feature, requiredTier }); | |
| } | |
| return false; | |
| } | |
| async promptUpgrade(feature: Feature): Promise<boolean> { | |
| if (this.hasAccess(feature)) { | |
| return true; | |
| } | |
| const requiredTier = this.getRequiredTier(feature); | |
| const featureDescription = FEATURE_DESCRIPTIONS[feature]; | |
| const action = await vscode.window.showWarningMessage( | |
| `DevRadar: "${featureDescription}" requires ${requiredTier} tier.`, | |
| 'Upgrade Now', | |
| 'Maybe Later' | |
| ); | |
| if (action === 'Upgrade Now') { | |
| const webAppUrl = this.getWebAppUrl(); | |
| const upgradeUrl = `${webAppUrl}/dashboard/billing?upgrade=${encodeURIComponent(requiredTier)}&feature=${encodeURIComponent(feature)}`; | |
| await vscode.env.openExternal(vscode.Uri.parse(upgradeUrl)); | |
| this.logger.info('Opened upgrade page', { feature, requiredTier }); | |
| } | |
| return false; | |
| } |
🤖 Prompt for AI Agents
In `@apps/extension/src/services/featureGatingService.ts` around lines 96 - 119,
In promptUpgrade, when building upgradeUrl use URL-encoding for the query
values: replace the raw interpolation of feature and requiredTier with encoded
values (e.g., via encodeURIComponent or URLSearchParams) so upgradeUrl is
constructed from getWebAppUrl() plus a safely encoded query like
?upgrade=<encodedRequiredTier>&feature=<encodedFeature>; update references to
feature and requiredTier in that construction and keep the rest of the method
(openExternal, logger.info) unchanged.
1 participant
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.