Skip to content

API - api/spotify/album - migrate to vercel - #24

Merged
sweetmantech merged 1 commit into
mainfrom
sweetmantech/myc-3608-api-apispotifyalbum-migrate-to-vercel
Dec 4, 2025
Merged

API - api/spotify/album - migrate to vercel#24
sweetmantech merged 1 commit into
mainfrom
sweetmantech/myc-3608-api-apispotifyalbum-migrate-to-vercel

Conversation

@sweetmantech

@sweetmantechsweetmantech commented Dec 4, 2025

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features
    • Added a new Spotify album API endpoint that enables retrieval of complete album information by ID, with optional market-specific data filtering for localized results. The endpoint includes automatic Spotify service authentication, comprehensive input validation, detailed error handling with appropriate HTTP status codes, and full CORS support to facilitate secure cross-origin requests from web and mobile clients.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel

vercelBot commented Dec 4, 2025

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

ProjectDeploymentPreviewUpdated (UTC)
recoup-apiReadyReadyPreviewDec 4, 2025 6:29am

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@coderabbitai

coderabbitaiBot commented Dec 4, 2025

Copy link
Copy Markdown

Walkthrough

A new Spotify album API endpoint is implemented with GET and OPTIONS handlers, including query validation for album ID and optional market parameters, access token generation, and integration with the Spotify API wrapper for error handling.

Changes

Cohort / File(s)Summary
Spotify Album API Route
app/api/spotify/album/route.ts
Implements GET and OPTIONS handlers to serve Spotify album requests, delegating to getSpotifyAlbumHandler and managing CORS preflight responses.
Spotify Album Request Handler
lib/spotify/getSpotifyAlbumHandler.ts
Processes Spotify album requests by validating query parameters, generating access tokens, calling getAlbum, and returning responses with appropriate HTTP status codes and CORS headers.
Spotify Album Client
lib/spotify/getAlbum.ts
Fetches album data from Spotify API by ID with optional market parameter, handling authentication via Bearer token and returning normalized response with album data or error.
Query Parameter Validation
lib/spotify/validateSpotifyAlbumQuery.ts
Defines Zod schema for album query parameters (required id, optional market), validates URLSearchParams, and returns either validated data or a 400 error response with CORS headers.

Sequence Diagram

sequenceDiagram
participant Client
participant Route as API Route<br/>(route.ts)
participant Handler as Handler<br/>(getSpotifyAlbumHandler)
participant Validator as Validator<br/>(validateSpotifyAlbumQuery)
participant TokenGen as Token Generator<br/>(generateAccessToken)
participant SpotifyClient as Spotify Client<br/>(getAlbum)
participant SpotifyAPI as Spotify API
Client->>Route: GET /api/spotify/album?id=...&market=...
Route->>Handler: forward request
Handler->>Validator: validate query params
Validator->>Validator: parse & schema check
alt Validation fails
Validator-->>Handler: 400 error response
Handler-->>Route: return error
Route-->>Client: 400 error
else Validation succeeds
Validator-->>Handler: validated params
Handler->>TokenGen: generate access token
alt Token generation fails
TokenGen-->>Handler: error
Handler-->>Route: 500 error response
Route-->>Client: 500 error
else Token generated
TokenGen-->>Handler: access token
Handler->>SpotifyClient: getAlbum(id, market, token)
SpotifyClient->>SpotifyAPI: GET /v1/albums/{id}
alt API request fails
SpotifyAPI-->>SpotifyClient: error response
SpotifyClient-->>Handler: { album: null, error }
Handler-->>Route: 502 error response
Route-->>Client: 502 error
else API request succeeds
SpotifyAPI-->>SpotifyClient: album data
SpotifyClient-->>Handler: { album, error: null }
Handler-->>Route: 200 success response
Route-->>Client: album data + CORS headers
end
end
end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

  • Error handling consistency: Verify error handling is uniform across validation, token generation, and Spotify API call stages
  • CORS header implementation: Confirm getCorsHeaders is correctly applied to all response types (success, 400, 500, 502)
  • Access token expiration: Validate that token generation and caching strategy (if any) is appropriate
  • API client robustness: Check that getAlbum properly handles network errors, malformed responses, and rate limiting scenarios

Poem

