Conversation
🦋 Changeset detectedLatest commit: 1dfb827 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a new server-side capability to automatically adjust user card limits based on approved cases from the Persona identity verification platform. By integrating a dedicated webhook handler, the system can now process specific Persona events, validate the incoming data, and programmatically update card limits through an external API, streamlining the process of managing user financial controls. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new "cardLimit" Persona case flow: validation and template checks, inquiry fetch to obtain a reference-id, DB lookups for credential and active card, and conditional Panda card limit updates; includes new persona constants, helper, API handling, tests, and a changeset. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Hook as Persona Hook
participant Persona as Persona API
participant DB as Database
participant Panda as Panda Service
Client->>Hook: POST webhook (data.type="case", template: cardLimit, status: Approved)
Hook->>Hook: Validate payload & template
Hook->>Persona: GET /inquiries/{inquiryId}
Persona-->>Hook: { data.attributes["reference-id"]: referenceId }
Hook->>DB: Query credentials where credentials.id = referenceId
DB-->>Hook: credential { pandaId? }
Hook->>DB: Query cards where cards.credentialId = referenceId AND status = "ACTIVE"
DB-->>Hook: card { id } or none
alt card exists
Hook->>Panda: updateCard(cardId, { limit: { amount, frequency: "per7DayPeriod" } })
Panda-->>Hook: success / error
end
Hook-->>Client: 200 { code: "ok" } or short-circuit codes ("no limit","no inquiry","no credential","no panda") / 500 on update error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ 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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #898 +/- ##
==========================================
+ Coverage 71.46% 71.98% +0.52%
==========================================
Files 225 225
Lines 8165 8368 +203
Branches 2605 2703 +98
==========================================
+ Hits 5835 6024 +189
- Misses 2103 2108 +5
- Partials 227 236 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: faf0a9e659
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| if (payload.template === "cardLimit") { | ||
| getActiveSpan()?.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, "persona.case.card-limit"); | ||
| if (payload.data.attributes.status !== "Approved") return c.json({ code: "ok" }, 200); |
There was a problem hiding this comment.
Returning 200 for every non-Approved case here drops the only signal about the review result. The new /kyc card-limit endpoints still infer state only from getInquiry() (server/utils/persona.ts:47-57), so once the underlying inquiry becomes approved they report ok/already approved even if the case is still Open/Pending or was later Declined, and the card limit was never changed. In those review outcomes, the user gets a false success state and no way to start a replacement limit-increase flow.
Useful? React with 👍 / 👎.
server/hooks/persona.ts
Outdated
| } catch (error: unknown) { | ||
| captureException(error, { level: "error", contexts: { details: { inquiryId } } }); | ||
| return c.json({ code: "ok" }, 200); |
There was a problem hiding this comment.
This catch acknowledges the webhook on any getInquiryById() failure, including transient Persona 5xx/timeouts. That lookup is the only way this handler recovers the credential reference from the case payload, and the approved limit is not persisted anywhere else, so a temporary Persona outage here permanently drops the limit increase: Persona stops retrying, the card stays unchanged, and later /kyc calls still only see the historical inquiry. This path needs to stay retryable or persist the case for replay.
Useful? React with 👍 / 👎.
server/hooks/persona.ts
Outdated
| } catch (error: unknown) { | ||
| captureException(error, { level: "error", contexts: { details: { inquiryId } } }); | ||
| return c.json({ code: "ok" }, 200); |
There was a problem hiding this comment.
🚩 Webhook silently drops card limit update when getInquiryById fails
In the card limit webhook handler at server/hooks/persona.ts:261-263, when getInquiryById throws (e.g. transient network error), the error is caught, captured in Sentry, and a 200 response is returned. This tells Persona the webhook was processed successfully, so it won't retry. The card limit update is permanently lost unless manually addressed. By contrast, updateCard failures at line 281 intentionally propagate as 500s, triggering Persona retries. This asymmetry appears deliberate (perhaps because an invalid inquiry ID wouldn't benefit from retries), but a transient Persona API timeout would also be silently dropped.
Was this helpful? React with 👍 or 👎 to provide feedback.
| case "failed": | ||
| case "declined": | ||
| return c.json({ code: "failed" }, 200); |
There was a problem hiding this comment.
🚩 GET and POST handlers return different status codes for failed/declined cardLimit inquiries
The GET handler at server/api/kyc.ts:77-79 returns status 200 for failed/declined cardLimit inquiries, while the POST handler at server/api/kyc.ts:201-202 returns 400 via the default case. This differs from non-cardLimit scopes where both GET and POST return 400 for failed/declined. The tests confirm this is intentional (GET tests assert 200, POST tests assert 400), but this asymmetry could confuse API consumers who expect consistent status codes across GET/POST for the same inquiry state.
Was this helpful? React with 👍 or 👎 to provide feedback.
| default: | ||
| return c.json({ code: "failed" }, 400); |
There was a problem hiding this comment.
🟡 POST cardLimit switch uses default as catch-all instead of explicitly listing failed/declined statuses
The POST handler's cardLimit switch statement (lines 185-203) uses default to handle failed and declined statuses, unlike every other inquiry-status switch in the same file which explicitly lists all 8 known statuses and reserves default for throwing on unknown values. Compare with the GET handler for cardLimit at server/api/kyc.ts:67-82 (explicitly lists failed/declined, throws on default) and the non-cardLimit POST handler at server/api/kyc.ts:226-247 (same pattern). This means an unexpected future status value would silently return { code: "failed" } instead of surfacing as an error. This violates the codebase's established consistency pattern per AGENTS.md.
| default: | |
| return c.json({ code: "failed" }, 400); | |
| case "failed": | |
| case "declined": | |
| return c.json({ code: "failed" }, 400); | |
| default: | |
| throw new Error("unknown inquiry status"); |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes / Reliability
Tests
Chores
This is part 1 of 2 in a stack made with GitButler: