feat(subscriptions): POST /api/subscriptions/card-on-file ($0 card registration) - #797
Conversation
…gistration)
Exposes the existing createCardOnFileSession helper over HTTP so a
browser can start a Stripe `mode: "setup"` checkout that saves a card
for the authenticated account with no charge and no subscription. Until
now the helper had no route: its only caller was server-internal
(ensureSongstatsPaymentMethod), so nothing a browser could hit.
Mirrors POST /api/subscriptions/sessions layer for layer
(route -> handler -> validate), reusing its strict `{ successUrl }` body
schema, its auth-error mapper, and its flat `{ id, url }` envelope.
- app/api/subscriptions/card-on-file/route.ts - POST plus the CORS
OPTIONS handler so recoupable.dev can call it cross-origin
- lib/stripe/createCardOnFileSessionHandler.ts - validate, mint, 400 on a
missing session URL, 500 on a Stripe failure
- lib/stripe/validateCreateCardOnFileSessionRequest.ts - zod body check
then validateAuthContext; the account always comes from the
credentials and an `accountId` in the body 400s (strict schema)
13 tests: happy path, 400 invalid JSON / missing successUrl / non-URL
successUrl / caller-supplied accountId, 401 with no credentials, 401 with
an invalid bearer, 500 on a Stripe throw, and OPTIONS.
Part of recoupable/chat#1910
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughA new card-on-file subscription API flow validates request JSON and authentication, creates a Stripe setup session, returns session details or structured errors, supports CORS preflight requests, and disables route caching. ChangesCard-on-file session flow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant CardOnFileRoute
participant createCardOnFileSessionHandler
participant Stripe
Client->>CardOnFileRoute: POST card-on-file request
CardOnFileRoute->>createCardOnFileSessionHandler: Forward NextRequest
createCardOnFileSessionHandler->>Stripe: Create setup session
Stripe-->>createCardOnFileSessionHandler: Return session id and url
createCardOnFileSessionHandler-->>Client: Return JSON session details
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
lib/stripe/validateCreateCardOnFileSessionRequest.ts (1)
22-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSplit this validator below the 20-line limit.
Extract JSON/schema error response creation into a private helper; this also removes repeated CORS response setup.
As per coding guidelines, “Flag functions longer than 20 lines” and “Use constants for repeated values.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/stripe/validateCreateCardOnFileSessionRequest.ts` around lines 22 - 50, The validateCreateCardOnFileSessionRequest function exceeds the 20-line limit and repeats CORS response construction. Extract the invalid-JSON and schema-validation error response creation into a private helper, reuse it for both failure paths, and preserve the existing 400 status and error messages.Source: Coding guidelines
lib/stripe/createCardOnFileSessionHandler.ts (2)
13-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReduce handler size and centralize response construction.
This function is 27 lines and repeats
NextResponse.json(..., { headers: getCorsHeaders() }). Extract a small response helper and keep the orchestration method focused.As per coding guidelines, “Flag functions longer than 20 lines” and “Keep functions small and focused.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/stripe/createCardOnFileSessionHandler.ts` around lines 13 - 39, Reduce createCardOnFileSessionHandler by extracting a local or module-level response-construction helper that consistently applies getCorsHeaders() and accepts the payload and status. Replace the repeated NextResponse.json calls with this helper while preserving the existing validation, missing-URL, success, and error response behavior.Source: Coding guidelines
3-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInject the session-creation dependency.
The API flow hard-wires
createCardOnFileSessioninto this handler. Accept it as a dependency (or use a handler factory) and compose the concrete Stripe implementation in the route layer.As per path instructions, “Use dependency injection for services.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/stripe/createCardOnFileSessionHandler.ts` around lines 3 - 4, Update the handler using validateCreateCardOnFileSessionRequest so it receives the session-creation service through dependency injection instead of importing and hard-coding createCardOnFileSession. Expose a handler factory or equivalent dependency parameter, then compose the concrete createCardOnFileSession implementation in the route layer while preserving the existing request flow.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/stripe/createCardOnFileSessionHandler.ts`:
- Around line 21-25: Update the missing-session-URL branch in the
createCardOnFileSessionHandler flow to return a server-side failure status,
using the existing NextResponse JSON structure and CORS headers; change the
current 400 status to the project’s chosen Stripe upstream status (500 or 502)
while preserving the error message.
---
Nitpick comments:
In `@lib/stripe/createCardOnFileSessionHandler.ts`:
- Around line 13-39: Reduce createCardOnFileSessionHandler by extracting a local
or module-level response-construction helper that consistently applies
getCorsHeaders() and accepts the payload and status. Replace the repeated
NextResponse.json calls with this helper while preserving the existing
validation, missing-URL, success, and error response behavior.
- Around line 3-4: Update the handler using
validateCreateCardOnFileSessionRequest so it receives the session-creation
service through dependency injection instead of importing and hard-coding
createCardOnFileSession. Expose a handler factory or equivalent dependency
parameter, then compose the concrete createCardOnFileSession implementation in
the route layer while preserving the existing request flow.
In `@lib/stripe/validateCreateCardOnFileSessionRequest.ts`:
- Around line 22-50: The validateCreateCardOnFileSessionRequest function exceeds
the 20-line limit and repeats CORS response construction. Extract the
invalid-JSON and schema-validation error response creation into a private
helper, reuse it for both failure paths, and preserve the existing 400 status
and error messages.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fd44f3b1-b5b3-46be-ab67-be784010549a
⛔ Files ignored due to path filters (5)
app/api/subscriptions/card-on-file/__tests__/route.options.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included byapp/**app/api/subscriptions/card-on-file/__tests__/route.post.outcomes.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included byapp/**app/api/subscriptions/card-on-file/__tests__/route.post.validation.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included byapp/**app/api/subscriptions/card-on-file/__tests__/route.test.tsis excluded by!**/*.test.*,!**/__tests__/**and included byapp/**app/api/subscriptions/card-on-file/__tests__/routeTestMocks.tsis excluded by!**/__tests__/**and included byapp/**
📒 Files selected for processing (3)
app/api/subscriptions/card-on-file/route.tslib/stripe/createCardOnFileSessionHandler.tslib/stripe/validateCreateCardOnFileSessionRequest.ts
Preview verificationPreview: All probes are against
Row 9: could not mint credentials headless
So the 200 path is covered by unit tests only ( Suite
|
A card-on-file request that already passed validation and auth cannot be corrected by the caller, so a session missing its URL is an upstream failure, not a bad request. Deliberate divergence from the sibling subscription route, which returns 400 for the same condition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Confidence score: 5/5
- In
lib/stripe/createCardOnFileSessionHandler.ts, duplicating near-identical checkout-session handling across two handlers creates the main risk: future edits to error mapping, CORS, or response shape can drift and cause inconsistent client behavior between endpoints—extract the shared validate/create/respond flow and pass only handler-specific config.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="lib/stripe/createCardOnFileSessionHandler.ts">
<violation number="1" location="lib/stripe/createCardOnFileSessionHandler.ts:13">
P3: Checkout-session response handling now exists in two near-identical handlers, so changes to error mapping, CORS, or response shape can silently diverge. Extract the shared validate/create/respond flow and supply the card-on-file-specific validator and session factory.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| * @param request - The incoming HTTP request. | ||
| * @returns A NextResponse with session `id` and `url`, or an error body. | ||
| */ | ||
| export async function createCardOnFileSessionHandler(request: NextRequest): Promise<NextResponse> { |
There was a problem hiding this comment.
P3: Checkout-session response handling now exists in two near-identical handlers, so changes to error mapping, CORS, or response shape can silently diverge. Extract the shared validate/create/respond flow and supply the card-on-file-specific validator and session factory.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/stripe/createCardOnFileSessionHandler.ts, line 13:
<comment>Checkout-session response handling now exists in two near-identical handlers, so changes to error mapping, CORS, or response shape can silently diverge. Extract the shared validate/create/respond flow and supply the card-on-file-specific validator and session factory.</comment>
<file context>
@@ -0,0 +1,39 @@
+ * @param request - The incoming HTTP request.
+ * @returns A NextResponse with session `id` and `url`, or an error body.
+ */
+export async function createCardOnFileSessionHandler(request: NextRequest): Promise<NextResponse> {
+ try {
+ const validated = await validateCreateCardOnFileSessionRequest(request);
</file context>
Preview verification (re-run on the final commit)Preview:
Row 9 remains blocked for the reasons in the earlier comment: a Privy bearer is not mintable headless, and the prod |
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Confidence score: 4/5
- In
app/api/subscriptions/card-on-file/__tests__/route.post.outcomes.test.ts, the documented behavior (“400 for session without a URL”) conflicts withcreateCardOnFileSessionHandler.tsreturning 500, which creates regression risk in API contracts and can mislead downstream clients and future maintenance—align the test/PR description with the handler’s intended status code (or update the handler if 400 is the real requirement).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/api/subscriptions/card-on-file/__tests__/route.post.outcomes.test.ts">
<violation number="1" location="app/api/subscriptions/card-on-file/__tests__/route.post.outcomes.test.ts:56">
P2: Custom agent: **Flag AI Slop and Fabricated Changes**
The PR description claims a 400 error for 'session without a URL', but the handler (`createCardOnFileSessionHandler.ts`) returns 500 for that case with a deliberate comment explaining the choice: 'The caller cannot correct a session Stripe returned without a URL, so this is a 500 rather than the sibling subscription route's 400.' Update the PR description to say 500 instead of 400 for the null-session-URL error case so it accurately reflects the implementation.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| expect(createCardOnFileSession).toHaveBeenCalledWith(ACCOUNT, SUCCESS_URL); | ||
| }); | ||
|
|
||
| it("returns 500 when session.url is null", async () => { |
There was a problem hiding this comment.
P2: Custom agent: Flag AI Slop and Fabricated Changes
The PR description claims a 400 error for 'session without a URL', but the handler (createCardOnFileSessionHandler.ts) returns 500 for that case with a deliberate comment explaining the choice: 'The caller cannot correct a session Stripe returned without a URL, so this is a 500 rather than the sibling subscription route's 400.' Update the PR description to say 500 instead of 400 for the null-session-URL error case so it accurately reflects the implementation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/api/subscriptions/card-on-file/__tests__/route.post.outcomes.test.ts, line 56:
<comment>The PR description claims a 400 error for 'session without a URL', but the handler (`createCardOnFileSessionHandler.ts`) returns 500 for that case with a deliberate comment explaining the choice: 'The caller cannot correct a session Stripe returned without a URL, so this is a 500 rather than the sibling subscription route's 400.' Update the PR description to say 500 instead of 400 for the null-session-URL error case so it accurately reflects the implementation.</comment>
<file context>
@@ -53,7 +53,7 @@ describe("POST /api/subscriptions/card-on-file (handler outcomes)", () => {
});
- it("returns 400 when session.url is null", async () => {
+ it("returns 500 when session.url is null", async () => {
vi.mocked(validateCreateCardOnFileSessionRequest).mockResolvedValue({
accountId: ACCOUNT,
</file context>
Preview verification — authenticated 200 path now coveredPreview: The earlier pass could not exercise the authenticated 200 because a prod API key 401s against previews (previews salt key hashes differently). Solved by minting a key on the preview itself via the documented agent-onboarding flow, which is unauthenticated by design: curl -X POST "$PREVIEW/api/agents/signup" -H "Content-Type: application/json" \
-d '{"email": "agent+'$(date +%s)-$RANDOM'@recoupable.com"}'
# -> { account_id, api_key } (agent+ addresses get a key immediately, no email round-trip)That key returned Results — documented vs actual
9/9. The 4xx cases were re-run with a valid key, so they now prove body validation rather than just failing at the auth gate as in the previous pass. The session is genuinely setup mode, not a paymentLoaded the returned URL. Stripe renders a card-setup screen, materially different from the Pro subscription checkout:
Asserted programmatically on the rendered page:
Stripe-side state
Disclosure and caveats
Contract, docs and live behavior agree. Docs are already merged (docs#280, Part of recoupable/chat#1910. |
| try { | ||
| body = await request.json(); | ||
| } catch { | ||
| return NextResponse.json( | ||
| { error: "Invalid JSON body" }, | ||
| { status: 400, headers: getCorsHeaders() }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
DRY - use the shared body parsing lib.
There was a problem hiding this comment.
Fixed in c11e4c7: now createSubscriptionSessionBodySchema.safeParse(await safeParseJson(request)), dropping the hand-rolled try/catch. lib/networking/safeParseJson is the right shared helper here (33 other validators use it).
One behavior change worth knowing about, since it is observable: safeParseJson returns {} for an unparseable body rather than signalling the parse failure, so an invalid body no longer gets a JSON-specific message. It now lands on the same missing-field 400 as an empty body:
| Input | Before | After |
|---|---|---|
not json |
400 {"error":"Invalid JSON body"} |
400 {"error":"Invalid input: expected string, received undefined"} |
{} |
400 {"error":"Invalid input: expected string, received undefined"} |
unchanged |
Still 400 either way, so the documented contract holds and the docs never pinned that string. I updated the validation test to the new message and left a comment in the validator explaining why the two inputs converge, so nobody "fixes" it back.
Re-verified on the preview for c11e4c79 (deployment confirmed built from that sha) with a freshly minted preview agent key: invalid JSON, missing successUrl, and a genuinely empty body all return the same 400; successUrl must be a valid URL, Unrecognized key: "accountId", the 401s, and the authenticated 200 { id, url } are all unchanged. Full domain suite 231/231, tsc and lint clean.
Review feedback: drop the hand-rolled try/catch around request.json() in favor
of lib/networking/safeParseJson, the shared helper used by 33 other validators.
safeParseJson returns {} for an absent or unparseable body, so an unparseable
body now surfaces the same missing-field 400 as an empty one instead of a
JSON-specific message. Still 400 either way, so the documented contract holds;
the validation test was updated to the new message.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Re-verified after the safeParseJson refactor. See review thread. |
There was a problem hiding this comment.
0 issues found across 2 files (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Requires human review: Auto-approval blocked by 2 unresolved issues from previous reviews.
Re-trigger cubic
1 participant
Exposes the existing
lib/stripe/createCardOnFileSession.tshelper over HTTP so a browser can start a Stripemode: "setup"checkout that saves a card for the authenticated account with no charge today and no subscription. Until now the helper had no route: its only caller was server-internal (ensureSongstatsPaymentMethod), so nothing a browser could hit. The saved card is what lets a later credit shortfall auto-recharge throughautoRechargeOrFailinstead of dead-ending.No new payment logic. The Stripe call is unchanged.
Contract
POST /api/subscriptions/card-on-filex-api-key, viavalidateAuthContext{ "successUrl": "https://..." }— zod, strict, must be a valid URL{ "id": "cs_...", "url": "https://checkout.stripe.com/..." }(flat, nodatawrapper)successUrl, non-URLsuccessUrl, or an unknown body keyOPTIONSreturnsgetCorsHeaders()sorecoupable.devcan call it cross-originThe account is always resolved from the credentials.
accountIdis never accepted from the caller — the body schema is strict, so sending one is a 400.Layering
Mirrors
POST /api/subscriptions/sessionslayer for layer, reusing its strict{ successUrl }body schema, its auth-error mapper, and its flat response envelope.app/api/subscriptions/card-on-file/route.ts— POST plus the CORSOPTIONShandlerlib/stripe/createCardOnFileSessionHandler.ts— validate, mint, 500 on a missing session URL or a Stripe failurelib/stripe/validateCreateCardOnFileSessionRequest.ts— zod body check, thenvalidateAuthContextOne deliberate divergence from the sibling: a session Stripe returns without a URL is a 500 here, not a 400. The request already passed validation and auth, so the caller cannot correct it.
Tests
TDD, red then green. 13 new tests in
app/api/subscriptions/card-on-file/__tests__/: happy path (200, andcreateCardOnFileSessioncalled with the credential-derived account id), 400 on invalid JSON / missingsuccessUrl/ non-URLsuccessUrl/ caller-suppliedaccountId, 401 with no credentials, 401 with an invalid bearer, 500 on a Stripe throw, 500 on a session with no URL, andOPTIONS.Full domain suite green:
pnpm exec vitest run lib/stripe app/api/subscriptions→ 61 files, 231 tests passed.pnpm exec tsc --noEmitreports zero errors in the changed files (pre-existing unrelatedlib/triggerbaseline noise only).pnpm lintclean.Preview verification (documented vs actual, every reachable status) is in a PR comment below.
Docs
Documentation-driven: the contract above is published in recoupable/docs#280.
Merge order: recoupable/docs#280 first, then this PR.
Follow-up (not this PR)
The free-CTA copy decision in the issue (Option A vs Option B) is a marketing-side call. This route is required either way and is deliberately neutral about which CTA calls it.
Part of recoupable/chat#1910
🤖 Generated with Claude Code