🐰 A Spotify burrow we've built with delight,
Albums now fetch through API so tight!
Validation and tokens, all in their place,
CORS headers grace every response with grace.
Hop-hop, the endpoint now ready to play! 🎵

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check nameStatusExplanation
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
Title check✅ PassedThe title 'API - api/spotify/album - migrate to vercel' accurately describes the main change: adding a new Spotify album API route with CORS handlers and validation logic.
Docstring Coverage✅ PassedDocstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch sweetmantech/myc-3608-api-apispotifyalbum-migrate-to-vercel

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sweetmantech
sweetmantech merged commit 65310af into mainDec 4, 2025
2 of 3 checks passed

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (4)
app/api/spotify/album/route.ts (1)

5-15: Route wiring and CORS handling look good; consider a small export simplification

OPTIONS correctly returns a 200 with shared CORS headers, and GET cleanly delegates to getSpotifyAlbumHandler, matching the pattern you likely use elsewhere. As an optional cleanup, you could re-export the handler directly:

export{getSpotifyAlbumHandlerasGET}from"@/lib/spotify/getSpotifyAlbumHandler";

This keeps the route file even thinner, but your current implementation is perfectly fine if you prefer explicit functions here.

Also applies to: 17-31

lib/spotify/getSpotifyAlbumHandler.ts (1)

57-66: Consider wrapping album data instead of spreading into the root

Right now the success payload flattens the Spotify album object into the top-level response next to status. That can make it harder to evolve the API later (e.g., adding your own metadata) without risking key collisions with Spotify’s fields. Consider returning something like:

returnNextResponse.json({status: "success",
album,},{status: 200,headers: getCorsHeaders()},);

This is a non-breaking internal tweak if consumers are new, but it’s fine to keep as-is if you’re matching an existing contract.

lib/spotify/getAlbum.ts (1)

1-35: Album fetch logic is correct; consider enriching error information

The URL construction, optional market handling, and Bearer auth are all correct, and the { album, error } contract is straightforward. As an incremental improvement, you might capture more context on failures:

if(!response.ok){constbody=awaitresponse.text().catch(()=>"");return{album: null,error: newError(`Spotify API request failed (${response.status}): ${body}`),};}

Optionally, you could also give the function an explicit return type that’s shared with the handler to keep typing consistent across the module boundary.

lib/spotify/validateSpotifyAlbumQuery.ts (1)

5-8: Validation is sound; response shape naming could be clearer

The Zod schema and safeParse usage over URLSearchParams look good, and returning a 400 NextResponse with CORS on failure cleanly isolates validation concerns. One small nit: the field name missing_fields may be misleading when the error is due to an invalid value rather than an absent param. If you don’t have consumers depending on this shape yet, something like invalid_fields (or a more general fields) would better reflect all Zod issues.

If you later need stricter market validation, you can easily extend the schema with a regex for 2‑letter country codes without changing the rest of this flow.

Also applies to: 18-37

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d25bc6f and d2c177f.

📒 Files selected for processing (4)
  • app/api/spotify/album/route.ts (1 hunks)
  • lib/spotify/getAlbum.ts (1 hunks)
  • lib/spotify/getSpotifyAlbumHandler.ts (1 hunks)
  • lib/spotify/validateSpotifyAlbumQuery.ts (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/spotify/validateSpotifyAlbumQuery.ts (1)
lib/networking/getCorsHeaders.ts (1)
  • getCorsHeaders (6-12)
🔇 Additional comments (1)
lib/spotify/getSpotifyAlbumHandler.ts (1)

17-38: Control flow and error mapping look solid

The handler’s flow (validate → get token → call Spotify → map to 500/502/200 with CORS) is clear and defensive, and returning the validator’s NextResponse early keeps concerns nicely separated. Nothing blocking here from a correctness standpoint.

Also applies to: 40-55, 67-78

sweetmantech added a commit that referenced this pull request May 15, 2026
Phase 2 of the credits / usage unification on the api side. Every
credit deduction now writes both the credits_usage wallet AND a
usage_events row carrying the wallet impact and any available token
detail — making credits_usage.remaining_credits derivable from
(top-ups + refills + grants) − SUM(credits_deducted_cents) and
enabling per-customer credit-usage rollups on the admin dashboard.
- lib/supabase/usage_events/insertUsageEvent.ts: new Supabase wrapper
with server-side nanoid id generation (matches open-agents' shape).
- lib/credits/recordCreditDeduction.ts: wallet + meter wrapper that
calls deductCredits and then insertUsageEvent. Insert failures are
logged but not surfaced — the wallet stays authoritative and a
reconciliation job can recover any missing audit row.
- Swap 4 call sites from deductCredits → recordCreditDeduction:
- handleChatCredits.ts: source='web', forwards model + tokens
- postResearchWebHandler.ts / handleResearch.ts /
handleArtistResearch.ts: source='api', no token detail
- x402 (lib/x402/fetchWithPayment.ts) is intentionally out of scope
for this phase and continues to update credits_usage only.
- types/database.types.ts: regenerated to include usage_events with
the credits_deducted_cents column from database PR #24.
- package.json: add `update-types` script (mirrors chat's convention)
and `nanoid` dependency for usage_events.id generation.
Caller tests updated to mock recordCreditDeduction instead of
deductCredits and assert the new call shape.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sweetmantech added a commit that referenced this pull request May 15, 2026
* feat(credits): just-in-time auto-recharge with 402 short-circuit (#566)
* feat(credits): just-in-time auto-recharge with 402 short-circuit
When a credit-gated request would push the account below its cost, attempt
a silent $5 off-session top-up against the saved card. If that succeeds,
increment the balance in-thread (the webhook ignores PIs stamped
`purpose: "credits_auto_recharge"`) and proceed. Otherwise return HTTP 402
with a unified body the open-agents credits dialog already parses.
Helpers (all under 100 LOC each):
- autoRechargeOrFail: returns `{ kind: "available" }` or
`{ kind: "insufficient_credits", checkoutUrl, declineReason? }`.
- ensureCreditsOrShortCircuit: wraps the above and returns
`NextResponse | null` — handlers do `if (short) return short`.
- buildInsufficientCreditsResponse: shared 402 body shape.
Wiring:
- `lib/research/handleResearch` + `handleArtistResearch` (covers ~26 GET
endpoints) gate up-front and deduct on upstream success.
- 5 special POST handlers (`postResearchWeb/People/Extract/Enrich/Deep`)
gate up-front with their per-endpoint cost.
- `lib/chat/handleChatStream` preflights with `creditsToDeduct: 1` before
the stream starts; `handleChatCredits` continues to deduct actual usage
post-hoc.
402 body shape (mirrors PR #561's Checkout response shape so open-agents
PR #36 needs no UI changes to render the right view):
```json
{ "error": "insufficient_credits", "remaining_credits": 12,
"required_credits": 100, "checkoutUrl": "https://pay.recoupable.com/...",
"declineReason": { "code": "card_declined", "declineCode": "...", "message": "..." } }
```
Tests: 8 new (autoRechargeOrFail + ensureCreditsOrShortCircuit), full
suite 2838/2838 green, lint + format clean.
Known follow-ups (deferred, see plan discussion):
- No concurrency lock — flood of zero-credit requests can fire multiple
top-ups in parallel. Defer until problem.
- $5 auto-recharge is a constant in `lib/credits/const.ts`. Revisit if
any single request needs >500 credits (chat ~1, deep research 25).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(credits): address PR #566 review — gate at route layer, fix charge/credit edge cases
Addresses sweetman, cubic, and CodeRabbit review feedback.
REFACTOR (sweetman + cubic P1):
- Moved the credit gate out of `handleResearch` / `handleArtistResearch`. Both
helpers are back to their original `{ data } | { error, status }` signature,
so non-route consumers (e.g. `resolveTrack`) aren't accidentally exposed to
a 402 NextResponse branch they don't know how to handle. Eliminates the
"why both responses" / "what does this line do" questions on each route's
`if (result instanceof NextResponse) return result;` line.
- New shared helper `ensureResearchCredits(accountId)` wraps the per-route
gate with the family's fixed 5-credit cost — every research route now does
two lines (`const short = await ensureResearchCredits(...); if (short) return short;`).
- 26 GET research route handlers refactored to use the new helper.
FIX (CodeRabbit Major #1):
- `autoRechargeOrFail` now ALWAYS credits the account when the off-session
charge succeeds, even if the top-up alone doesn't cover the request.
Previously a request for >500 credits with a 2-credit balance would charge
the card but skip `incrementRemainingCredits`, leaving paid credits
unapplied. Now we credit unconditionally on `kind: "charged"`.
FIX (CodeRabbit Major #2):
- `autoRechargeOrFail` now throws when `createCreditsStripeSession` returns
no `url` instead of emitting a 402 with empty `checkoutUrl`. Empty checkout
URL is non-actionable; throwing converts to a proper 500 at the handler.
FIX (CodeRabbit quick win):
- Moved chat preflight INSIDE the existing try/catch in `handleChatStream`
so transient Supabase/Stripe failures don't leak as uncaught 500s.
FIX (sweetman):
- `CREDIT_AUTO_RECHARGE_FALLBACK_SUCCESS_URL`: chat.recoupable.com/credits/success
→ sandbox.recoupable.com/settings/profile (matches the live app URL).
- Removed dead `if (body.accountId)` guard in `handleChatStream`. After
`validateChatRequest` runs auth via `validateAuthContext`, `accountId` is
always set — the guard was YAGNI.
TESTS:
- 7 tests covering `autoRechargeOrFail` (+2 new: paid-credits-always-applied
edge case, throw-on-missing-url).
- Full suite: 2840/2840 passing. Lint + format clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(credits): SRP — gate moves from route handlers into validate functions
Per PR #566 review (sweetman): each validate function now owns request
rejection end-to-end — auth, schema, AND credit budget. Route handlers
become a clean two-step: validate → handle. The previous per-handler
preflight (`const short = await ensureResearchCredits(...)`) is gone;
the gate now lives where it belongs.
Validators gated:
- `validateChatRequest` (1 credit min)
- `validateArtistRequest` (5 credits, covers 10 artist-research routes
transitively via wrappers like validateGetResearchMetricsRequest)
- 12 per-route GET research validators (5 credits each, via the shared
`ensureResearchCredits` helper)
- `validatePostResearchWebRequest`, `validatePostResearchPeopleRequest`
(5 credits via `ensureResearchCredits`)
- `validatePostResearchExtractRequest` (5 × url count, custom cost)
- `validatePostResearchEnrichRequest` (5 / 10 / 25 by processor)
- `validatePostResearchDeepRequest` (NEW — extracted from inline zod in
postResearchDeepHandler; 25 credits, matches the previous inline gate)
Handler cleanups:
- 26 GET research handlers: removed the 2-line preflight
- 4 POST research handlers: removed the 5-line preflight + unused imports
- postResearchDeepHandler: now a thin "validate → fetch → deduct" handler
- handleChatStream: removed the preflight + unused imports
Tests: 23 validate/handler tests that transitively load the credit
chain now mock `ensureCreditsOrShortCircuit` at module level. Full
suite 2840/2840 passing. Lint + format clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(payment-method): implement GET /api/accounts/{id}/payment-method (#568)
* feat(payment-method): implement GET /api/accounts/{id}/payment-method
Implements the doc-first contract landed in recoupable/docs#211 — exposes
the default Stripe payment method on file for an account so the open-agents
top-up dialog can show pre-charge confirmation (which card will be charged)
before triggering a silent off-session top-up.
TDD red-green-refactor for both layers:
`lib/stripe/getDefaultPaymentMethodDetails` (4 tests):
- no PM on file → null
- card PM → SavedCard shape ({ brand, last4, exp_month, exp_year, funding })
- non-card PM (e.g., us_bank_account) → null
- card PM missing card object → null
Builds on the existing `findDefaultPaymentMethodForCustomer` helper, then
expands the PM ID into the full PaymentMethod via stripeClient.paymentMethods.retrieve.
`lib/payment_methods/getPaymentMethodHandler` (5 tests):
- 200 with card details when the account has one on file
- 200 with `card: null` when the customer has no PM
- 401 forwarded from validation
- 403 forwarded from validation
- 500 with masked internal-error when an upstream throws
Wired at app/api/accounts/[id]/payment-method/route.ts, mirroring the
existing credits-get route conventions (CORS preflight, force-dynamic,
no-store).
Tests: 487/487 passing (full vitest suite), lint + format clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(payment-method): move into lib/billing/ + fix GET write side-effect
Two concerns from PR review:
1. sweetman — co-locate with the rest of the billing surface:
- lib/payment_methods/{getPaymentMethodHandler,validateGetPaymentMethodParams,
mapToPaymentMethodError,buildPaymentMethodResponse}.ts → lib/billing/...
- test file moved alongside.
- All imports updated.
2. cubic P2 — `resolveStripeCustomerForAccount` creates a new Stripe Customer
on miss, so the GET handler was making a write side-effect during a read.
Introduced a new read-only helper `findStripeCustomerForAccount` that
returns null when no customer matches `metadata.accountId`. The handler
now short-circuits to `card: null` in that case — accounts that have
never been provisioned in Stripe also can't have a saved card.
resolveStripeCustomerForAccount stays as-is for mutating callers
(off-session charge, Checkout-session creation).
Tests:
- New `findStripeCustomerForAccount.test.ts` (2 cases: hit, miss-no-create).
- New handler test case: "skips PM lookup when no Stripe customer exists yet"
(asserts getDefaultPaymentMethodDetails is NOT called).
- Full suite 2852/2852 passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(billing): address bot review — guard .json() + document input contract
Two PR #568 bot threads:
coderabbit Major (mapToPaymentMethodError.ts): `.json()` throws on empty
or malformed response body. The validateAuthContext path is always JSON
in practice, but an upstream returning `new NextResponse("", {status})`
would propagate a SyntaxError instead of the documented `{ error }`
shape. Wrapped in try/catch with the existing "Unauthorized" fallback;
added a test asserting the empty-body 401 still returns
`{ error: "Unauthorized" }`.
coderabbit nit (findStripeCustomerForAccount.ts): `accountId` is
interpolated into the Stripe search query without internal validation.
Every route caller goes through `validateGetPaymentMethodParams` (zod
UUID schema) for this reason — added a JSDoc note pinning that as the
required input contract so future callers don't drop the guard.
Skipped cubic P3 (test file >100 lines) — precedent set on PR #561:
`lib/stripe/__tests__/chargeCustomerOffSession.test.ts` at 178 lines was
approved. My handler test is 119 lines after the new case.
Full suite 2853/2853 passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(credits): unify deductions through usage_events meter (#570)
Phase 2 of the credits / usage unification on the api side. Every
credit deduction now writes both the credits_usage wallet AND a
usage_events row carrying the wallet impact and any available token
detail — making credits_usage.remaining_credits derivable from
(top-ups + refills + grants) − SUM(credits_deducted_cents) and
enabling per-customer credit-usage rollups on the admin dashboard.
- lib/supabase/usage_events/insertUsageEvent.ts: new Supabase wrapper
with server-side nanoid id generation (matches open-agents' shape).
- lib/credits/recordCreditDeduction.ts: wallet + meter wrapper that
calls deductCredits and then insertUsageEvent. Insert failures are
logged but not surfaced — the wallet stays authoritative and a
reconciliation job can recover any missing audit row.
- Swap 4 call sites from deductCredits → recordCreditDeduction:
- handleChatCredits.ts: source='web', forwards model + tokens
- postResearchWebHandler.ts / handleResearch.ts /
handleArtistResearch.ts: source='api', no token detail
- x402 (lib/x402/fetchWithPayment.ts) is intentionally out of scope
for this phase and continues to update credits_usage only.
- types/database.types.ts: regenerated to include usage_events with
the credits_deducted_cents column from database PR #24.
- package.json: add `update-types` script (mirrors chat's convention)
and `nanoid` dependency for usage_events.id generation.
Caller tests updated to mock recordCreditDeduction instead of
deductCredits and assert the new call shape.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sweetmantech added a commit that referenced this pull request May 16, 2026
* feat(credits): just-in-time auto-recharge with 402 short-circuit (#566)
* feat(credits): just-in-time auto-recharge with 402 short-circuit
When a credit-gated request would push the account below its cost, attempt
a silent $5 off-session top-up against the saved card. If that succeeds,
increment the balance in-thread (the webhook ignores PIs stamped
`purpose: "credits_auto_recharge"`) and proceed. Otherwise return HTTP 402
with a unified body the open-agents credits dialog already parses.
Helpers (all under 100 LOC each):
- autoRechargeOrFail: returns `{ kind: "available" }` or
`{ kind: "insufficient_credits", checkoutUrl, declineReason? }`.
- ensureCreditsOrShortCircuit: wraps the above and returns
`NextResponse | null` — handlers do `if (short) return short`.
- buildInsufficientCreditsResponse: shared 402 body shape.
Wiring:
- `lib/research/handleResearch` + `handleArtistResearch` (covers ~26 GET
endpoints) gate up-front and deduct on upstream success.
- 5 special POST handlers (`postResearchWeb/People/Extract/Enrich/Deep`)
gate up-front with their per-endpoint cost.
- `lib/chat/handleChatStream` preflights with `creditsToDeduct: 1` before
the stream starts; `handleChatCredits` continues to deduct actual usage
post-hoc.
402 body shape (mirrors PR #561's Checkout response shape so open-agents
PR #36 needs no UI changes to render the right view):
```json
{ "error": "insufficient_credits", "remaining_credits": 12,
"required_credits": 100, "checkoutUrl": "https://pay.recoupable.com/...",
"declineReason": { "code": "card_declined", "declineCode": "...", "message": "..." } }
```
Tests: 8 new (autoRechargeOrFail + ensureCreditsOrShortCircuit), full
suite 2838/2838 green, lint + format clean.
Known follow-ups (deferred, see plan discussion):
- No concurrency lock — flood of zero-credit requests can fire multiple
top-ups in parallel. Defer until problem.
- $5 auto-recharge is a constant in `lib/credits/const.ts`. Revisit if
any single request needs >500 credits (chat ~1, deep research 25).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(credits): address PR #566 review — gate at route layer, fix charge/credit edge cases
Addresses sweetman, cubic, and CodeRabbit review feedback.
REFACTOR (sweetman + cubic P1):
- Moved the credit gate out of `handleResearch` / `handleArtistResearch`. Both
helpers are back to their original `{ data } | { error, status }` signature,
so non-route consumers (e.g. `resolveTrack`) aren't accidentally exposed to
a 402 NextResponse branch they don't know how to handle. Eliminates the
"why both responses" / "what does this line do" questions on each route's
`if (result instanceof NextResponse) return result;` line.
- New shared helper `ensureResearchCredits(accountId)` wraps the per-route
gate with the family's fixed 5-credit cost — every research route now does
two lines (`const short = await ensureResearchCredits(...); if (short) return short;`).
- 26 GET research route handlers refactored to use the new helper.
FIX (CodeRabbit Major #1):
- `autoRechargeOrFail` now ALWAYS credits the account when the off-session
charge succeeds, even if the top-up alone doesn't cover the request.
Previously a request for >500 credits with a 2-credit balance would charge
the card but skip `incrementRemainingCredits`, leaving paid credits
unapplied. Now we credit unconditionally on `kind: "charged"`.
FIX (CodeRabbit Major #2):
- `autoRechargeOrFail` now throws when `createCreditsStripeSession` returns
no `url` instead of emitting a 402 with empty `checkoutUrl`. Empty checkout
URL is non-actionable; throwing converts to a proper 500 at the handler.
FIX (CodeRabbit quick win):
- Moved chat preflight INSIDE the existing try/catch in `handleChatStream`
so transient Supabase/Stripe failures don't leak as uncaught 500s.
FIX (sweetman):
- `CREDIT_AUTO_RECHARGE_FALLBACK_SUCCESS_URL`: chat.recoupable.com/credits/success
→ sandbox.recoupable.com/settings/profile (matches the live app URL).
- Removed dead `if (body.accountId)` guard in `handleChatStream`. After
`validateChatRequest` runs auth via `validateAuthContext`, `accountId` is
always set — the guard was YAGNI.
TESTS:
- 7 tests covering `autoRechargeOrFail` (+2 new: paid-credits-always-applied
edge case, throw-on-missing-url).
- Full suite: 2840/2840 passing. Lint + format clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(credits): SRP — gate moves from route handlers into validate functions
Per PR #566 review (sweetman): each validate function now owns request
rejection end-to-end — auth, schema, AND credit budget. Route handlers
become a clean two-step: validate → handle. The previous per-handler
preflight (`const short = await ensureResearchCredits(...)`) is gone;
the gate now lives where it belongs.
Validators gated:
- `validateChatRequest` (1 credit min)
- `validateArtistRequest` (5 credits, covers 10 artist-research routes
transitively via wrappers like validateGetResearchMetricsRequest)
- 12 per-route GET research validators (5 credits each, via the shared
`ensureResearchCredits` helper)
- `validatePostResearchWebRequest`, `validatePostResearchPeopleRequest`
(5 credits via `ensureResearchCredits`)
- `validatePostResearchExtractRequest` (5 × url count, custom cost)
- `validatePostResearchEnrichRequest` (5 / 10 / 25 by processor)
- `validatePostResearchDeepRequest` (NEW — extracted from inline zod in
postResearchDeepHandler; 25 credits, matches the previous inline gate)
Handler cleanups:
- 26 GET research handlers: removed the 2-line preflight
- 4 POST research handlers: removed the 5-line preflight + unused imports
- postResearchDeepHandler: now a thin "validate → fetch → deduct" handler
- handleChatStream: removed the preflight + unused imports
Tests: 23 validate/handler tests that transitively load the credit
chain now mock `ensureCreditsOrShortCircuit` at module level. Full
suite 2840/2840 passing. Lint + format clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(payment-method): implement GET /api/accounts/{id}/payment-method (#568)
* feat(payment-method): implement GET /api/accounts/{id}/payment-method
Implements the doc-first contract landed in recoupable/docs#211 — exposes
the default Stripe payment method on file for an account so the open-agents
top-up dialog can show pre-charge confirmation (which card will be charged)
before triggering a silent off-session top-up.
TDD red-green-refactor for both layers:
`lib/stripe/getDefaultPaymentMethodDetails` (4 tests):
- no PM on file → null
- card PM → SavedCard shape ({ brand, last4, exp_month, exp_year, funding })
- non-card PM (e.g., us_bank_account) → null
- card PM missing card object → null
Builds on the existing `findDefaultPaymentMethodForCustomer` helper, then
expands the PM ID into the full PaymentMethod via stripeClient.paymentMethods.retrieve.
`lib/payment_methods/getPaymentMethodHandler` (5 tests):
- 200 with card details when the account has one on file
- 200 with `card: null` when the customer has no PM
- 401 forwarded from validation
- 403 forwarded from validation
- 500 with masked internal-error when an upstream throws
Wired at app/api/accounts/[id]/payment-method/route.ts, mirroring the
existing credits-get route conventions (CORS preflight, force-dynamic,
no-store).
Tests: 487/487 passing (full vitest suite), lint + format clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor(payment-method): move into lib/billing/ + fix GET write side-effect
Two concerns from PR review:
1. sweetman — co-locate with the rest of the billing surface:
- lib/payment_methods/{getPaymentMethodHandler,validateGetPaymentMethodParams,
mapToPaymentMethodError,buildPaymentMethodResponse}.ts → lib/billing/...
- test file moved alongside.
- All imports updated.
2. cubic P2 — `resolveStripeCustomerForAccount` creates a new Stripe Customer
on miss, so the GET handler was making a write side-effect during a read.
Introduced a new read-only helper `findStripeCustomerForAccount` that
returns null when no customer matches `metadata.accountId`. The handler
now short-circuits to `card: null` in that case — accounts that have
never been provisioned in Stripe also can't have a saved card.
resolveStripeCustomerForAccount stays as-is for mutating callers
(off-session charge, Checkout-session creation).
Tests:
- New `findStripeCustomerForAccount.test.ts` (2 cases: hit, miss-no-create).
- New handler test case: "skips PM lookup when no Stripe customer exists yet"
(asserts getDefaultPaymentMethodDetails is NOT called).
- Full suite 2852/2852 passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(billing): address bot review — guard .json() + document input contract
Two PR #568 bot threads:
coderabbit Major (mapToPaymentMethodError.ts): `.json()` throws on empty
or malformed response body. The validateAuthContext path is always JSON
in practice, but an upstream returning `new NextResponse("", {status})`
would propagate a SyntaxError instead of the documented `{ error }`
shape. Wrapped in try/catch with the existing "Unauthorized" fallback;
added a test asserting the empty-body 401 still returns
`{ error: "Unauthorized" }`.
coderabbit nit (findStripeCustomerForAccount.ts): `accountId` is
interpolated into the Stripe search query without internal validation.
Every route caller goes through `validateGetPaymentMethodParams` (zod
UUID schema) for this reason — added a JSDoc note pinning that as the
required input contract so future callers don't drop the guard.
Skipped cubic P3 (test file >100 lines) — precedent set on PR #561:
`lib/stripe/__tests__/chargeCustomerOffSession.test.ts` at 178 lines was
approved. My handler test is 119 lines after the new case.
Full suite 2853/2853 passing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(credits): unify deductions through usage_events meter (#570)
Phase 2 of the credits / usage unification on the api side. Every
credit deduction now writes both the credits_usage wallet AND a
usage_events row carrying the wallet impact and any available token
detail — making credits_usage.remaining_credits derivable from
(top-ups + refills + grants) − SUM(credits_deducted_cents) and
enabling per-customer credit-usage rollups on the admin dashboard.
- lib/supabase/usage_events/insertUsageEvent.ts: new Supabase wrapper
with server-side nanoid id generation (matches open-agents' shape).
- lib/credits/recordCreditDeduction.ts: wallet + meter wrapper that
calls deductCredits and then insertUsageEvent. Insert failures are
logged but not surfaced — the wallet stays authoritative and a
reconciliation job can recover any missing audit row.
- Swap 4 call sites from deductCredits → recordCreditDeduction:
- handleChatCredits.ts: source='web', forwards model + tokens
- postResearchWebHandler.ts / handleResearch.ts /
handleArtistResearch.ts: source='api', no token detail
- x402 (lib/x402/fetchWithPayment.ts) is intentionally out of scope
for this phase and continues to update credits_usage only.
- types/database.types.ts: regenerated to include usage_events with
the credits_deducted_cents column from database PR #24.
- package.json: add `update-types` script (mirrors chat's convention)
and `nanoid` dependency for usage_events.id generation.
Caller tests updated to mock recordCreditDeduction instead of
deductCredits and assert the new call shape.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* feat(admin): add credits rollup + events endpoints (#573)
* feat(admin): add credits rollup + events endpoints
Adds two admin-only endpoints powering the admin credits dashboard:
- GET /api/admins/credits/rollup — per-account totals over the selected
period, sorted by total credits descending, joined with account names
+ emails. Page-based pagination with total_count.
- GET /api/admins/credits/events — drilldown rows from usage_events for
one account in the selected period.
Period selector follows existing admin convention
(all|daily|weekly|monthly, default monthly). Aggregation runs in JS for
the rollup; acceptable at admin scale, future Postgres-function target.
Contract is documented in the recoupable/docs OpenAPI spec (PR #213).
* refactor(admin): promote getCutoffMs + PERIOD_DAYS to lib/admins/
The same period→cutoff math lived in three places (privy, credits) with
PERIOD_DAYS still in privy/ even though slack/ and credits/ already
imported it from there. Move both to lib/admins/ as a shared util.
- Unify return type to `number | null` (was `0` for "all" in privy,
`null` in credits). Callers updated to narrow on `=== null`.
- Delete the credits-local copy.
* refactor(admin): address PR review on credits endpoints
Sweetman feedback:
- SRP: validators absorb the validateAdminAuth call; handlers no longer
invoke auth directly.
- KISS: collapse selectAdminCreditsEvents + selectAdminCreditsRollupEvents
into a generic selectUsageEvents.ts. Extract count into its own file
(countUsageEvents.ts) per "one supabase query per lib file".
Bot bugs addressed:
- Supabase row-cap truncation: selectUsageEvents now paginates internally
in 1000-row batches when no explicit page is requested, so the rollup
aggregation no longer silently misses data once the period exceeds
Supabase's default response cap.
- Deterministic ordering: usage_events SELECT adds id DESC tiebreaker to
created_at DESC so equal-timestamp rows don't shuffle across pages.
- Rollup tiebreaker: account_id ASC when totals are equal.
- Primary email: pick the most-recently-updated row from account_emails
(the table has no is_primary, but updated_at is deterministic).
File-size hygiene: extracted aggregateRollupByAccount and
enrichRollupPageWithAccountDetails so the rollup handler drops from
111 to 54 lines.
* refactor(supabase): split selectUsageEvents into range + paginator
SRP: pull the single-batch Supabase select out of selectUsageEvents.ts
into its own selectUsageEventsRange.ts. selectUsageEvents now only owns
the paginate-until-done loop and delegates each batch.
* refactor(admin): move fetch-all loop out of supabase layer
selectUsageEvents is now a single, range-based Supabase primitive (one
file, one job, no polymorphic wrapper). The paginate-until-done loop
that the rollup needs to dodge Supabase's row cap moves to
lib/admins/credits/selectAllUsageEvents.ts — it's an admin/aggregation
concern, not a database concern.
The events handler now computes its own (from, to) from page+limit and
calls selectUsageEvents directly; the rollup handler calls
selectAllUsageEvents.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@sweetmantech