feat(payment): add read-only imperative commands - #2276
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
This is a well-scoped read-only surface for AgentCore Payments. I reviewed the diff end-to-end and did not find issues that need to change before merging.
Highlights that made this easy to sign off on:
- Boundaries.
PaymentClientsits cleanly on top of the shared control/data client factories. Data-plane methods route through a singlesendDatahelper that (a) rejects ARN-shapedmanagerIdbefore any client is configured, (b) fetches the manager fresh on every call, (c) refusesCUSTOM_JWTmanagers with aUSER-sourcedInputValidationErrorthat explains the bearer-token limitation, and (d) surfaces aMalformedServiceResponseErrorwhen the service omits the ARN. Each of these branches is asserted inpayment.read.test.ts. - Handlers. Required-flag validation happens before Core is touched, and the tests confirm no SDK client is ever constructed on the failure path (both
omittedandemptyvariants). ARN selectors and the removed--manager-arnflag are explicitly rejected. - Big-number fidelity.
GetPaymentInstrumentBalanceresponses are handed straight to the JSON renderer, andbalance.test.tsxpins down that the atomicamountstring beyondMAX_SAFE_INTEGERand itsdecimalssurvive round-tripping, including the"0"case. The handler correctly does not convert service errors to a zero balance. - Region/endpoint propagation. Both control and data client factories receive the same
{ region, endpoint }derived from context; verified for theeu-west-1+ custom endpoint case. - Test style. Mocks live at the SDK
sendboundary and everything else — router, CoreClient, JSON rendering, IO — runs for real. Fixture replay viafixtureFactoriesgives realistic wire shapes without over-mocking. - Command tree.
payment.read.test.tsxasserts the exact nine-leaf shape (manager get/list,connector get/list,session get/list,instrument get/list/balance) and that no leaf accepts--wait/--browseror exposes TUI, matching the "CLI only for now" stance in the README. - Identity surface. Only
getandlistare wired forpayment-credential-provider, consistent with the read-only scope; the create/update/delete methods added toCoreIdentityClientare unexposed but ready for a follow-up. - Connector hints.
connector getemits theAUTHENTICATION_EXPIRED/AUTHENTICATION_FAILEDwarning only in non-JSON mode and never suggests write commands that don't exist yet — tested for both--jsonand bare invocations.
Nothing blocking. Ship it.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## refactor #2276 +/- ##
============================================
+ Coverage 97.06% 97.08% +0.02%
============================================
Files 569 587 +18
Lines 39322 39906 +584
============================================
+ Hits 38167 38743 +576
- Misses 1155 1163 +8 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
Description
First layer of the AgentCore Payments command stack, targeting
refactor.Mutation follow-ups #2277 (identity providers), #2280 (managers/connectors), and
#2281 (sessions/instruments) are temporarily closed with their branches preserved.
The current focus is the read CLI, followed by a separate read-only TUI PR.
Updated against
refactorat4a235162. Payment read behavior, required-field/TUIrouting conventions, and fixture recordings are unchanged.
Read help now distinguishes application-user scope from optional observability labels,
orders required selectors before optional inputs, and uses the existing Gateway/Identity
pagination wording. README examples connect manager selection, instrument list/get, and
balance. Flag names, schemas, defaults, request handling, and JSON output are unchanged.
Adds 11 project-free, headless read commands:
payment manager get|listpayment connector get|listpayment session get|listpayment instrument get|list|balanceidentity payment-credential-provider get|listPrimary selectors use IDs. For session/instrument reads, Core resolves the manager with
GetPaymentManagerin the configured region and supplies the returned ARN to the data plane.Callers therefore need the manager-read permission as well as the requested data-plane action.
JWT-only managers are rejected before data-plane access.
Balance requires an explicit chain, defaults to USDC, preserves atomic amount strings/decimals,
and propagates service errors rather than converting them to zero.
The layer includes read-side Core contracts, injected client wiring, read-only tests, and recorded
Get/List fixtures. It adds no payment mutation handlers, IAM provisioning, credential-write input
handling, or dedicated TUI screens.
Related Issue
Part of #2272. The preserved mutation branches cover the remaining headless scope.
Documentation PR
N/A. README command tree, read examples, and permission requirements are updated here.
Type of Change
Testing
RECORD=0 bun test: 3,259 passed, 0 failed across 229 files.bun run typecheckbun run lint:check,bun run format:check,bun run secrets:checkbun run build38c4d079: https://github.com/aws/agentcore-cli/actions/runs/34625569401Test source was consolidated from 1,475 to 561 added lines, reducing the read-layer snapshot
from 3,448 to 2,233 additions at that cleanup. The subsequent help polish adds no tests or fixtures.
Each read command retains fixture-backed root/Core coverage, list pagination is checked at the
SDK boundary, shared manager-resolution failures are tested once in Core, and balance keeps its
chain/default-token/precision/zero/error checks. Unused test-client configuration and write-only
fixtures were removed from this layer.
Uses the
refactorBun scripts rather than the npm scripts from the repository'smaintemplate.No AWS resources were changed while preparing this split.
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.