add a swift external account type#473
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✱ Stainless preview builds for gridThis PR will update the cli csharp go kotlin openapi php python ruby typescript
|
shreyav
force-pushed
the
05-18-add_a_swift_external_account_type
branch
from
2c9b7bb to
8b11c8e
Compare
Greptile SummaryThis PR introduces a new
Confidence Score: 3/5The new SWIFT account type is mostly well-structured, but the base schema allows a create request to pass validation with no bank account identifier at all — accountNumber and iban are both optional despite the docs saying one is mandatory. The SwiftAccountInfoBase schema leaves accountNumber and iban both optional, meaning clients can submit a create request that passes OpenAPI validation but carries no usable bank routing information. This would only surface as a runtime failure, not a schema error, making it easy to miss in client-side validation or SDK generation. openapi/components/schemas/common/SwiftAccountInfoBase.yaml needs the account identifier constraint tightened before the new account type goes live.
|
| Filename | Overview |
|---|---|
| openapi/components/schemas/common/SwiftAccountInfoBase.yaml | Core SWIFT account schema — neither accountNumber nor iban is required, so schema validation allows creating an account with no bank identifier; also contains an incorrect corridor example (BR is not an IBAN country). |
| openapi/components/schemas/common/SwiftBeneficiary.yaml | Individual beneficiary schema for SWIFT — requires beneficiaryType and fullName; address is optional (unlike AedBeneficiary which requires it), which appears intentional for SWIFT corridors. |
| openapi/components/schemas/external_accounts/SwiftExternalAccountCreateInfo.yaml | Create-side SWIFT account schema — composes BaseExternalAccountInfo + SwiftAccountInfoBase, adds required beneficiary with discriminated oneOf (INDIVIDUAL/BUSINESS). Structure follows existing patterns. |
| openapi/components/schemas/external_accounts/SwiftExternalAccountInfo.yaml | Read-side SWIFT account schema — uses SwiftAccountInfo (which includes paymentRails) instead of SwiftAccountInfoBase. Correctly mirrors the create schema pattern used across other account types. |
| openapi/components/schemas/external_accounts/ExternalAccountType.yaml | Enum reordered alphabetically with SWIFT_ACCOUNT added and wallet types grouped at the end. Clean housekeeping change. |
| openapi/components/schemas/external_accounts/ExternalAccountCreateInfoOneOf.yaml | SwiftExternalAccountCreateInfo added to oneOf list and discriminator mapping; list reordered alphabetically. SWIFT_ACCOUNT discriminator mapping is correct. |
| openapi/components/schemas/external_accounts/ExternalAccountInfoOneOf.yaml | SwiftExternalAccountInfo added to oneOf list and discriminator mapping; list reordered alphabetically. Pre-existing LIGHTNING / LIGHTNING_ACCOUNT dual mapping retained unchanged. |
Class Diagram
%%{init: {'theme': 'neutral'}}%%
classDiagram
class SwiftExternalAccountCreateInfo {
+title: SWIFT Account
}
class SwiftExternalAccountInfo {
+title: SWIFT Account
}
class BaseExternalAccountInfo {
+accountType: ExternalAccountType
}
class SwiftAccountInfoBase {
+accountType: SWIFT_ACCOUNT (required)
+swiftCode: string (required)
+bankName: string (required)
+country: string (required)
+accountNumber: string (optional)
+iban: string (optional)
}
class SwiftAccountInfo {
+paymentRails: SWIFT[] (required)
}
class SwiftBeneficiary {
+beneficiaryType: INDIVIDUAL (required)
+fullName: string (required)
+address: Address (optional)
}
class BusinessBeneficiary {
+beneficiaryType: BUSINESS (required)
+legalName: string (required)
+address: Address (optional)
}
SwiftExternalAccountCreateInfo --|> BaseExternalAccountInfo : allOf
SwiftExternalAccountCreateInfo --|> SwiftAccountInfoBase : allOf
SwiftExternalAccountCreateInfo --> SwiftBeneficiary : beneficiary oneOf
SwiftExternalAccountCreateInfo --> BusinessBeneficiary : beneficiary oneOf
SwiftExternalAccountInfo --|> BaseExternalAccountInfo : allOf
SwiftExternalAccountInfo --|> SwiftAccountInfo : allOf
SwiftExternalAccountInfo --> SwiftBeneficiary : beneficiary oneOf
SwiftExternalAccountInfo --> BusinessBeneficiary : beneficiary oneOf
SwiftAccountInfo --|> SwiftAccountInfoBase : allOf
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
openapi/components/schemas/common/SwiftAccountInfoBase.yaml:32-48
**No enforcement that at least one of `accountNumber` or `iban` is provided**
Both `accountNumber` and `iban` are absent from the `required` list, yet the descriptions state that one is mandatory depending on the corridor. A client can submit a valid (schema-passing) create request with neither field, which would only fail at runtime. A JSON Schema `oneOf` / `if-then-else` or a simple `anyOf` with two options (one requiring `accountNumber`, one requiring `iban`) is the standard way to enforce "exactly one of" at the spec layer, matching the pattern used by corridor-specific schemas like `AedAccountInfoBase.yaml` where the relevant identifier is listed directly in `required`.
### Issue 2 of 2
openapi/components/schemas/common/SwiftAccountInfoBase.yaml:32-44
Brazil (BR) does not use IBAN — it is not part of the IBAN registry. Listing it alongside GB as an "IBAN-only corridor" will mislead integrators trying to set up BRL SWIFT transfers. A well-known IBAN-mandatory country pair for SWIFT is `DE` / `GB`.
```suggestion
accountNumber:
type: string
description: >-
The bank account number. Required for most corridors.
Use iban instead for IBAN-only corridors (e.g. DE, GB).
example: '1234567890'
minLength: 1
maxLength: 34
iban:
type: string
description: >-
The IBAN of the bank account. Required for IBAN-only corridors (e.g. DE, GB).
Use accountNumber for all other corridors.
```
Reviews (1): Last reviewed commit: "add a swift external account type" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| accountNumber: | ||
| type: string | ||
| description: >- | ||
| The bank account number. Required for most corridors. | ||
| Use iban instead for IBAN-only corridors (e.g. BR, GB). | ||
| example: '1234567890' | ||
| minLength: 1 | ||
| maxLength: 34 | ||
| iban: | ||
| type: string | ||
| description: >- | ||
| The IBAN of the bank account. Required for IBAN-only corridors (e.g. BR, GB). | ||
| Use accountNumber for all other corridors. | ||
| example: GB29NWBK60161331926819 | ||
| minLength: 15 | ||
| maxLength: 34 | ||
| pattern: ^[A-Z]{2}[0-9]{2}[A-Za-z0-9]{11,30}$ |
There was a problem hiding this comment.
accountNumber or iban is provided
Both accountNumber and iban are absent from the required list, yet the descriptions state that one is mandatory depending on the corridor. A client can submit a valid (schema-passing) create request with neither field, which would only fail at runtime. A JSON Schema oneOf / if-then-else or a simple anyOf with two options (one requiring accountNumber, one requiring iban) is the standard way to enforce "exactly one of" at the spec layer, matching the pattern used by corridor-specific schemas like AedAccountInfoBase.yaml where the relevant identifier is listed directly in required.
Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/common/SwiftAccountInfoBase.yaml
Line: 32-48
Comment:
**No enforcement that at least one of `accountNumber` or `iban` is provided**
Both `accountNumber` and `iban` are absent from the `required` list, yet the descriptions state that one is mandatory depending on the corridor. A client can submit a valid (schema-passing) create request with neither field, which would only fail at runtime. A JSON Schema `oneOf` / `if-then-else` or a simple `anyOf` with two options (one requiring `accountNumber`, one requiring `iban`) is the standard way to enforce "exactly one of" at the spec layer, matching the pattern used by corridor-specific schemas like `AedAccountInfoBase.yaml` where the relevant identifier is listed directly in `required`.
How can I resolve this? If you propose a fix, please make it concise.
There was a problem hiding this comment.
we dont want another level of nesting / one of here
| accountNumber: | ||
| type: string | ||
| description: >- | ||
| The bank account number. Required for most corridors. | ||
| Use iban instead for IBAN-only corridors (e.g. BR, GB). | ||
| example: '1234567890' | ||
| minLength: 1 | ||
| maxLength: 34 | ||
| iban: | ||
| type: string | ||
| description: >- | ||
| The IBAN of the bank account. Required for IBAN-only corridors (e.g. BR, GB). | ||
| Use accountNumber for all other corridors. |
There was a problem hiding this comment.
DE / GB.
| accountNumber: | |
| type: string | |
| description: >- | |
| The bank account number. Required for most corridors. | |
| Use iban instead for IBAN-only corridors (e.g. BR, GB). | |
| example: '1234567890' | |
| minLength: 1 | |
| maxLength: 34 | |
| iban: | |
| type: string | |
| description: >- | |
| The IBAN of the bank account. Required for IBAN-only corridors (e.g. BR, GB). | |
| Use accountNumber for all other corridors. | |
| accountNumber: | |
| type: string | |
| description: >- | |
| The bank account number. Required for most corridors. | |
| Use iban instead for IBAN-only corridors (e.g. DE, GB). | |
| example: '1234567890' | |
| minLength: 1 | |
| maxLength: 34 | |
| iban: | |
| type: string | |
| description: >- | |
| The IBAN of the bank account. Required for IBAN-only corridors (e.g. DE, GB). | |
| Use accountNumber for all other corridors. |
Prompt To Fix With AI
This is a comment left during a code review.
Path: openapi/components/schemas/common/SwiftAccountInfoBase.yaml
Line: 32-44
Comment:
Brazil (BR) does not use IBAN — it is not part of the IBAN registry. Listing it alongside GB as an "IBAN-only corridor" will mislead integrators trying to set up BRL SWIFT transfers. A well-known IBAN-mandatory country pair for SWIFT is `DE` / `GB`.
```suggestion
accountNumber:
type: string
description: >-
The bank account number. Required for most corridors.
Use iban instead for IBAN-only corridors (e.g. DE, GB).
example: '1234567890'
minLength: 1
maxLength: 34
iban:
type: string
description: >-
The IBAN of the bank account. Required for IBAN-only corridors (e.g. DE, GB).
Use accountNumber for all other corridors.
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Yes, the IBAN (International Bank Account Number) is used in Brazil.
It is officially regulated by the Banco Central do Brasil (Central Bank of Brazil) and is exclusively required when sending or receiving international wire transfers
| type: string | ||
| description: The country of residence of the beneficiary | ||
| address: | ||
| $ref: ./Address.yaml |
There was a problem hiding this comment.
[Re: line +31]
is this different from a regular Individual beneficiary?
See this comment inline on Graphite.
There was a problem hiding this comment.
no but I was following the existing pattern of having a dedicated individual bene schema per account type
There was a problem hiding this comment.
oh yeah it was so we could separate out required fields based on country
There was a problem hiding this comment.
@claude add a note to openapi/readme.md to note for external accounts, we created individual beneficiares as not each country has the same fields / required information
3 participants
No description provided.