Skip to content

fix(agent-commerce): 与信と売上確定を区別し capture へ与信結果を引き渡す - #7032

Merged
dotani1111 merged 3 commits into
EC-CUBE:4.4from
nanasess:fix/agent-commerce-capture-contract
Aug 7, 2026
Merged

fix(agent-commerce): 与信と売上確定を区別し capture へ与信結果を引き渡す#7032
dotani1111 merged 3 commits into
EC-CUBE:4.4from
nanasess:fix/agent-commerce-capture-contract

Conversation

@nanasess

@nanasess nanasess commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

概要

エージェントチェックアウトの決済ハンドラ契約 (AgentCheckoutPaymentHandlerInterface / PaymentOutcome) に 2 つの欠落があり、実 PSP を接続したときに実害が出ます。EC-CUBE/sample-payment-plugin#54 のレビューで顕在化したものを本体側で修正します。

いずれもエージェントコマース (#6777 / #6776 / #6574) が 4.4 で導入した未リリースの API であり、既存の決済プラグイン (通常購入の PaymentMethodInterface) には影響しません。

破壊的変更 (決済ハンドラを実装するプラグイン向け)

AgentCheckoutPaymentHandlerInterface の 2 メソッドのシグネチャを変更しています。古いシグネチャのまま実装しているとインターフェース実装制約により fatal error になります (authorize() は第 3 引数にデフォルト値がありますが、実装側で省略すると同様に fatal です)。

-public function authorize(Order $order, array $paymentData): PaymentOutcome;
+public function authorize(Order $order, array $paymentData, array $paymentReference = []): PaymentOutcome;

-public function capture(Order $order, array $paymentData): PaymentOutcome;
+public function capture(Order $order, array $paymentData, PaymentOutcome $authorization): PaymentOutcome;
  • 影響範囲: エージェントコマースの決済ハンドラ (AgentCheckoutPaymentHandlerInterface / AcpPaymentHandlerInterface / UcpPaymentHandlerInterface) を実装するプラグインのみ。通常購入の PaymentMethodInterface を実装する既存の決済プラグインには影響しません。
  • リリース済み API ではありません: 本インターフェースは 4.4 の開発サイクル中に feat(agent-commerce): CheckoutSession 中核を実装 (#6777 トラックB前提・Phase 1b) #6825 で追加されたもので、Constant::VERSION4.4.0-dev・4.4 のタグは未発行です。コア本体に具象実装はなく (テストダブルのみ・本 PR で更新済み)、リリース済みのプラグイン API を壊すものではありません。
  • 移行方法: 上記のとおり引数を追加します。$paymentReference再開 complete のときだけ中断前の PSP 参照 (transaction_id と metadata) が入り、初回は空配列です。capture()$paymentData から取引を導出し直さず $authorization->transactionId を使ってください。移行の実例は EC-CUBE/sample-payment-plugin#54 を参照してください。

修正 1: 与信 (authorized) と売上確定 (completed) を区別する

PaymentOutcomeStatus::COMPLETED が「与信のみ (requires_capture)」と「売上確定済 (succeeded)」の両方を表していたため、AgentCheckoutCompletionService は COMPLETED を受けると必ず capture() を呼んでいました。

与信と売上が 1 度の通信で完結する auto-capture 型 PSP を差し込むと capture が二重発行されます。

  • PaymentOutcomeStatus::AUTHORIZED を追加し、AUTHORIZED のときだけ capture を呼ぶ
  • COMPLETED は「capture 済/不要」を表し、capture を呼ばずに注文を確定する。
  • PaymentOutcome::authorized() を追加。

修正 2: capture へ authorize の結果を引き渡す

capture() には authorize と同じ $paymentData しか渡っておらず、authorize が返した取引識別子・metadata を引き継ぐ経路がありませんでした。ハンドラは対象取引を支払データから導出し直すほかなく、ACP の Shared Payment Token のようなワンショット償還のトークンでは 2 度目の償還が失敗します (実際に sample-payment で SPT が 1 決済につき 2 回償還されていました)。

-public function capture(Order $order, array $paymentData): PaymentOutcome;
+public function capture(Order $order, array $paymentData, PaymentOutcome $authorization): PaymentOutcome;

併せて: 中断・失敗時に PSP 参照を追跡できるようにする

  • PaymentOutcome::requiresAction() / pending() / failed()transactionId を受け取れるようにした (DTO 自体は元から保持していたが、ファクトリが受け取らず捨てていた)。
  • payment_data へ metadata に加えて transaction_id を保持し、capture 失敗時にも残す (与信済みの取引を照会・取消するために必要)。
  • authorize が支払データを検証できない場合は fail-closed で failed() を返す旨を interface に明記。トークン欠落を「異常でない」と解釈して無与信のまま受注確定する事故を防ぐ。
  • UcpPaymentHandlerInterface::exchangePaymentToken() は complete の状態機械の外側 (controller のペイロード解決時) で呼ばれるため、例外を投げてはならない旨を明記 (投げるとビジネス系エラーでなく HTTP 500 になる)。
  • AcpPaymentHandlerInterface::redeemSharedPaymentToken() はワンショットであり authorize 内で 1 度だけ呼ぶ旨を明記。

結合 E2E の拡充

想定入力からしかテストを作っていなかったため、負の入力capture 失敗の経路が一度も通っていませんでした。e2e/agent/{acp,ucp}-checkout.php に追加します。

  • ACP / UCP 共通: トークン欠落 (fail-closed の回帰) / capture 失敗
  • UCP: 3DS 中断 → 再開 (ACP と対称に。UCP は認証結果が credential 経由でしか届かないため、そこを落とすと requires_escalation から復帰できない)
  • acp-checkout.phpAGENT_E2E_ITEM_ID 既定値を UCP 側と揃えて 2 に修正 (id=1visible=0 の規格でローカル実行が必ず失敗していた。CI は 2 を明示指定しているため CI には影響なし)

検証

項目 結果
PHPStan level 6 (analyse src) No errors
PHP-CS-Fixer 0 件
PHPUnit (tests/Eccube/Tests/Service/AgentCommerce のうち本 PR の変更範囲 = CheckoutSession / Payment / Discovery / AgentCheckoutCoreConformance) 33 tests, 101 assertions, 0 失敗
結合 E2E (ACP・ローカル HTTPS + api44 + samplepayment44) 41 assertions PASS
結合 E2E (UCP・同上) 35 assertions PASS

追加したテストケース:

  • testAuthorizedOutcomeHandsTransactionReferenceToCapture — capture が authorize の取引識別子・metadata を受け取り、payment_data に永続化される
  • testAutoCaptureGatewayIsNotCapturedTwice — authorize が COMPLETED を返したとき capture を呼ばない
  • testCaptureFailureRollsBackStockAndKeepsAuthorizationReference — capture 失敗で引当を戻しつつ取引識別子は残す

関連

🤖 Generated with Claude Code

Summary by CodeRabbit

  • 新機能
    • 決済結果に「与信済み」状態を追加しました。
    • PSPの取引IDや決済メタデータを保存し、決済状況を追跡しやすくしました。
    • 3DS認証後の決済再開に対応しました。
  • 不具合修正
    • オートキャプチャ決済での二重実行を防止しました。
    • トークン不足時は安全側で処理を中断します。
    • キャプチャ失敗時は注文を未完了として扱い、エラー情報を返します。
    • 決済トークン交換時のエラーを、適切な業務エラーとして表示します。

@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 811b03be-4aa7-478a-b5f9-9b30f23ada1e

📥 Commits

Reviewing files that changed from the base of the PR and between 2985dd5 and bd41395.

📒 Files selected for processing (1)
  • tests/Eccube/Tests/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionServiceTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/Eccube/Tests/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionServiceTest.php

📝 Walkthrough

Walkthrough

PaymentOutcomeAUTHORIZED を追加しました。認可後だけ capture を実行し、PSP参照を payment_data に保存します。再開処理、UCPの例外処理、capture失敗、トークン欠落をテストとE2Eシナリオで検証します。

Changes

決済認可と capture

Layer / File(s) Summary
決済結果とハンドラ契約
src/Eccube/Service/AgentCommerce/Payment/*
AUTHORIZED と PSP参照を PaymentOutcome で保持します。authorize()capture() の契約を更新します。
注文完了と payment reference
src/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionService.php
AUTHORIZED の場合だけ capture を実行します。COMPLETED の場合は capture を省略します。決済結果の参照を payment_data に保存します。
UCPトークン交換のエラー処理
src/Eccube/Controller/AgentCommerce/UcpCheckoutController.php, tests/Eccube/Tests/Web/AgentCommerce/UcpCheckoutControllerTest.php
トークン交換の例外をログ記録し、セッションを未完了のまま業務エラーへ変換します。
決済完了フローのテスト
tests/Eccube/Tests/Service/AgentCommerce/CheckoutSession/*, tests/Eccube/Tests/Service/AgentCommerce/Conformance/*, tests/Eccube/Tests/Service/AgentCommerce/Discovery/*, tests/Eccube/Tests/Service/AgentCommerce/Payment/*
認可結果の capture 引き渡し、再開時の参照利用、例外処理、ロールバック、5状態の適合性を検証します。
ACP と UCP の E2E シナリオ
e2e/agent/acp-checkout.php, e2e/agent/ucp-checkout.php
認証結果付き再開、トークン欠落時の fail-closed、capture 失敗を検証します。UCP の complete リクエストは追加 credential 項目を送信します。

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant UcpCheckoutController
  participant AgentCheckoutCompletionService
  participant PaymentHandler
  participant Order
  Client->>UcpCheckoutController: complete checkout
  UcpCheckoutController->>PaymentHandler: exchangePaymentToken()
  PaymentHandler-->>UcpCheckoutController: payment data or exception
  UcpCheckoutController->>AgentCheckoutCompletionService: complete with resolved data
  AgentCheckoutCompletionService->>PaymentHandler: authorize()
  PaymentHandler-->>AgentCheckoutCompletionService: AUTHORIZED or COMPLETED
  alt AUTHORIZED
    AgentCheckoutCompletionService->>PaymentHandler: capture(authorization)
    PaymentHandler-->>AgentCheckoutCompletionService: capture result
  else COMPLETED
    AgentCheckoutCompletionService->>Order: complete without capture
  end
  AgentCheckoutCompletionService->>Order: save payment reference
Loading

Possibly related PRs

Suggested labels: Agentic Commerce, BC break, bug

Suggested reviewers: dotani1111, ttokoro20240902

Poem

うさぎは認可の印を持つ
AUTHORIZED なら capture へ進む
COMPLETED なら二度進まない
欠けた token は完了させない
PSP参照を静かに保存する 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed タイトルは、与信と売上確定を分離し、captureへ与信結果を渡すという変更の主目的を正確かつ簡潔に示しています。
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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
`@src/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionService.php`:
- Line 180: 再開時の authorize 処理で、リクエストの paymentData に保存済みの PSP
参照をサーバー側からマージするよう更新してください。CheckoutSession の mergePaymentData と paymentReference
を利用し、transaction_id などの保存済み取引識別子がリクエスト値で上書きされない順序を維持してください。保存済み参照を省略した再開リクエストでも
authorize ハンドラへ同じ PSP 取引が渡されるテストを追加してください。
- Around line 130-138: AUTHORIZED 分岐の handler->capture() で PSP
例外をそのまま伝播させず、PaymentOutcome::failed() 相当の FAILED 結果へ変換して既存の failOrder()
経路で処理してください。capture 実行前に mergePaymentData() で保存した与信参照が rollback で失われないよう、各実ハンドラの
capture 実装または周辺の復旧処理を更新し、authorize() 再実行時にその参照を再利用できる状態を維持してください。

In `@src/Eccube/Service/AgentCommerce/Payment/AcpPaymentHandlerInterface.php`:
- Around line 46-47: Update the ACP authorize flow to catch exceptions from
redeemSharedPaymentToken() and return PaymentOutcome::failed() instead of
propagating them. Preserve the already-resolved transactionId and metadata in
the failed outcome, ensuring redemption failures trigger the existing
fail-closed payment transition rather than an HTTP 500.
🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5eb57b41-4a82-4311-9af8-375eed2b0c1f

📥 Commits

Reviewing files that changed from the base of the PR and between b85d7dd and 627e565.

📒 Files selected for processing (13)
  • e2e/agent/acp-checkout.php
  • e2e/agent/ucp-checkout.php
  • src/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionService.php
  • src/Eccube/Service/AgentCommerce/Payment/AcpPaymentHandlerInterface.php
  • src/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerInterface.php
  • src/Eccube/Service/AgentCommerce/Payment/PaymentOutcome.php
  • src/Eccube/Service/AgentCommerce/Payment/PaymentOutcomeStatus.php
  • src/Eccube/Service/AgentCommerce/Payment/UcpPaymentHandlerInterface.php
  • tests/Eccube/Tests/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionServiceTest.php
  • tests/Eccube/Tests/Service/AgentCommerce/Conformance/AgentCheckoutCoreConformanceTest.php
  • tests/Eccube/Tests/Service/AgentCommerce/Discovery/UcpPaymentHandlerDiscoveryRegistryTest.php
  • tests/Eccube/Tests/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistryTest.php
  • tests/Eccube/Tests/Service/AgentCommerce/Payment/DefaultAgentPaymentMethodResolverTest.php

エージェントチェックアウトの決済ハンドラ契約に 2 つの欠落があった。sample-payment-plugin
(EC-CUBE/sample-payment-plugin#54) のレビューで顕在化したもので、いずれも実 PSP を接続した
ときに実害が出る。

1. 与信のみ (requires_capture) と売上確定済 (succeeded) を PaymentOutcomeStatus::COMPLETED
   へ潰していたため、オーケストレータは COMPLETED を受けると必ず capture を呼んでいた。
   与信と売上が 1 度で完結する auto-capture 型 PSP を差し込むと capture が二重発行される。
   → PaymentOutcomeStatus::AUTHORIZED を追加し、AUTHORIZED のときだけ capture を呼ぶ。
     COMPLETED は「capture 済/不要」を表し、capture を呼ばずに確定する。

2. capture へ authorize の結果 (取引識別子・metadata) を渡す経路がなく、同じ paymentData を
   再度渡していた。ハンドラは対象取引を支払データから導出し直すほかなく、ACP の Shared
   Payment Token のようなワンショット償還のトークンでは 2 度目の償還が失敗する。
   → capture() の引数に authorize が返した PaymentOutcome を追加する。

併せて、中断・失敗時に PSP 参照を追跡できるようにする。

- PaymentOutcome::requiresAction()/pending()/failed() に transactionId を渡せるようにした
  (DTO 自体は元から保持していたが、ファクトリが受け取らず捨てていた)。
- payment_data には metadata に加えて transaction_id を保持し、capture 失敗時にも残す
  (与信済みの取引を照会・取消するために必要)。
- authorize が支払データを検証できない場合は fail-closed で failed を返す旨を interface に明記。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 77.61%. Comparing base (b85d7dd) to head (bd41395).
⚠️ Report is 92 commits behind head on 4.4.

Additional details and impacted files
@@            Coverage Diff             @@
##              4.4    #7032      +/-   ##
==========================================
+ Coverage   77.17%   77.61%   +0.44%     
==========================================
  Files         564      595      +31     
  Lines       28082    29173    +1091     
==========================================
+ Hits        21671    22642     +971     
- Misses       6411     6531     +120     
Flag Coverage Δ
Unit 77.61% <100.00%> (+0.44%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@nanasess
nanasess force-pushed the fix/agent-commerce-capture-contract branch from 627e565 to 70f6e8a Compare August 6, 2026 08:57
nanasess added a commit to nanasess/sample-payment-plugin that referenced this pull request Aug 6, 2026
本体 EC-CUBE/ec-cube#7032 のレビュー指摘 (CodeRabbit) 対応。authorize() の第 3 引数として
中断前の complete で保持した PSP 参照 (transaction_id と metadata) が渡るようになったため、
再開時はトークンを再償還せずその取引を続行する (実 PSP の「既存 PaymentIntent を confirm」に相当)。

モックも渡された transaction_id を導出値より優先するようにし、ユニットで固定した。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerInterface.php (1)

62-76: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

capture() の戻り値として許容するステータスを契約に明記してください。

呼び出し側 (AgentCheckoutCompletionService::runStateMachine() Line 142) は COMPLETED 以外のすべてを失敗として扱います。PENDING を返す非同期 capture のハンドラは、failOrder() に入り、errorCodenull のため空のエラーメッセージになります。契約に「capture は COMPLETED または failed() のみを返す」と明記してください。非同期 capture を将来許容する場合は、状態機械側の分岐を先に定義してください。

📝 契約追記の例
      * authorize と同様、PSP 通信の失敗は例外でなく {`@link` PaymentOutcome::failed()} で返し、
      * **与信済みの取引識別子を保持する** (与信は PSP 側に残るため、取消・再 capture の照会に要る)。
+     *
+     * 戻り値は {`@link` PaymentOutcome::completed()} または {`@link` PaymentOutcome::failed()} のいずれかとする。
+     * これ以外のステータス (AUTHORIZED / REQUIRES_ACTION / PENDING) はオーケストレータが失敗として扱う。
      *
🤖 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
`@src/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerInterface.php`
around lines 62 - 76, Update the capture() contract documentation in
AgentCheckoutPaymentHandlerInterface to state that it may return only COMPLETED
or PaymentOutcome::failed(), and must not return PENDING. Keep asynchronous
capture unsupported unless AgentCheckoutCompletionService::runStateMachine() is
first updated with an explicit PENDING branch.
🤖 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
`@src/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerInterface.php`:
- Around line 46-60: Update all external plugin implementations of
AgentCheckoutPaymentHandlerInterface to match the new authorize() third argument
and required capture() third argument, preserving compatibility with the
interface to avoid fatal PHP signature errors. Update any plugin-facing fixtures
or adapters as needed, and add a migration note documenting the signature
changes and required implementation updates.

---

Nitpick comments:
In
`@src/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerInterface.php`:
- Around line 62-76: Update the capture() contract documentation in
AgentCheckoutPaymentHandlerInterface to state that it may return only COMPLETED
or PaymentOutcome::failed(), and must not return PENDING. Keep asynchronous
capture unsupported unless AgentCheckoutCompletionService::runStateMachine() is
first updated with an explicit PENDING branch.
🪄 Autofix

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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bf576dda-01b8-459e-b1f9-ea63076892ec

📥 Commits

Reviewing files that changed from the base of the PR and between b85d7dd and 70f6e8a.

📒 Files selected for processing (13)
  • e2e/agent/acp-checkout.php
  • e2e/agent/ucp-checkout.php
  • src/Eccube/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionService.php
  • src/Eccube/Service/AgentCommerce/Payment/AcpPaymentHandlerInterface.php
  • src/Eccube/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerInterface.php
  • src/Eccube/Service/AgentCommerce/Payment/PaymentOutcome.php
  • src/Eccube/Service/AgentCommerce/Payment/PaymentOutcomeStatus.php
  • src/Eccube/Service/AgentCommerce/Payment/UcpPaymentHandlerInterface.php
  • tests/Eccube/Tests/Service/AgentCommerce/CheckoutSession/AgentCheckoutCompletionServiceTest.php
  • tests/Eccube/Tests/Service/AgentCommerce/Conformance/AgentCheckoutCoreConformanceTest.php
  • tests/Eccube/Tests/Service/AgentCommerce/Discovery/UcpPaymentHandlerDiscoveryRegistryTest.php
  • tests/Eccube/Tests/Service/AgentCommerce/Payment/AgentCheckoutPaymentHandlerRegistryTest.php
  • tests/Eccube/Tests/Service/AgentCommerce/Payment/DefaultAgentPaymentMethodResolverTest.php
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/Eccube/Service/AgentCommerce/Payment/PaymentOutcomeStatus.php
  • tests/Eccube/Tests/Service/AgentCommerce/Conformance/AgentCheckoutCoreConformanceTest.php
  • e2e/agent/acp-checkout.php
  • e2e/agent/ucp-checkout.php
  • src/Eccube/Service/AgentCommerce/Payment/UcpPaymentHandlerInterface.php
  • src/Eccube/Service/AgentCommerce/Payment/PaymentOutcome.php
  • src/Eccube/Service/AgentCommerce/Payment/AcpPaymentHandlerInterface.php

@ttokoro20240902 ttokoro20240902 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.

レビューしました。auto-capture 型 PSP での capture 二重発行と、capture へ与信結果が渡らない問題はいずれも実在し、修正の方向に異論はありません。負の入力・capture 失敗・ハンドラ例外を本体テストと E2E の両方で通しているのも良いと思います。CodeRabbit の Major 3 件は 70f6e8a で実装されていることをコードで確認しました。

その上で、ステートマシンに 4 点だけ確認したい点があります。詳細は該当行にインラインで書きました。

# 重大度 指摘 該当
1 capture 失敗後の再試行(READY)では PSP 参照がハンドラへ渡らず、本 PR が REQUIRES_ACTION/PENDING で解決したワンショット問題が残る。プラグイン側コメントの「同一取引の再 capture を可能にする」がコアの経路と噛み合っていない AgentCheckoutCompletionService.php:91,134
2 capture 成功後に commitOrder() が投げると PSP は課金済みなのに transaction_id ごと巻き戻る。authorize()/capture() には「最後の砦」を入れたが commit には無い AgentCheckoutCompletionService.php:43
3 capture の PENDING が一律「失敗」になり在庫が戻る。さらに failOrder() のメッセージが空文字になる AgentCheckoutCompletionService.php:142
4 exchangePaymentToken() の「例外を投げてはならない」が PHPDoc だけで、UcpCheckoutController::resolvePaymentData() に受けが無い UcpPaymentHandlerInterface.php:46

2 と 3 は origin/4.4 で同じ分岐を確認したので本 PR の regression ではありません。ただし「capture を一級のステップとして契約化する」「500 で巻き戻さない最後の砦を置く」というのがこの PR の主題なので、ここで塞ぐか、契約の範囲を PHPDoc で明示するかを決められると良いと思います。

なお PaymentOutcome::needsCapture() は追加されていますが呼び出し元がありません(isSuccessful() も同様に未使用のまま AUTHORIZED を true にする意味変更が入っています)。ステートマシンは switch ($outcome->status) を直接使っているので、削除するかステートマシン側で使って判定源を一本化するのが良さそうです。

Comment thread src/Eccube/Service/AgentCommerce/Payment/UcpPaymentHandlerInterface.php Outdated
Comment thread src/Eccube/Service/AgentCommerce/Payment/PaymentOutcome.php Outdated
@dotani1111

Copy link
Copy Markdown
Contributor

@nanasess

実装の契約変更に合わせて、合意用 Issue #6777 の記載更新をお願いできればと思います。
マージ後でも問題ありません。

  1. §7-1 PaymentOutcomeStatus(4 値 → 5 値)
  2. §7-2 ハンドラ interface のシグネチャ
// before
public function authorize(Order $order, array $paymentData): PaymentOutcome;
// 売上確定。authorize が COMPLETED を返した後にのみ呼ぶ。
public function capture(Order $order, array $paymentData): PaymentOutcome;

// after
public function authorize(Order $order, array $paymentData, array $paymentReference = []): PaymentOutcome;
// 売上確定。authorize が AUTHORIZED を返した後にのみ呼ぶ。
public function capture(Order $order, array $paymentData, PaymentOutcome $authorization): PaymentOutcome;
  1. §7-3 の状態機械図(2 箇所)

  2. テスト観点
    authorize→COMPLETED→capture→commit→completedauthorize→AUTHORIZED→capture→commit→completed へ変更し、auto-capture 型(COMPLETED 直行)の観点を 1 行追加。

本 PR のマージにあわせて、#6777 の該当箇所の更新(または「#7032 で改訂」の追記)をお願いします。

@nanasess

nanasess commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

@dotani1111 agent-commerce 関連のタスクが一通り完了しましたら、一連の更新をしておきますね

nanasess and others added 2 commits August 7, 2026 12:25
PR EC-CUBE#7032 のレビュー指摘 5 件に対応する。5 件はいずれも「docblock が主張する一般則」と
「実装した範囲」の差分に落ちていたため、範囲を実装で埋めるか、主張を実装に合わせて絞る。

- UCP の exchangePaymentToken() に砦を追加。状態機械の外 (controller) で呼ぶため
  CompletionService の authorize/capture 用の砦が効かず、ハンドラが契約に反して投げると
  HTTP 500 になっていた。controller で捕捉しビジネス系エラー (HTTP 200 + messages[]) へ
  写像する。handler_id の重複登録 (デプロイ不備) は従来どおり伝播させる。
- capture が契約 (COMPLETED か FAILED) に反する status を返したときの正規化を追加。
  PENDING は errorCode/errorMessage を持たないため、素通しすると空文字の ERROR が
  エージェントへ返っていた。契約違反はログへ残し明示的な失敗へ写像する。
- failOrder のメッセージ本文が空にならないようフォールバックを追加。
- PaymentOutcome から未使用の述語 3 つ (isSuccessful/needsCapture/needsAction) を削除。
  needsCapture は本 PR で追加し呼び出し元ゼロ、isSuccessful は本 PR が AUTHORIZED を含む
  意味へ変更したまま呼び出し元ゼロで、「commit してよい」と誤読される余地があった。
  判定源は状態機械の switch に一本化する。
- docblock を実装に合わせて訂正。「最後の砦」の範囲を authorize/capture に限定し、
  範囲外 (supports/getHandlerId・exchangePaymentToken・PurchaseFlow) を明記。
  complete に再入できる 4 状態と、保持済み PSP 参照を渡すのが requires_action /
  in_progress からの再開に限られることを表で示す。capture() の PHPDoc には、ready からの
  再試行が新規 authorize になるため、与信が残り再 authorize できない場合 (ACP の Shared
  Payment Token 等) は retryable=false を返す契約を明記。

テストは分岐ごとではなく不変条件ごとに書く。「保持した PSP 参照がハンドラへ渡るのは
在庫を保持したまま中断した状態からの再開時のみ」を再入 3 経路で検証し、capture 失敗後の
ready 再試行が新規 authorize になることを固定する。従来は「参照が書かれる」テストと
「参照が読まれる」テストが個別にあり、両者が接続するテストが無かったため穴が残っていた。

検証: PHPUnit AgentCommerce 354 tests / 1344 assertions / 0 failures、
php-cs-fixer 0 件、PHPStan level 6 (AgentCommerce 配下) No errors。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI の rector が YieldDataProviderRector で指摘。既存の
MinorUnitConverterTest::malformedAmountProvider() と同じく \Iterator + yield へ揃える。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@ttokoro20240902 ttokoro20240902 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.

指摘 5 件すべての対応を bd41395 のコードで確認しました。LGTM です。

  • ready 再試行の扱いは契約として明文化(入口ステータス表 + capture()retryable=false 指示)。「与信拒否でも transaction_id は保存されるので、ready で渡すと死んだ取引を掴ませる」という反証は私の見落としでした
  • 砦の範囲を authorize() / capture() に限定し、commit 失敗時に参照が巻き戻る点を既知の制約として明記
  • asCaptureFailure() / errorMessageFor() で capture の契約違反と空文字メッセージを解消
  • resolvePaymentData()?array にして controller 側に砦を実装
  • PaymentOutcome の述語 3 つを削除し、判定を switch ($outcome->status) に一本化

追加された 5 つのテストがいずれも修正前に落ちることを実測されている点も良いと思います。

@dotani1111
dotani1111 merged commit e5d42e9 into EC-CUBE:4.4 Aug 7, 2026
185 of 187 checks passed
nanasess added a commit to nanasess/sample-payment-plugin that referenced this pull request Aug 7, 2026
EC-CUBE/ec-cube#7032 のマージで決済ハンドラの契約が 2 点明確になったため追随する。

- **capture の戻り値は COMPLETED か FAILED のみ**。従来は authorize と写像を共用しており、
  ゲートウェイが REQUIRES_CAPTURE / REQUIRES_ACTION / PROCESSING を返すと AUTHORIZED /
  REQUIRES_ACTION / PENDING をそのまま返していた。本体はこれらを失敗として扱うが、
  errorCode / errorMessage が無いぶん理由を伝えられない。capture 専用の写像を分け、
  非終端ステータスはログに残したうえで capture_unexpected_status の失敗へ畳む。
- **与信が残り再 authorize できない場合の capture 失敗は retryable=false**。本体には
  capture 単独の再実行入口が無く、ready からの再 complete は新規 authorize から始まる
  (保持した PSP 参照は渡らない)。ACP は入口が Shared Payment Token の償還でワンショット
  のため、ready へ戻しても再試行は必ず失敗し与信だけが残る。プロトコル別の
  captureFailureIsRetryable() で分岐し、ACP=false / UCP=true とする。UCP はエージェントが
  complete のたびに credential を送り直し exchange をやり直せるため再試行が成立する。

ゲートウェイが不可逆と判断した失敗 (金額不一致等) は、再 authorize できるプロトコルでも
再試行させないよう AND で畳む。capture の例外経路では metadata も引き継ぐようにした
(取消・照会に要る)。

紛らわしかった toOutcome() は toAuthorizeOutcome() へ改名し、capture からの流用を防ぐ。

検証: PHPUnit 86 tests / 177 assertions / 0 failures、PHPStan level 6 No errors。
追加した 6 テストは修正前に落ちることを確認済み。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants