Skip to content

feat(payment): add read-only imperative commands - #2276

Draft
aidandaly24 wants to merge 6 commits into
refactorfrom
feat/payment-read-only
Draft

feat(payment): add read-only imperative commands#2276
aidandaly24 wants to merge 6 commits into
refactorfrom
feat/payment-read-only

Conversation

@aidandaly24

@aidandaly24 aidandaly24 commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Description

First layer of the AgentCore Payments command stack, targeting refactor.
The dependent CUD layer is #2277.

Adds 11 project-free, headless read commands:

  • payment manager get|list
  • payment connector get|list
  • payment session get|list
  • payment instrument get|list|balance
  • identity payment-credential-provider get|list

Primary selectors use IDs. For session/instrument reads, Core resolves the manager with
GetPaymentManager in 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. The CUD layer builds on this branch; read-only TUI is a later PR.

Related Issue

Part of #2272. The dependent CUD PR completes the headless scope.

Documentation PR

N/A. README command tree, read examples, and permission requirements are updated here.

Type of Change

  • Bug fix
  • New feature
  • Breaking change
  • Documentation update
  • Other (please describe):

Testing

  • RECORD=0 bun test --coverage --coverage-reporter=lcov: 3,240 passed, 0 failed across 228 files.
  • bun run typecheck
  • bun run lint:check, bun run format:check, bun run secrets:check
  • bun run build
  • Focused review of consolidated tests and comparison with earlier Runtime/Gateway/Identity PRs.
  • Linux, Windows, and macOS CI on 975d8192: https://github.com/aws/agentcore-cli/actions/runs/34532197010

Test source was consolidated from 1,475 to 561 added lines; the PR is now 2,233 additions
instead of 3,448. Production code is unchanged by this cleanup.
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 refactor Bun scripts rather than the npm scripts from the repository's main template.
No AWS resources were changed while preparing this split.

Checklist

  • I have read the CONTRIBUTING document
  • I have added any necessary tests that prove my fix is effective or my feature works
  • I have updated the documentation accordingly
  • I have added an appropriate example to the documentation to outline the feature, or no new docs are needed
  • My changes generate no new warnings
  • Any dependent changes have been merged and published

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the
terms of your choice.

@github-actions github-actions Bot added the size/xl PR size: XL label Sep 10, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added agentcore-harness-reviewing AgentCore Harness review in progress claude-security-reviewing Claude Code /security-review in progress labels Sep 10, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Sep 10, 2026

@agentcore-devx-automation agentcore-devx-automation Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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. PaymentClient sits cleanly on top of the shared control/data client factories. Data-plane methods route through a single sendData helper that (a) rejects ARN-shaped managerId before any client is configured, (b) fetches the manager fresh on every call, (c) refuses CUSTOM_JWT managers with a USER-sourced InputValidationError that explains the bearer-token limitation, and (d) surfaces a MalformedServiceResponseError when the service omits the ARN. Each of these branches is asserted in payment.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 omitted and empty variants). ARN selectors and the removed --manager-arn flag are explicitly rejected.
  • Big-number fidelity. GetPaymentInstrumentBalance responses are handed straight to the JSON renderer, and balance.test.tsx pins down that the atomic amount string beyond MAX_SAFE_INTEGER and its decimals survive 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 the eu-west-1 + custom endpoint case.
  • Test style. Mocks live at the SDK send boundary and everything else — router, CoreClient, JSON rendering, IO — runs for real. Fixture replay via fixtureFactories gives realistic wire shapes without over-mocking.
  • Command tree. payment.read.test.tsx asserts the exact nine-leaf shape (manager get/list, connector get/list, session get/list, instrument get/list/balance) and that no leaf accepts --wait/--browser or exposes TUI, matching the "CLI only for now" stance in the README.
  • Identity surface. Only get and list are wired for payment-credential-provider, consistent with the read-only scope; the create/update/delete methods added to CoreIdentityClient are unexposed but ready for a follow-up.
  • Connector hints. connector get emits the AUTHENTICATION_EXPIRED / AUTHENTICATION_FAILED warning only in non-JSON mode and never suggests write commands that don't exist yet — tested for both --json and bare invocations.

Nothing blocking. Ship it.

@agentcore-devx-automation agentcore-devx-automation Bot removed the agentcore-harness-reviewing AgentCore Harness review in progress label Sep 10, 2026
@codecov-commenter

codecov-commenter commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.49123% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.06%. Comparing base (9d8cdbd) to head (975d819).
⚠️ Report is 1 commits behind head on refactor.

Files with missing lines Patch % Lines
src/handlers/payment/instrument/balance/index.tsx 88.23% 8 Missing ⚠️
src/core/identity.tsx 70.00% 3 Missing ⚠️
src/handlers/payment/instrument/get/index.tsx 95.74% 2 Missing ⚠️
src/handlers/payment/instrument/list/index.tsx 95.55% 2 Missing ⚠️
src/handlers/payment/session/get/index.tsx 94.87% 2 Missing ⚠️
src/handlers/payment/connector/get/index.tsx 97.43% 1 Missing ⚠️
src/handlers/payment/connector/list/index.tsx 96.55% 1 Missing ⚠️
src/handlers/payment/session/list/index.tsx 97.36% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           refactor    #2276      +/-   ##
============================================
+ Coverage     97.04%   97.06%   +0.02%     
============================================
  Files           566      584      +18     
  Lines         39353    39918     +565     
============================================
+ Hits          38190    38747     +557     
- Misses         1163     1171       +8     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aidandaly24
aidandaly24 added this pull request to stack #2278 September 10, 2026 20:12
@github-actions github-actions Bot added size/xl PR size: XL and removed size/xl PR size: XL labels Sep 10, 2026
@github-actions github-actions Bot added size/xl PR size: XL and removed size/xl PR size: XL labels Sep 10, 2026
@agentcore-devx-automation agentcore-devx-automation Bot added the claude-security-reviewing Claude Code /security-review in progress label Sep 10, 2026
@github-actions github-actions Bot added size/xl PR size: XL and removed size/xl PR size: XL labels Sep 10, 2026
@agentcore-devx-automation

Copy link
Copy Markdown
Contributor

Claude Security Review: no high-confidence findings. (run)

@agentcore-devx-automation agentcore-devx-automation Bot removed the claude-security-reviewing Claude Code /security-review in progress label Sep 10, 2026
@github-actions github-actions Bot added size/xl PR size: XL and removed size/xl PR size: XL labels Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/xl PR size: XL

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants