fix(cli): reconcile existing Grid CLI commands with the current API#709
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
|
@greptileai review |
Greptile SummaryThis PR reconciles the Grid CLI command layer with the current
Confidence Score: 5/5Safe to merge — every bug fix corrects a request that the API was already rejecting, so there is no regression risk, and the new boundary tests lock in the corrected request shapes. All five core bug fixes are straightforward field-name and discriminator corrections, each backed by a dedicated test. The new test harness correctly isolates env vars and process state per run. The only finding — two options silently dropped in a wrong-context combination — is a DX consistency gap that would not cause data corruption; the requests that reach the API would still be valid (the ignored option is simply absent). cli/src/commands/quotes.ts — the --fbo-customer and --payment-rail guard gap noted in the review comment.
|
| Filename | Overview |
|---|---|
| cli/src/commands/quotes.ts | Adds remittanceInformation, paymentRail, cryptoNetwork, fboCustomer, and scaFactor; fixes purposeOfPayment to top-level; adds guards for --crypto-network + --source-account. --fbo-customer and --payment-rail lack symmetric guards (silently dropped in wrong context). |
| cli/src/commands/customers.ts | Adds customerType discriminator, businessInfo routing, validateBusinessRequired, address validation, KYB fields, and new list filters. Logic is sound and well-tested. |
| cli/src/commands/accounts.ts | Migrates to currency-suffixed discriminators, adds beneficiary validation and building, platformAccountId, defaultUmaDepositAccount, and --type filter for internal-account list. Validation ordering is correct. |
| cli/src/commands/sandbox.ts | Fixes field names to receivingCurrencyAmount/receivingCurrencyCode, adds --sender-uma as required, adds --customer-id alternative, and adds mutual-exclusion guard. Correct and tested. |
| cli/src/commands/config.ts | Adds --supported-currencies and --embedded-wallet-config with JSON parsing and error handling; updates PlatformConfig interface. |
| cli/src/client.ts | Fixes error.details to extract the inner details object instead of the entire error envelope. Simple, correct, and covered by a test. |
| cli/test/helpers.ts | New test harness: stubs fetch, captures request details, isolates env vars and exitCode per test, and exposes buildProgram as the stable boundary. Clean design. |
| cli/src/validation.ts | Relaxes currency validation from a 13-entry allowlist to a format check (uppercase 2-10 char), with a clear comment explaining why. Intentional and correct. |
| cli/src/index.ts | Refactors from module-level program singleton to buildProgram() factory, enabling per-test program isolation. Uses require.main guard to avoid running on import. |
Reviews (3): Last reviewed commit: "feat(cli): fix drift on existing Grid CL..." | Re-trigger Greptile
Greptile SummaryA comprehensive reconciliation of every CLI command against the
Confidence Score: 4/5The change corrects a broad set of commands that were previously sending structurally invalid requests to the API; no changes touch authentication, data storage, or cross-service state. The core bug fixes are solid and well-tested. The two gaps in
|
| Filename | Overview |
|---|---|
| cli/src/commands/accounts.ts | Rewrites external-account creation to use currency-suffixed discriminators and adds beneficiary validation; isFiatAccountType uses .endsWith("_ACCOUNT") which could misidentify future crypto account types with a similar naming pattern. |
| cli/src/commands/quotes.ts | Fixes purposeOfPayment placement; adds remittanceInformation, paymentRail, cryptoNetwork, and SCA options; the REALTIME_FUNDING branch is triggered by --source-currency alone, and --crypto-network is silently dropped when --source-account is also provided. |
| cli/src/commands/customers.ts | Adds customerType discriminator to update (fixes API rejections), extends KYB fields, adds contact/nationality/id fields, and validates address completeness — all well-structured and tested. |
| cli/src/commands/sandbox.ts | Corrects field names to receivingCurrencyAmount/receivingCurrencyCode/senderUmaAddress; makes --sender-uma required; adds --customer-id as an alternative to --uma-address with mutual-exclusion guard. |
| cli/src/index.ts | Extracts program construction into buildProgram() (async factory) and guards main() execution with require.main === module, enabling test imports without auto-running the CLI. |
| cli/src/validation.ts | Replaces hard-coded currency allowlist with a format-only regex ^[A-Z0-9]{2,10}$, correctly deferring currency validity to the API while still catching lowercase inputs client-side. |
| cli/test/helpers.ts | Clean test harness that stubs fetch, captures the constructed HTTP request at the network boundary, and properly restores env/exitCode between tests. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant CLI (Commander)
participant Validator
participant GridClient
participant API
User->>CLI (Commander): argv args
CLI (Commander)->>Validator: validateCurrency / validateDate / validateAll
Validator-->>CLI (Commander): ValidationResult
alt Validation fails
CLI (Commander)-->>User: formatError process.exitCode=1
else Validation passes
CLI (Commander)->>GridClient: post/patch/get/delete(path, body/params)
GridClient->>GridClient: buildUrl filters undefined values
GridClient->>API: fetch request
API-->>GridClient: HTTP response
alt response.ok
GridClient-->>CLI (Commander): success true data
else error
GridClient-->>CLI (Commander): success false error
end
CLI (Commander)-->>User: outputResponse JSON or table
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant CLI (Commander)
participant Validator
participant GridClient
participant API
User->>CLI (Commander): argv args
CLI (Commander)->>Validator: validateCurrency / validateDate / validateAll
Validator-->>CLI (Commander): ValidationResult
alt Validation fails
CLI (Commander)-->>User: formatError process.exitCode=1
else Validation passes
CLI (Commander)->>GridClient: post/patch/get/delete(path, body/params)
GridClient->>GridClient: buildUrl filters undefined values
GridClient->>API: fetch request
API-->>GridClient: HTTP response
alt response.ok
GridClient-->>CLI (Commander): success true data
else error
GridClient-->>CLI (Commander): success false error
end
CLI (Commander)-->>User: outputResponse JSON or table
end
Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 3
cli/src/commands/accounts.ts:265-268
`isFiatAccountType` uses `.endsWith("_ACCOUNT")` as the sole criterion for requiring a beneficiary. A crypto account type that happens to share this suffix (e.g. a future `LIGHTNING_ACCOUNT`) would be incorrectly required to carry a beneficiary, causing the CLI to block valid wallet-type requests. An explicit allowlist of the known fiat discriminators is more resilient to API surface growth.
```suggestion
const FIAT_ACCOUNT_TYPES = new Set([
"USD_ACCOUNT", "MXN_ACCOUNT", "BRL_ACCOUNT", "EUR_ACCOUNT",
"INR_ACCOUNT", "NGN_ACCOUNT", "SGD_ACCOUNT",
]);
function isFiatAccountType(accountType: string): boolean {
// Fiat and SWIFT types use a currency-prefixed *_ACCOUNT discriminator.
// Crypto wallets (SPARK_WALLET, SOLANA_WALLET, …) never require a beneficiary.
return FIAT_ACCOUNT_TYPES.has(accountType);
}
```
### Issue 2 of 3
cli/src/commands/quotes.ts:95-106
The validation guards the direction "source-customer/crypto-network → need source-currency," but not the reverse. If a user passes only `--source-currency` (without `--source-account` or `--source-customer`), the branch condition on line 135 is truthy and a `REALTIME_FUNDING` source is emitted with no `customerId` — the CLI sends the request without any warning. If that body is invalid for the API, the user receives an opaque HTTP error rather than a clear CLI message.
```suggestion
// A realtime-funding source requires a currency.
if (
(options.sourceCustomer || options.cryptoNetwork) &&
!options.sourceAccount &&
!options.sourceCurrency
) {
validations.push({
valid: false,
error:
"--source-currency is required for a realtime-funding source (--source-customer)",
});
}
// --source-currency alone (without --source-account or --source-customer)
// would build a REALTIME_FUNDING source with no customerId.
if (
options.sourceCurrency &&
!options.sourceAccount &&
!options.sourceCustomer &&
!options.cryptoNetwork
) {
validations.push({
valid: false,
error:
"--source-currency requires --source-customer (or --source-account / --crypto-network) to identify the funding source",
});
}
```
### Issue 3 of 3
cli/src/commands/quotes.ts:128-143
**`--crypto-network` silently dropped with `--source-account`**
When a user passes both `--source-account` and `--crypto-network`, the `ACCOUNT` branch runs and `cryptoNetwork` is never included in the source object — the option is silently ignored. The existing validation only checks the case where `cryptoNetwork` is present but `sourceCurrency` is absent; it doesn't warn when `cryptoNetwork` is redundant with `sourceAccount`. A guard like `if (options.cryptoNetwork && options.sourceAccount)` with a clear error message would prevent this silent no-op.
Reviews (2): Last reviewed commit: "feat(cli): fix drift on existing Grid CL..." | Re-trigger Greptile
Reconcile every implemented command against the current 2025-10-13 schema. Bug fixes (commands that built invalid requests): - customers update: send required customerType discriminator; route individual/business fields under the correct variant - customers create (business): send required incorporatedOn - quotes create: move purposeOfPayment to the top-level field (was buried in senderCustomerInfo and ignored) - sandbox receive: use receivingCurrencyAmount/receivingCurrencyCode and require senderUmaAddress; add --customer-id - accounts external create: use currency-suffixed *_ACCOUNT discriminators, drop nonexistent NGN purposeOfPayment / US accountCategory, require beneficiary for fiat accounts Field/param drift added: - quotes create: remittanceInformation, paymentRail, cryptoNetwork, FBO customerId, scaFactor; quotes execute: scaFactor - transfers out: remittanceInformation, destination paymentRail - customers: KYB fields, contact fields, nationality, id fields, list filters - config update: supportedCurrencies, embeddedWalletConfig - accounts: internal list --type, platform-endpoint param trimming, external create swift/ifsc/rail, platformAccountId, defaultUmaDepositAccount - transactions list --account-identifier; approve receiverCustomerInfo - receiver lookup-account sender-uma/customer-id Also: relax client-side currency allowlist to a format check, map error.details to the schema's inner details object, refresh stale response interfaces, and add a vitest boundary-test harness (25 tests) covering the fixes.
0364499 to
5e3d38f
Compare
|
@greptileai review |

Summary
Reconciles every command the Grid CLI (
cli/) already implements against the current2025-10-13OpenAPI surface. Focuses on correctness of existing commands — net-new endpoints/domains (agents, cards, sca, auth, tokens, …) are out of scope and tracked as follow-ups.Bugs fixed (commands that built invalid requests)
customers update— now sends the requiredcustomerTypediscriminator and routes individual/business fields under the correct variant (previously every update was rejected).customers create(business) — sends the requiredincorporatedOn; also validates the requiredlegalName/taxId/incorporatedOnup front.quotes create—purposeOfPaymentmoved to the top-level field (was buried insenderCustomerInfoand silently ignored).sandbox receive— usesreceivingCurrencyAmount/receivingCurrencyCode(wereamount/currency, which don't exist), requiressenderUmaAddress, and adds--customer-id.accounts external create— uses the currency-suffixed*_ACCOUNTdiscriminators (USD_ACCOUNT,MXN_ACCOUNT,BRL_ACCOUNT,EUR_ACCOUNT,INR_ACCOUNT, …); drops the non-existent NGNpurposeOfPaymentand USaccountCategory; requires abeneficiary(with a name) for fiat accounts.Field / param drift added
quotes create:remittanceInformation,paymentRail,cryptoNetwork(stablecoin realtime funding), FBOcustomerId,scaFactor;quotes execute:scaFactor.transfers out:remittanceInformation, destinationpaymentRail.customers: KYB fields (NAICS, structured source-of-funds, expected-activity, counterparty countries, entity/business type, …), contact fields, nationality, id fields, and newlistfilters.config update:supportedCurrencies,embeddedWalletConfig.accounts: internallist --type, platform-endpoint param trimming, external-createswiftCode/ifsc/rail,platformAccountId,defaultUmaDepositAccount.transactions list --account-identifier;approvereceiverCustomerInfo.receiver lookup-account:sender-uma/customer-idquery params.Other
error.detailsnow maps the schema's innerdetailsobject instead of the whole error body.vitestboundary-test harness (30 tests) — the CLI had none. Tests drive each command through argv parsing withfetchstubbed and assert the constructed request; every bug fix and guard is covered.Breaking change
accounts external create --account-typevalues changed from the legacy rail names (US_ACCOUNT,CLABE,PIX,IBAN,UPI) to the API discriminators (USD_ACCOUNT,MXN_ACCOUNT,BRL_ACCOUNT,EUR_ACCOUNT,INR_ACCOUNT). The old values were already rejected by the API, so this is a fix rather than a regression. README updated; CLI version bumped 1.0.0 → 1.1.0.Test plan
cd cli && npm install && npm run build— clean strict-mode compile.npm test— 30 passing boundary tests.Original PR: #702