feat(agent-commerce): エージェントコマース共通基盤 Phase 1a (#6777)#6802
Open
nanasess wants to merge 13 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAgent Commerce 共通基盤を追加します：鍵ストアと署名器、国コードマスタとマイグレーション、住所マッピング、通貨 minor-unit 変換、OAuth スコープレジストリ、BaseInfo フラグ、管理画面項目と幅広いテスト群を導入します。 ChangesAgent Commerce Common Base
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
nanasess
commented
Jun 3, 2026
nanasess
commented
Jun 3, 2026
nanasess
added a commit
to nanasess/ec-cube
that referenced
this pull request
Jun 4, 2026
CI の rector ジョブ (PR EC-CUBE#6802) で検出された 8 ファイルの指摘を vendor/bin/rector で適用。 - AddressMappingService: 冗長な (int) キャストと三項を null 合体演算子に簡約 - UcpMessageSigner: コンストラクタプロパティ昇格 - テスト各種: self:: → $this->、final class 化、strict_types 宣言等 AgentCommerce テスト 53 件すべて成功を確認。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
ACP/UCP 対応の共通基盤のうち CheckoutSession 非依存の先行スライスを実装。 トラック A (Product Feed / Discovery) を解放する最小集合。 - MinorUnitConverter: 通貨 minor unit 変換 (bcmath / ゼロデシマル / 負数) - AddressMappingService: 住所マッピング・国コード numeric→alpha-2 (全249件網羅) - AgentCommerceScopeRegistry: <protocol>:<capability> scope 照合 - KeyStoreInterface / FilesystemKeyStore: 鍵保管 (env パス上書き→既定ファイル、EC-CUBE#6797 雛形) - AgentCommerceMessageSignerInterface / UcpMessageSigner: RFC 9421 EC P-256 / JWK 公開鍵 / 鍵ローテーション grace - BaseInfo にフラグ5 (acp/ucp 有効化等、default false) + google_pay_merchant_id + migration - 秘密鍵は dtb_baseinfo でなく app/keystore/ に保管 (EC-CUBE#6797、.htaccess/.gitignore 多重防御) プライバシー/利用規約 URL はカラム化せず標準ページ (help_privacy/help_agreement) から自動生成する方針。 検証: PHPUnit 53 tests / 611 assertions / 0 失敗、PHPStan level6 No errors、php-cs-fixer 0件。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
minor-unit は通貨の小数桁数だけ桁が増える (一律 ×100 ではない) ことが 一目で分かるよう、docblock の例を JPY (×1) / USD (×100) の 2 桁までに統一。 3 桁通貨 (BHD 等) の 4 桁例は紛らわしいため削除。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
ACP/UCP 用の 6 カラムは BaseInfo エンティティに #[ORM\Column] で定義済みのため、 公式アップデート手順 (doctrine:schema:update --force) で自動反映される。 カラム追加に ALTER TABLE マイグレーションを書かないのが EC-CUBE の慣例 (前例 PR EC-CUBE#4912: カラム追加に ALTER マイグレーション無し、INSERT のみ)。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI の rector ジョブ (PR EC-CUBE#6802) で検出された 8 ファイルの指摘を vendor/bin/rector で適用。 - AddressMappingService: 冗長な (int) キャストと三項を null 合体演算子に簡約 - UcpMessageSigner: コンストラクタプロパティ昇格 - テスト各種: self:: → $this->、final class 化、strict_types 宣言等 AgentCommerce テスト 53 件すべて成功を確認。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
決済はプラグイン化方針 (Stripe 同様) のため、Google Pay の merchant_id は core BaseInfo に持たせない。UCP discovery の payment_handlers は決済ハンドラ プラグインが寄与する設計とする。あわせて rector 適用後の php-cs-fixer 整形 (ライセンスヘッダ直後の空行) をテストへ反映。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nanasess
force-pushed
the
feature/agentic-commerce-base
branch
from
54968e2 to
74c74af
Compare
AddressMappingService の numeric->alpha-2 ハードコード const (249件) を廃止し、 新規マスタ mtb_country_iso_code で管理する。PR EC-CUBE#6802 レビュー指摘 (マスタテーブル化) に対応。 - 新規マスタ CountryIsoCode / CountryIsoCodeRepository を追加 - mtb_* の固定スキーマ (id/name/sort_no/discriminator_type) に準拠し、id=ISO numeric / name=alpha-2 を格納 (discriminator=countryisocode) - 新規インストールは import_csv (definition.yml 登録)、既存環境は INSERT データ migration で backfill (mtb_country は改変しない) - AddressMappingService はリポジトリ経由解決へ変更、テストを Layer 2 化 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Jun 4, 2026
CI rector ジョブ (ContainerGetNameToTypeInTestsRector / AssertFuncCallToPHPUnitAssertRector)
の指摘に対応。get('doctrine') + ManagerRegistry 手動構築をやめ、services_test.yaml で
AddressMappingService を public 化しコンテナから FQCN 取得する方式へ変更。
- services_test.yaml: AddressMappingService を public 化 (consumer 未実装で private では除去されるため)
- AddressMappingServiceTest: self::getContainer()->get(AddressMappingService::class) に簡素化
検証: PHPUnit 52/608、PHPStan level6 No errors、php-cs-fixer 0、rector dry-run クリーン。
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## 4.4 #6802 +/- ##
============================================
- Coverage 75.10% 74.88% -0.22%
============================================
Files 483 469 -14
Lines 26279 24223 -2056
============================================
- Hits 19736 18140 -1596
+ Misses 6543 6083 -460
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
src/Eccube/Service/AgentCommerce/Security/FilesystemKeyStore.php (1)
73-82: 💤 Low value
purposeパラメータのバリデーションを検討してください。
purposeがパス構築に直接使用されているため、../等を含む値が渡された場合、意図しないディレクトリへの書き込みが可能です。現在の DI 設定ではpurposeは固定値ですが、防御的プログラミングとしてバリデーションを追加することを推奨します。🛡️ バリデーション追加の提案
private function resolvePath(string $purpose): string { + if (!preg_match('/^[a-zA-Z0-9_-]+$/', $purpose)) { + throw new \InvalidArgumentException(sprintf('無効な purpose 識別子: "%s"', $purpose)); + } + $override = $this->envPathOverrides[$purpose] ?? ''; if ($override !== '') { return $override; } return $this->projectDir.'/app/keystore/agent-commerce/'.$purpose.'.key'; }🤖 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/Security/FilesystemKeyStore.php` around lines 73 - 82, resolvePath関数で渡されるpurposeがそのままファイルパスに使われており、"../"等のパス操作を含むと任意のディレクトリに書き込めてしまうので、目的(purpose)の検証を追加してください: resolvePath内で purpose が事前定義された許可リスト（例: 固定キー名配列）に含まれるか、もしくは正規表現でパス区切りや上位参照(".." または "/" や "\" )を含まないことをチェックし、無効な場合は例外を投げるか空の override を返すようにし、envPathOverrides と projectDir を使って組み立てる処理はその検証後にのみ実行するように変更してください。
🤖 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 `@app/DoctrineMigrations/Version20260604120000.php`:
- Around line 48-55: down() currently deletes all rows from mtb_country_iso_code
even when up() is a no-op; change it to be explicitly irreversible instead of
deleting data: update the down() method in Version20260604120000 to throw an
IrreversibleMigrationException (e.g.
\Doctrine\Migrations\IrreversibleMigrationException) with a clear message
referencing self::NAME/mtb_country_iso_code, instead of calling
$this->addSql('DELETE FROM mtb_country_iso_code'); this makes the migration
abort on rollback and prevents unintended data loss.
- Around line 39-43: The current migration in Version20260604120000 uses a
simple presence check ($this->connection->fetchOne('SELECT COUNT(*) FROM
'.self::NAME) and if ($count > 0) return;) which skips backfill entirely when
any row exists; change this to verify against the expected dataset and only
insert missing rows: compute the expected key set (or expected total count) for
self::NAME, query existing keys/rows from self::NAME, determine which specific
entries are missing, and perform inserts only for those missing entries so
partial failures can be repaired rather than skipping the whole backfill.
In `@src/Eccube/Service/AgentCommerce/MinorUnitConverter.php`:
- Around line 57-68: The toMinorUnits method accepts $amount strings but only
guards '' and '.'; before any BCMath calls (bcmul/bcadd) add a strict format
validation for $amount (in MinorUnitConverter::toMinorUnits) using a regex that
allows an optional sign, digits, optional single decimal point with digits (e.g.
/^[+-]?\d+(\.\d+)?$/), rejecting commas or multiple dots; if the check fails
return 0, otherwise proceed with existing scale/factor, $scaled and bcmath
operations so malformed inputs like 'abc', '1,000', or '1..2' never reach
bcmul/bcadd.
In `@src/Eccube/Service/AgentCommerce/Security/FilesystemKeyStore.php`:
- Around line 54-65: The write() method currently ignores failures from mkdir(),
file_put_contents(), and chmod(), so update FilesystemKeyStore::write to detect
errors and throw exceptions: after calling resolvePath($purpose) and computing
$dir, check mkdir($dir, 0700, true) return (when directory didn't exist) and
throw a RuntimeException (or a domain-specific exception) on failure; check
file_put_contents($path, $pem) result (=== false) and throw if it failed; also
check chmod($path, 0600) return and throw if it fails. Keep using the existing
method names resolvePath and write, include clear error messages that include
$path or $dir for debugging, and ensure no silent failure paths remain.
In `@tests/Eccube/Tests/Service/AgentCommerce/AddressMappingServiceTest.php`:
- Around line 172-174: After calling fopen($csvPath, 'r') in
AddressMappingServiceTest, validate that $handle is not false before calling
fgetcsv; if fopen failed, fail the test or throw a clear exception (e.g. use
$this->fail(...) or throw new \RuntimeException(...)) with the $csvPath included
in the message so the failure is diagnosable, then proceed to fgetcsv($handle,
...) when the handle is valid.
In `@tests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.php`:
- Around line 62-68: The current readBooleanFlag casts the getter result to
(bool) which hides nulls; instead call the getter on BaseInfo (using the same
dynamically-built names 'is'.$studly and 'get'.$studly), then check the raw
return value with is_bool(), return it if true, and if the getter returns null
or any non-bool throw a clear exception (or fail the test) indicating the
contract violation so nulls are not treated as false.
---
Nitpick comments:
In `@src/Eccube/Service/AgentCommerce/Security/FilesystemKeyStore.php`:
- Around line 73-82:
resolvePath関数で渡されるpurposeがそのままファイルパスに使われており、"../"等のパス操作を含むと任意のディレクトリに書き込めてしまうので、目的(purpose)の検証を追加してください:
resolvePath内で purpose が事前定義された許可リスト（例: 固定キー名配列）に含まれるか、もしくは正規表現でパス区切りや上位参照(".."
または "/" や "\" )を含まないことをチェックし、無効な場合は例外を投げるか空の override を返すようにし、envPathOverrides と
projectDir を使って組み立てる処理はその検証後にのみ実行するように変更してください。
🪄 Autofix (Beta)
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
Run ID: bbe18c12-17ac-40c7-be75-fe7fc5b9190e
⛔ Files ignored due to path filters (4)
src/Eccube/Resource/doctrine/import_csv/en/dtb_base_info.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/en/mtb_country_iso_code.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/ja/dtb_base_info.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/ja/mtb_country_iso_code.csvis excluded by!**/*.csv
📒 Files selected for processing (25)
.gitignore.htaccessapp/DoctrineMigrations/Version20260604120000.phpapp/config/eccube/services.yamlapp/config/eccube/services_test.yamlapp/keystore/.gitkeepapp/keystore/.htaccesssrc/Eccube/Entity/BaseInfo.phpsrc/Eccube/Entity/Master/CountryIsoCode.phpsrc/Eccube/Repository/Master/CountryIsoCodeRepository.phpsrc/Eccube/Resource/doctrine/import_csv/en/definition.ymlsrc/Eccube/Resource/doctrine/import_csv/ja/definition.ymlsrc/Eccube/Service/AgentCommerce/AddressMappingService.phpsrc/Eccube/Service/AgentCommerce/MinorUnitConverter.phpsrc/Eccube/Service/AgentCommerce/Security/AgentCommerceMessageSignerInterface.phpsrc/Eccube/Service/AgentCommerce/Security/AgentCommerceScopeRegistry.phpsrc/Eccube/Service/AgentCommerce/Security/FilesystemKeyStore.phpsrc/Eccube/Service/AgentCommerce/Security/KeyStoreInterface.phpsrc/Eccube/Service/AgentCommerce/Security/UcpMessageSigner.phptests/Eccube/Tests/Service/AgentCommerce/AddressMappingServiceTest.phptests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.phptests/Eccube/Tests/Service/AgentCommerce/Conformance/AgentCommerceBaseConformanceTest.phptests/Eccube/Tests/Service/AgentCommerce/MinorUnitConverterTest.phptests/Eccube/Tests/Service/AgentCommerce/Security/AgentCommerceScopeRegistryTest.phptests/Eccube/Tests/Service/AgentCommerce/Security/UcpMessageSignerTest.php
15 tasks
nanasess
added a commit
to nanasess/ec-cube
that referenced
this pull request
Jun 8, 2026
…トラックA) エージェントコマース共通基盤 Phase 1a (EC-CUBE#6802) の上に、CheckoutSession 非依存の トラック A (Product Feed / Catalog / Discovery) を 1 PR で実装する。 - Foundation: protocol 非依存 DTO (AgentCatalogItemDto 等) / ProductReferenceResolver (sku/product_class_id/barcode→ProductClass、barcode は Customize seam) / CatalogProvider (toIterable でメモリ蓄積回避) / CatalogMapper (表示商品のみ・price02→minor unit・availability) - ACP Feed: products.jsonl/metadata.json 生成 + pre-push スキーマ検証 (justinrainbow/json-schema) + push クライアント (AcpFeedClientInterface、outbound Bearer、Error shape 変換、多段ガード)。 実機送出は acp_feed_enabled + 設定有無でガード、Bearer は #[SensitiveParameter] - UCP Catalog: POST /catalog/{search,lookup,product} (v2026-04-08 の RPC 形式・GET ではない)。 UcpCatalogResponseBuilder (ucp wrapper) / FilesystemAdapter+Lock キャッシュ (stampede 防止) / gzip。無効時 404。認証必須モード・GraphQL は api4 未導入のため TODO。 lookup の variants[].inputs (input_correlation) は未実装 (schema 契約テストで incomplete 追跡) - Discovery: GET /.well-known/ucp。UcpProfileBuilder (signing_keys=getPublicJwks 公開鍵のみ・ capability 宣言・endpoint は RequestContext 動的生成)、Cache-Control public max-age=300。 payment_handlers はプラグイン寄与の tagged seam (既定空) - 管理画面 + CLI の 2 系統を標準提供: eccube:acp-feed:push --full / eccube:ucp-catalog:cache:warmup / :clear、管理画面ボタン (CSRF 検証・検証目的限定の注記) - キャッシュ無効化 EventListener (Product/ProductClass 更新→UCP キャッシュ clear) - 依存追加: symfony/http-client (ACP push 用、first-party)。framework.lock: flock (外部インフラ不要) - テスト: Layer 0 適合性 / Layer 1 マッピング / Layer 2 スキーマ契約・DB 突合 / Layer 3 Web / Layer 5 push。183 tests / 954 assertions / 0 failures (incomplete 7 は意図的)。 spec schema はリポジトリに同梱しない (Apache-2.0 露出・case-collision 回避): ACP は src/Eccube/Resource/AgentCommerce/Acp/schema.feed.json の runtime リソースを再利用、 UCP は CI で公式リポジトリから取得 (var/・gitignore) + ローカルは specifications/ フォールバック、 無ければ skip。手順は tests/.../AgentCommerce/README.md / 出所明記は各 README PHPStan level 6 No errors / php-cs-fixer 0 / rector clean。本 PR は eccube-api4 非依存。 Refs EC-CUBE#6794 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nanasess
added a commit
to nanasess/ec-cube
that referenced
this pull request
Jun 9, 2026
…トラックA) エージェントコマース共通基盤 Phase 1a (EC-CUBE#6802) の上に、CheckoutSession 非依存の トラック A (Product Feed / Catalog / Discovery) を 1 PR で実装する。 - Foundation: protocol 非依存 DTO (AgentCatalogItemDto 等) / ProductReferenceResolver (sku/product_class_id/barcode→ProductClass、barcode は Customize seam) / CatalogProvider (toIterable でメモリ蓄積回避) / CatalogMapper (表示商品のみ・price02→minor unit・availability) - ACP Feed: products.jsonl/metadata.json 生成 + pre-push スキーマ検証 (justinrainbow/json-schema) + push クライアント (AcpFeedClientInterface、outbound Bearer、Error shape 変換、多段ガード)。 実機送出は acp_feed_enabled + 設定有無でガード、Bearer は #[SensitiveParameter] - UCP Catalog: POST /catalog/{search,lookup,product} (v2026-04-08 の RPC 形式・GET ではない)。 UcpCatalogResponseBuilder (ucp wrapper) / FilesystemAdapter+Lock キャッシュ (stampede 防止) / gzip。無効時 404。認証必須モード・GraphQL は api4 未導入のため TODO。 lookup の variants[].inputs (input_correlation) は未実装 (schema 契約テストで incomplete 追跡) - Discovery: GET /.well-known/ucp。UcpProfileBuilder (signing_keys=getPublicJwks 公開鍵のみ・ capability 宣言・endpoint は RequestContext 動的生成)、Cache-Control public max-age=300。 payment_handlers はプラグイン寄与の tagged seam (既定空) - 管理画面 + CLI の 2 系統を標準提供: eccube:acp-feed:push --full / eccube:ucp-catalog:cache:warmup / :clear、管理画面ボタン (CSRF 検証・検証目的限定の注記) - キャッシュ無効化 EventListener (Product/ProductClass 更新→UCP キャッシュ clear) - 依存追加: symfony/http-client (ACP push 用、first-party)。framework.lock: flock (外部インフラ不要) - テスト: Layer 0 適合性 / Layer 1 マッピング / Layer 2 スキーマ契約・DB 突合 / Layer 3 Web / Layer 5 push。183 tests / 954 assertions / 0 failures (incomplete 7 は意図的)。 spec schema はリポジトリに同梱しない (Apache-2.0 露出・case-collision 回避): ACP は src/Eccube/Resource/AgentCommerce/Acp/schema.feed.json の runtime リソースを再利用、 UCP は CI で公式リポジトリから取得 (var/・gitignore) + ローカルは specifications/ フォールバック、 無ければ skip。手順は tests/.../AgentCommerce/README.md / 出所明記は各 README PHPStan level 6 No errors / php-cs-fixer 0 / rector clean。本 PR は eccube-api4 非依存。 Refs EC-CUBE#6794 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nanasess
added a commit
to nanasess/ec-cube
that referenced
this pull request
Jun 9, 2026
…トラックA) エージェントコマース共通基盤 Phase 1a (EC-CUBE#6802) の上に、CheckoutSession 非依存の トラック A (Product Feed / Catalog / Discovery) を 1 PR で実装する。 - Foundation: protocol 非依存 DTO (AgentCatalogItemDto 等) / ProductReferenceResolver (sku/product_class_id/barcode→ProductClass、barcode は Customize seam) / CatalogProvider (toIterable でメモリ蓄積回避) / CatalogMapper (表示商品のみ・price02→minor unit・availability) - ACP Feed: products.jsonl/metadata.json 生成 + pre-push スキーマ検証 (justinrainbow/json-schema) + push クライアント (AcpFeedClientInterface、outbound Bearer、Error shape 変換、多段ガード)。 実機送出は acp_feed_enabled + 設定有無でガード、Bearer は #[SensitiveParameter] - UCP Catalog: POST /catalog/{search,lookup,product} (v2026-04-08 の RPC 形式・GET ではない)。 UcpCatalogResponseBuilder (ucp wrapper) / FilesystemAdapter+Lock キャッシュ (stampede 防止) / gzip。無効時 404。認証必須モード・GraphQL は api4 未導入のため TODO。 lookup の variants[].inputs (input_correlation) は未実装 (schema 契約テストで incomplete 追跡) - Discovery: GET /.well-known/ucp。UcpProfileBuilder (signing_keys=getPublicJwks 公開鍵のみ・ capability 宣言・endpoint は RequestContext 動的生成)、Cache-Control public max-age=300。 payment_handlers はプラグイン寄与の tagged seam (既定空) - 管理画面 + CLI の 2 系統を標準提供: eccube:acp-feed:push --full / eccube:ucp-catalog:cache:warmup / :clear、管理画面ボタン (CSRF 検証・検証目的限定の注記) - キャッシュ無効化 EventListener (Product/ProductClass 更新→UCP キャッシュ clear) - 依存追加: symfony/http-client (ACP push 用、first-party)。framework.lock: flock (外部インフラ不要) - テスト: Layer 0 適合性 / Layer 1 マッピング / Layer 2 スキーマ契約・DB 突合 / Layer 3 Web / Layer 5 push。183 tests / 954 assertions / 0 failures (incomplete 7 は意図的)。 spec schema はリポジトリに同梱しない (Apache-2.0 露出・case-collision 回避): ACP は src/Eccube/Resource/AgentCommerce/Acp/schema.feed.json の runtime リソースを再利用、 UCP は CI で公式リポジトリから取得 (var/・gitignore) + ローカルは specifications/ フォールバック、 無ければ skip。手順は tests/.../AgentCommerce/README.md / 出所明記は各 README PHPStan level 6 No errors / php-cs-fixer 0 / rector clean。本 PR は eccube-api4 非依存。 Refs EC-CUBE#6794 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
BaseInfo の有効化フラグを「checkout の有無を制御する」意味へ整理する。 discovery / catalog は公開して害がないため常時公開とし (ゲート撤去は EC-CUBE#6794)、 ACP feed push は認証情報の有無で実質ガードされるためフラグ不要とする。 - 改名: acp_enabled → acp_checkout_enabled / ucp_enabled → ucp_checkout_enabled (getter は isAcpCheckoutEnabled / isUcpCheckoutEnabled) - 削除: acp_feed_enabled (push は base URL + API key の有無でガード) / ucp_catalog_api_enabled (UCP Catalog は常時公開) - 維持: ucp_catalog_requires_auth (catalog の OAuth 必須モードを api4 着手時に実装) - dtb_base_info.csv (ja/en) ヘッダを 3 フラグへ更新 - 店舗設定 (ShopMasterType / @admin/Setting/Shop/shop_master.twig) に acp_checkout_enabled / ucp_checkout_enabled のトグルを追加 (checkout は日本未提供の注記つき) - BaseInfoAgentCommerceFlagsTest を 3 フラグへ更新 PHPStan level 6 No errors / php-cs-fixer 0 / 関連テスト green。 カラム変更は schema:update 方式 (ALTER マイグレーションは書かない)。 Refs EC-CUBE#6777 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.php (1)
62-72: 💤 Low value型キャストによる
null隠蔽の懸念は strict_types で軽減されています。過去のレビューコメントで Line 67 の
(bool)キャストがnullをfalseに変換してしまう懸念が指摘されていますが、本ファイルはdeclare(strict_types=1);を宣言しており、かつエンティティの getter には戻り値型boolが明示されているため、仮にプロパティがnullであれば PHP がTypeErrorを投げます。そのため現在の実装でも実質的に安全です。ただし、より明示的な型チェック（
$this->assertIsBool($value, ...)）を追加することで、テストの意図がより明確になり、将来的な変更に対してもロバストになります。♻️ より明示的な型チェック案（任意）
private function readBooleanFlag(BaseInfo $BaseInfo, string $property): bool { $studly = str_replace('_', '', ucwords($property, '_')); foreach (['is'.$studly, 'get'.$studly] as $getter) { if (method_exists($BaseInfo, $getter)) { - return (bool) $BaseInfo->{$getter}(); + $value = $BaseInfo->{$getter}(); + $this->assertIsBool($value, sprintf('BaseInfo getter "%s" must return bool, got %s', $getter, get_debug_type($value))); + return $value; } }🤖 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 `@tests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.php` around lines 62 - 72, The readBooleanFlag helper currently casts the getter result to (bool) which can mask null; update readBooleanFlag to call the discovered getter (is{Studly} or get{Studly}) into a local $value, use $this->assertIsBool($value, sprintf('Getter %s must return bool for flag "%s"', $getter, $property)) to explicitly validate the type, then return $value; reference the function name readBooleanFlag and the dynamic getters 'is'.$studly and 'get'.$studly when making the change.
🤖 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.
Nitpick comments:
In `@tests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.php`:
- Around line 62-72: The readBooleanFlag helper currently casts the getter
result to (bool) which can mask null; update readBooleanFlag to call the
discovered getter (is{Studly} or get{Studly}) into a local $value, use
$this->assertIsBool($value, sprintf('Getter %s must return bool for flag "%s"',
$getter, $property)) to explicitly validate the type, then return $value;
reference the function name readBooleanFlag and the dynamic getters 'is'.$studly
and 'get'.$studly when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 683938ec-5be0-4856-90cf-8d42c31a408e
⛔ Files ignored due to path filters (2)
src/Eccube/Resource/doctrine/import_csv/en/dtb_base_info.csvis excluded by!**/*.csvsrc/Eccube/Resource/doctrine/import_csv/ja/dtb_base_info.csvis excluded by!**/*.csv
📒 Files selected for processing (6)
src/Eccube/Entity/BaseInfo.phpsrc/Eccube/Form/Type/Admin/ShopMasterType.phpsrc/Eccube/Resource/locale/messages.en.yamlsrc/Eccube/Resource/locale/messages.ja.yamlsrc/Eccube/Resource/template/admin/Setting/Shop/shop_master.twigtests/Eccube/Tests/Service/AgentCommerce/BaseInfoAgentCommerceFlagsTest.php
✅ Files skipped from review due to trivial changes (1)
- src/Eccube/Resource/locale/messages.en.yaml
- MinorUnitConverter: 不正な金額文字列 (abc / 1,000 / 1..2 等) を BCMath 呼び出し前に正規表現で弾き、ValueError を防止 (仕様の「不正は 0」契約遵守) - FilesystemKeyStore: 鍵書き込み時の mkdir / file_put_contents 失敗を RuntimeException で検知し、サイレント失敗を防止 - Version20260604120000: down() を不可逆 migration として明示し、 新規インストール (up() が no-op) 環境での import_csv 由来データ巻き添え削除を回避 - AddressMappingServiceTest: fopen 後に assertIsResource を追加し診断性を向上 - MinorUnitConverterTest: 不正・空入力の回帰テスト 8 ケースを追加 CodeRabbit 指摘のうち COUNT(*) 判定と (bool) キャストの 2 件は非妥当のため非対応。 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
MinorUnitConverter::toMinorUnits の入力検証を、正規表現での形式チェックから bcmul/bcadd の ValueError 捕捉へ変更する。 - BCMath が「受け付ける数値文字列形式」の唯一の権威であり、正規表現で再実装すると 仕様の二重管理・他箇所への正規表現の拡散を招くため、判定を BCMath 自身へ委譲する - 空文字 '' / '.' は BCMath が 0 として扱うため明示チェックも不要になり簡素化 - is_numeric は指数表記 (1e3) を true と判定するが BCMath では ValueError になり 不適 (実証済み)。ValueError 捕捉なら指数表記・hex も正しく 0 に倒せる - 回帰テストに指数表記 (1e3 / 1.5e3) と hex (0x1A) ケースを追加 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
15 tasks
FilesystemKeyStore::write が file_put_contents で umask 既定 (通常 0644) の パーミッションでファイルを作成し、その後 chmod(0600) するため、その間だけ 秘密鍵が group/other から読める瞬間が生じていた。書き込みの間だけ umask(0077) に切り替え、作成時点から 0600 になるようにする。あわせて chmod の戻り値を検査し失敗時に例外を投げる。 CodeRabbit レビュー指摘 (PR EC-CUBE#6825 経由・本ファイルは EC-CUBE#6802 由来) 対応。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
15 tasks
…ー対応) CodeRabbit レビュー指摘 (EC-CUBE#6777/EC-CUBE#6797 共通鍵保管) への対応。 - resolvePath の $purpose は既定パスへ直接連結されるため、"../" 等によるパストラバーサルを防ぐべく 許可文字 ([a-z0-9_-]) のみに制限し、許可外は InvalidArgumentException を投げる - FilesystemKeyStoreTest を追加 (不正 purpose の reject / 正常 purpose の受理) 検証: FilesystemKeyStoreTest 6 tests 0 失敗 / PHPStan level6 src No errors / php-cs-fixer 0 Refs EC-CUBE#6797 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
15 tasks
nanasess
added a commit
to nanasess/ec-cube
that referenced
this pull request
Jun 19, 2026
PR EC-CUBE#6843 のレビュー指摘のうち ACP 層所管の 6 件を修正: - complete の decodeBody() を try 内へ移動。不正 JSON が 500 ではなく 400 プロトコルエラーに変換され、create/update と一貫する - findSession にセッション所有者 (agent_id) 照合を追加。他エージェントの セッションへの越境アクセスを存在秘匿で 404 拒否 (多層防御) - line_items の id/quantity を正の整数のみ許可。非数値→0・負数の素通りを 防ぎ、不正値は AgentCheckoutException → 400 に寄せる - AcpMessageMapper の raw HTML 検知を包括化。任意タグ・HTML コメントを 検知しつつ CommonMark の autolink は誤検知しない - Discovery テストの max-age を固定値一致から >= 3600 の下限判定へ - テスト Stub に declare(strict_types=1) を追加 越境遮断・不正値・不正 JSON・raw HTML 包括検知の回帰テストを追加。 B 群 (EC-CUBE#6825 checkout-core) / C 群 (EC-CUBE#6802 base) 所管の指摘は各 PR 側で対応。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
CI の rector で検出された 2 件を適用:
- YieldDataProviderRector: data provider を return [...] から yield + 戻り型 \Iterator へ
- AttributeArgumentsOrderRector: #[DataProvider('...')] を #[DataProvider(methodName: '...')] へ
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要(Overview・Refs Issue)
エージェントコマース (ACP / UCP) 対応の共通基盤 #6777 のうち、CheckoutSession に依存しない先行スライス (Phase 1a) を実装します。これにより Product Feed / Catalog (#6794) と UCP Discovery (トラック A) が CheckoutSession 中核の完成を待たずに着手・先行リリース可能になります。
app/keystore/の雛形を含む (Refs 共通の鍵保管ディレクトリapp/keystore/の導入 (各種暗号鍵・シークレットの標準置き場) #6797)方針(Policy)
dtb_baseinfoに置かずapp/keystore/<feature>/に保管 (共通の鍵保管ディレクトリapp/keystore/の導入 (各種暗号鍵・シークレットの標準置き場) #6797)。解決順は環境変数パス上書き (Secrets Manager / Key Vault 連携) → 既定ファイル (有効化時自動生成・perm 600) = eccube-api4 の OAuth2 鍵と同方式。app/は web ルート内 (public-dir:".") のためapp/keystore/.htaccessdeny + ルート.htaccess拡張子ブロックにkey追加 + nginx/appdeny (既存) で多重防御。BaseInfoカラム化せず、既存標準ページ (help_privacy/help_agreement) から絶対 URL を実行時生成する方針 (本 PR ではマッピング基盤のみ)。Symfony\Component\Intl\Currencies::getFractionDigits()を再利用 (ゼロデシマル通貨を権威データで処理)、bcmath は既存nanasess/bcmath-polyfill。Country::getIsoCode()が存在せずmtb_country.idが ISO 3166-1 numeric のため、numeric→alpha-2 は新規マスタmtb_country_iso_code(id=ISO numeric / name=alpha-2 / 全 249 件) で管理しCountryIsoCodeRepository経由で解決する (ハードコードしない)。mtb_*の固定スキーマ (id/name/sort_no/discriminator_type) に準拠し、既存mtb_countryは改変しない。AgentCommerceMessageSignerInterfaceをアルゴリズム非依存にし、ACP の HMAC は後続で差し替え可能。実装に関する補足(Appendix)
追加クラス (
src/Eccube/Service/AgentCommerce/):MinorUnitConverterAddressMappingServicemtb_country_iso_codeをCountryIsoCodeRepository経由で解決) / Pref→regionEntity/Master/CountryIsoCode+Repository/Master/CountryIsoCodeRepositorycountryisocode)Security/AgentCommerceScopeRegistry<protocol>:<capability>scope の妥当性・protocol 照合Security/KeyStoreInterface+FilesystemKeyStoreSecurity/AgentCommerceMessageSignerInterface+UcpMessageSigner本体・設定:
BaseInfoにフラグ 3 (acp_checkout_enabled/ucp_checkout_enabled/ucp_catalog_requires_auth、いずれも default false) を追加。dtb_base_info.csvヘッダ追記、services.yamlに alias / bind。*_checkout_enabledは checkout の有無のみを制御する (checkout は日本未提供のため既定 OFF)。discovery / catalog は公開して害がないため常時公開とし、フラグでゲートしない。ACP feed push は認証情報 (base URL + API key) の有無で実質ガードされるため専用フラグは持たない (旧acp_enabled→acp_checkout_enabled/ucp_enabled→ucp_checkout_enabledに改名、acp_feed_enabled/ucp_catalog_api_enabledは削除)。ucp_catalog_requires_authは catalog の OAuth 必須モード用に維持 (api4 着手時に実装)。/admin/setting/shop) にトグルを追加:ShopMasterTypeにacp_checkout_enabled/ucp_checkout_enabledの有効化スイッチ (日本未提供の注記つき)。doctrine:schema:updateで反映、前例 PR Googleアナリティクス機能を追加 #4912)。google_pay_merchant_idは追加しない — 決済はプラグイン化方針 (Stripe 同様) のため、Google Pay の merchant_id は決済ハンドラプラグインが UCP discovery のpayment_handlersに寄与する。mtb_country_iso_code(Entity/Master/CountryIsoCode+ Repository): テーブルはschema:updateで生成、import_csv(definition.yml登録) で新規投入、既存環境は INSERT データ migration (Version20260604120000、COUNT ガード付き) で backfill。レビュー時の注意:
Shipping::getName01/getKana01(): stringは値 null で TypeError、Customerは?string)、AddressMappingServiceは TypeError を null 扱いで吸収しています。toString('JWK')で{kty,crv,x,y}を取得しています。テスト（Test)
tests/Eccube/Tests/Service/AgentCommerce/に Layer 0 (仕様適合性) / Layer 1 (純ロジック) / Layer 2 (Doctrine) / Layer 5 (署名) を追加。markTestIncomplete。AddressMappingServiceTestはマスタ参照のため Layer 2 化)paths: src): No errors相談（Discussion）
Eccube\Service\AgentCommerce) について意見をいただけると助かります。app/keystore/の新設は 共通の鍵保管ディレクトリapp/keystore/の導入 (各種暗号鍵・シークレットの標準置き場) #6797 として別途整理予定 (本 PR は雛形のみ)。マイナーバージョン互換性保持のための制限事項チェックリスト
dtb_base_infoinstall fixture のヘッダに列を追記したのみ・既存列は不変)レビュワー確認項目
Summary by CodeRabbit