Skip to content

feat(agent-commerce): ACP/UCP エージェント決済ハンドラと Gateway 抽象を追加 - #54

Open
nanasess wants to merge 6 commits into
EC-CUBE:4.4from
nanasess:feature/agentic-commerce
Open

feat(agent-commerce): ACP/UCP エージェント決済ハンドラと Gateway 抽象を追加#54
nanasess wants to merge 6 commits into
EC-CUBE:4.4from
nanasess:feature/agentic-commerce

Conversation

@nanasess

@nanasess nanasess commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

概要

EC-CUBE 本体のエージェントコマース対応(ACP / UCP)に対する、サンプル決済での決済ハンドラ参考実装です。本体が定義する決済ハンドラ interface の具象を本プラグイン側に実装し、AI エージェント経由の購入(authorize / capture / トークン償還・交換、3DS 中断・再開)を sample 決済で完結させます。

Note

依存していた本体側の変更はすべて 4.4 へマージ済みです(本 PR のドラフトを解除しました)。

一時期あった「本プラグインの具象ハンドラに agent_commerce.payment_handler タグが付かない」問題(EC-CUBE/ec-cube#6915Plugin\ の PSR-4 自動登録が services.yaml から services.php へ移り、services.yaml_instanceof がファイルスコープゆえプラグイン具象へ届かなくなっていた件)は、上記 #6872 の registerForAutoconfiguration() 移行(PaymentMethodInterface と同方式・コンテナ全体に効く)で解消済みです。

Important

セットだった EC-CUBE/ec-cube#7032 は 4.4 へマージ済みです(2026-08-07)。本 PR は 4.4 単体で CI が緑になります(PHPUnit 4 マトリクス + Playwright 12 マトリクス、すべて pass)。
#7032 で確定した決済ハンドラの契約(capture() の戻り値は COMPLETED か FAILED のみ / 与信が残り再 authorize できない場合の capture 失敗は retryable=false)への追随は、下記「本体契約への追随」で対応しています。

レビュー指摘への対応(2026-08-06 更新)

レビューでいただいた 12 件はすべて対応しました。原因は「実装・モック・結合 E2E を同じ設計から起こしたため、検証が実装と同じ仮定を共有し、境界値・異常系が素通りしていた」ことなので、まず PHPUnit の基盤とユニットテストを入れ、そのうえで不具合を直しています

PHPUnit 基盤の追加

  • phpunit.xml.dist / Tests/bootstrap.php(eccube-api4 と同じ流儀)
  • .github/workflows/phpunit.yml(PHP 8.2–8.5 × PostgreSQL)
  • Playwright の tests/e2e/ へ改名。大文字小文字を区別しないファイルシステムで Tests/ と衝突するため(本体も PHPUnit=tests / Playwright=e2e で分離)
  • 86 tests / 177 assertionsPaymentTokenExtractorTest / MockAgentPaymentGatewayTest / AcpSampleCardHandlerTest / UcpSampleCardHandlerTest

主な修正

指摘 対応
【1】トークン欠落で無与信のまま受注確定 抽出を PaymentTokenExtractor へ集約し、解決不能なら PaymentOutcome::failed('invalid_payment_data', retryable: true)(fail-closed)。モック側にも二次防衛
【2】ACP の SPT 償還が 1 決済で 2 回 ゲートウェイの capture() から支払データを外し string $transactionId を受け取る形へ。capture でトークンを再利用する実装が型として書けない
【3】ゲートウェイ例外が未 catch authorize / capture で捕捉し failed へ写像。capture 失敗は与信が PSP に残るため取引識別子と metadata を保持(再試行可否は後述の契約に従う)
【5】requires_action で PSP 参照が残らない requiresAction() に metadata を渡す(本体側で transaction_id も永続化)
【6】取引 ID が別注文で衝突 導出に注文参照(order_noorder_id)を混入。同一注文では不変なので 3DS 再開でも一致
【7】authentication_result の判定が緩い 空でない文字列 または 空でない配列のみ認証済み([] / false / 数値は未認証)
【8】failed() だけ metadata / transactionId を持てない 他のファクトリと対称化
【9】モック capture() が常に成功 *-capture-fail* 規約を追加。併せて未与信・二重 capture・金額不一致・トークン再償還も失敗させる
【10】トークン規約の doc と実装の不一致 / UCP で 3DS が成立しない マーカーをハイフン付きに統一し評価順・大文字小文字を doc 化。exchangePaymentToken()authentication_result を伝播
【11】SUCCEEDED と REQUIRES_CAPTURE の潰し込み 本体 PaymentOutcomeStatus::AUTHORIZED の追加(#7032)に合わせて分けて写像
【12】docblock の矛盾 services.yaml を正として修正
【4】capture に取引 ID を渡す経路がない 本体 interface を変更(#7032)

結合 E2E の拡充(本体 #7032 側)

想定入力からしかテストを作っていなかったため、負の入力と capture 失敗の経路が一度も通っていませんでした。ACP / UCP 双方に トークン欠落(fail-closed)capture 失敗 を、UCP には 3DS 中断 → 再開 を追加しています(ローカル実行で ACP 41 assertions / UCP 35 assertions PASS)。

本体契約への追随(2026-08-07 更新)

EC-CUBE/ec-cube#7032 のマージで決済ハンドラの契約が 2 点明確になったため、e22cb7f で追随しました。

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 参照はハンドラへ渡りません(保持は照会・監査用)。したがって再試行可否は「同じ入力から instrument を作り直せるか」で決まります。

プロトコル captureFailureIsRetryable() 理由
ACP false 再 authorize の入口が redeemSharedPaymentToken()(Shared Payment Token の償還)。SPT はワンショットで 2 度目が失敗するため、ready へ戻しても再試行は必ず失敗し与信だけが残る
UCP true エージェントが complete のたびに payment.instruments[].credential を送り直し、本体 controller が exchangePaymentToken() で交換をやり直す。同じ入力から作り直せる

ゲートウェイが不可逆と判断した失敗(金額不一致等)は、再 authorize できるプロトコルでも再試行させないよう AND で畳んでいます。あわせて capture の例外経路で metadata も引き継ぐようにし、紛らわしかった toOutcome()toAuthorizeOutcome() へ改名して capture からの流用を名前で防いでいます。

追加した 6 テストは、修正を巻き戻すと落ちることを実測しています。

実装内容(Service/AgentCommerce/

Gateway 抽象(PSP 境界の分離)

  • Gateway/GatewayStatus.phpsucceeded / requires_capture / requires_action / processing / failed の enum。決済ゲートウェイの状態語彙を中立化(PSP の状態にそのまま写像できる形)。
  • Gateway/GatewayResult.php — 結果 DTO(readonly・ファクトリ付き)。
  • Gateway/AgentPaymentGatewayInterface.phpauthorize() / capture()(通貨・最小単位整数額・instrument・context)。
  • Gateway/MockAgentPaymentGateway.php — トークン規約でシナリオを再現するモックゲートウェイ:
    • *-3ds* → 追加認証(requires_action、再開時 authentication_result 同伴で成功)
    • *-decline* → 与信拒否(failed・再試行可) / *-fraud* → 不正検知(failed・再試行不可)
    • *-processing* → 非同期処理中(processing) / *-capture-fail* → 与信は成功し capture で失敗 / それ以外 → 与信成功(requires_capture)→ capture で succeeded
    • 取引 ID は決定的(pi_mock_<sha256>)。

ハンドラ

  • AbstractAgentCardHandler.php — 共通基底。supports() は支払方法が CreditCard かつ Order.getAgentProtocol() がプロトコル一致のときのみ true。GatewayResult → 本体 PaymentOutcome の写像は authorize と capture で分けています
    • toAuthorizeOutcome()requires_capture→AUTHORIZED / succeeded→COMPLETED / requires_action→REQUIRES_ACTION / processing→PENDING / failed→FAILED
    • toCaptureOutcome() … 本体契約に従い COMPLETED か FAILED のみへ畳む。非終端ステータスはゲートウェイ実装の誤りとしてログに残し capture_unexpected_status の失敗にする
  • Acp/AcpSampleCardHandler.phpAcpPaymentHandlerInterface 実装。handler_id = card_tokenizedredeemSharedPaymentToken()(ACP Shared Payment Token の償還)。
  • Ucp/UcpSampleCardHandler.phpUcpPaymentHandlerInterface 実装。handler_id = dev.ucp.payment.cardexchangePaymentToken()(UCP のトークン交換)。

具象ハンドラは agent_commerce.payment_handler タグで登録され、本体のレジストリが checkout 解決・discovery 広告に利用します。タグ付けは本体側の Kernel::registerForAutoconfiguration() が自動で行うため、本プラグインの Resource/config/services.yaml には Gateway 実装のエイリアスのみを置いています。

検証

本体↔本プラグイン↔eccube-api4 の結合 E2E(本体側 agentic-commerce-e2e ワークフロー、run_payment=true)で、ACP / UCP の 成功 / 3DS 中断・再開 / 拒否 の全シナリオが通ることを確認済みです。

このワークフローは EC-CUBE/ec-cube#6872 で本体の通常 CI(main.yml)へ統合され、nanasess/sample-payment-plugin@feature/agentic-commerce(=本 PR の head)を参照して push / PR ごとに自動実行されます。#6872 マージ後の 4.4 での実行結果:

  • run 30458540838(2026-07-29)… Integration smoke (本体 + api44 + samplepayment44) 全 8 マトリクス(PHP 8.2 / 8.3 / 8.4 / 8.5 × pgsql / mysql)+ Checkout E2Erun_payment 既定 true のため complete まで実行)がいずれも success

本プラグイン単体の CI にも PHPUnit(PHP 8.2–8.5)を追加したため、エージェント決済ハンドラの境界値・異常系はプラグイン単体で検証できます。経路の疎通(本体↔api44↔本プラグイン)は引き続き本体側の結合 E2E が根拠です。

補足

  • 本プラグインは「サンプル決済」での参考実装です。実 PSP 連携は各決済プラグインが本体 interface を実装して提供します(Gateway 抽象はその移植境界として設計)。
  • 本プラグイン単体の PHPUnit を本 PR で追加しました。境界値・異常系はユニット、経路の疎通は本体側の結合 E2E という分担です(「E2E が緑」は想定した経路が動くことの証拠でしかないため)。

Summary by CodeRabbit

  • 新機能
    • ACPおよびUCPに対応したカード決済処理を追加しました。
    • 与信、売上確定、追加認証、処理中、失敗などの決済状態を扱えるようになりました。
    • 決済結果や取引情報を統一的に管理できるようになりました。
    • 外部決済サービスを使わずに各種決済結果を再現できるモック環境を追加しました。

本体のエージェントチェックアウト (#6776 ACP / #6574 UCP) の決済ハンドラ seam に対する
サンプル実装。通常購入のトークン決済 (CreditCard) を流用し、Stripe/ChatGPT 非依存で
結合 E2E を緑化する。stripe-payment-plugin への移植リファレンスを兼ねる。

- Gateway 抽象 (PSP 隔離): AgentPaymentGatewayInterface + GatewayResult/GatewayStatus
  (Stripe PaymentIntent.status 語彙に整合) + MockAgentPaymentGateway
  (トークン規約で 成功/3DS/拒否/不正/処理中 を決定的に分岐)。
- AbstractAgentCardHandler: supports() = methodClass + Order.agentProtocol 一致、
  Order 総額→minor unit→gateway、GatewayResult→PaymentOutcome 写像。
- AcpSampleCardHandler (handler_id=card_tokenized, redeemSharedPaymentToken) /
  UcpSampleCardHandler (handler_id=dev.ucp.payment.card, exchangePaymentToken)。
- services.yaml は gateway interface→Mock の alias のみ (具象とタグは本体 glob/_instanceof が自動付与)。
  PluginManager は sort_no 操作なし。

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
決済ハンドラの agent_commerce.payment_handler タグは、本体 services.yaml の
_instanceof ではなく Kernel::build() の registerForAutoconfiguration が付与する。

EC-CUBE 本体 #6915 (PR #6917) で Plugin\ の PSR-4 自動登録が
app/config/eccube/services.yaml から services.php へ移った結果、
ファイルスコープの _instanceof はプラグインの具象クラスへ届かなくなった。
コメントが旧実装のままだったため、実際のタグ付け経路と理由を明記する。

コメントのみの変更で、コンテナ定義に変更はない。

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.45% 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
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed タイトルはACP/UCP決済ハンドラとGateway抽象の追加という主要変更を明確かつ簡潔に示しています。
✨ 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.

@nanasess
nanasess marked this pull request as ready for review July 30, 2026 02:56

@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: 2

🧹 Nitpick comments (3)
Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php (1)

60-69: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

capture() が失敗系トークンでも成功します。

capture() はトークン規約を判定しません。そのため *-decline**-fraud* のトークンでも SUCCEEDED を返します。authorize が失敗した取引に対して capture を呼ぶ異常系を E2E で再現できません。テストの網羅性を上げる場合、authorize() と同じ拒否判定を capture() にも適用してください。

♻️ 提案する変更
     public function capture(string $currencyCode, int $amount, array $instrument, array $context = []): GatewayResult
     {
         $token = $this->token($instrument);
         $transactionId = $this->transactionId($currencyCode, $amount, $token);
 
+        if (str_contains($token, 'fraud')) {
+            return GatewayResult::failed('card_not_supported', 'The card was blocked by fraud detection.', false);
+        }
+
+        if (str_contains($token, 'decline')) {
+            return GatewayResult::failed('card_declined', 'The card was declined.', true);
+        }
+
         return GatewayResult::succeeded(
🤖 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 `@Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php` around lines 60 -
69, Update MockAgentPaymentGateway::capture() to apply the same token-based
rejection rules as authorize(), including tokens matching the decline and fraud
patterns, before creating a successful GatewayResult. Preserve the existing
success behavior and metadata for accepted tokens.
Service/AgentCommerce/Gateway/GatewayResult.php (1)

42-75: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

failed() で metadata を保持できません。

failed()metadata を受け取りません。そのため FAILED 時に PSP 参照 (transaction_id 等) を payment_data へ残せません。障害調査や再試行の追跡を行う場合、metadata を任意引数として追加してください。

♻️ 提案する変更
-    public static function failed(string $errorCode, string $errorMessage = '', bool $retryable = true): self
-    {
-        return new self(GatewayStatus::FAILED, null, [], [], $errorCode, $errorMessage, $retryable);
-    }
+    /**
+     * `@param` array<string, mixed> $metadata
+     */
+    public static function failed(string $errorCode, string $errorMessage = '', bool $retryable = true, array $metadata = []): self
+    {
+        return new self(GatewayStatus::FAILED, null, $metadata, [], $errorCode, $errorMessage, $retryable);
+    }
🤖 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 `@Service/AgentCommerce/Gateway/GatewayResult.php` around lines 42 - 75, Update
GatewayResult::failed() to accept an optional array<string, mixed> $metadata
parameter, and pass it to the constructor instead of always using an empty
metadata array. Preserve the existing error and retryable arguments and default
behavior for callers that omit metadata.
Service/AgentCommerce/Gateway/AgentPaymentGatewayInterface.php (1)

38-46: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win

capture() に取引識別子を明示的に渡す設計を検討してください。

現在の capture() は取引識別子を引数に持たず、instrument から導出します。Mock 実装は決定的なハッシュで導出できるため成立します。しかし実 PSP は authorize で発行された ID (例: PaymentIntent ID) を必要とします。移植時にインターフェース変更が発生します。?string $transactionId を引数に追加すると、契約を安定させられます。

♻️ 提案する変更
-    public function capture(string $currencyCode, int $amount, array $instrument, array $context = []): GatewayResult;
+    public function capture(string $currencyCode, int $amount, array $instrument, array $context = [], ?string $transactionId = null): GatewayResult;
🤖 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 `@Service/AgentCommerce/Gateway/AgentPaymentGatewayInterface.php` around lines
38 - 46, Update the AgentPaymentGatewayInterface::capture() contract to accept
an explicit nullable transaction identifier, such as ?string $transactionId,
alongside the existing currency, amount, instrument, and context parameters.
Ensure the method documentation describes its purpose and preserve the existing
GatewayResult return type.
🤖 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 `@Service/AgentCommerce/Acp/AcpSampleCardHandler.php`:
- Around line 64-79: Update extractToken() in
Service/AgentCommerce/Acp/AcpSampleCardHandler.php (64-79) and
Service/AgentCommerce/Ucp/UcpSampleCardHandler.php (63-70) so unresolved or
missing tokens produce a validation error, either by throwing an exception or by
ensuring redeemSharedPaymentToken() returns an error instead of proceeding. Keep
valid token extraction behavior unchanged, and align both handlers’ extraction
coverage, preferably through a shared helper.

In `@Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php`:
- Around line 38-39: Update the authentication_result check in
MockAgentPaymentGateway to accept only a non-empty string, rejecting null, empty
strings, arrays (including empty arrays), and other types so interrupted 3DS
flows cannot be treated as authenticated.

---

Nitpick comments:
In `@Service/AgentCommerce/Gateway/AgentPaymentGatewayInterface.php`:
- Around line 38-46: Update the AgentPaymentGatewayInterface::capture() contract
to accept an explicit nullable transaction identifier, such as ?string
$transactionId, alongside the existing currency, amount, instrument, and context
parameters. Ensure the method documentation describes its purpose and preserve
the existing GatewayResult return type.

In `@Service/AgentCommerce/Gateway/GatewayResult.php`:
- Around line 42-75: Update GatewayResult::failed() to accept an optional
array<string, mixed> $metadata parameter, and pass it to the constructor instead
of always using an empty metadata array. Preserve the existing error and
retryable arguments and default behavior for callers that omit metadata.

In `@Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php`:
- Around line 60-69: Update MockAgentPaymentGateway::capture() to apply the same
token-based rejection rules as authorize(), including tokens matching the
decline and fraud patterns, before creating a successful GatewayResult. Preserve
the existing success behavior and metadata for accepted tokens.
🪄 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: 86d250d5-4eb1-44a6-8498-8c99012d590a

📥 Commits

Reviewing files that changed from the base of the PR and between cd186f0 and 1a0ab2b.

📒 Files selected for processing (8)
  • Resource/config/services.yaml
  • Service/AgentCommerce/AbstractAgentCardHandler.php
  • Service/AgentCommerce/Acp/AcpSampleCardHandler.php
  • Service/AgentCommerce/Gateway/AgentPaymentGatewayInterface.php
  • Service/AgentCommerce/Gateway/GatewayResult.php
  • Service/AgentCommerce/Gateway/GatewayStatus.php
  • Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php
  • Service/AgentCommerce/Ucp/UcpSampleCardHandler.php

Comment thread Service/AgentCommerce/Acp/AcpSampleCardHandler.php Outdated
Comment thread Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php Outdated

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

実装と本体 4.4 (origin/4.4) の該当箇所を照合したうえでのレビューです。行番号は本 PR head (1a0ab2b) 基準です。

個別の指摘【1】〜【12】は各行のインラインコメントに記載しました。以下は差分行に紐づけられないもの(【13】【14】)と参考事項です。


【13】【低】README の「ファイルごとの概要」に追加7ファイルが未記載

README.md:436 の一覧に、本 PR で追加された 7 クラス(AbstractAgentCardHandler / AcpSampleCardHandler / UcpSampleCardHandler / GatewayStatus / GatewayResult / AgentPaymentGatewayInterface / MockAgentPaymentGateway)がいずれも載っていません。参考実装が主目的の PR なので影響が大きいと考えます。


【14】【低】ユニットテストなし

別タスクと明記されている点は承知しています。ただし extractToken() / toOutcome() のマッピング / モックの分岐 / supports() は外部依存のない純粋ロジックで、単体テスト可能です(現状 tests/ は Playwright の .test.ts 4 本のみ)。


参考(欠陥ではない留意点)

supports() が false のとき、本体は「決済ハンドラ未登録=与信不要」として無与信で受注確定します(AgentCheckoutCompletionService.php:121-123)。supports() の条件を絞る変更は fail-open 方向に効くため、変更時はご注意ください。なお【1】はこれとは別経路で、ハンドラが正しく解決され authorize/capture とも成功を返しているのに実体が無い、という指摘です。

Comment thread Service/AgentCommerce/Acp/AcpSampleCardHandler.php Outdated
Comment thread Service/AgentCommerce/AbstractAgentCardHandler.php Outdated
Comment thread Service/AgentCommerce/AbstractAgentCardHandler.php Outdated
Comment thread Service/AgentCommerce/AbstractAgentCardHandler.php Outdated
Comment thread Service/AgentCommerce/AbstractAgentCardHandler.php Outdated
Comment thread Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php Outdated
Comment thread Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php Outdated
Comment thread Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php Outdated
Comment thread Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php Outdated
Comment thread Service/AgentCommerce/Gateway/MockAgentPaymentGateway.php
EC-CUBE#54 のレビュー指摘に対応する。いずれも「実装・モック・結合 E2E を
同じ設計から起こしたため検証が実装と同じ仮定を共有し、境界値・異常系が素通りしていた」ことが原因
なので、まず PHPUnit の基盤とユニットテストを入れ、そのうえで不具合を直す。

## PHPUnit 基盤

- `phpunit.xml.dist` / `Tests/bootstrap.php` を追加 (eccube-api4 と同じ流儀)。
- Playwright の `tests/` を `e2e/` へ改名。大文字小文字を区別しないファイルシステムで
  `Tests/` と衝突するため (本体も PHPUnit=tests / Playwright=e2e で分けている)。
- `.github/workflows/phpunit.yml` を追加。PHPStan は本体組み込み時の絶対パスを前提とする
  既存 phpstan.neon.dist を使うため CI からは実行せず、docker の手順を維持する。

## 修正した指摘事項

- トークンを解決できないときに空文字を返していたため、モックのどのトークン規約にも一致せず
  「正常な支払」と解釈され、無与信のまま受注が確定しえた。抽出を `PaymentTokenExtractor` へ
  集約し、解決できない場合は例外 → `PaymentOutcome::failed('invalid_payment_data')` とする
  (fail-closed)。ACP/UCP で抽出の対応範囲がずれていた点も解消。
- ACP の Shared Payment Token 償還が authorize と capture の 2 回走っていた。実 PSP の償還は
  ワンショットで 2 度目が失敗する。capture は本体から渡される与信結果の取引識別子を使う。
- UCP の `exchangePaymentToken()` が `authentication_result` を落としており、UCP では 3DS の
  requires_action から永久に復帰できなかった (本体で認証結果を扱うのは ACP 経路のみで、UCP は
  クレデンシャル経由でしか届かない)。
- 与信のみ (requires_capture) と売上確定済 (succeeded) を潰さず、本体の AUTHORIZED / COMPLETED
  へ分けて写像する。auto-capture 型 PSP へ差し替えたときの capture 二重発行を防ぐ。
- ゲートウェイ例外を捕捉して `PaymentOutcome::failed` へ写像する (未捕捉だと 500 になり、
  エージェントへ決済エラーとして返らない)。authorize と capture で retryable の意味が異なる点も
  コメントで明示。
- `requires_action` 時に metadata を渡しておらず PSP 参照が payment_data に残らなかった。
- 取引識別子の導出に `$context` を使っておらず、同額・同トークンの別注文で衝突していた。
- `authentication_result` の判定が緩く `[]` や `false` が認証済み扱いになっていた。
- `GatewayResult::failed()` だけ transactionId / metadata を持てず、失敗時に PSP 参照を残せなかった。
- モックの `capture()` が常に成功していたため、本体の capture 失敗分岐を結合 E2E で通せなかった。
  `*-capture-fail*` 規約を追加し、併せて未与信・二重 capture・金額不一致・トークン再償還を
  失敗させる (寛容なモックは呼び出し側の誤りを隠す)。
- トークン規約の doc と実装が食い違っていた (`*-3ds*` と `str_contains('3ds')`)。マーカーを
  ハイフン付きに統一し、評価順と大文字小文字の扱いを doc に明記してテストで固定した。
- `AbstractAgentCardHandler` の docblock が services.yaml と逆のことを述べていたので実装に合わせた。

本体側の対応 (与信/売上確定の区別・capture への与信結果引き渡し) を前提とするため、本 PR 単体では
CI が失敗する。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
postgres サービスコンテナが POSTGRES_DB で eccube_db を作成済みのため、
doctrine:database:create が Duplicate database で失敗していた。

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@nanasess
nanasess marked this pull request as draft August 6, 2026 08:26
@nanasess

nanasess commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

レビュー指摘 12 件への対応を push しました (8a90123 / 7f719d2)。個別のコメントにそれぞれ返信しています。

ドラフトへ戻しました。 本 PR は本体 EC-CUBE/ec-cube#7032 に依存します (AgentCheckoutPaymentHandlerInterface::capture() の引数追加・PaymentOutcomeStatus::AUTHORIZED の追加)。#7032 マージ前の 4.4 ではコンテナのコンパイル時に fatal になりプラグインが動作しません (Playwright ジョブが ec-cube コンテナ起動失敗で赤なのはこのためです)。#7032 マージ後に CI を再実行してドラフトを解除します。

検証状況:

項目 結果
PHPUnit (本 PR で新設) 79 tests / 157 assertions・ローカル (本体 4.4 + #7032) で pass
PHPStan level 6 (Service/AgentCommerce + Tests/Service/AgentCommerce) 0 件 (既存の legacy 部分の指摘は本 PR 対象外)
結合 E2E (ACP・HTTPS・api44 + samplepayment44 同居) 41 assertions PASS
結合 E2E (UCP・同上) 35 assertions PASS

E2E には負の入力 (トークン欠落)capture 失敗UCP の 3DS 再開を追加しました (本体 #7032 側)。今回の 12 件は「実装・モック・E2E を同じ設計から起こしたため、検証が実装と同じ仮定を共有していた」ことが共通の原因なので、境界値と異常系はユニットで押さえる分担に改めています。

本体 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>
nanasess added a commit to nanasess/ec-cube that referenced this pull request Aug 6, 2026
エージェントチェックアウトの決済ハンドラ契約に 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>
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>
@nanasess
nanasess marked this pull request as ready for review August 7, 2026 06:57

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

対応ありがとうございました。前回の 12 件すべて、返信の内容が現物のコードと一致していることを確認しました。LGTM です。

裏取りは PR head (e22cb7f2) をクローンして変更ファイルを通読し、本体側の契約は 4.4 の実物 (git show origin/4.4:...) で確認しています。連動する EC-CUBE/ec-cube#7032 が 4.4 へマージ済みなのも確認しました。

特に重い 2 件が「規約」ではなく「型」で担保された点が良かったです。

  • SPT の二重償還: toGatewayInstrument() の呼び出しが authorize の 1 箇所だけになり、ゲートウェイの capture()string $transactionId を受け取る形になったことで、capture でトークンを再利用する実装が書けなくなりました
  • auto-capture の二重発行: 本体に PaymentOutcomeStatus::AUTHORIZED が入り、AgentCheckoutCompletionService が AUTHORIZED のときだけ capture を呼ぶことを確認しました

モックを台帳持ちにして「未与信への capture」「capture 済みへの再 capture」「金額不一致」「別注文でのトークン再償還」を失敗させた点も、寛容なモックが呼び出し側の誤りを隠す問題への直接の回答になっていると思います。

CI も確認しました。

  • 本 PR: 17 チェックすべて pass(PHPUnit を PHP 8.2〜8.5 で新規追加、47 テスト)
  • 本体 4.4: agentic-commerce-e2e / Integration smoke (本体 + api44 + samplepayment44)8 マトリクス全て success。プラグイン側の契約変更と本体の対応が噛み合っていることが実証できています

追加で気づいた点(いずれもブロッカーではありません)

1. toGatewayInstrument() の非 InvalidPaymentDataException が未捕捉(低・移植時のみ発現)

AbstractAgentCardHandler::authorize() の最初の try は InvalidPaymentDataException だけを捕捉しています。サンプルのモックは配列を組むだけなので発現しませんが、実 PSP へ差し替えると ACP の redeemSharedPaymentToken() / UCP のトークン交換は API 呼び出しになります。そこで PSP SDK の通信例外が投げられると、この try をすり抜け、コントローラも AgentCheckoutException しか捕捉しないため HTTP 500 になります。

指摘 3 でゲートウェイ呼び出し側は塞がりましたが、同じ穴が instrument 生成側に残っている形です。同じ try に \Throwable を足して payment_gateway_error / retryable: true へ写像しておくと、移植境界の参考実装として対称になると思います。本 PR で対応してもよいですし、別途でも構いません。

2. capture 失敗の retryable について(情報・修正不要)

指摘 3 への返信では「capture も retryable: true」とありましたが、最終コード (e22cb7f2) は captureFailureIsRetryable() でプロトコル別になり ACP は false ですね。本体 AgentCheckoutPaymentHandlerInterface::capture() の docblock が「与信が残り、かつ同じ支払データで再 authorize できない場合 (ACP の SPT 等) は retryable=false」と規定しているので、最終コードのほうが正しいと判断しています。スレッドだけ読むと食い違って見えるため、念のため書き残しておきます。

3. ファクトリの引数順(情報・任意)

GatewayResult::requiresAction(actionData, transactionId, metadata) と本体 PaymentOutcome::requiresAction(actionData, metadata, transactionId) で順序が逆になっています(processing / pending も同様)。型が違うので取り違えれば PHPStan か実行時に落ちるため実害は小さいですが、docblock に一言あると安全かもしれません。


なお e2e/*.test.ts+0 -0 の新規追加に見えたので一度退行を疑いましたが、git diff -Mtests/e2e/ の 100% rename と確認できました。旧パスへの参照もワークフロー・README・composer.json に残っていません。PHPUnit 用の Tests/ との衝突回避という理解で合っていますか(大小文字を区別しない環境向け)。

前回の 12 スレッドは resolve します。

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.

2 participants