feat(stripe): add coffee/meeting checkout flow with APOD-style FAB#155
feat(stripe): add coffee/meeting checkout flow with APOD-style FAB#155
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughAdds end-to-end Stripe payments: backend config, Stripe service, GraphQL types/resolvers, validation and tests; frontend FAB, dialog, hook, GraphQL mutation/types, success/cancel pages, README env var docs, and related frontend/backend tests and docs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FAB as Frontend<br/>StripeFab/Dialog
participant Hook as Frontend<br/>useStripeCheckout
participant Apollo as Apollo Client
participant Resolver as Backend<br/>GraphQL Resolver
participant StripeSvc as Backend<br/>Stripe Service
participant StripeAPI as Stripe API
User->>FAB: open dialog, select product, enter email
FAB->>Hook: startCheckout(productKey, email)
Hook->>Apollo: mutation CREATE_CHECKOUT_SESSION
Apollo->>Resolver: GraphQL createCheckoutSession(input)
Resolver->>Resolver: validate productKey
Resolver->>StripeSvc: createCheckoutSession(input)
StripeSvc->>StripeAPI: create checkout session (priceId, urls, email, metadata)
StripeAPI-->>StripeSvc: session { id, url }
StripeSvc-->>Resolver: { sessionId, url }
Resolver-->>Apollo: { sessionId, url }
Apollo-->>Hook: mutation result
Hook->>User: redirect to session.url (window.location)
User->>StripeAPI: Stripe Checkout flow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/config/config.ts (1)
26-38:⚠️ Potential issue | 🟠 MajorStripe credentials missing from required env vars — runtime bomb waiting to explode.
You validate
OPENAI_API_KEY,RESEND_API_KEY,NASA_API_KEYat startup, butSTRIPE_SECRET_KEY,STRIPE_COFFEE_PRICE_ID, andSTRIPE_MEETING_PRICE_IDslip through with empty string defaults. If someone deploys without these, the checkout mutation will blow up at runtime with a cryptic Stripe error instead of a clear "Missing env var" message at startup.Either:
- Add them to
requiredEnvVars(if Stripe is always needed), or- Make the Stripe service fail gracefully with a clear error when credentials are missing
Option 2 is more flexible if you want checkout to be an optional feature.
🔧 Option 1: Add to required env vars
const requiredEnvVars = [ 'PORT', 'NODE_ENV', 'MONGODB_URI', 'JWT_SECRET', 'REDIS_URL', 'OPENAI_API_KEY', 'RATE_LIMIT_WINDOW', 'RATE_LIMIT_MAX_REQUESTS', 'RATE_LIMIT_ANONYMOUS_REQUESTS', 'RESEND_API_KEY', - 'NASA_API_KEY' + 'NASA_API_KEY', + 'STRIPE_SECRET_KEY', + 'STRIPE_COFFEE_PRICE_ID', + 'STRIPE_MEETING_PRICE_ID' ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/config/config.ts` around lines 26 - 38, The startup env validation currently lists requiredEnvVars but omits Stripe keys, so missing STRIPE_SECRET_KEY, STRIPE_COFFEE_PRICE_ID, and STRIPE_MEETING_PRICE_ID can cause runtime failures; either add these three names to the requiredEnvVars array in config.ts (so startup fails fast), or keep them optional and modify the Stripe-related code (e.g., the Stripe service/checkout resolver functions that reference STRIPE_SECRET_KEY, STRIPE_COFFEE_PRICE_ID, STRIPE_MEETING_PRICE_ID) to check for presence and throw a clear error or return a graceful “Stripe not configured” response when any are missing; choose one approach and implement the corresponding change.
🧹 Nitpick comments (1)
frontend/src/lib/graphql/types/stripe.types.ts (1)
1-1: Hardcoded product keys — acceptable but watch for drift.You've got
'coffee' | 'meeting'hardcoded here, while the backend derivesStripeProductKeyfromkeyof typeof PRODUCTS(seebackend/src/services/stripe.tsline 20). For two products, this works. But if you add'mentorship'later and forget to update the frontend, TypeScript won't save you.For now, this is fine — just stay disciplined when adding products. Or consider a shared types package if this grows.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/lib/graphql/types/stripe.types.ts` at line 1, The frontend's StripeProductKey type is hardcoded as 'coffee' | 'meeting', which can drift from the backend's canonical keys (derived as keyof typeof PRODUCTS); update the code so the frontend consumes a single source of truth—either replace the hardcoded union by importing a generated/shared type from the backend (or a shared types package) that mirrors keyof typeof PRODUCTS, or add an automated type generation step that exports StripeProductKey to frontend; locate the type alias StripeProductKey in frontend/src/lib/graphql/types/stripe.types.ts and align it with the backend PRODUCTS/keyof typeof PRODUCTS source to prevent divergence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/resolvers/stripe/mutations.ts`:
- Around line 14-35: The createCheckoutSession resolver in stripeMutations is
not wrapped with createErrorHandler; update the export so the resolver function
returned by createCheckoutSession is passed through createErrorHandler from
backend/src/utils/errors (import createErrorHandler if missing) to enforce the
project's resolver error-handling contract; locate the stripeMutations object
and the createCheckoutSession method and wrap that method with
createErrorHandler while preserving its current logic and thrown Errors/internal
logging behavior.
In `@backend/src/services/stripe.ts`:
- Line 63: Sanitize the email before sending to Stripe: instead of passing email
directly in the Stripe request (the line setting customer_email: email ||
undefined), trim whitespace and convert empty/whitespace-only strings to
undefined; locate the Stripe call in backend/src/services/stripe.ts where
customer_email is set and replace the direct use of the email variable with a
trimmed value (use email?.trim() and check length) so Stripe never receives a
whitespace-only string.
In `@backend/src/validation/shield.ts`:
- Line 107: Add a Zod schema file named checkout.schema.ts exporting a Zod
schema called CheckoutInputSchema (and a typed CheckoutInput) under
backend/src/validation/schemas/, then add a Shield rule factory function
validateCheckout (or export a rule named validateCheckout) that uses that schema
to validate inputs; finally update the Shield map where createCheckoutSession
currently uses allow to instead use validateCheckout (i.e., replace
"createCheckoutSession: allow" with "createCheckoutSession: validateCheckout")
and import the new validateCheckout in backend/src/validation/shield.ts so the
resolver relies on the centralized CheckoutInputSchema rather than inline
checks.
In `@frontend/src/app/payment/success/page.tsx`:
- Around line 19-40: The page currently renders a "Payment complete" UI whenever
PaymentSuccessPage finds a session_id in searchParams (or none), so you must
verify the checkout session server-side before showing success copy: in
PaymentSuccessPage, use the session_id (from params?.session_id) to call your
backend/Stripe lookup (e.g., a server-side fetch to your payment API or Stripe
SDK) to confirm the session status is paid/complete, and only render the success
block when that verification returns a valid paid status; otherwise render a
neutral recovery state (error/processing instructions and retry/contact support)
when the session is missing, invalid, or not paid. Ensure you reference
sessionId and the PaymentSuccessPage component when implementing the server-side
verification and conditional rendering.
In `@frontend/src/components/stripe/StripeDialog.tsx`:
- Around line 63-66: handleContinue currently sends the raw email to
startCheckout; trim and validate the email first to avoid leading/trailing
spaces and obvious malformed values. In handleContinue (and the similar block
around lines 137-143), replace uses of the raw email variable with a
trimmedEmail = email.trim(), check trimmedEmail is non-empty and matches a
simple email pattern or the existing form validity before calling
startCheckout(trimmedEmail, ...), and surface a user-facing validation
error/early return when validation fails instead of proceeding to Stripe.
In `@frontend/src/lib/hooks/useStripeCheckout.ts`:
- Around line 32-37: The race allows a second click between mutation completion
and window.location.assign; to fix, add a local isRedirecting boolean state in
StripeDialog and set isRedirecting = true immediately when calling startCheckout
(the useStripeCheckout hook/mutation), include isRedirecting in the button
disabled condition alongside selected and loading, and clear isRedirecting only
on explicit navigation failure or in cleanup; reference the startCheckout call
in StripeDialog, the loading flag from useStripeCheckout, and the button's
disabled prop to locate where to add the guard.
---
Outside diff comments:
In `@backend/src/config/config.ts`:
- Around line 26-38: The startup env validation currently lists requiredEnvVars
but omits Stripe keys, so missing STRIPE_SECRET_KEY, STRIPE_COFFEE_PRICE_ID, and
STRIPE_MEETING_PRICE_ID can cause runtime failures; either add these three names
to the requiredEnvVars array in config.ts (so startup fails fast), or keep them
optional and modify the Stripe-related code (e.g., the Stripe service/checkout
resolver functions that reference STRIPE_SECRET_KEY, STRIPE_COFFEE_PRICE_ID,
STRIPE_MEETING_PRICE_ID) to check for presence and throw a clear error or return
a graceful “Stripe not configured” response when any are missing; choose one
approach and implement the corresponding change.
---
Nitpick comments:
In `@frontend/src/lib/graphql/types/stripe.types.ts`:
- Line 1: The frontend's StripeProductKey type is hardcoded as 'coffee' |
'meeting', which can drift from the backend's canonical keys (derived as keyof
typeof PRODUCTS); update the code so the frontend consumes a single source of
truth—either replace the hardcoded union by importing a generated/shared type
from the backend (or a shared types package) that mirrors keyof typeof PRODUCTS,
or add an automated type generation step that exports StripeProductKey to
frontend; locate the type alias StripeProductKey in
frontend/src/lib/graphql/types/stripe.types.ts and align it with the backend
PRODUCTS/keyof typeof PRODUCTS source to prevent divergence.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
README.mdbackend/package.jsonbackend/src/__tests__/unit/stripe.mutations.test.tsbackend/src/config/config.tsbackend/src/resolvers/index.tsbackend/src/resolvers/stripe/mutations.tsbackend/src/schemas/typeDefs.tsbackend/src/schemas/types/stripeTypes.tsbackend/src/services/stripe.tsbackend/src/validation/shield.tsfrontend/src/__tests__/components/Stripe.test.tsxfrontend/src/app/layout.tsxfrontend/src/app/payment/cancel/page.tsxfrontend/src/app/payment/success/page.tsxfrontend/src/components/stripe/StripeDialog.tsxfrontend/src/components/stripe/StripeFab.tsxfrontend/src/lib/graphql/mutations/stripe.mutations.tsfrontend/src/lib/graphql/types/stripe.types.tsfrontend/src/lib/hooks/useStripeCheckout.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
backend/src/validation/schemas/checkout.schema.ts (1)
5-6: Make GraphQL tell the same truth as Zod.This schema only accepts
coffee | meeting, butbackend/src/schemas/types/stripeTypes.tsstill exposesproductKey: String!. So clients learn “any string is fine” and only get punched later by runtime validation. Add a GraphQL enum forproductKeyand use it inCheckoutInputso the contract stops lying.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/validation/schemas/checkout.schema.ts` around lines 5 - 6, Add a GraphQL enum for productKey and use it in the CheckoutInput so the schema matches the Zod union. Define an enum ProductKey with values "coffee" and "meeting" (or uppercase variants if you follow project conventions) and change the CheckoutInput type to use productKey: ProductKey! instead of String!; also update any generated/exported type (the existing productKey type) to reference the new ProductKey enum so server and clients share the same contract as CheckoutInputSchema and productKey in the codebase.backend/src/services/stripe.ts (2)
77-82: Test mode short-circuit is fine, but be aware of config inconsistency.Using
process.env.NODE_ENV === 'test'directly here while elsewhere usingconfig.nodeEnvcreates a minor inconsistency. Not a blocker — it works — but if someone overridesconfig.nodeEnvin tests without settingNODE_ENV, behavior would diverge.Consider using
config.nodeEnv === 'test'for consistency with how other config values are accessed.♻️ Optional: Use config.nodeEnv for consistency
export async function createCheckoutSession({ productKey, email, }: CreateCheckoutSessionInput): Promise<CheckoutSessionResult> { - if (process.env.NODE_ENV === 'test') { + if (config.nodeEnv === 'test') { return { sessionId: `test_session_${productKey}`, url: `https://checkout.stripe.com/test-${productKey}`, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/services/stripe.ts` around lines 77 - 82, Replace the direct environment check in the Stripe service short-circuit with the project's config value: in backend/src/services/stripe.ts update the condition that currently uses process.env.NODE_ENV === 'test' (the block that returns the test sessionId/url) to use config.nodeEnv === 'test' instead so test behavior is consistent with other code that reads nodeEnv from the config object.
146-151: Error handling is solid but consider logging unexpected errors.You catch
StripeInvalidRequestErrorand map it properly, then re-throw everything else. That's correct. But when you re-throw unknown errors, they'll bubble up without context about what happened ingetCheckoutSessionStatus.Consider adding a log before the re-throw so you're not flying blind in production.
🔍 Add logging before re-throw
+import { logger } from '../utils/logger'; + // ... in getCheckoutSessionStatus catch block: } catch (error) { if (error instanceof Stripe.errors.StripeInvalidRequestError) { throw new StripeServiceError('SESSION_NOT_FOUND', 'Checkout session not found', 404, { sessionId }); } + logger.error('Unexpected error retrieving checkout session', { error, sessionId }); throw error; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/services/stripe.ts` around lines 146 - 151, In getCheckoutSessionStatus, before re-throwing unexpected errors, log the error with context (include sessionId and a descriptive message) so production has visibility; use the module's existing logger (e.g., logger or processLogger). Keep the StripeInvalidRequestError mapping to StripeServiceError as-is, but add a logger.error call that logs the error object/stack and sessionId immediately before the final throw so callers still receive the original error while you retain diagnostic details.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/validation/schemas/checkout.schema.ts`:
- Around line 3-12: The current CheckoutInputSchema uses a custom emailRegex
(emailRegex) which is brittle and omits valid addresses (like plus-addressing);
remove the emailRegex and replace the string validation with Zod's built-in
email validation via z.preprocess to handle blank strings and then
z.string().email() (update the email field in CheckoutInputSchema accordingly),
or simpler use z.string().trim().email().optional().or(z.literal('')) so valid
emails (including plus-addressing) are accepted and empty strings are still
allowed; delete the unused emailRegex constant.
In `@backend/src/validation/shield.ts`:
- Line 102: The public rule granting access to checkoutSessionStatus makes
sessionId a PII lookup token because the Stripe status type includes
customerEmail; update backend/src/validation/shield.ts to prevent anonymous
retrieval of customerEmail via checkoutSessionStatus by either (a) restricting
access to checkoutSessionStatus to authenticated users via a GraphQL Shield rule
(e.g., add an isAuthenticated rule to the checkoutSessionStatus field), or (b)
if anonymous access is required for success pages, strip or mask the
customerEmail in the resolver response for checkoutSessionStatus (or add a
field-level rule that returns a redacted customerEmail for public contexts) so
sessionId cannot be used to fetch plaintext emails. Ensure you modify the
checkoutSessionStatus field rule and/or resolver to enforce this change.
In `@frontend/src/lib/hooks/useStripeCheckout.ts`:
- Around line 33-65: The mutation result from createSession is ignoring GraphQL
errors because you only read data; update startCheckout to check and re-throw
GraphQL errors returned by createSession (e.g., destructure { data, errors } =
await createSession(...); if (errors && errors.length) throw errors[0];) before
reading data, or pass an override { errorPolicy: 'none' } in the createSession
call to force exceptions; ensure thrown errors are handled by your existing
catch path (used with getCheckoutErrorMessage and trackClientEvent) so real
backend errors aren't masked as "Checkout URL missing".
---
Nitpick comments:
In `@backend/src/services/stripe.ts`:
- Around line 77-82: Replace the direct environment check in the Stripe service
short-circuit with the project's config value: in backend/src/services/stripe.ts
update the condition that currently uses process.env.NODE_ENV === 'test' (the
block that returns the test sessionId/url) to use config.nodeEnv === 'test'
instead so test behavior is consistent with other code that reads nodeEnv from
the config object.
- Around line 146-151: In getCheckoutSessionStatus, before re-throwing
unexpected errors, log the error with context (include sessionId and a
descriptive message) so production has visibility; use the module's existing
logger (e.g., logger or processLogger). Keep the StripeInvalidRequestError
mapping to StripeServiceError as-is, but add a logger.error call that logs the
error object/stack and sessionId immediately before the final throw so callers
still receive the original error while you retain diagnostic details.
In `@backend/src/validation/schemas/checkout.schema.ts`:
- Around line 5-6: Add a GraphQL enum for productKey and use it in the
CheckoutInput so the schema matches the Zod union. Define an enum ProductKey
with values "coffee" and "meeting" (or uppercase variants if you follow project
conventions) and change the CheckoutInput type to use productKey: ProductKey!
instead of String!; also update any generated/exported type (the existing
productKey type) to reference the new ProductKey enum so server and clients
share the same contract as CheckoutInputSchema and productKey in the codebase.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e83984e4-39b6-4264-bda4-c69c92a6f26a
📒 Files selected for processing (17)
_docs/featureBreakdown/v3.1-stripe-payments.mdbackend/src/__tests__/unit/stripe.mutations.test.tsbackend/src/resolvers/index.tsbackend/src/resolvers/stripe/mutations.tsbackend/src/resolvers/stripe/queries.tsbackend/src/schemas/types/stripeTypes.tsbackend/src/services/stripe.tsbackend/src/validation/schemas/checkout.schema.tsbackend/src/validation/shield.tsfrontend/src/__tests__/app/PaymentSuccessPage.test.tsxfrontend/src/__tests__/components/Stripe.test.tsxfrontend/src/app/layout.tsxfrontend/src/app/payment/success/page.tsxfrontend/src/components/stripe/StripeDialog.tsxfrontend/src/lib/apollo/client.tsfrontend/src/lib/graphql/queries/server.queries.tsfrontend/src/lib/hooks/useStripeCheckout.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- backend/src/schemas/types/stripeTypes.ts
- frontend/src/app/payment/success/page.tsx
- frontend/src/app/layout.tsx
- backend/src/resolvers/stripe/mutations.ts
- frontend/src/tests/components/Stripe.test.tsx
- frontend/src/components/stripe/StripeDialog.tsx
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
frontend/src/lib/hooks/useStripeCheckout.ts (1)
71-81: Pick one error surface, not two.This hook returns
errorMessageand fires a toast, whilefrontend/src/components/stripe/StripeDialog.tsx, Lines 88-90 already render the same failure inline. Every checkout miss gets hit twice. Keep the hook on transport/analytics and let the caller decide whether the UI wants a toast, an alert, or both.♻️ Leaner split of responsibilities
-import { toast } from 'sonner'; @@ } catch (error) { const errorMessage = getCheckoutErrorMessage(error); trackClientEvent('stripe_checkout_error', { productKey, message: errorMessage, }); - toast.error(errorMessage); return { ok: false, errorMessage, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/lib/hooks/useStripeCheckout.ts` around lines 71 - 81, The hook useStripeCheckout currently both returns an errorMessage and triggers a UI toast, causing duplicate UI error surfaces; remove the UI responsibility from the hook by deleting the toast.error(errorMessage) call in the catch block while keeping trackClientEvent('stripe_checkout_error', ...) and returning { ok: false, errorMessage } so callers (e.g., StripeDialog component) control presentation; ensure the hook's return shape/type remains unchanged so existing callers can show toasts/inline errors as they prefer.frontend/src/components/stripe/StripeFab.tsx (1)
42-52: Move the ring styling out of inlinestyle.Everything else here speaks Tailwind, then this span shows up freelancing with raw CSS. Extract the ring into a utility/class so the FAB stays consistent with the component styling contract and is easier to tweak without spelunking through object literals.
As per coding guidelines, "Use TailwindCSS for styling and import shadcn/ui components from the component library".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/stripe/StripeFab.tsx` around lines 42 - 52, The span inside StripeFab.tsx is using inline style for the ring (gradient, padding, masks); extract that into a Tailwind-compatible class instead of inline CSS: create a new CSS class (e.g., .fab-ring) in your global CSS or component stylesheet that contains the background linear-gradient, padding:3px, -webkit-mask, -webkit-mask-composite and mask-composite rules, then replace the inline style on the span in StripeFab with that class (keeping the existing className values like "pointer-events-none absolute inset-0 rounded-full"); this keeps styling consistent with Tailwind/shadcn/ui usage and makes the ring easily adjustable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/__tests__/app/Home.test.tsx`:
- Around line 18-20: The test in Home.test.tsx is asserting the wrong badge
text; update the expected string used in screen.getByText to match the actual
badge in page.tsx: "Senior Software Engineer & Project Lead · Master's SWE & AI"
(ensure the apostrophe/HTML entity matches how the component renders), so the
assertion that badge is in the document uses the exact text the component
outputs.
In `@frontend/src/app/payment/success/page.tsx`:
- Around line 8-15: The metadata export (export const metadata) currently
declares a successful payment regardless of session validity; change it to a
neutral or conditional title/description so the tab doesn't claim success for
missing/invalid session_id. Update the exported metadata object (export const
metadata) to something like a neutral title/description (e.g., "Payment Status"
/ "Payment") or wire it to runtime/async logic that inspects the session in the
Page component to set a dynamic title, ensuring tab metadata reflects actual
verification state rather than always saying "Payment Successful".
---
Nitpick comments:
In `@frontend/src/components/stripe/StripeFab.tsx`:
- Around line 42-52: The span inside StripeFab.tsx is using inline style for the
ring (gradient, padding, masks); extract that into a Tailwind-compatible class
instead of inline CSS: create a new CSS class (e.g., .fab-ring) in your global
CSS or component stylesheet that contains the background linear-gradient,
padding:3px, -webkit-mask, -webkit-mask-composite and mask-composite rules, then
replace the inline style on the span in StripeFab with that class (keeping the
existing className values like "pointer-events-none absolute inset-0
rounded-full"); this keeps styling consistent with Tailwind/shadcn/ui usage and
makes the ring easily adjustable.
In `@frontend/src/lib/hooks/useStripeCheckout.ts`:
- Around line 71-81: The hook useStripeCheckout currently both returns an
errorMessage and triggers a UI toast, causing duplicate UI error surfaces;
remove the UI responsibility from the hook by deleting the
toast.error(errorMessage) call in the catch block while keeping
trackClientEvent('stripe_checkout_error', ...) and returning { ok: false,
errorMessage } so callers (e.g., StripeDialog component) control presentation;
ensure the hook's return shape/type remains unchanged so existing callers can
show toasts/inline errors as they prefer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 26b43965-15eb-4c20-a7c2-cb48602bfcf1
📒 Files selected for processing (15)
backend/src/__tests__/unit/stripe.mutations.test.tsbackend/src/resolvers/stripe/queries.tsbackend/src/schemas/types/stripeTypes.tsbackend/src/services/stripe.tsbackend/src/validation/schemas/checkout.schema.tsbackend/src/validation/shield.tsfrontend/src/__tests__/app/Home.test.tsxfrontend/src/__tests__/app/PaymentSuccessPage.test.tsxfrontend/src/__tests__/components/Stripe.test.tsxfrontend/src/app/payment/success/page.tsxfrontend/src/components/stripe/StripeDialog.tsxfrontend/src/components/stripe/StripeFab.tsxfrontend/src/lib/apollo/client.tsfrontend/src/lib/graphql/queries/server.queries.tsfrontend/src/lib/hooks/useStripeCheckout.ts
🚧 Files skipped from review as they are similar to previous changes (9)
- backend/src/tests/unit/stripe.mutations.test.ts
- backend/src/schemas/types/stripeTypes.ts
- frontend/src/tests/components/Stripe.test.tsx
- backend/src/validation/shield.ts
- frontend/src/tests/app/PaymentSuccessPage.test.tsx
- backend/src/validation/schemas/checkout.schema.ts
- backend/src/services/stripe.ts
- backend/src/resolvers/stripe/queries.ts
- frontend/src/lib/apollo/client.ts
fix(config): remove deprecated eslint block from next.config.ts fix(config): add turbopack.root to silence workspace root warning docs: add .github/copilot-instructions.md fix(stripe): fix StripeFab dialog by using Button as direct TooltipTrigger asChild target fix(stripe): add pointer-events-none to decorative ring span in StripeFab feat(stripe): replace HandCoins icon with Coffee icon in StripeFab fix(stripe): handle Stripe SDK errors (StripePermissionError) in createCheckoutSession fix(stripe): log service errors in createErrorHandler and on checkout entry fix(stripe): add startup warning when STRIPE_SECRET_KEY is missing fix(payment): update cancel page Try Again link to /#support Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…and tar vulns - Fix apodResolvers.test.ts: jest.mock() factory was referencing const variables in TDZ (hoisting issue). Changed to inline jest.fn() calls and pulled mock refs via require() after mock registration. - Add overrides in frontend/package.json: immutable@3.8.3 (prototype pollution, high) and tar@7.5.11 (path traversal, high) to resolve two Dependabot alerts. @tootallnate/once (low) requires jest major bump — deferred. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@_docs/releaseNotes/v.2.92.1_Security-Serialize-JS.md`:
- Around line 10-12: The release note currently claims "zero runtime behavior
changes" for serialize-javascript v7.0.3 which is misleading because v7 requires
Node.js v20+; update the wording around the serialize-javascript mention
(references to "serialize-javascript" and "v7.0.3") in the release note
paragraph (lines describing the CVE patch and build verification) and the later
summary (the line at ~53) to explicitly say: "v7.0.3 is a security patch; it
requires Node.js v20+, which this repo already enforces. Build and tests
verified under this configuration." Ensure the change replaces the current "zero
API changes"/"pure patch" phrasing and retains the notes that builds/tests pass.
In @.github/copilot-instructions.md:
- Around line 99-102: Update the environment variables documentation and
backend/.env.example to include the missing vars so contributors can run the
app: add FRONTEND_URL to the backend list and add Stripe-related vars
(STRIPE_SECRET_KEY, STRIPE_WEBHOOK_SECRET, and any STRIPE_PRICE_ID or
STRIPE_PRICE_<NAME> entries used by the code) and ensure the frontend/.env.local
docs note any Stripe public keys or URLs if required; reference these exact
variable names (FRONTEND_URL, STRIPE_SECRET_KEY, STRIPE_WEBHOOK_SECRET,
STRIPE_PRICE_ID/STRIPE_PRICE_<NAME>) when updating the docs and examples.
In `@backend/.env.example`:
- Around line 37-40: The .env example is missing the STRIPE_WEBHOOK_SECRET
variable that backend/src/config/config.ts expects; update backend/.env.example
to include a STRIPE_WEBHOOK_SECRET entry with a placeholder value (e.g.
STRIPE_WEBHOOK_SECRET=whsec_xxx) so onboarding/setup includes the webhook secret
required by the config loader and Stripe webhook verification.
In `@backend/src/services/stripe.ts`:
- Around line 177-182: The catch block incorrectly treats every
Stripe.errors.StripeInvalidRequestError as a missing session; update the
handling in the catch that surrounds the checkout session retrieval so it only
converts the error to a StripeServiceError('SESSION_NOT_FOUND', ..., 404, {
sessionId }) when error.code === 'resource_missing' (or equivalent Stripe
resource-missing indicator), and for all other StripeInvalidRequestError cases
log the full error (via logger.error) with context (sessionId) and rethrow or
propagate the original error so it remains identifiable; reference
Stripe.errors.StripeInvalidRequestError, StripeServiceError, logger.error and
sessionId to locate and change the conditional branching accordingly.
In `@backend/src/utils/errors/graphqlErrors.ts`:
- Around line 113-118: The logger.error call in graphqlErrors.ts currently
includes error.details (see logger.error(`Service error in ${operationName}`, {
code: error.code, message: error.message, statusCode: error.statusCode, details:
error.details })), which may persist sensitive, unknown payloads; remove or
sanitize that field before logging. Replace the direct details field with a
sanitizedDetails value produced by whitelisting allowed keys (e.g., pick known
safe keys) or omit it entirely; implement this via a small helper (e.g.,
sanitizeErrorDetails) or inline logic that checks typeof error.details ===
'object' and selects only approved properties, then pass sanitizedDetails to
logger.error instead of error.details.
In `@frontend/src/app/page.tsx`:
- Line 18: Your frontend structured-data field personLd.jobTitle (in
frontend/src/app/page.tsx) and the backend profile entry "Software Engineer /
Project Lead" in backend/src/data/luis-profile.json are inconsistent; pick the
canonical title and make both sources match. Either update
backend/src/data/luis-profile.json (replace "Software Engineer / Project Lead"
with "Senior Software Engineer") or update frontend personLd.jobTitle to include
"Software Engineer / Project Lead", then verify any consumers (SEO component
that emits personLd and any API that reads luis-profile.json) still render the
same value and run the app/tests to confirm consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 893a7a8a-e7ad-4e4b-b9d9-2ac63933ce74
📒 Files selected for processing (14)
.github/copilot-instructions.md_docs/featureBreakdown/v2.91-seo-monitor-agent.md_docs/featureBreakdown/v2.92-seo-takeover.md_docs/releaseNotes/v.2.92.1_Security-Serialize-JS.mdbackend/.env.examplebackend/src/resolvers/stripe/mutations.tsbackend/src/services/stripe.tsbackend/src/utils/errors/graphqlErrors.tsfrontend/next.config.tsfrontend/src/__tests__/app/Home.test.tsxfrontend/src/app/metadata.tsfrontend/src/app/page.tsxfrontend/src/app/payment/cancel/page.tsxfrontend/src/components/stripe/StripeFab.tsx
💤 Files with no reviewable changes (2)
- _docs/featureBreakdown/v2.91-seo-monitor-agent.md
- _docs/featureBreakdown/v2.92-seo-takeover.md
✅ Files skipped from review due to trivial changes (1)
- frontend/src/app/metadata.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/components/stripe/StripeFab.tsx
- frontend/src/app/payment/cancel/page.tsx
- backend/src/resolvers/stripe/mutations.ts
| ## Environment Variables | ||
| Backend (`backend/.env`): `MONGODB_URI`, `REDIS_URL`, `JWT_SECRET`, `OPENAI_API_KEY`, `RESEND_API_KEY`, `NASA_API_KEY`, `DISCORD_WEBHOOK_URL` — see `backend/.env.example` | ||
|
|
||
| Frontend (`frontend/.env.local`): `NEXT_PUBLIC_GRAPHQL_URL` (defaults to `http://localhost:4000/graphql`) |
There was a problem hiding this comment.
The env docs are stale on day one.
The backend env list omits FRONTEND_URL and the Stripe vars this PR depends on (STRIPE_SECRET_KEY, price IDs, and likely STRIPE_WEBHOOK_SECRET). That leaves contributors and agents under-configured before they even hit “run”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/copilot-instructions.md around lines 99 - 102, Update the
environment variables documentation and backend/.env.example to include the
missing vars so contributors can run the app: add FRONTEND_URL to the backend
list and add Stripe-related vars (STRIPE_SECRET_KEY, STRIPE_WEBHOOK_SECRET, and
any STRIPE_PRICE_ID or STRIPE_PRICE_<NAME> entries used by the code) and ensure
the frontend/.env.local docs note any Stripe public keys or URLs if required;
reference these exact variable names (FRONTEND_URL, STRIPE_SECRET_KEY,
STRIPE_WEBHOOK_SECRET, STRIPE_PRICE_ID/STRIPE_PRICE_<NAME>) when updating the
docs and examples.
| logger.error(`Service error in ${operationName}`, { | ||
| code: error.code, | ||
| message: error.message, | ||
| statusCode: error.statusCode, | ||
| details: error.details, | ||
| }); |
There was a problem hiding this comment.
Don’t spray error.details into persistent logs.
Line 117 is a little data cannon. details is unknown, and current Stripe call sites already put sessionId and productKey in there. backend/src/utils/logger.ts persists metadata verbatim to JSON file transports, so this change turns service errors into durable leakage. Log a redacted/whitelisted payload here, or drop details entirely from the logger call.
Suggested hardening
if (isServiceError(error)) {
logger.error(`Service error in ${operationName}`, {
code: error.code,
message: error.message,
statusCode: error.statusCode,
- details: error.details,
+ hasDetails: error.details !== undefined,
});
throw createGraphQLError(error.message, {
code: mapErrorCode(error.code),
statusCode: error.statusCode,
details: error.details,📝 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.
| logger.error(`Service error in ${operationName}`, { | |
| code: error.code, | |
| message: error.message, | |
| statusCode: error.statusCode, | |
| details: error.details, | |
| }); | |
| logger.error(`Service error in ${operationName}`, { | |
| code: error.code, | |
| message: error.message, | |
| statusCode: error.statusCode, | |
| hasDetails: error.details !== undefined, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/src/utils/errors/graphqlErrors.ts` around lines 113 - 118, The
logger.error call in graphqlErrors.ts currently includes error.details (see
logger.error(`Service error in ${operationName}`, { code: error.code, message:
error.message, statusCode: error.statusCode, details: error.details })), which
may persist sensitive, unknown payloads; remove or sanitize that field before
logging. Replace the direct details field with a sanitizedDetails value produced
by whitelisting allowed keys (e.g., pick known safe keys) or omit it entirely;
implement this via a small helper (e.g., sanitizeErrorDetails) or inline logic
that checks typeof error.details === 'object' and selects only approved
properties, then pass sanitizedDetails to logger.error instead of error.details.
Implements #147
Implements #148
Implements #149
Implements #150
Implements #151
Implements #152
Implements #153
Implements #154
Summary by CodeRabbit
New Features
Documentation
Tests