Rozagerardo/feat/gh 2289 off ramp implementation#2317
Merged
rozagerardo merged 21 commits intoJun 19, 2026
Conversation
Bridge external account and liquidation address provisioning, payout-accounts API, Deposits/Payouts treasury UI, and two-step add dialog. Governance recipient test included. Sandbox E2E and polish remain. Refs #2289 Co-authored-by: Cursor <cursoragent@cursor.com>
- Fix IBAN adapter: use body.iban={account_number,bic?,country} object, not body.account.iban
- Remap address.subdivision → address.state for Bridge ExternalAccountAddress type
- Derive iban.country from IBAN prefix (bank country), not beneficiary address country
- Add MOD-97 IBAN checksum validation (server Zod schema + client pre-submit)
- Filter country dropdown by rail: EUR→SEPA only, GBP→GBR, USD→USA, SWIFT→all
- Add SEPA_ALPHA3 set + SEPA_COUNTRIES export (EPC 2024 adherence list, 37 entries)
- Add fieldErrors state in dialog for inline red-ring validation UX on IBAN field
- Update BridgeExternalAccountAddress type to use state field
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…out accounts - Add review step (step 3 of 3) between form and POST: shows read-only summary of all entered data before submission; back button returns to form - Remove success step from add dialog; after successful POST, onSuccess callback opens the detail dialog instead - Add PayoutAccountDetailDialog: shows currency/rail, source token, copyable EVM address, and how-to instructions; opened on card click or after creation - Make PayoutAccountCard clickable (role=button, hover/focus styles, keyboard) - Thread onPayoutAccountClick down from BankingSection through BankAccountsSection and ApprovedBankingPayouts to PayoutAccountCard - Add i18n keys for review step and detail dialog (5 locales) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Thread accountName (nickname) through BankPayoutAccountPublic type chain:
providers/types.ts → adapter.ts → map-bridge-resources.ts →
create-space-bank-payout-account.ts → core/types.ts → epics/hooks/types.ts
- Payout card: use accountName as primary header (fallback: bankName),
source→dest subheader format (e.g. EURC → EUR, no rail name),
rename liquidationAddressLabel → payoutAddressLabel
- Detail dialog: use accountName as title, currency flow header, bank
destination section (bank name, masked account, owner name), simplified
how-to text, payoutAddressLabel
- Add dialog: set aria-invalid={undefined} (not false) when no error to
prevent browser from triggering red invalid state before user interaction
- i18n: add payoutAddressLabel and detail.titleFallback across all 5 locales
(en/de/es/fr/pt) with proper UTF-8 encoding (no BOM)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
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:
WalkthroughThis PR introduces end-to-end payout account provisioning for spaces via Bridge liquidation addresses. It adds off-ramp rail constants, Bridge API client functions for external accounts and liquidation addresses, server-side orchestration with duplicate-guard logic, a Zod validation schema with IBAN/MOD-97 validation, a Next.js API route, SWR client hooks, and a full multi-step UI dialog with card and detail components, tabbed layout refactoring, and five-language internationalization support. ChangesBanking Payout Accounts Feature
Sequence Diagram(s)sequenceDiagram
participant User
participant Dialog as AddPayoutAccountDialog
participant Hook as useCreatePayoutAccount
participant Route as POST /api/.../payout-accounts
participant Fn as createSpaceBankPayoutAccount
participant KycAdapter as BridgeAdapter
participant BridgeAPI as Bridge API
User->>Dialog: select currency, enter bank details, confirm
Dialog->>Hook: createPayoutAccount(input)
Hook->>Route: POST with Bearer token + body
Route->>Route: schemaCreatePayoutAccount.safeParse(body)
Route->>Route: check NEXT_PUBLIC_BANKING_SUPPORTED_PAYOUT_RAILS
Route->>Fn: input + authToken + db
Fn->>Fn: resolveBankPayoutRail + authorizeSpaceBankOnboarding
Fn->>Fn: verify customer approved + sync provider customer ID
Fn->>KycAdapter: registerExternalAccount(input, idempotencyKey)
KycAdapter->>BridgeAPI: POST /v0/customers/{id}/external_accounts
BridgeAPI-->>KycAdapter: BridgeExternalAccountResponse
Fn->>Fn: loadBankingProviderState → check liquidationAddressPairs (409 if duplicate)
Fn->>KycAdapter: createLiquidationAddress(input, idempotencyKey)
KycAdapter->>BridgeAPI: POST /v0/customers/{id}/liquidation_addresses
BridgeAPI-->>KycAdapter: BridgeLiquidationAddressResponse
Fn->>Fn: mapBridgePayoutAccountToPublic
Fn-->>Route: CreateSpaceBankPayoutAccountResult
Route-->>Hook: 200 JSON
Hook->>Hook: SWR mutate payout-accounts cache
Hook-->>Dialog: CreatePayoutAccountResult
Dialog->>User: onSuccess → open PayoutAccountDetailDialog
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
Fixes CI format:check failures on 6 files. 4 were touched in the previous commit; 2 (banking-section, country-data) were pre-existing. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
🤖 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 `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/banking/payout-accounts/route.ts:
- Around line 19-50: The GET route in the payout-accounts endpoint uses custom
ad-hoc pagination instead of the required shared v1 pagination contracts. Import
PaginationParams and PaginatedResponse<T> from core/common, then replace the
manual parsing of limit and starting_after from searchParams with
PaginationParams contract extraction, update the call to
getSpaceBankPayoutAccounts to use the parsed PaginationParams, and wrap the
result in PaginatedResponse<T> before returning via NextResponse.json to conform
to the shared pagination contract requirements for v1 API endpoints.
- Around line 76-86: The authenticateBankCustomerRequest call is outside the
try-catch block, meaning any exceptions from that function (DB/network/internal
failures) will not be caught and will bypass structured error handling. Wrap the
authenticateBankCustomerRequest call in a try-catch block to gracefully handle
failures, returning a structured JSON error response with an appropriate error
message and status code, consistent with how the Invalid JSON body error is
handled in the existing try-catch block.
- Around line 35-43: The limit parameter being passed to
getSpaceBankPayoutAccounts accepts any finite integer, including zero, negative
numbers, and excessively large values, which can cause provider errors or
expensive calls. After parsing the limit from searchParams on line 35, add
validation logic to clamp it to a safe positive range (for example, minimum of 1
and maximum of a reasonable threshold like 100 or 1000) before passing it to
getSpaceBankPayoutAccounts. Replace or enhance the current Number.isFinite check
to include both lower and upper bound constraints on the limit value.
In `@packages/core/src/banking/server/create-space-bank-payout-account.ts`:
- Around line 96-100: The externalAccountIdempotencyKey in the
create-space-bank-payout-account.ts file embeds raw sensitive bank account
identifiers (accountNumber, iban, or routingNumber) which can leak financial
data through provider logs and telemetry. Replace the raw account identifier
with a stable one-way cryptographic hash fingerprint. Use a hash function (such
as SHA256) to create a deterministic, non-reversible fingerprint of the account
identifier part (accountNumber ?? iban ?? routingNumber ?? 'unknown'), while
keeping the other key components (customerId, railKey, sourceCurrency)
unchanged, ensuring the key remains stable for idempotency while protecting
sensitive data.
In `@packages/core/src/banking/server/get-space-bank-payout-accounts.ts`:
- Around line 40-46: The bridgeListExternalAccounts call on line 45 uses a
hard-coded limit of 100, which causes liquidation rows to reference external
accounts that were never fetched, resulting in null accountName, bankName,
last4, and externalAccount values in the returned payout records. Fix this by
using the same limit variable that is passed to bridgeListLiquidationAddresses
instead of the hard-coded 100 value, ensuring that both queries fetch the same
number of records and can properly join on external accounts.
In `@packages/core/src/banking/server/providers/bridge/adapter.ts`:
- Around line 123-129: The SWIFT external account handling in the bridge adapter
is using an incomplete placeholder implementation. Replace the current approach
of setting a top-level `bic` field and optional `account` property with the
proper nested structure that the Bridge API expects. Create a `swift` object
nested within the body and populate it with the required fields: account
(containing account_number), address, category, purpose_of_funds, and
short_business_description. Map the input parameters to these nested properties
according to the Bridge API contract, removing the top-level `bic` assignment
and restructuring the account assignment to be part of the `swift` object
instead.
In `@packages/core/src/banking/server/providers/bridge/banking-provider-state.ts`:
- Around line 115-128: The liquidationAddressPairs set is currently populated
using only the first page of results from bridgeListLiquidationAddresses
(limited to 100 items), which causes downstream duplicate prevention to miss
accounts on subsequent pages and allow duplicate payouts. Modify the code to
paginate through all results from bridgeListLiquidationAddresses by repeatedly
calling the function with pagination parameters (such as an offset or cursor)
until all pages are exhausted, accumulating all liquidation addresses into
liquidationAddressPairs before the set is used for duplicate checking.
In `@packages/core/src/banking/validation.ts`:
- Around line 235-242: The SWIFT account validation in the if block checking
rail.externalAccountType === 'swift' currently allows iban without
accountNumber, but the Bridge adapter's SWIFT serialization does not support
IBAN and only serializes account_number. Align the validation requirements by
either requiring accountNumber to be mandatory for SWIFT accounts (modify the
condition to reject iban-only input), or alternatively add IBAN field
serialization support to the Bridge adapter in
packages/core/src/banking/server/providers/bridge/adapter.ts. Choose one
approach to ensure validation rules match what the adapter actually sends to the
provider.
- Line 177: The destinationCurrency field in the validation schema accepts
whitespace-only input which gets trimmed to an empty string, causing this empty
value to override the rail default downstream instead of being treated as
missing. To fix this, add validation to the destinationCurrency field to reject
empty strings after trimming, ensuring that whitespace-only input is properly
rejected and allows the rail default to be used as a fallback. Use a validation
method that checks the trimmed string has a minimum length requirement or
implements a custom refinement to disallow empty strings.
In `@packages/epics/src/banking/components/add-payout-account-dialog.tsx`:
- Around line 41-42: Move all user-facing literals to next-intl translations in
the add-payout-account-dialog.tsx file. Replace the REQUIRED_MSG constant with a
translation key for the required field validation message. Identify the garbled
IBAN error message (containing mojibake characters) and the CA placeholder text,
and create appropriate translation keys for these strings. For the IBAN
validation error that appears in multiple branches, extract a single reusable
translation key for the IBAN error message and use it consistently across all
locations (instead of separate hardcoded strings), ensuring the translation
fixes the character encoding issue. Update all hardcoded strings to use
next-intl's translation mechanism to comply with the packages/epics coding
guidelines.
- Around line 297-303: The SWIFT validation logic currently allows submissions
without an IBAN, but since the SWIFT form UI has no accountNumber input field,
users can reach submit with neither IBAN nor accountNumber, causing API
rejection. In the SWIFT validation branch (selectedCurrency === 'swift'), add a
required validation check for IBAN similar to the existing BIC check so that
errs.iban is set when ibanValue is empty. The form UI at lines 583-623 is
referenced as evidence that accountNumber is not available for SWIFT users,
confirming that IBAN must be made mandatory in this validation branch.
In `@packages/epics/src/banking/components/bank-accounts-section.tsx`:
- Around line 34-35: There is a contract mismatch between the type definition of
openPayoutAccountDisabledReason and its implementation. The property type allows
multiple reason values (loadingAccounts, allCurrenciesCovered, etc.) but the
implementation at lines 139-142 only handles the finishVerificationFirst case,
which could leave a disabled CTA without an explanation. Either narrow the
OpenSpaceAccountDisabledReason type definition at lines 34-35 to only include
the reason values that are actually handled in the implementation, or expand the
conditional logic at lines 139-142 to handle all allowed reason values from the
type definition.
- Around line 92-97: The tooltip explanation for the disabled button is only
accessible via the `title` attribute on a non-focusable span wrapper, making it
inaccessible to keyboard and screen reader users. Replace this pattern with an
accessible alternative using `aria-describedby` paired with a visible or
aria-hidden helper element that properly associates the disabled reason with the
button. Modify the conditional block that checks `if (tooltipText && disabled)`
to implement this accessible pattern instead of wrapping the button in a span
with a title attribute.
In `@packages/i18n/src/messages/de.json`:
- Line 718: In the German locale file (de.json), replace the English text values
with proper German translations. The "sortCode" key currently has the English
value "Sort code" which should be translated to German. Additionally, the "Wire"
label mentioned in the related locations (also appearing in the same file)
should also be localized to German. Update all these string values to use German
translations instead of English text to maintain consistent UI language in the
German locale.
In `@packages/i18n/src/messages/es.json`:
- Around line 2857-2862: Replace the inconsistent terminology "tesoro" with
"tesorería" throughout the payout account descriptions in
packages/i18n/src/messages/es.json to maintain consistency with the rest of the
locale file. Additionally, at lines 2857-2862, fix the awkward phrasing "envía
el fiat el tesoro" to use more natural Spanish phrasing. Apply the same
terminology and phrasing improvements at lines 2891-2893 where similar
inconsistencies exist. Ensure all user-facing messages use "tesorería"
consistently and employ grammatically correct, natural-sounding Spanish across
all payout-related copy.
In `@packages/i18n/src/messages/pt.json`:
- Around line 2902-2903: Update the routingNumber translation in pt.json to
match the existing Portuguese translation pattern used elsewhere in the file.
Change the routingNumber value from the English "Routing number" to the existing
Portuguese translation "Número de roteamento" that is already used in the
depositInstructions section, ensuring consistent terminology for the same
banking field across the application.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 421bd204-a37c-42b0-a5a6-967840ee85cc
📒 Files selected for processing (36)
apps/web/src/app/api/v1/spaces/[spaceSlug]/banking/payout-accounts/route.tspackages/core/src/banking/__tests__/payout-proposal-recipient.test.tspackages/core/src/banking/constants.tspackages/core/src/banking/server/__tests__/build-rail-statuses.test.tspackages/core/src/banking/server/__tests__/map-bridge-resources.test.tspackages/core/src/banking/server/__tests__/request-space-bank-onboarding.test.tspackages/core/src/banking/server/create-space-bank-payout-account.tspackages/core/src/banking/server/get-space-bank-payout-accounts.tspackages/core/src/banking/server/index.tspackages/core/src/banking/server/map-bridge-resources.tspackages/core/src/banking/server/providers/bridge/__tests__/adapter.test.tspackages/core/src/banking/server/providers/bridge/adapter.tspackages/core/src/banking/server/providers/bridge/banking-provider-state.tspackages/core/src/banking/server/providers/types.tspackages/core/src/banking/types.tspackages/core/src/banking/validation.tspackages/core/src/common/server/bridge-client.tspackages/epics/src/banking/components/add-payout-account-dialog.tsxpackages/epics/src/banking/components/approved-banking-payouts.tsxpackages/epics/src/banking/components/bank-accounts-section.tsxpackages/epics/src/banking/components/bank-transfers-section.tsxpackages/epics/src/banking/components/banking-page-skeleton.tsxpackages/epics/src/banking/components/banking-section.tsxpackages/epics/src/banking/components/payout-account-card.tsxpackages/epics/src/banking/components/payout-account-detail-dialog.tsxpackages/epics/src/banking/components/payout-currency-option-row.tsxpackages/epics/src/banking/country-data.tspackages/epics/src/banking/hooks/index.tspackages/epics/src/banking/hooks/types.tspackages/epics/src/banking/hooks/use-create-payout-account.tspackages/epics/src/banking/hooks/use-payout-accounts.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
Comment on lines
+19
to
+50
| export async function GET( | ||
| request: NextRequest, | ||
| { params }: { params: Promise<Params> }, | ||
| ) { | ||
| const { spaceSlug } = await params; | ||
| const { searchParams } = new URL(request.url); | ||
|
|
||
| try { | ||
| const authResult = await authenticateBankCustomerRequest( | ||
| request, | ||
| spaceSlug, | ||
| ); | ||
| if (!authResult.ok) { | ||
| return authResult.response; | ||
| } | ||
|
|
||
| const limit = Number.parseInt(searchParams.get('limit') ?? '25', 10); | ||
| const startingAfter = searchParams.get('starting_after') ?? undefined; | ||
|
|
||
| const result = await getSpaceBankPayoutAccounts( | ||
| authResult.space, | ||
| { | ||
| spaceSlug, | ||
| limit: Number.isFinite(limit) ? limit : 25, | ||
| startingAfter, | ||
| }, | ||
| { db }, | ||
| ); | ||
|
|
||
| return NextResponse.json(result, { | ||
| headers: { 'Cache-Control': 'private, no-store' }, | ||
| }); |
Contributor
There was a problem hiding this comment.
Use shared v1 pagination contracts (PaginationParams / PaginatedResponse<T>).
This GET list route currently uses ad-hoc query parsing and a custom paginated shape. For app/api/v1/**, the repo rule requires the shared pagination contract.
As per coding guidelines: “apps/web/src/app/api/v1/**: … Use PaginatedResponse<T> and PaginationParams from core/common for all list endpoints”.
🤖 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 `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/banking/payout-accounts/route.ts
around lines 19 - 50, The GET route in the payout-accounts endpoint uses custom
ad-hoc pagination instead of the required shared v1 pagination contracts. Import
PaginationParams and PaginatedResponse<T> from core/common, then replace the
manual parsing of limit and starting_after from searchParams with
PaginationParams contract extraction, update the call to
getSpaceBankPayoutAccounts to use the parsed PaginationParams, and wrap the
result in PaginatedResponse<T> before returning via NextResponse.json to conform
to the shared pagination contract requirements for v1 API endpoints.
Source: Coding guidelines
Contributor
There was a problem hiding this comment.
@rozagerardo, understood — thanks for tracking it in #2330. I'll consider this deferred for the current PR.
🧠 Learnings used
Learnt from: plitzenberger
Repo: hypha-dao/hypha-web PR: 2053
File: apps/web/src/app/[lang]/profile/[personSlug]/@aside/edit/page.tsx:44-44
Timestamp: 2026-03-26T18:21:43.709Z
Learning: In hypha-dao/hypha-web, always flag usage of TypeScript `any` (including cases suppressed via `// eslint-disable-next-line typescript-eslint/no-explicit-any`) and do not approve/overlook it. Suggest properly typed alternatives such as inferring types from Zod schemas (`z.infer<typeof schema>`) or defining explicit TypeScript interfaces/types. Apply this standard consistently across the repo’s TypeScript/TSX files.
Comment on lines
+40
to
+46
| const [liquidationListed, externalListed] = await Promise.all([ | ||
| bridgeListLiquidationAddresses(customerId, { | ||
| limit, | ||
| starting_after: input.startingAfter, | ||
| }), | ||
| bridgeListExternalAccounts(customerId, { limit: 100 }), | ||
| ]); |
Contributor
There was a problem hiding this comment.
External-account join is truncated to the first 100 records.
Line 45 hard-caps bridgeListExternalAccounts to 100, so liquidation rows on the current page can reference external accounts that were never fetched. That produces partial payout records (accountName/bankName/last4 null) and can misclassify status because externalAccount becomes null downstream.
Suggested fix direction
- const [liquidationListed, externalListed] = await Promise.all([
- bridgeListLiquidationAddresses(customerId, {
- limit,
- starting_after: input.startingAfter,
- }),
- bridgeListExternalAccounts(customerId, { limit: 100 }),
- ]);
+ const liquidationListed = await bridgeListLiquidationAddresses(customerId, {
+ limit,
+ starting_after: input.startingAfter,
+ });
+
+ const neededExternalIds = new Set(
+ liquidationListed.data
+ .map((l) => l.external_account_id)
+ .filter((id): id is string => Boolean(id)),
+ );
+
+ const externalById = new Map<string, (typeof (await bridgeListExternalAccounts(customerId, { limit: 1 })).data)[number]>();
+ let cursor: string | undefined = undefined;
+ while (neededExternalIds.size > 0) {
+ const page = await bridgeListExternalAccounts(customerId, {
+ limit: 100,
+ starting_after: cursor,
+ });
+ for (const account of page.data) {
+ if (neededExternalIds.has(account.id)) {
+ externalById.set(account.id, account);
+ neededExternalIds.delete(account.id);
+ }
+ }
+ if (!page.has_more || page.data.length === 0) break;
+ cursor = page.data[page.data.length - 1]?.id;
+ }🤖 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 `@packages/core/src/banking/server/get-space-bank-payout-accounts.ts` around
lines 40 - 46, The bridgeListExternalAccounts call on line 45 uses a hard-coded
limit of 100, which causes liquidation rows to reference external accounts that
were never fetched, resulting in null accountName, bankName, last4, and
externalAccount values in the returned payout records. Fix this by using the
same limit variable that is passed to bridgeListLiquidationAddresses instead of
the hard-coded 100 value, ensuring that both queries fetch the same number of
records and can properly join on external accounts.
Contributor
There was a problem hiding this comment.
@rozagerardo, understood — thanks for tracking it in #2330. I'll leave this comment open until the fix lands.
🧠 Learnings used
Learnt from: plitzenberger
Repo: hypha-dao/hypha-web PR: 2053
File: apps/web/src/app/[lang]/profile/[personSlug]/@aside/edit/page.tsx:44-44
Timestamp: 2026-03-26T18:21:43.709Z
Learning: In hypha-dao/hypha-web, always flag usage of TypeScript `any` (including cases suppressed via `// eslint-disable-next-line typescript-eslint/no-explicit-any`) and do not approve/overlook it. Suggest properly typed alternatives such as inferring types from Zod schemas (`z.infer<typeof schema>`) or defining explicit TypeScript interfaces/types. Apply this standard consistently across the repo’s TypeScript/TSX files.
|
🔗 Custom preview URL: https://pr-2317.preview-app.hypha.earth |
- Change footer Continue button (form step) from type="submit" with form= attribute to type="button" with explicit onClick. The submit button was being fired immediately when React transitioned from the currency step to the form step (button re-rendered still focused), causing all field errors to populate before the user interacted. - Make handleFormContinue accept an optional FormEvent so it can be called from both the button onClick and the form's onSubmit (Enter key in text fields still works). - Clear fieldErrors in handleBack when going review→form, not just form→currency — prevents stale errors from persisting. - Fix garbled IBAN error string encoding (â€" → —). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…mponent - Restructures add-payout-account dialog to 4 steps: currency → form → compliance (beneficiary address + transfer details) → review - Step 3 shows business name badge, beneficiary address, SWIFT transfer details (category, purpose of funds, business description) - Currency badge moved to top of step 2; footer validation error summary - Extracts shared AddressFormFields component (address-form-fields.tsx) with consistent field order (city|subdivision, postal|country) used by both beneficiary and SWIFT bank address sections - Fixes SWIFT account_type: removes incorrect override to 'iban'/'unknown'; must stay 'swift' so Bridge routes to SWIFT validator - Confirms beneficiary address required for SWIFT (top-level `address` alongside swift.address per Bridge API spec) - Review step maps category/purpose raw values to display labels - Button copy: 'Submit' (was 'Confirm & Submit') - Zod schema: swiftBankAddress, swiftCategory, swiftPurposeOfFunds, swiftBusinessDescription fields added; address always required Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/i18n/src/messages/de.json (1)
744-747:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd the missing SWIFT add-dialog keys before merge.
The i18n verification is failing because
de.jsonlacks the 19 SWIFT keys used by the dialog underBankingTab.payouts.addDialog.swift(accountFormatLabel,accountNumber,bankAddressSection,complianceSection,purposeInvoice, etc.).Proposed fix
"swift": { "code": "SWIFT", - "hint": "Internationale Überweisung via BIC/SWIFT" + "hint": "Internationale Überweisung via BIC/SWIFT", + "accountFormatLabel": "Kontonummernformat", + "accountFormatIban": "IBAN", + "accountFormatOther": "Andere Kontonummer", + "accountNumber": "SWIFT-Kontonummer", + "accountNumberPlaceholder": "Kontonummer eingeben", + "bankAddressSection": "Bankadresse", + "complianceSection": "SWIFT-Compliance", + "categoryLabel": "Empfängerbeziehung", + "categoryPlaceholder": "Beziehung auswählen…", + "categoryClient": "Kunde", + "categoryParentCompany": "Muttergesellschaft", + "categorySubsidiary": "Tochtergesellschaft", + "categorySupplier": "Lieferant", + "purposeLabel": "Verwendungszweck", + "purposeIntraGroup": "Konzerninterne Überweisung", + "purposeInvoice": "Rechnung für Waren und Dienstleistungen", + "businessDescription": "Geschäftsbeschreibung", + "businessDescriptionPlaceholder": "Beschreiben Sie den geschäftlichen Zweck dieser Zahlung", + "businessDescriptionHint": "Beschreiben Sie kurz, warum dieses Auszahlungskonto verwendet wird." },🤖 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 `@packages/i18n/src/messages/de.json` around lines 744 - 747, The de.json file is missing 19 SWIFT-related translation keys under the swift object that are required by the BankingTab.payouts.addDialog.swift dialog. Add the missing keys (accountFormatLabel, accountNumber, bankAddressSection, complianceSection, purposeInvoice, and 14 others) to the swift object in de.json with appropriate German translations to match the functionality of the SWIFT payment method dialog and resolve the i18n verification failure.Source: Pipeline failures
packages/i18n/src/messages/es.json (1)
2934-2937:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winRestore full
BankingTab.payouts.addDialog.swiftkey parity to unblock i18n verification.
verify:messagesis currently failing becausees.jsonis missing requiredswiftkeys (19 total missing in this subtree), which blocks CI. Add the full key set expected byverify-banking-tab-parity.mjs(at least the reported missing keys) and sync with the source locale.Suggested patch starter
"swift": { "code": "SWIFT", - "hint": "Transferencia internacional vía BIC/SWIFT" + "hint": "Transferencia internacional vía BIC/SWIFT", + "accountFormatLabel": "Formato de cuenta", + "accountNumber": "Número de cuenta", + "bankAddressSection": "Dirección del banco", + "complianceSection": "Información de cumplimiento", + "purposeInvoice": "Propósito / número de factura" },As per coding guidelines,
packages/i18n/**message files must stay consistent across locales with no missing keys.🤖 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 `@packages/i18n/src/messages/es.json` around lines 2934 - 2937, The es.json file is missing 19 required keys under the swift subtree within BankingTab.payouts.addDialog, causing the verify:messages check to fail. Locate the source locale's swift key structure in the i18n messages (likely en.json or similar), identify all missing keys that exist in the source but not in es.json under the swift section, and add each missing key with appropriate Spanish translations to es.json to achieve full parity with the source locale structure as required by verify-banking-tab-parity.mjs.Sources: Coding guidelines, Pipeline failures
packages/i18n/src/messages/fr.json (1)
2930-2940:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore missing
BankingTab.payouts.addDialog.swiftkeys in French locale
verify:messagesis failing becausefr.jsonis missing keys in this subtree (pipeline reports 19 missing keys, e.g.payouts.addDialog.swift.accountFormatLabel,accountNumber,bankAddressSection,complianceSection,purposeInvoice). This blocks merge and can surface missing-message failures in the payout dialog. Please sync the fullswiftkey set from the source locale (en.json) and translate all missing entries.As per coding guidelines, “Message files are consistent across locales (no missing keys)”.
🤖 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 `@packages/i18n/src/messages/fr.json` around lines 2930 - 2940, The French locale file is missing multiple keys under the payouts.addDialog.swift subtree that exist in the English source locale, including accountFormatLabel, accountNumber, bankAddressSection, complianceSection, and purposeInvoice. Locate the complete swift key set in the source locale and add all missing keys to the French locale file under the same structure, then provide appropriate French translations for each missing entry to ensure consistency across all locales as required by the coding guidelines.Sources: Coding guidelines, Pipeline failures
packages/i18n/src/messages/pt.json (1)
2932-2935:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd the missing
payouts.addDialog.swift.*translation keys to restore locale parity.
verify:messagesis failing becausept.jsonis missing required keys underBankingTab.payouts.addDialog.swift(19 missing, includingaccountFormatLabel,accountNumber,bankAddressSection,complianceSection,purposeInvoice). This is currently blocking CI.As per coding guidelines, message files must stay consistent across locales with no missing keys.
🤖 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 `@packages/i18n/src/messages/pt.json` around lines 2932 - 2935, The pt.json file is missing 19 translation keys under the payouts.addDialog.swift object, including accountFormatLabel, accountNumber, bankAddressSection, complianceSection, and purposeInvoice. To fix this, reference the complete payouts.addDialog.swift key structure from another complete locale file (such as en.json) and add all missing keys to pt.json with appropriate Portuguese translations. Ensure all keys that exist in the reference locale are present in pt.json to maintain consistency across locales and satisfy the verify:messages validation.Sources: Coding guidelines, Pipeline failures
♻️ Duplicate comments (2)
packages/core/src/banking/server/create-space-bank-payout-account.ts (1)
96-100:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIdempotency key still embeds raw bank identifiers
accountNumber/iban/routingNumberare directly included in the key, which can leak sensitive financial data through request logs and telemetry. Use a deterministic one-way fingerprint instead.Suggested patch
+import { createHash } from 'node:crypto'; ... - const externalAccountIdempotencyKey = `ea:${customerId}:${ - input.railKey - }:${sourceCurrency}:${ - input.accountNumber ?? input.iban ?? input.routingNumber ?? 'unknown' - }`; + const idempotencyFingerprint = createHash('sha256') + .update( + [ + input.accountNumber ?? '', + input.iban ?? '', + input.routingNumber ?? '', + input.sortCode ?? '', + ] + .join('|') + .toLowerCase(), + ) + .digest('hex'); + + const externalAccountIdempotencyKey = `ea:${customerId}:${input.railKey}:${sourceCurrency}:${idempotencyFingerprint}`;🤖 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 `@packages/core/src/banking/server/create-space-bank-payout-account.ts` around lines 96 - 100, The externalAccountIdempotencyKey construction embeds raw sensitive bank identifiers (accountNumber, iban, routingNumber) directly into the key string, which can expose financial data in logs and telemetry. Replace the direct concatenation of the bank identifier (currently using a fallback chain of input.accountNumber ?? input.iban ?? input.routingNumber ?? 'unknown') with a deterministic one-way hash or fingerprint of that identifier, ensuring the key remains idempotent while avoiding exposure of sensitive financial information.packages/core/src/banking/validation.ts (1)
187-187:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlank
destinationCurrencyis still accepted and overrides rail defaultsWhitespace-only input is trimmed to
'', thencreate-space-bank-payout-account.tstreats it as a provided value instead of falling back torail.destinationCurrency, producing invalid downstream payloads.Suggested patch
- destinationCurrency: z.string().trim().optional(), + destinationCurrency: z.string().trim().min(1).optional(),#!/bin/bash # Verify the validation-to-server contract path for blank destinationCurrency rg -n "destinationCurrency:\\s*z\\.string\\(\\)\\.trim\\(\\)\\.optional\\(\\)" packages/core/src/banking/validation.ts rg -n "input\\.destinationCurrency\\?\\.toLowerCase\\(\\)\\s*\\?\\?\\s*rail\\.destinationCurrency" packages/core/src/banking/server/create-space-bank-payout-account.ts🤖 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 `@packages/core/src/banking/validation.ts` at line 187, The `destinationCurrency` field in the validation schema uses `.trim().optional()` which converts whitespace-only input to an empty string, and this empty string is then treated as a provided value instead of undefined in create-space-bank-payout-account.ts, preventing fallback to rail.destinationCurrency. Add a `.pipe()` with a `.refine()` or use `.transform()` to convert empty strings to undefined after trimming, so that whitespace-only input properly falls back to the rail default value instead of being treated as a valid provided value.
🤖 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 `@packages/core/src/banking/validation.ts`:
- Line 191: The swiftIbanCountry field in the validation schema is marked as
optional but creates an unnecessary blocking requirement for the SWIFT-IBAN flow
since downstream code (CreateSpaceBankPayoutAccountInput and provider adapters)
does not consume this field and derives the country directly from the IBAN
instead. Remove the swiftIbanCountry field requirement from the validation
schema or ensure it does not create mandatory constraints in the SWIFT-IBAN
flow. Also review and apply the same fix to the related validation logic
referenced at lines 262-268 to maintain consistency across the codebase.
In `@packages/epics/src/banking/components/add-payout-account-dialog.tsx`:
- Around line 983-984: The country selector in the payout account dialog at line
983-984 and the review lookup at lines 1349-1351 are currently limited to
SEPA_COUNTRIES when building the country options. Since SWIFT is an
international standard that supports non-SEPA correspondent banks, replace the
SEPA_COUNTRIES reference with a comprehensive list of all available countries.
Update the .map() function that iterates over SEPA_COUNTRIES to instead iterate
over all countries to enable selection and validation of valid non-SEPA banks
for SWIFT transfers.
- Around line 335-344: The BIC field validation is currently only required when
swiftAccountFormat is not 'iban', but the BIC field is rendered and required for
all SWIFT payout formats. Move the BIC validation check (if (!bic.trim())
errs.bic = REQUIRED_MSG;) outside of the else block so that it validates the BIC
field regardless of whether the swiftAccountFormat is 'iban' or 'other'. This
ensures that users cannot submit the form without providing a BIC value for any
SWIFT account format.
- Around line 1381-1391: The swiftCategory and swiftPurposeOfFunds values
displayed in the ReviewRow components are showing raw enum strings instead of
localized text. Replace the direct value assignments with translations by
mapping each enum value to its corresponding translation key using the
translation function. For swiftCategory, translate the single value before
passing it to the ReviewRow label prop. For swiftPurposeOfFunds, map each item
in the array to its translated equivalent before joining them with commas. This
ensures all user-facing SWIFT field values comply with the i18n requirement for
the packages/epics directory.
In `@packages/i18n/src/messages/en.json`:
- Around line 669-689: The new SWIFT payment method translation keys added to
en.json (starting with "code": "SWIFT (USD)" and ending with
"businessDescriptionHint": "Brief description of the business relationship and
purpose of transfer.") are missing from the corresponding locale files. Add all
the same key-value pairs under the swift section in de.json, es.json, fr.json,
and pt.json files to maintain locale parity and resolve the
verify-banking-tab-parity check failure. Ensure the keys are identical to the
English version and provide appropriate translations for the values in each
respective language.
---
Outside diff comments:
In `@packages/i18n/src/messages/de.json`:
- Around line 744-747: The de.json file is missing 19 SWIFT-related translation
keys under the swift object that are required by the
BankingTab.payouts.addDialog.swift dialog. Add the missing keys
(accountFormatLabel, accountNumber, bankAddressSection, complianceSection,
purposeInvoice, and 14 others) to the swift object in de.json with appropriate
German translations to match the functionality of the SWIFT payment method
dialog and resolve the i18n verification failure.
In `@packages/i18n/src/messages/es.json`:
- Around line 2934-2937: The es.json file is missing 19 required keys under the
swift subtree within BankingTab.payouts.addDialog, causing the verify:messages
check to fail. Locate the source locale's swift key structure in the i18n
messages (likely en.json or similar), identify all missing keys that exist in
the source but not in es.json under the swift section, and add each missing key
with appropriate Spanish translations to es.json to achieve full parity with the
source locale structure as required by verify-banking-tab-parity.mjs.
In `@packages/i18n/src/messages/fr.json`:
- Around line 2930-2940: The French locale file is missing multiple keys under
the payouts.addDialog.swift subtree that exist in the English source locale,
including accountFormatLabel, accountNumber, bankAddressSection,
complianceSection, and purposeInvoice. Locate the complete swift key set in the
source locale and add all missing keys to the French locale file under the same
structure, then provide appropriate French translations for each missing entry
to ensure consistency across all locales as required by the coding guidelines.
In `@packages/i18n/src/messages/pt.json`:
- Around line 2932-2935: The pt.json file is missing 19 translation keys under
the payouts.addDialog.swift object, including accountFormatLabel, accountNumber,
bankAddressSection, complianceSection, and purposeInvoice. To fix this,
reference the complete payouts.addDialog.swift key structure from another
complete locale file (such as en.json) and add all missing keys to pt.json with
appropriate Portuguese translations. Ensure all keys that exist in the reference
locale are present in pt.json to maintain consistency across locales and satisfy
the verify:messages validation.
---
Duplicate comments:
In `@packages/core/src/banking/server/create-space-bank-payout-account.ts`:
- Around line 96-100: The externalAccountIdempotencyKey construction embeds raw
sensitive bank identifiers (accountNumber, iban, routingNumber) directly into
the key string, which can expose financial data in logs and telemetry. Replace
the direct concatenation of the bank identifier (currently using a fallback
chain of input.accountNumber ?? input.iban ?? input.routingNumber ?? 'unknown')
with a deterministic one-way hash or fingerprint of that identifier, ensuring
the key remains idempotent while avoiding exposure of sensitive financial
information.
In `@packages/core/src/banking/validation.ts`:
- Line 187: The `destinationCurrency` field in the validation schema uses
`.trim().optional()` which converts whitespace-only input to an empty string,
and this empty string is then treated as a provided value instead of undefined
in create-space-bank-payout-account.ts, preventing fallback to
rail.destinationCurrency. Add a `.pipe()` with a `.refine()` or use
`.transform()` to convert empty strings to undefined after trimming, so that
whitespace-only input properly falls back to the rail default value instead of
being treated as a valid provided value.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: eca6f949-31c8-481c-9279-c857086dcc78
📒 Files selected for processing (14)
packages/core/src/banking/server/create-space-bank-payout-account.tspackages/core/src/banking/server/providers/bridge/adapter.tspackages/core/src/banking/server/providers/types.tspackages/core/src/banking/types.tspackages/core/src/banking/validation.tspackages/core/src/common/server/bridge-client.tspackages/epics/src/banking/components/add-payout-account-dialog.tsxpackages/epics/src/banking/components/address-form-fields.tsxpackages/epics/src/banking/hooks/types.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
- step3Description (compliance step), step4Description (review step) - formHasErrors validation summary message - confirm button → simplified to locale equivalent of "Submit" - swift.code updated to "SWIFT (USD)" - swift.* sub-keys: accountFormat, bankAddressSection, complianceSection, category (label/placeholder/4 options), purpose (label/2 options), businessDescription (label/placeholder/hint) Verified: pnpm verify:messages passes — 449 BankingTab keys match across all 5 locale files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds account_owner_type toggle (business/individual) to the payout account dialog. For individual accounts Bridge requires first_name and last_name separately; account_owner_name is derived automatically. For business accounts the existing accountOwnerName + businessName fields are unchanged. Types updated end-to-end: RegisterExternalAccountInput, CreateSpaceBankPayoutAccountInput, CreatePayoutAccountInput, create-space-bank-payout-account, Bridge adapter. i18n updated in all five languages (en/de/es/fr/pt). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/epics/src/banking/hooks/types.ts (1)
217-256:⚠️ Potential issue | 🔴 CriticalFix critical type mismatch:
firstNameandlastNamefields are not in server validation schema but are being sent by the client.The
CreatePayoutAccountInputtype includes optionalfirstNameandlastNamefields that the dialog collects and sends to the API, but the server-side validation schema (schemaCreatePayoutAccountinpackages/core/src/banking/validation.ts) does not include these fields. Since the schema uses.strict()mode, the API will reject requests containing these fields with a validation error, breaking the user flow.Either:
- Add
firstNameandlastNameto the server validation schema if they should be accepted, or- Remove these fields from
CreatePayoutAccountInputand the dialog if they shouldn't be collected.The type definition should also be consolidated: instead of duplicating
CreatePayoutAccountInputin epics, importCreatePayoutAccountBodyfrom@hypha-platform/core/banking/validation(inferred fromschemaCreatePayoutAccount) to ensure client and server stay synchronized.🤖 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 `@packages/epics/src/banking/hooks/types.ts` around lines 217 - 256, The `CreatePayoutAccountInput` type includes optional `firstName` and `lastName` fields that are not present in the server validation schema `schemaCreatePayoutAccount` which uses strict mode, causing API validation failures. Either add `firstName` and `lastName` fields to the server validation schema in packages/core/src/banking/validation.ts if they should be accepted, or remove these fields from the `CreatePayoutAccountInput` type definition and update the client dialog to not collect them. Additionally, import `CreatePayoutAccountBody` from the core banking validation module instead of maintaining a duplicate type definition in epics to ensure client and server schemas remain synchronized.packages/epics/src/banking/components/add-payout-account-dialog.tsx (2)
348-354:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate that the selected SWIFT IBAN country matches the IBAN prefix.
A valid
DE...IBAN can currently be submitted withswiftIbanCountry: 'FRA', because the country is only required and then sent independently. Reject mismatches before moving to compliance so the provider payload is internally consistent.Based on the PR context,
country-data.tsalready exposesibanToAlpha3for this normalization.Proposed fix
-import { COUNTRIES, SEPA_COUNTRIES } from '../country-data'; +import { COUNTRIES, SEPA_COUNTRIES, ibanToAlpha3 } from '../country-data';} else if (selectedCurrency === 'swift') { if (swiftAccountFormat === 'iban') { - if (!iban.trim()) { + const ibanValue = iban.trim().replace(/\s/g, ''); + if (!ibanValue) { errs.iban = REQUIRED_MSG; - } else if (!isValidIban(iban.trim().replace(/\s/g, ''))) { + } else if (!isValidIban(ibanValue)) { errs.iban = 'Invalid IBAN — check the number and try again'; } - if (!swiftIbanCountry) errs.swiftIbanCountry = REQUIRED_MSG; + const ibanCountry = ibanToAlpha3(ibanValue); + if (!swiftIbanCountry) { + errs.swiftIbanCountry = REQUIRED_MSG; + } else if (ibanCountry && ibanCountry !== swiftIbanCountry) { + errs.swiftIbanCountry = t('swift.ibanCountryMismatch'); + } } else {Also applies to: 428-431
🤖 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 `@packages/epics/src/banking/components/add-payout-account-dialog.tsx` around lines 348 - 354, In the IBAN validation block where swiftAccountFormat equals 'iban', after confirming the IBAN is valid with isValidIban(), add a check to ensure the country code extracted from the IBAN matches the selected swiftIbanCountry value. Use the ibanToAlpha3 function from country-data.ts to extract the country code from the IBAN string and compare it against swiftIbanCountry. If they don't match, assign an error message to errs.swiftIbanCountry or create a new error property to indicate the mismatch. Apply the same validation logic at the second location mentioned in the diff (around lines 428-431) where similar IBAN country validation occurs.
778-808:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winExpose these button groups as real radio controls.
Both groups declare
role="radiogroup", but the children are plain buttons withoutrole="radio"oraria-checked, so assistive tech cannot determine the selected option. Use native radios or add proper radio semantics.Minimal semantic fix
<button key={fmt} type="button" + role="radio" + aria-checked={swiftAccountFormat === fmt} disabled={isSubmitting}<button key={type} type="button" + role="radio" + aria-checked={accountOwnerType === type} disabled={isSubmitting}Also applies to: 937-968
🤖 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 `@packages/epics/src/banking/components/add-payout-account-dialog.tsx` around lines 778 - 808, The button elements within the radiogroup div lack proper ARIA semantics. Add role="radio" to each button element inside the map function, and add aria-checked attribute that evaluates to true when swiftAccountFormat === fmt and false otherwise. This same fix needs to be applied to both radiogroup instances mentioned in the comment (the SWIFT account format buttons around line 778 and the other radiogroup around line 937).
♻️ Duplicate comments (1)
packages/i18n/src/messages/es.json (1)
2857-2857:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse consistent "tesorería" terminology and fix copy phrasing in payout descriptions.
On line 2857 and lines 2893-2895, the new copy uses "tesoro" and awkward phrasing ("envía el fiat el tesoro"), while the rest of this locale consistently uses "tesorería" (e.g., lines 12, 46). This will read inconsistently in-product.
Suggested copy adjustment
- "description": "Registre dónde envía el fiat el tesoro tras una propuesta aprobada. Cada cuenta obtiene una dirección de liquidación para propuestas de Financiación o Gastos." + "description": "Registre dónde envía la tesorería el dinero fiat tras una propuesta aprobada. Cada cuenta obtiene una dirección de liquidación para propuestas de financiación o gastos." ... - "sourceCurrencyLabel": "Token del tesoro", - "sourceCurrencySection": "Token del tesoro", - "sourceCurrencyHint": "Seleccione qué token enviará el tesoro a esta cuenta.", + "sourceCurrencyLabel": "Token de tesorería", + "sourceCurrencySection": "Token de tesorería", + "sourceCurrencyHint": "Seleccione qué token enviará la tesorería a esta cuenta.",As per coding guidelines, "packages/i18n/**" translations should remain consistent and well-structured across user-facing messages.
Also applies to: 2893-2895
🤖 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 `@packages/i18n/src/messages/es.json` at line 2857, In the Spanish translation file es.json, the payout description around line 2857 and the related descriptions on lines 2893-2895 use inconsistent terminology ("tesoro" instead of "tesorería") and contain awkward phrasing ("envía el fiat el tesoro") that does not match the natural Spanish used elsewhere in the file. Replace all instances of "tesoro" with "tesorería" in these sections and rephrase the text to use natural Spanish phrasing consistent with other messages in the locale, such as the existing references to "tesorería" on lines 12 and 46.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@packages/epics/src/banking/components/add-payout-account-dialog.tsx`:
- Around line 348-354: In the IBAN validation block where swiftAccountFormat
equals 'iban', after confirming the IBAN is valid with isValidIban(), add a
check to ensure the country code extracted from the IBAN matches the selected
swiftIbanCountry value. Use the ibanToAlpha3 function from country-data.ts to
extract the country code from the IBAN string and compare it against
swiftIbanCountry. If they don't match, assign an error message to
errs.swiftIbanCountry or create a new error property to indicate the mismatch.
Apply the same validation logic at the second location mentioned in the diff
(around lines 428-431) where similar IBAN country validation occurs.
- Around line 778-808: The button elements within the radiogroup div lack proper
ARIA semantics. Add role="radio" to each button element inside the map function,
and add aria-checked attribute that evaluates to true when swiftAccountFormat
=== fmt and false otherwise. This same fix needs to be applied to both
radiogroup instances mentioned in the comment (the SWIFT account format buttons
around line 778 and the other radiogroup around line 937).
In `@packages/epics/src/banking/hooks/types.ts`:
- Around line 217-256: The `CreatePayoutAccountInput` type includes optional
`firstName` and `lastName` fields that are not present in the server validation
schema `schemaCreatePayoutAccount` which uses strict mode, causing API
validation failures. Either add `firstName` and `lastName` fields to the server
validation schema in packages/core/src/banking/validation.ts if they should be
accepted, or remove these fields from the `CreatePayoutAccountInput` type
definition and update the client dialog to not collect them. Additionally,
import `CreatePayoutAccountBody` from the core banking validation module instead
of maintaining a duplicate type definition in epics to ensure client and server
schemas remain synchronized.
---
Duplicate comments:
In `@packages/i18n/src/messages/es.json`:
- Line 2857: In the Spanish translation file es.json, the payout description
around line 2857 and the related descriptions on lines 2893-2895 use
inconsistent terminology ("tesoro" instead of "tesorería") and contain awkward
phrasing ("envía el fiat el tesoro") that does not match the natural Spanish
used elsewhere in the file. Replace all instances of "tesoro" with "tesorería"
in these sections and rephrase the text to use natural Spanish phrasing
consistent with other messages in the locale, such as the existing references to
"tesorería" on lines 12 and 46.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 57d86001-34cc-47b8-a340-d011986de78f
📒 Files selected for processing (11)
packages/core/src/banking/server/create-space-bank-payout-account.tspackages/core/src/banking/server/providers/bridge/adapter.tspackages/core/src/banking/server/providers/types.tspackages/core/src/banking/types.tspackages/epics/src/banking/components/add-payout-account-dialog.tsxpackages/epics/src/banking/hooks/types.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
…ail env filter - Add NEXT_PUBLIC_BANKING_SUPPORTED_PAYOUT_RAILS: UI filter via getEnabledPayoutCurrencyKeys(), server-side 403 enforcement in payout-accounts route; open-world default (empty = all enabled) - Add NEXT_PUBLIC_BANKING_SUPPORTED_DEPOSIT_RAILS: UI filter via getEnabledDepositCurrencies() applied inside getAvailableAddAccountRailOptions(), server-side 403 in accounts route - Gate payout currency selection by Bridge endorsement status (mirrors deposit side): getPayoutRailEndorsementStatus() looks up endorsementStatuses; isBankRailSelectable() gates each option; AddPayoutAccountDialog accepts status prop from BankingSection - Add endorsement field to BankPayoutRailConfig; export BANK_PAYOUT_RAILS + BANK_VIRTUAL_ACCOUNT_CURRENCIES from core/client.ts for client-side use in epics - Fix GBP payout validation: sort code must be exactly 6 digits (hyphens stripped server + adapter), account number must be exactly 8 digits; maxLength + inputMode enforced on form inputs; adapter normalises sort_code before Bridge API call - Fix schemaCreatePayoutAccount strict mode: add firstName/lastName as optional fields so EUR/GBP individual submissions are not rejected by .strict() - Remove Zod error details from 400 responses in both payout and accounts routes (server console logging is sufficient; avoids exposing internals to clients) - Update i18n (en/de/es/fr/pt): fix sortCodePlaceholder to 200000 (no hyphens), add gbpAccountNumberPlaceholder: 12345678 (exact 8-digit example) - Add tests: schemaCreatePayoutAccount (USD/GBP/EUR SEPA happy paths + field-level rejections, sort-code normalisation, individual/business account owner variants); adapter registerExternalAccount GBP sort-code strip + EUR SEPA business/individual Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (3)
apps/web/src/app/api/v1/spaces/[spaceSlug]/banking/payout-accounts/route.ts (3)
74-86:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap pre-body POST auth in the same structured error path.
authenticateBankCustomerRequestis still outside thetryblock, so thrown failures there bypass yourBankOnboardingError/500 handling path.
Based on prior review comments in this PR, this remains unresolved.🤖 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 `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/banking/payout-accounts/route.ts around lines 74 - 86, The authenticateBankCustomerRequest function call and its error check are currently outside the try-catch block, which means any errors thrown by this function bypass the structured error handling path. Move the authenticateBankCustomerRequest call and its conditional check (the if block that checks !authResult.ok) inside the try block, right before or at the beginning of the block where request.json() is called, so all authentication and body parsing errors follow the same error handling flow.
35-45:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse shared v1 pagination contracts for this list endpoint.
GET still parses pagination ad hoc and returns a custom shape instead of the shared
PaginationParams/PaginatedResponse<T>contract required forapp/api/v1/**.
As per coding guidelines: “apps/web/src/app/api/v1/**... All list endpoints must usePaginatedResponse<T>andPaginationParamsfromcore/common.”🤖 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 `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/banking/payout-accounts/route.ts around lines 35 - 45, The GET endpoint for payout-accounts is manually parsing pagination parameters and likely returning a custom response shape instead of using the shared v1 pagination contracts. Replace the ad hoc parsing of limit and starting_after from searchParams with the PaginationParams contract imported from core/common, pass this to getSpaceBankPayoutAccounts, and wrap the returned result with PaginatedResponse<T> before returning to the client. This ensures the endpoint complies with the required v1 API contracts.Source: Coding guidelines
35-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp
limitto a safe positive range before provider calls.
Number.isFinite(limit)still accepts0, negative values, and very large integers. Please bound it (e.g.,1..100) before callinggetSpaceBankPayoutAccounts.
Based on prior review comments in this PR, this remains unresolved.🤖 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 `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/banking/payout-accounts/route.ts around lines 35 - 43, The `limit` parameter validation using `Number.isFinite(limit)` does not properly constrain the value to a safe positive range, as it still accepts 0, negative values, and very large integers. Replace the conditional `Number.isFinite(limit) ? limit : 25` check with a bounds validation that clamps the limit to a safe range (for example, between 1 and 100) before passing it to `getSpaceBankPayoutAccounts`. This ensures only valid pagination sizes are accepted.
🤖 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 `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/banking/accounts/route.ts:
- Around line 103-110: The supported deposit rails validation is case-sensitive,
which can cause valid currency deposits to be rejected if the environment
variable contains mixed-case values. Normalize both the supported currencies
from the environment variable and the incoming currency value to lowercase
before performing the set comparison. Specifically, when processing the
supportedRailsEnv split values and when checking if the parsed.data.currency
exists in the supportedCurrencies set, convert both to lowercase to ensure
case-insensitive matching.
In `@packages/core/src/banking/constants.ts`:
- Around line 275-276: The endorsement field in the banking constants file is
typed as a generic string, which loses compile-time type safety. Narrow the
endorsement field type from string to a literal union of the valid endorsement
values (such as 'VALUE1' | 'VALUE2' | 'VALUE3') that this payout rail actually
supports. This change will prevent accidental invalid values from being assigned
at compile time while maintaining the same runtime behavior.
In `@packages/epics/src/banking/banking-ui.ts`:
- Around line 341-345: The getEnabledDepositCurrencies() function does not
normalize the case of currency codes when parsing the
NEXT_PUBLIC_BANKING_SUPPORTED_DEPOSIT_RAILS environment variable, causing
mixed-case values to fail matching against BANK_VIRTUAL_ACCOUNT_CURRENCIES. To
fix this, convert the currency codes to a consistent case (lowercase) when
creating the allowed Set from the split environment variable, and ensure the
same case normalization is applied to the currency codes during the filter
comparison with allowed.has(c).
In `@packages/epics/src/banking/components/payout-currency-option-row.tsx`:
- Around line 52-55: The environment variable values for
NEXT_PUBLIC_BANKING_SUPPORTED_PAYOUT_RAILS are not being normalized to a
consistent case before comparison, causing mixed-case entries like USD_ACH to
fail matching and unexpectedly disable payout options. Normalize the values when
creating the allowed Set by converting them to a consistent case (typically
lowercase) using a case-normalization function, and ensure the same
normalization is applied to the result of payoutCurrencyToRailKey(c) when
filtering PAYOUT_CURRENCY_KEYS so both sides of the comparison use matching
case.
---
Duplicate comments:
In `@apps/web/src/app/api/v1/spaces/`[spaceSlug]/banking/payout-accounts/route.ts:
- Around line 74-86: The authenticateBankCustomerRequest function call and its
error check are currently outside the try-catch block, which means any errors
thrown by this function bypass the structured error handling path. Move the
authenticateBankCustomerRequest call and its conditional check (the if block
that checks !authResult.ok) inside the try block, right before or at the
beginning of the block where request.json() is called, so all authentication and
body parsing errors follow the same error handling flow.
- Around line 35-45: The GET endpoint for payout-accounts is manually parsing
pagination parameters and likely returning a custom response shape instead of
using the shared v1 pagination contracts. Replace the ad hoc parsing of limit
and starting_after from searchParams with the PaginationParams contract imported
from core/common, pass this to getSpaceBankPayoutAccounts, and wrap the returned
result with PaginatedResponse<T> before returning to the client. This ensures
the endpoint complies with the required v1 API contracts.
- Around line 35-43: The `limit` parameter validation using
`Number.isFinite(limit)` does not properly constrain the value to a safe
positive range, as it still accepts 0, negative values, and very large integers.
Replace the conditional `Number.isFinite(limit) ? limit : 25` check with a
bounds validation that clamps the limit to a safe range (for example, between 1
and 100) before passing it to `getSpaceBankPayoutAccounts`. This ensures only
valid pagination sizes are accepted.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 287b7a85-7ba5-4f09-a893-53c043bad3bc
📒 Files selected for processing (21)
apps/web/.env.templateapps/web/src/app/api/v1/spaces/[spaceSlug]/banking/accounts/route.tsapps/web/src/app/api/v1/spaces/[spaceSlug]/banking/payout-accounts/route.tspackages/core/src/banking/__tests__/validation.test.tspackages/core/src/banking/constants.tspackages/core/src/banking/server/providers/bridge/__tests__/adapter.test.tspackages/core/src/banking/server/providers/bridge/adapter.tspackages/core/src/banking/server/providers/types.tspackages/core/src/banking/types.tspackages/core/src/banking/validation.tspackages/core/src/client.tspackages/epics/src/banking/banking-ui.tspackages/epics/src/banking/components/add-payout-account-dialog.tsxpackages/epics/src/banking/components/banking-section.tsxpackages/epics/src/banking/components/payout-currency-option-row.tsxpackages/epics/src/banking/hooks/types.tspackages/i18n/src/messages/de.jsonpackages/i18n/src/messages/en.jsonpackages/i18n/src/messages/es.jsonpackages/i18n/src/messages/fr.jsonpackages/i18n/src/messages/pt.json
…vings toggle
- Fix EUR last-4 display: BridgeExternalAccountResponse now types iban
as { last_4?, country? } at the top level (matching actual Bridge
response). readExternalAccountLast4 checks response.iban?.last_4 as
third fallback so SEPA accounts always show ••••XXXX
- Remove masked account number from PayoutAccountCard subheading;
last-4 now only shows in the PayoutAccountDetailDialog
- Detail dialog: remove masked account from currency header box;
use 'IBAN' label for sepa rail, 'Account Number' for USD/GBP
- Add checking/savings toggle to USD ACH payout form (default:
Checking); wired through CreatePayoutAccountInput → adapter →
RegisterExternalAccountResult → BankPayoutAccountPublic
- Detail dialog: show account type (Checking / Savings) for USD accounts
- i18n: add checkingOrSavings/Checking/Savings keys to all 5 locales
- Fix Prettier on 8 files (6 from CI + 2 pre-existing)
- map-bridge-resources tests: +2 (iban.last_4 extraction, checkingOrSavings)
Banking tests: 394 pass
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ype; clamp GET limit; move POST auth inside try-catch Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ble fields until country chosen Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… case-insensitive env rails; i18n required/invalidIban Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…T country picker, a11y - de.json: translate Sort code → Sortiercode (Sort code), USD Wire → Überweisung (Wire) - es.json: replace inconsistent 'tesoro' with 'tesorería' in payout descriptions - pt.json: translate 'Routing number' → 'Número de roteamento (Routing number)' - SWIFT IBAN country picker: expand from SEPA_COUNTRIES to all COUNTRIES - SWIFT review: expand country lookup to all COUNTRIES - bank-accounts-section: extend payout tooltip to handle all disabled reasons - bank-accounts-section: replace <span title> with accessible Tooltip component Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
… add swift Add 'swift' to bridgeEndorsementSchema so it covers all Bridge endorsement types (same pattern as 'cards' — valid type, WIP on Bridge side). Use BridgeEndorsement as the type for BankPayoutRailConfig.endorsement instead of a loose string. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #2289
Summary by CodeRabbit