Skip to content

feat(payment): add session and instrument mutations - #2281

Closed
aidandaly24 wants to merge 5 commits into
feat/payment-manager-connectorfrom
feat/payment-session-instrument
Closed

feat(payment): add session and instrument mutations#2281
aidandaly24 wants to merge 5 commits into
feat/payment-manager-connectorfrom
feat/payment-session-instrument

Conversation

@aidandaly24

@aidandaly24 aidandaly24 commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Description

Final CUD layer of the payment stack, following manager/connector mutations in #2280.

Adds:

  • payment session create|delete
  • payment instrument create|delete

Includes session expiry/spend inputs, embedded-wallet shorthand or JSON inputs,
and user/connector/instrument scope. Manager-ID resolution, balance, IAM
provisioning, and provider/connector management are inherited from the lower layers.

The split preserved the pre-split implementation 10e4dacf byte-for-byte,
including production, tests, fixtures, and README. A subsequent naming-only
follow-up renamed the shared manager-ARN preflight helper to withPaymentManagerArn.
The latest upstream sync is 4a235162. Wallet JSON/shorthand conflicts use
assertMutuallyExclusiveFlags from #2263 pairwise, preserving valid combinations
of network, email, and phone flags. The existing test table covers each conflict.
No additional test suites or features were introduced.
Read-only/interactive TUI remains a separate follow-up.

Related Issue

Closes #2272.

Documentation PR

N/A. README command tree and session creation example are included.

Type of Change

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

Testing

  • RECORD=0 bun test: 3,367 passed across 236 files.
  • bun run typecheck, bun run lint:check, bun run format:check
  • bun run build, bun run secrets:check
  • Exact pre-rename split-tree comparison against 10e4dacf; existing regression coverage preserved.
  • Naming-only follow-up: 79 focused payment tests, typecheck, formatting, lint, and build passed.
  • Linux, Windows, and macOS CI on 5a583e26: https://github.com/aws/agentcore-cli/actions/runs/34625566083

Uses the repository's Bun scripts rather than the npm scripts from the main-branch template.
No AWS resources were changed or live mutation fixtures re-recorded.

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

PR #2280 is the outstanding dependency.


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

@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

Nice, self-contained addition. The new create/delete handlers for payment session and payment instrument mirror existing patterns in the module cleanly:

  • SDK enums are pinned with as const satisfies readonly ..., so a new PaymentInstrumentType, CryptoWalletNetwork, or Currency will surface as a compile-time diff.
  • Validation is done up-front in the handlers (including the nice detail of rejecting --instrument-details alongside shorthand flags before touching stdin, which the test asserts via readableLength).
  • max-spend is kept as a string all the way through to avoid float coercion; the tests explicitly cover "0" and "10.00" for that.
  • Tests use fixture-based CoreClients and only spy at the SDK send boundary or at core.payment.* where needed to assert short-circuit behavior — no excessive mocking, and lifecycle tests round-trip create → get → delete → post-delete ResourceNotFoundException through a real Core.
  • Wiring updates (session/index.tsx, instrument/index.tsx, payment.read.test.tsx command inventory, TestPaymentClient stubs, CorePaymentClient interface, WithPaymentManagerId inputs) are all consistent.

No serious issues to flag — LGTM to merge.

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

codecov-commenter commented Sep 11, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.26923% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.14%. Comparing base (c537d6e) to head (5a583e2).

Files with missing lines Patch % Lines
src/handlers/payment/instrument/delete/index.tsx 84.00% 8 Missing ⚠️
src/handlers/payment/instrument/create/index.tsx 95.37% 5 Missing ⚠️
src/handlers/payment/session/create/index.tsx 93.42% 5 Missing ⚠️
src/handlers/payment/session/delete/index.tsx 91.89% 3 Missing ⚠️
Additional details and impacted files
@@                        Coverage Diff                         @@
##           feat/payment-manager-connector    #2281      +/-   ##
==================================================================
- Coverage                           97.17%   97.14%   -0.03%     
==================================================================
  Files                                 599      603       +4     
  Lines                               40808    41119     +311     
==================================================================
+ Hits                                39656    39946     +290     
- Misses                               1152     1173      +21     

☔ 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.

@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Sep 11, 2026
@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Sep 11, 2026
@github-actions github-actions Bot added size/l PR size: L and removed size/l PR size: L labels Sep 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/l PR size: L

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants