From 358fce47babc23c2ae59b52def80febfca7c650f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 11:08:18 +0200 Subject: [PATCH 01/14] chore: update .gitignore to include Claude configuration and capability definitions --- .gitignore | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 53998dfc..8918c8d0 100644 --- a/.gitignore +++ b/.gitignore @@ -20,6 +20,13 @@ vendor # Environment variables .env !.env.example -.sdd/ -.claude/ -CLAUDE.md \ No newline at end of file + +.DS_Store + +# Claude files +CLAUDE.local.md +.claude/settings.local.json +.claude/cache/ +.claude/tmp/ +.claude/logs/ +.claude/artifacts/ \ No newline at end of file From 32e8983ff6109235c433dd4b46c0b96701c8e88c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 11:18:46 +0200 Subject: [PATCH 02/14] chore: update Developer Guide to clarify usage of bin scripts and PHPUnit execution --- DEVELOPER_GUIDE.md | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 9bb169aa..e0cfe9bc 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -13,7 +13,7 @@ This guide provides comprehensive information for developers working with the Se ## Available Development Tools -The `bin/` directory contains several essential scripts for development and code quality. These scripts are symlinked to their corresponding vendor packages. +The `bin/` directory contains several essential scripts for development and code quality. **These are Docker wrappers, not symlinks to local binaries.** `bin/phpcs`, `bin/phpcbf`, `bin/phpstan`, and `bin/phpunit` run inside the `php` Compose service via `docker compose exec php …`, so the container **must be running first** (`./setup.sh` or `docker compose up -d`). `bin/composer` and `bin/php-syntax-check` instead launch their own throwaway Docker images and do not require the Compose service. ### 1. **bin/composer** Package dependency manager for PHP projects. @@ -134,10 +134,17 @@ Unit testing framework for PHP. **Usage**: ```bash -# Run all tests +# Run all tests (the container must be running) ./bin/phpunit ``` +**Note**: `bin/phpunit` ignores any arguments you pass it and always runs the full suite. To run a single test class or method, invoke PHPUnit inside the container directly: + +```bash +docker compose exec php vendor/bin/phpunit --configuration phpunit.xml --filter testSomeMethod +docker compose exec php vendor/bin/phpunit --configuration phpunit.xml tests/Infrastructure/ServiceRegisterTest.php +``` + **Configuration**: See `phpunit.xml` --- @@ -323,8 +330,8 @@ Create or verify `.vscode/launch.json` in your project: docker compose up -d # VSCode: Press F5 to start listening -# Terminal 2: Run specific test -docker compose exec php php vendor/bin/phpunit tests/BusinessLogic/AdminAPI/AdminAPITest.php --filter testConnectionValidation +# Terminal 2: Run a specific test +docker compose exec php vendor/bin/phpunit --configuration phpunit.xml tests/Infrastructure/ServiceRegisterTest.php # In VSCode: Execution pauses at breakpoints # View variables in Debug panel @@ -454,13 +461,15 @@ docker compose exec -it php bash 4. **Run Tests** ```bash - # Run specific tests - docker compose exec php php vendor/bin/phpunit tests/BusinessLogic/ + # Run a subset of tests in the PHP 7.2 container + docker compose exec php vendor/bin/phpunit --configuration phpunit.xml tests/BusinessLogic/ - # Or full test suite - ./run-tests.sh + # Full suite in the container + ./bin/phpunit ``` + `run-tests.sh` is the CI gate: it runs the suite against **local** `/usr/bin/php7.4`–`php8.5` (not Docker), then phpcs + phpstan. It only works on a machine that has all of those PHP versions installed. + 5. **Debug Issues** (if needed) - Set breakpoints in VS Code or PhpStorm - Run tests with debugger attached From c84881dc3320cb75b38736c9538976177e199554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 11:29:23 +0200 Subject: [PATCH 03/14] docs: add CLAUDE.md with architecture, commands, and working principles Document the codebase for Claude Code: the platform-agnostic library layout, ServiceRegister/BootstrapComponent wiring, API facade + Aspects flow, multi-store context, and the integration-contract boundary. Capture the Docker-based bin/ tooling accurately (container must be up; bin/phpunit ignores args) and add adapted working principles tied to the project's phpunit/phpcs/phpstan verification gates. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 74 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..e08ca8fc --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,74 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## What this is + +`sequra/integration-core` is a **platform-agnostic PHP library** (PHP >= 7.2, pure PHP + vanilla JS, no framework deps) that holds the shared business logic and SeQura API communication layer for all SeQura e-commerce integrations (WooCommerce, PrestaShop, etc.). It is consumed as a Composer library — there is no app to "run". Host platforms wire it up, implement a set of integration interfaces, and call its API facades. + +Architecture follows the Onion model with two top-level namespaces (PSR-4): +- `SeQura\Core\Infrastructure\` → `src/Infrastructure` — technical foundation (HTTP, ORM abstraction, logger, task runner, service registry, serializer). +- `SeQura\Core\BusinessLogic\` → `src/BusinessLogic` — SeQura domain logic and the public API facades. + +## Working principles + +General working guidelines (adapted from [andrej-karpathy-skills/CLAUDE.md](https://github.com/forrestchang/andrej-karpathy-skills/blob/main/CLAUDE.md)). They bias toward caution over speed; for trivial tasks, use judgment. + +1. **Think before coding.** State assumptions explicitly and ask when uncertain. If multiple interpretations exist, surface them instead of silently picking one. If a simpler approach exists, say so. If something is unclear, stop and name what's confusing. +2. **Simplicity first.** Write the minimum code that solves the problem — nothing speculative. No features beyond what was asked, no abstractions for single-use code, no unrequested "configurability", no error handling for impossible scenarios. This matters here: the core is deliberately platform-agnostic and dependency-free, so resist pulling in new deps or framework-isms. +3. **Surgical changes.** Touch only what the request requires. Don't refactor working code, reformat adjacent lines, or "improve" comments. Match the existing style (PSR-12) even if you'd do it differently. Remove imports/variables your change orphaned, but leave pre-existing dead code alone — mention it rather than deleting it. Every changed line should trace to the request. +4. **Goal-driven execution.** Turn the task into a verifiable goal and loop until it's met, using this project's gates: a failing/repro test under `./bin/phpunit`, then green `./bin/phpcs` and `./bin/phpstan` (all in the PHP 7.2 container). "Add validation" → write tests for invalid inputs, then make them pass. "Fix the bug" → write a test that reproduces it, then make it pass. For multi-step work, state a brief plan with a verify step for each item. + +## Commands + +The `bin/` scripts are **Docker wrappers**, not local binaries. `phpunit`, `phpcs`, `phpcbf`, and `phpstan` run via `docker compose exec php …`, so the PHP 7.2 container **must be running first**: `./setup.sh` (or `docker compose up -d --build`). `composer` and `php-syntax-check` instead launch their own throwaway Docker images and don't need the container. + +```bash +./bin/composer install # install deps (runs in composer:latest image) +./bin/phpunit # full test suite, PHP 7.2 (uses phpunit.xml, --testdox) +./bin/phpcs # PSR-12 style check (config: .phpcs.xml.dist) +./bin/phpcbf # auto-fix style violations +./bin/phpstan # static analysis, level 6 over src/ (config: phpstan.neon) +./bin/php-syntax-check --php=7.2 # syntax-only check at a target PHP version (php:-cli-alpine) +``` + +**Running a single test:** `bin/phpunit` ignores any arguments you pass it and always runs the whole suite. To target one test, invoke PHPUnit inside the container directly: + +```bash +docker compose exec php vendor/bin/phpunit --configuration phpunit.xml --filter testSomeMethod +docker compose exec php vendor/bin/phpunit --configuration phpunit.xml tests/Infrastructure/ServiceRegisterTest.php +``` + +(`bin/phpstan` and `bin/composer` *do* forward arguments; `bin/phpcs`/`bin/phpcbf`/`bin/phpunit` do not.) + +`run-tests.sh` is the CI-equivalent gate: it runs the suite against **local** `/usr/bin/php7.4`–`php8.5` (not Docker) and then phpcs + phpstan. It only works on a machine that has all those PHP versions installed — it will not run on a typical dev box that lacks them. + +Tests are split into two PHPUnit suites: `tests/Infrastructure` and `tests/BusinessLogic`. The lowest supported version is **PHP 7.2** — `composer.json` pins the platform to 7.2, so do not use syntax newer than 7.2 in `src/`. The container runs XDebug 2.9.8 on port 9003; see `DEVELOPER_GUIDE.md` for IDE setup. + +## How the pieces fit together + +**Service locator, not a DI framework.** `Infrastructure\ServiceRegister` is a static singleton mapping a class/interface name → a lazy callable that builds the instance. Resolve with `ServiceRegister::getService(SomeInterface::class)`. Everything is wired through this; there is no autowiring. + +**`BootstrapComponent` is the wiring manifest.** `BusinessLogic\BootstrapComponent::init()` (extends `Infrastructure\BootstrapComponent`) registers every repository, service, controller, proxy, event listener, and webhook topic handler. When you add a new service/controller/repository, you must register it here, injecting its collaborators via `ServiceRegister::getService(...)`. The host platform calls `init()` once at startup after registering its own platform-specific implementations. + +**Four public API facades are the entry points** (`BusinessLogic/AdminAPI`, `CheckoutAPI`, `WebhookAPI`, `ConfigurationWebhookAPI`). Pattern, e.g. `AdminAPI::get()->connection($storeId)->validateConnection($request)`: +- The facade returns controllers wrapped by **Aspects** (`BusinessLogic/Bootstrap/Aspect`), which apply cross-cutting behavior before each method call. +- `ErrorHandlingAspect` wraps every call so controllers **return Response objects and never throw** to the caller — domain exceptions become `TranslatableErrorResponse`. Keep this contract: controllers return `Request`/`Response` DTOs. +- `StoreContextAspect($storeId)` sets the active store for the duration of the call. + +**Multi-store is pervasive.** `BusinessLogic\Domain\Multistore\StoreContext` holds the current store id; `StoreContext::doWithStore($storeId, $callback)` scopes execution. Store-scoped `DataAccess` repositories take `StoreContext` in their constructor and filter persistence by it. Anything reading/writing per-store config must respect the active store. + +**Domain layer shape** (`BusinessLogic/Domain//`): `Services/` (logic), `RepositoryContracts/` and `ProxyContracts/` (interfaces), `Models/`. Concrete repository implementations and ORM entities live separately under `BusinessLogic/DataAccess//{Repositories,Entities}`. + +**`BusinessLogic/Domain/Integration/` is the host platform's contract.** These interfaces (e.g. `Order`, `Product`, `Category`, `Store`, `Version`, `Log`, `SellingCountries`) are **implemented by the integrating platform**, not the core. The core registers and calls them; the platform supplies the implementations during its own bootstrap. When core code needs platform-specific data, it depends on one of these interfaces. + +**Talking to SeQura's HTTP API** goes through `BusinessLogic/SeQuraAPI` proxies (`OrderProxy`, `MerchantProxy`, etc.), built via `AuthorizedProxyFactory`/`ConnectionProxyFactory` on top of `Infrastructure\Http\HttpClient`. Domain services depend on `*ProxyInterface` contracts, never on the HTTP client directly. + +**Persistence is abstracted.** `Infrastructure\ORM` provides the repository pattern (`RepositoryRegistry`, `Entity`, `QueryFilter`); the host platform registers concrete storage repositories. The core never assumes a database. + +**Async work** runs through `Infrastructure\TaskExecution` (`QueueService`, `Task`, `TaskRunner`). Queue item state transitions emit events on `QueueItemStateTransitionEventBus`; `BootstrapComponent::initEvents()` wires `TransactionLog` listeners to those events. + +## Conventions + +- Match PSR-12 (`.phpcs.xml.dist`) and keep PHPStan level 6 clean before considering a change done; run `./bin/phpcs` and `./bin/phpstan`. +- New feature work typically touches three places: a `Domain/` service + contracts, a `DataAccess/` repository (if persisted), and a registration block in `BootstrapComponent`. API-exposed features also need a controller under the relevant `*API` facade and a method on the facade class. From b4501053095045c2c685a66cc6b16eaed1a068cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 11:35:38 +0200 Subject: [PATCH 04/14] chore: add project permission allowlist to Claude settings Allow the Docker-based bin/ tooling (phpunit, phpcs, phpcbf, phpstan, composer, php-syntax-check), docker compose, the setup/teardown scripts, read-only git commands, and the sq-git commit/open-pr/sync-branch skills so routine project commands run without per-call prompts. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/settings.json | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 .claude/settings.json diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..4676c79b --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,24 @@ +{ + "permissions": { + "allow": [ + "Bash(./bin/phpunit)", + "Bash(docker compose *)", + "Bash(./bin/phpcs)", + "Bash(./bin/phpcbf)", + "Bash(./bin/phpstan)", + "Bash(./bin/phpstan *)", + "Bash(./bin/composer *)", + "Bash(./bin/php-syntax-check *)", + "Bash(./setup.sh)", + "Bash(./teardown.sh)", + "Bash(git status)", + "Bash(git status *)", + "Bash(git diff *)", + "Bash(git log *)", + "Bash(git add *)", + "Skill(sq-git:commit)", + "Skill(sq-git:open-pr)", + "Skill(sq-git:sync-branch)" + ] + } +} From 676cc0532c2ee593c998906c310095845d8e5a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 12:22:43 +0200 Subject: [PATCH 05/14] chore: add additional Bash commands to Claude permissions --- .claude/settings.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.claude/settings.json b/.claude/settings.json index 4676c79b..eca9bf5d 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -16,6 +16,8 @@ "Bash(git diff *)", "Bash(git log *)", "Bash(git add *)", + "Bash(git --version)", + "Bash(git check-attr *)", "Skill(sq-git:commit)", "Skill(sq-git:open-pr)", "Skill(sq-git:sync-branch)" From eba163c7cd1c0e666334154eb774e82ae2bc0405 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 12:22:54 +0200 Subject: [PATCH 06/14] chore: update .gitattributes to exclude Claude configuration and hooks from export --- .gitattributes | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitattributes b/.gitattributes index a833833d..dfba2cea 100644 --- a/.gitattributes +++ b/.gitattributes @@ -6,6 +6,9 @@ /.gitignore export-ignore /phpstan.neon export-ignore DEVELOPER_GUIDE.md export-ignore +/CLAUDE.md export-ignore +/.claude/ export-ignore +/.githooks/ export-ignore /docker/ export-ignore /docker-compose.yml export-ignore /.phpcs.xml.dist export-ignore From aa7db10dcf9dd52b7515f96be3b455560f899504 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 12:24:15 +0200 Subject: [PATCH 07/14] chore: add pre-commit and pre-push hooks for quality checks --- .githooks/pre-commit | 80 ++++++++++++++++++++++++++++++++++++++++++++ .githooks/pre-push | 75 +++++++++++++++++++++++++++++++++++++++++ DEVELOPER_GUIDE.md | 28 +++++++--------- setup.sh | 7 ++++ 4 files changed, 175 insertions(+), 15 deletions(-) create mode 100755 .githooks/pre-commit create mode 100755 .githooks/pre-push diff --git a/.githooks/pre-commit b/.githooks/pre-commit new file mode 100755 index 00000000..349004aa --- /dev/null +++ b/.githooks/pre-commit @@ -0,0 +1,80 @@ +#!/bin/bash +# +# integration-core pre-commit quality gate (operates on staged .php files). +# +# Shipped in the repo and enabled by ./setup.sh via: +# git config core.hooksPath .githooks +# Bypass once with: +# git commit --no-verify +# +# Checks, in order: +# 1. PHP syntax across every supported version (7.2 + 7.4–8.5), using +# throwaway php:-cli-alpine images (the FIRST run pulls them all). +# 2. phpcbf then phpcs on the staged files — phpcbf's fixes are re-staged +# so they become part of the commit. +# 3. phpstan on staged files under src/ (the scope phpstan.neon targets). +# +# The full phpstan analysis and the PHPUnit suite run in the pre-push hook +# (.githooks/pre-push), not here, to keep per-commit feedback fast. +# +# The checks run inside / against the Docker "php" service. If it isn't +# running the hook FAILS (so unverified code can't slip through); start it +# with ./setup.sh or bypass with --no-verify. Git hooks have no TTY, so +# `docker compose exec -T` is used throughout. + +set -euo pipefail + +cd "$(git rev-parse --show-toplevel)" + +PHP_VERSIONS="7.2 7.4 8.0 8.1 8.2 8.3 8.4 8.5" + +# Staged PHP files (added/copied/modified), NUL-delimited to tolerate spaces. +STAGED_PHP=() +while IFS= read -r -d '' f; do + STAGED_PHP+=("$f") +done < <(git diff --cached --name-only --diff-filter=ACM -z -- '*.php') + +if [ ${#STAGED_PHP[@]} -eq 0 ]; then + echo "No staged PHP files — skipping PHP checks." + exit 0 +fi + +# Precondition: the Docker "php" service must be reachable. It backs every +# check here — the syntax images via `docker run` (needs the daemon) and +# phpcbf/phpcs/phpstan via `docker compose exec`. +if ! docker compose exec -T php true >/dev/null 2>&1; then + echo "✗ Docker 'php' service is not running — cannot run the pre-commit checks." + echo " Start it with ./setup.sh (or docker compose up -d)," + echo " or bypass with: git commit --no-verify" + exit 1 +fi + +echo "▶ PHP syntax check (${PHP_VERSIONS// /, }) on ${#STAGED_PHP[@]} staged file(s)..." +for ver in $PHP_VERSIONS; do + echo " php $ver..." + docker run --rm -v "$PWD":/app -w /app "php:${ver}-cli-alpine" \ + sh -c 'rc=0; for f in "$@"; do php -l -n "$f" >/dev/null || rc=1; done; exit $rc' _ "${STAGED_PHP[@]}" +done + +echo "▶ PHP Code Beautifier (phpcbf)..." +# phpcbf exits non-zero when it fixes things; phpcs below is the authority. +docker compose exec -T php sh -c 'vendor/bin/phpcbf --standard=.phpcs.xml.dist "$@"' _ "${STAGED_PHP[@]}" || true +# Re-stage whatever phpcbf fixed so the fixes are committed. +git add -- "${STAGED_PHP[@]}" + +echo "▶ PHP_CodeSniffer (phpcs)..." +docker compose exec -T php sh -c 'vendor/bin/phpcs --standard=.phpcs.xml.dist --warning-severity=0 "$@"' _ "${STAGED_PHP[@]}" + +# phpstan only over staged files under src/ (phpstan.neon targets src/). +SRC_PHP=() +for f in "${STAGED_PHP[@]}"; do + case "$f" in src/*) SRC_PHP+=("$f") ;; esac +done +if [ ${#SRC_PHP[@]} -gt 0 ]; then + echo "▶ PHPStan on ${#SRC_PHP[@]} staged src/ file(s)..." + docker compose exec -T php sh -c 'vendor/bin/phpstan analyse -c phpstan.neon --memory-limit=1G "$@"' _ "${SRC_PHP[@]}" +else + echo "▶ PHPStan — no staged src/ files, skipping." +fi + +echo "✓ Pre-commit checks passed." diff --git a/.githooks/pre-push b/.githooks/pre-push new file mode 100755 index 00000000..75aeb26e --- /dev/null +++ b/.githooks/pre-push @@ -0,0 +1,75 @@ +#!/bin/bash +# +# integration-core pre-push quality gate. +# +# Shipped in the repo and enabled by ./setup.sh via: +# git config core.hooksPath .githooks +# Bypass once with: +# git push --no-verify +# +# Runs the comprehensive checks that are too heavy (or can't be scoped to +# staged files) for the pre-commit hook: +# 1. Full PHPStan analysis over src/ (per phpstan.neon). +# 2. The full PHPUnit suite. +# +# These only analyse src/ and run the PHP test suite, so the hook first +# inspects the commits being pushed and SKIPS when none of them touch PHP code +# or the tooling config that drives the checks (composer.json/lock, +# phpunit.xml, phpstan.neon). A docs-only push therefore runs nothing. +# +# When the checks do run they execute inside the Docker "php" service; if it +# isn't running the hook FAILS (so unverified code can't be pushed). Start it +# with ./setup.sh or bypass with --no-verify. Git hooks have no TTY, so +# `docker compose exec -T` is used. + +set -euo pipefail + +cd "$(git rev-parse --show-toplevel)" + +# Files whose changes can affect phpstan/phpunit results. +RELEVANT='\.php$|^composer\.(json|lock)$|^phpunit\.xml$|^phpstan\.neon$' + +EMPTY_TREE=$(git hash-object -t tree /dev/null) +is_zero() { case "$1" in *[!0]*) return 1 ;; *) return 0 ;; esac; } + +# Inspect each ref being pushed (git pre-push feeds these on stdin as +# " "). +relevant=0 +while read -r _local_ref local_sha _remote_ref remote_sha; do + is_zero "$local_sha" && continue # branch deletion — nothing to check + + if is_zero "$remote_sha"; then + # New branch on the remote: diff the commits not yet on any remote. + oldest=$(git rev-list "$local_sha" --not --remotes | tail -1) + [ -z "$oldest" ] && continue # nothing new to push + if base=$(git rev-parse --quiet --verify "${oldest}^" 2>/dev/null); then :; else + base="$EMPTY_TREE" # oldest is a root commit + fi + else + base="$remote_sha" + fi + + if git diff --name-only "$base" "$local_sha" | grep -qE "$RELEVANT"; then + relevant=1 + fi +done + +if [ "$relevant" -eq 0 ]; then + echo "No PHP-relevant changes in the pushed commits — skipping phpstan/phpunit." + exit 0 +fi + +if ! docker compose exec -T php true >/dev/null 2>&1; then + echo "✗ Docker 'php' service is not running — cannot run the pre-push checks." + echo " Start it with ./setup.sh (or docker compose up -d)," + echo " or bypass with: git push --no-verify" + exit 1 +fi + +echo "▶ PHPStan (full analysis over src/)..." +docker compose exec -T php sh -c "vendor/bin/phpstan analyse -c phpstan.neon --memory-limit=1G" + +echo "▶ PHPUnit (full suite)..." +docker compose exec -T php sh -c "vendor/bin/phpunit --configuration phpunit.xml" + +echo "✓ Pre-push checks passed." diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index e0cfe9bc..7fa3763b 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -481,28 +481,26 @@ docker compose exec -it php bash git commit -m "feat: description of changes" ``` -### Git Pre-commit Hook (Optional) +### Git Hooks -Create `.git/hooks/pre-commit` to validate before commits: +The repo ships quality-gate hooks in `.githooks/`. `./setup.sh` enables them automatically (it sets `core.hooksPath` and marks them executable). To enable them manually — after cloning without running setup, for example: ```bash -#!/bin/bash -echo "Running syntax check..." -./bin/php-syntax-check --php=7.2 || exit 1 +git config core.hooksPath .githooks +``` -echo "Running code style check..." -./bin/phpcs || exit 1 +**`pre-commit`** — fast checks on the *staged* `.php` files: +- PHP syntax across every supported version (7.2 + 7.4–8.5), via throwaway `php:-cli-alpine` images. +- `phpcbf` then `phpcs`; phpcbf's auto-fixes are re-staged so they land in the commit. +- `phpstan` on staged files under `src/`. -echo "Running static analysis..." -docker compose exec php vendor/bin/phpstan analyse -c phpstan.neon --memory-limit=512M || exit 1 +**`pre-push`** — the heavier checks that can't be scoped to staged files: +- Full `phpstan` analysis over `src/`. +- The full PHPUnit suite. -echo "All checks passed!" -``` +The pre-push hook first inspects the commits being pushed and **skips entirely** when none of them touch PHP code or the tooling config that drives the checks (`composer.json`/`composer.lock`, `phpunit.xml`, `phpstan.neon`) — so a docs-only push runs nothing. -Make it executable: -```bash -chmod +x .git/hooks/pre-commit -``` +When their checks do run, both hooks execute inside / against the Docker `php` service. If it isn't running the hook **fails** rather than passing silently, so unverified code can't slip through — start it with `./setup.sh` (or `docker compose up -d`) first. Bypass a hook for one operation with `git commit --no-verify` / `git push --no-verify`. --- diff --git a/setup.sh b/setup.sh index 89606ea0..ba6143c9 100755 --- a/setup.sh +++ b/setup.sh @@ -3,4 +3,11 @@ if [ ! -f .env ]; then cp .env.sample .env fi +# Enable the shared git hooks (pre-commit and pre-push quality gates in .githooks/). +if git rev-parse --git-dir >/dev/null 2>&1; then + chmod +x .githooks/* 2>/dev/null + git config core.hooksPath .githooks + echo "Git hooks enabled (core.hooksPath=.githooks)." +fi + docker compose up -d --build || exit 1 \ No newline at end of file From af93428e0f2feb3081c2d6f37fe2b2f066cf538d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 12:58:41 +0200 Subject: [PATCH 08/14] chore: add scaffold-feature skill and integration-core-reviewer agent Add the first repo-specific Claude capabilities on top of the PAR-805 configuration baseline: - scaffold-feature skill: generates a new feature's skeleton across the Domain/DataAccess/BootstrapComponent/facade layers, grounded in the CountryConfiguration reference, with PHP 7.2 and the verify gate baked in. - integration-core-reviewer agent: architecture-aware diff review enforcing this repo's invariants (7.2 syntax, onion boundaries, Response-not-throw, StoreContext scoping, BootstrapComponent registration). Co-Authored-By: Claude --- .claude/agents/integration-core-reviewer.md | 77 +++++++++++++++++ .claude/skills/scaffold-feature/SKILL.md | 92 +++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 .claude/agents/integration-core-reviewer.md create mode 100644 .claude/skills/scaffold-feature/SKILL.md diff --git a/.claude/agents/integration-core-reviewer.md b/.claude/agents/integration-core-reviewer.md new file mode 100644 index 00000000..7b124b70 --- /dev/null +++ b/.claude/agents/integration-core-reviewer.md @@ -0,0 +1,77 @@ +--- +name: integration-core-reviewer +description: Architecture-aware reviewer for sequra/integration-core. Reviews the current diff against this repo's specific invariants — PHP 7.2 syntax, onion-layer boundaries, the Response-not-throw facade contract, multistore StoreContext scoping, and BootstrapComponent registration — beyond what the generic /code-review catches. Use for reviewing a branch/PR or staged changes in this repo. +tools: Bash, Read, Grep, Glob +--- + +# integration-core reviewer + +You review changes to `sequra/integration-core` — a platform-agnostic PHP library +(PHP >= 7.2, no framework deps) following the Onion model. You enforce **this repo's +invariants**, not generic PHP style (phpcs/phpstan already cover style/types). Report +concrete, file:line findings; do not rewrite the code. + +## How to run + +1. Scope the diff: `git diff master...HEAD` (or `git diff --staged` / `git diff` as appropriate). +2. Orient before reading source: graphify-out/graph.json exists, so run + `graphify query ""`, `graphify explain ""`, or + `graphify path "" ""` first; only read raw files to inspect the changed lines. +3. Review each changed file against the checklist below. + +## Invariants to enforce (in priority order) + +1. **PHP 7.2 syntax in `src/`.** `composer.json` pins the platform to 7.2. Flag anything + newer: arrow functions (`fn() =>`), null-safe `?->`, named arguments, `match`, enums, + constructor property promotion, typed properties, union/intersection types, `readonly`, + first-class callable syntax, trailing-comma-in-params (7.3+). Tests may run on newer PHP, + but `src/` must stay 7.2-clean. + +2. **Onion-layer boundaries.** Dependencies point inward only: + - A `Domain/.../Services` class must depend on **interfaces** (`*RepositoryInterface`, + `*ProxyInterface`) and other domain services — **never** on a concrete `DataAccess` + repository, an ORM class, or `Infrastructure\Http\HttpClient` directly. + - SeQura HTTP calls go through a `SeQuraAPI` proxy behind a `ProxyContracts` interface, + never through `HttpClient` from a service. + - Platform-specific data must come through a `Domain/Integration/*` interface (these are + implemented by the host platform, not the core) — flag core code that hardcodes + platform assumptions instead of depending on an Integration contract. + +3. **Facade contract: controllers return, never throw.** Classes under `*API/.../` + controllers must return `Request`/`Response` DTOs. A controller method that can throw a + domain exception to the caller breaks the `ErrorHandlingAspect` contract — flag it. + New facade methods must wrap the controller in `Aspects` with `ErrorHandlingAspect` + and, for store-scoped calls, `StoreContextAspect($storeId)`. + +4. **Multistore scoping.** Any `DataAccess` repository reading/writing per-store config must + filter by `storeContext->getStoreId()` (via `QueryFilter`) and stamp `storeId` on + save/update. Flag a query that omits the store filter or a save that doesn't set `storeId`. + New store-scoped repos must take `StoreContext` in the constructor. + +5. **BootstrapComponent registration.** A new service/repository/controller/proxy/entity is + dead unless registered. Check that `BootstrapComponent` has the matching + `ServiceRegister::registerService(...)` block in the right `init*()` method + (`initRepositories`/`initServices`/`initControllers`/`initProxies`), that collaborators are + injected via `ServiceRegister::getService(...)`, and that new ORM entities are registered so + `RepositoryRegistry::getRepository(...)` resolves them. A new interface with no concrete + registration is a finding. + +6. **ORM entity correctness.** Entities `extends Entity` must declare + `public const CLASS_NAME = __CLASS__;` and implement `inflate()`, `toArray()`, `getConfig()`; + `getConfig()`'s `IndexMap` must index any field used in a `QueryFilter` (notably `storeId`). + +7. **Scope discipline (CLAUDE.md).** Flag speculative abstractions, unrequested + configurability, error handling for impossible cases, refactors of untouched code, and + reformatting of adjacent lines. Every changed line should trace to the stated task. Note + pre-existing dead code rather than deleting it. + +8. **Tests + gate.** New domain logic / controllers should have tests under + `tests/BusinessLogic/...` or `tests/Infrastructure/...`. Remind that the change isn't done + until `./bin/phpcs`, `./bin/phpstan`, and `./bin/phpunit` are green in the PHP 7.2 container. + +## Output format + +Group findings by severity: **Blocking** (broken invariant / 7.2 violation / missing +registration / store-scope leak), **Should-fix** (boundary smell, missing test, scope creep), +**Nit**. For each: `file:line` + one-line problem + the minimal fix. If a layer is clean, say +so briefly. Be specific and terse — no generic PHP advice. diff --git a/.claude/skills/scaffold-feature/SKILL.md b/.claude/skills/scaffold-feature/SKILL.md new file mode 100644 index 00000000..9ae2b2f6 --- /dev/null +++ b/.claude/skills/scaffold-feature/SKILL.md @@ -0,0 +1,92 @@ +--- +name: scaffold-feature +description: Scaffold a new SeQura integration-core feature end-to-end following repo conventions — Domain service/contracts/models, a store-scoped DataAccess repository + ORM entity, the BootstrapComponent registration block, and (when API-exposed) a facade controller with Request/Response DTOs plus a facade method. Use when adding a new persisted and/or API-exposed feature under src/BusinessLogic/Domain/. +--- + +# Scaffold an integration-core feature + +This repo adds a feature in a fixed, convention-heavy shape spread across three to +four places. This skill generates that skeleton so nothing is forgotten and every +file matches the existing style. **`CountryConfiguration` is the canonical reference** +— read those files when a template detail is unclear. + +## Before you start + +1. Get the feature name in `PascalCase` (e.g. `ShippingRules`) and decide: + - **Persisted?** → needs a `DataAccess/` repository + ORM entity. + - **API-exposed?** → needs a controller under a facade (`AdminAPI`, `CheckoutAPI`, + `WebhookAPI`, or `ConfigurationWebhookAPI`) + a facade method. + - **Talks to SeQura's HTTP API?** → also needs a `ProxyContracts/` interface and a + `SeQuraAPI` proxy (see `OrderProxy`). Out of scope for the basic skeleton — flag it. +2. Hard constraints (these are non-negotiable in `src/`): + - **PHP 7.2 syntax only.** No arrow fns, no `?->`, no named args, no enums, no + constructor promotion, no union/typed-property syntax newer than 7.2. + - **PSR-12** (`.phpcs.xml.dist`) and **PHPStan level 6** clean. + - Namespaces are PSR-4: `SeQura\Core\BusinessLogic\...` → `src/BusinessLogic/...`. + - Every class/interface/method gets a docblock matching the surrounding style. + +## Layers to generate + +For feature `` with a domain model ``: + +### 1. Domain (`src/BusinessLogic/Domain//`) +- `Models/.php` — plain immutable-ish model: constructor + getters, no framework deps. +- `RepositoryContracts/RepositoryInterface.php` — the persistence contract. + Docblocks say **"for current store context"** — the store scoping is implicit, never a param. +- `Services/Service.php` — business logic. Depends on the **`...RepositoryInterface`** + (and other `*Interface`/`*Service` collaborators), **never** on a concrete repository, + the ORM, or `HttpClient`. +- `Exceptions/*.php` — domain exceptions. If the error must surface to an API caller as a + translatable message, extend the translatable base (see `Domain/Translations`) so + `ErrorHandlingAspect` can turn it into a `TranslatableErrorResponse`. + +### 2. DataAccess (`src/BusinessLogic/DataAccess//`) — only if persisted +- `Entities/.php` — `extends Entity`. Must declare `public const CLASS_NAME = __CLASS__;`, + a `protected $storeId;`, and implement `inflate()`, `toArray()`, `getConfig()`. `getConfig()` + returns an `EntityConfiguration` whose `IndexMap` adds at least `addStringIndex('storeId')`. +- `Repositories/Repository.php` — `implements RepositoryInterface`. + Constructor takes `(RepositoryInterface $repository, StoreContext $storeContext)`. **Every** + read/write filters by `storeContext->getStoreId()` via a `QueryFilter` and stamps `storeId` + on save/update. This is the multistore contract — never skip it. + +### 3. BootstrapComponent registration (`src/BusinessLogic/BootstrapComponent.php`) +Add a `ServiceRegister::registerService(...)` lazy-callable block in the matching `init*()` method: +- repository interface → concrete repo in **`initRepositories()`** (resolve the ORM repo with + `RepositoryRegistry::getRepository(::getClassName())` and inject `StoreContext`). +- service → in **`initServices()`** (inject the repo interface + collaborators via `ServiceRegister::getService(...)`). +- controller → in **`initControllers()`** (inject the service(s)). +- Also ensure the ORM **entity class is registered** wherever the entity list lives + (host platform / `RepositoryRegistry`); the core references it via `getClassName()`. +- If the feature emits/handles events or webhook topics, wire `initEvents()` / `initTopicHandlers()` too. + +### 4. API facade (only if API-exposed) — e.g. `src/BusinessLogic/AdminAPI//` +- `Controller.php` — **returns `Response` objects and NEVER throws** to the caller. + Each method returns a typed `*Response`; mutating methods take a typed `*Request`. +- `Requests/*.php` — `extends Request`, expose `transformToDomainModel()`. +- `Responses/*.php` — `extends Response`, implement `toArray()`. +- Add a facade method on the facade class (e.g. `AdminAPI::(string $storeId): Aspects`) + returning `Aspects::run(new ErrorHandlingAspect())->andRun(new StoreContextAspect($storeId)) + ->beforeEachMethodOfService(Controller::class)`. + +### 5. Tests (`tests/BusinessLogic//`) +Mirror an existing controller/service test (see `tests/BusinessLogic/AdminAPI/CountryConfiguration/`). +Register the ORM entity in the test bootstrap so the in-memory repository can resolve it. + +## Verify (the project gate — required before calling it done) + +The Docker `php` service must be up (`./setup.sh`). Then, per `CLAUDE.md`: +```bash +./bin/php-syntax-check --php=7.2 # 7.2 syntax sanity on new files +./bin/phpcbf && ./bin/phpcs # PSR-12 +./bin/phpstan # level 6 over src/ +./bin/phpunit # full suite (bin/phpunit ignores args) +``` +Single test while iterating: +`docker compose exec php vendor/bin/phpunit --configuration phpunit.xml --filter ` + +## Output discipline + +Generate only the files the feature actually needs — skip DataAccess if not persisted, +skip the facade if not API-exposed. Don't invent configurability or extra methods beyond +what was asked (CLAUDE.md: simplicity first, surgical changes). After generating, list every +file created/edited and confirm the registration block was added to `BootstrapComponent`. From c13ff0c7bad063e453f26accc32d4e94232c561e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 13:09:07 +0200 Subject: [PATCH 09/14] chore: address review nits in setup.sh and reviewer agent - setup.sh: add missing trailing newline. - integration-core-reviewer: correct PHP trailing-comma versions (calls 7.3+, parameter lists 8.0+). Co-Authored-By: Claude --- .claude/agents/integration-core-reviewer.md | 3 ++- setup.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.claude/agents/integration-core-reviewer.md b/.claude/agents/integration-core-reviewer.md index 7b124b70..8e077a29 100644 --- a/.claude/agents/integration-core-reviewer.md +++ b/.claude/agents/integration-core-reviewer.md @@ -24,7 +24,8 @@ concrete, file:line findings; do not rewrite the code. 1. **PHP 7.2 syntax in `src/`.** `composer.json` pins the platform to 7.2. Flag anything newer: arrow functions (`fn() =>`), null-safe `?->`, named arguments, `match`, enums, constructor property promotion, typed properties, union/intersection types, `readonly`, - first-class callable syntax, trailing-comma-in-params (7.3+). Tests may run on newer PHP, + first-class callable syntax, trailing comma in calls (7.3+) / in parameter lists (8.0+). + Tests may run on newer PHP, but `src/` must stay 7.2-clean. 2. **Onion-layer boundaries.** Dependencies point inward only: diff --git a/setup.sh b/setup.sh index ba6143c9..c75e90c5 100755 --- a/setup.sh +++ b/setup.sh @@ -10,4 +10,4 @@ if git rev-parse --git-dir >/dev/null 2>&1; then echo "Git hooks enabled (core.hooksPath=.githooks)." fi -docker compose up -d --build || exit 1 \ No newline at end of file +docker compose up -d --build || exit 1 From 8d894bc3f6a7cc3a6c1300832fe52cf2798c61f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 13:20:23 +0200 Subject: [PATCH 10/14] chore: update pre-commit and pre-push hooks for improved PHP syntax checks and documentation --- .githooks/pre-commit | 25 ++++++++++--------------- .githooks/pre-push | 39 +++++++++++++++++++++++++++++++++------ DEVELOPER_GUIDE.md | 3 ++- 3 files changed, 45 insertions(+), 22 deletions(-) diff --git a/.githooks/pre-commit b/.githooks/pre-commit index 349004aa..091bf646 100755 --- a/.githooks/pre-commit +++ b/.githooks/pre-commit @@ -8,14 +8,15 @@ # git commit --no-verify # # Checks, in order: -# 1. PHP syntax across every supported version (7.2 + 7.4–8.5), using -# throwaway php:-cli-alpine images (the FIRST run pulls them all). +# 1. PHP 7.2 syntax (the language floor) on the staged files, reusing the +# already-running php:7.2 container — no image pulls, so this stays fast. # 2. phpcbf then phpcs on the staged files — phpcbf's fixes are re-staged # so they become part of the commit. # 3. phpstan on staged files under src/ (the scope phpstan.neon targets). # -# The full phpstan analysis and the PHPUnit suite run in the pre-push hook -# (.githooks/pre-push), not here, to keep per-commit feedback fast. +# The multi-version syntax sweep (7.2–8.5), the full phpstan analysis, and the +# PHPUnit suite run in the pre-push hook (.githooks/pre-push), not here, to keep +# per-commit feedback fast. # # The checks run inside / against the Docker "php" service. If it isn't # running the hook FAILS (so unverified code can't slip through); start it @@ -26,8 +27,6 @@ set -euo pipefail cd "$(git rev-parse --show-toplevel)" -PHP_VERSIONS="7.2 7.4 8.0 8.1 8.2 8.3 8.4 8.5" - # Staged PHP files (added/copied/modified), NUL-delimited to tolerate spaces. STAGED_PHP=() while IFS= read -r -d '' f; do @@ -39,9 +38,9 @@ if [ ${#STAGED_PHP[@]} -eq 0 ]; then exit 0 fi -# Precondition: the Docker "php" service must be reachable. It backs every -# check here — the syntax images via `docker run` (needs the daemon) and -# phpcbf/phpcs/phpstan via `docker compose exec`. +# Precondition: the Docker "php" service must be reachable. Every check here +# runs through it via `docker compose exec` — the 7.2 syntax lint, phpcbf, +# phpcs, and phpstan. if ! docker compose exec -T php true >/dev/null 2>&1; then echo "✗ Docker 'php' service is not running — cannot run the pre-commit checks." echo " Start it with ./setup.sh (or docker compose up -d)," @@ -49,12 +48,8 @@ if ! docker compose exec -T php true >/dev/null 2>&1; then exit 1 fi -echo "▶ PHP syntax check (${PHP_VERSIONS// /, }) on ${#STAGED_PHP[@]} staged file(s)..." -for ver in $PHP_VERSIONS; do - echo " php $ver..." - docker run --rm -v "$PWD":/app -w /app "php:${ver}-cli-alpine" \ - sh -c 'rc=0; for f in "$@"; do php -l -n "$f" >/dev/null || rc=1; done; exit $rc' _ "${STAGED_PHP[@]}" -done +echo "▶ PHP 7.2 syntax check on ${#STAGED_PHP[@]} staged file(s)..." +docker compose exec -T php sh -c 'rc=0; for f in "$@"; do php -l -n "$f" >/dev/null || rc=1; done; exit $rc' _ "${STAGED_PHP[@]}" echo "▶ PHP Code Beautifier (phpcbf)..." # phpcbf exits non-zero when it fixes things; phpcs below is the authority. diff --git a/.githooks/pre-push b/.githooks/pre-push index 75aeb26e..be5db0fd 100755 --- a/.githooks/pre-push +++ b/.githooks/pre-push @@ -9,12 +9,16 @@ # # Runs the comprehensive checks that are too heavy (or can't be scoped to # staged files) for the pre-commit hook: -# 1. Full PHPStan analysis over src/ (per phpstan.neon). -# 2. The full PHPUnit suite. +# 1. PHP syntax sweep across every supported version (7.2–8.5) over the +# pushed .php files, using throwaway php:-cli-alpine images (the FIRST +# push pulls them all). Pre-commit only lints 7.2, so this is where the +# newer-version syntax check lives. +# 2. Full PHPStan analysis over src/ (per phpstan.neon). +# 3. The full PHPUnit suite. # -# These only analyse src/ and run the PHP test suite, so the hook first -# inspects the commits being pushed and SKIPS when none of them touch PHP code -# or the tooling config that drives the checks (composer.json/lock, +# These only analyse src/, lint the changed PHP, and run the test suite, so the +# hook first inspects the commits being pushed and SKIPS when none of them touch +# PHP code or the tooling config that drives the checks (composer.json/lock, # phpunit.xml, phpstan.neon). A docs-only push therefore runs nothing. # # When the checks do run they execute inside the Docker "php" service; if it @@ -33,8 +37,10 @@ EMPTY_TREE=$(git hash-object -t tree /dev/null) is_zero() { case "$1" in *[!0]*) return 1 ;; *) return 0 ;; esac; } # Inspect each ref being pushed (git pre-push feeds these on stdin as -# " "). +# " "). Accumulate the changed +# .php files (added/copied/modified) for the syntax sweep as we go. relevant=0 +CHANGED_PHP=() while read -r _local_ref local_sha _remote_ref remote_sha; do is_zero "$local_sha" && continue # branch deletion — nothing to check @@ -52,6 +58,10 @@ while read -r _local_ref local_sha _remote_ref remote_sha; do if git diff --name-only "$base" "$local_sha" | grep -qE "$RELEVANT"; then relevant=1 fi + + while IFS= read -r -d '' f; do + CHANGED_PHP+=("$f") + done < <(git diff --name-only --diff-filter=ACM -z "$base" "$local_sha" -- '*.php') done if [ "$relevant" -eq 0 ]; then @@ -66,6 +76,23 @@ if ! docker compose exec -T php true >/dev/null 2>&1; then exit 1 fi +# Multi-version syntax sweep over the pushed .php files (7.2–8.5), using +# throwaway php:-cli-alpine images. Only files still present in the working +# tree are linted (php -l needs the content on disk; deletions are skipped). +PHP_VERSIONS="7.2 7.3 7.4 8.0 8.1 8.2 8.3 8.4 8.5" +SYNTAX_PHP=() +for f in ${CHANGED_PHP[@]+"${CHANGED_PHP[@]}"}; do + [ -f "$f" ] && SYNTAX_PHP+=("$f") +done +if [ ${#SYNTAX_PHP[@]} -gt 0 ]; then + echo "▶ PHP syntax sweep (${PHP_VERSIONS// /, }) on ${#SYNTAX_PHP[@]} changed file(s)..." + for ver in $PHP_VERSIONS; do + echo " php $ver..." + docker run --rm -v "$PWD":/app -w /app "php:${ver}-cli-alpine" \ + sh -c 'rc=0; for f in "$@"; do php -l -n "$f" >/dev/null || rc=1; done; exit $rc' _ "${SYNTAX_PHP[@]}" + done +fi + echo "▶ PHPStan (full analysis over src/)..." docker compose exec -T php sh -c "vendor/bin/phpstan analyse -c phpstan.neon --memory-limit=1G" diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 7fa3763b..9bf86f46 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -490,11 +490,12 @@ git config core.hooksPath .githooks ``` **`pre-commit`** — fast checks on the *staged* `.php` files: -- PHP syntax across every supported version (7.2 + 7.4–8.5), via throwaway `php:-cli-alpine` images. +- PHP 7.2 syntax lint, reusing the already-running `php` container (no image pulls). - `phpcbf` then `phpcs`; phpcbf's auto-fixes are re-staged so they land in the commit. - `phpstan` on staged files under `src/`. **`pre-push`** — the heavier checks that can't be scoped to staged files: +- PHP syntax sweep across every supported version (7.2 + 7.4–8.5) over the pushed `.php` files, via throwaway `php:-cli-alpine` images. - Full `phpstan` analysis over `src/`. - The full PHPUnit suite. From eb7ee0d10491f51418032e60282b7c10634ca237 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 15:12:46 +0200 Subject: [PATCH 11/14] chore: update PHP version range in pre-push checks for consistency --- DEVELOPER_GUIDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index 9bf86f46..d4d4ff78 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -495,7 +495,7 @@ git config core.hooksPath .githooks - `phpstan` on staged files under `src/`. **`pre-push`** — the heavier checks that can't be scoped to staged files: -- PHP syntax sweep across every supported version (7.2 + 7.4–8.5) over the pushed `.php` files, via throwaway `php:-cli-alpine` images. +- PHP syntax sweep across every supported version (7.2–8.5) over the pushed `.php` files, via throwaway `php:-cli-alpine` images. - Full `phpstan` analysis over `src/`. - The full PHPUnit suite. From 9f2088e3e0190337a396bc96f796a8c1438d29f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Mon, 15 Jun 2026 17:01:42 +0200 Subject: [PATCH 12/14] chore: update Claude configuration to include new permissions and remove deprecated commands --- .claude/settings.json | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.claude/settings.json b/.claude/settings.json index eca9bf5d..2a3603c0 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -1,4 +1,5 @@ { + "$schema": "https://json.schemastore.org/claude-code-settings.json", "permissions": { "allow": [ "Bash(./bin/phpunit)", @@ -9,8 +10,6 @@ "Bash(./bin/phpstan *)", "Bash(./bin/composer *)", "Bash(./bin/php-syntax-check *)", - "Bash(./setup.sh)", - "Bash(./teardown.sh)", "Bash(git status)", "Bash(git status *)", "Bash(git diff *)", @@ -21,6 +20,17 @@ "Skill(sq-git:commit)", "Skill(sq-git:open-pr)", "Skill(sq-git:sync-branch)" + ], + "deny": [ + "Read(.env)", + "Read(.env.*)", + "Read(**/*.pem)", + "Read(**/*.key)" + ], + "ask": [ + "Bash(./setup.sh:*)", + "Bash(./teardown.sh:*)", + "Bash(git push:*)" ] } -} +} \ No newline at end of file From 5b8adaaa957c592aaa575cbc4c3e3c9edfcdde28 Mon Sep 17 00:00:00 2001 From: Boljanovic Date: Wed, 17 Jun 2026 09:48:03 +0200 Subject: [PATCH 13/14] AI setup --- .claude/agents/integration-core-reviewer.md | 4 + .claude/docs/codingStandard.md | 222 +++++++++++++++++ .claude/docs/unitTests.md | 257 ++++++++++++++++++++ .claude/skills/scaffold-feature/SKILL.md | 5 +- CLAUDE.md | 7 + DEVELOPER_GUIDE.md | 61 +++++ 6 files changed, 555 insertions(+), 1 deletion(-) create mode 100644 .claude/docs/codingStandard.md create mode 100644 .claude/docs/unitTests.md diff --git a/.claude/agents/integration-core-reviewer.md b/.claude/agents/integration-core-reviewer.md index 8e077a29..b694e74e 100644 --- a/.claude/agents/integration-core-reviewer.md +++ b/.claude/agents/integration-core-reviewer.md @@ -11,6 +11,10 @@ You review changes to `sequra/integration-core` — a platform-agnostic PHP libr invariants**, not generic PHP style (phpcs/phpstan already cover style/types). Report concrete, file:line findings; do not rewrite the code. +The binding standard is `.claude/docs/codingStandard.md` and test expectations are in +`.claude/docs/unitTests.md` — read them as the source of truth. The checklist below is +the enforcement summary; cite the relevant doc section when flagging a violation. + ## How to run 1. Scope the diff: `git diff master...HEAD` (or `git diff --staged` / `git diff` as appropriate). diff --git a/.claude/docs/codingStandard.md b/.claude/docs/codingStandard.md new file mode 100644 index 00000000..b404754a --- /dev/null +++ b/.claude/docs/codingStandard.md @@ -0,0 +1,222 @@ +# Coding Standard + +The conventions for `sequra/integration-core`. This is a **platform-agnostic PHP +library**: it holds the shared SeQura business logic and is consumed by every +platform integration (Magento, WooCommerce, PrestaShop, …). Code here must never +assume a particular shop platform, framework, or runtime. + +> This file is the authoritative standard for CORE. It supersedes any generic / +> framework-flavoured notes (e.g. Laravel/Shopify `app/...` conventions) that do +> not apply to this library. + +## 0. The quality gate (non-negotiable) + +Every change must pass, on **every supported PHP version**: + +```bash +./run-tests.sh # phpunit on PHP 7.4–8.5 + phpcs (PSR-12, 7.2-compat) + phpstan (level 6) +``` + +- **PHPUnit** — all tests green. New code ships with tests; see `.claude/docs/unitTests.md`. +- **PHPCS** — `PSR12` ruleset + `PHPCompatibility` with `testVersion 7.2-` + (`.phpcs.xml.dist`). `./bin/phpcbf` auto-fixes most style issues. +- **PHPStan** — **level 6**, `src/` (`phpstan.neon`). No new errors, no blanket + ignores; fix the type, don't silence the analyser. +- **Syntax floor PHP 7.2** — even though unit tests run on 7.4+, the syntax must + parse on 7.2 (`./bin/php-syntax-check --php=7.2`). See §3. + +A PR that doesn't pass all four is not done. + +## 1. Architecture (Clean / Onion) + +Two top-level layers, dependencies point **inward only**: + +``` + ┌─────────────────────────────────────────────┐ + outer │ Platform integration (Magento, Woo, …) │ <- separate repos + │ implement Domain/Integration interfaces, │ + │ wire services in their own Bootstrap │ + └───────────────▲─────────────────────────────┘ + │ implements / calls + src/BusinessLogic ┌───────┴───────────────────────────────┐ + API / facade │ AdminAPI · CheckoutAPI · WebhookAPI · │ controllers + Requests/Responses + │ ConfigurationWebhookAPI │ + adapters │ DataAccess (Entities, Repositories) · │ implement Domain contracts + │ SeQuraAPI (Proxies) │ + core ┌──────┴───────────────────────────────────────┐ + Domain │ Domain/{Feature}/{Models, Services, │ ← depends on NOTHING outward + │ RepositoryContracts, ProxyContracts, │ + │ Exceptions} + Domain/Integration/*Interface │ + └──────────────────────────────────────────────┘ + src/Infrastructure: technical foundation (HTTP, ORM, ServiceRegister/DI, queue, + events, serializer, logger) — depended on by BusinessLogic, depends on no domain. +``` + +**Dependency rule:** an inner layer must never reference an outer one. The +**Domain** is the innermost core and the most protected — see §2. + +### Where things live (`src/BusinessLogic/...`) + +| What | Location | +|------|----------| +| Domain models / value objects | `Domain/{Feature}/Models/` | +| Domain services | `Domain/{Feature}/Services/` | +| Domain exceptions | `Domain/{Feature}/Exceptions/` | +| Repository **contracts** (interfaces) | `Domain/{Feature}/RepositoryContracts/{Foo}RepositoryInterface.php` | +| Proxy **contracts** (interfaces) | `Domain/{Feature}/ProxyContracts/` | +| **Platform** integration contracts | `Domain/Integration/{Feature}/{Foo}Interface.php` | +| ORM entities | `DataAccess/{Feature}/Entities/` | +| Concrete repositories | `DataAccess/{Feature}/Repositories/` | +| API proxies (SeQura HTTP) | `SeQuraAPI/{Feature}/` | +| AdminAPI / CheckoutAPI / Webhook controllers + Requests/Responses | `{AdminAPI,CheckoutAPI,...}/{Feature}/` | +| DI wiring (composition root) | `BootstrapComponent.php` | + +## 2. Domain layer must be independent + +This is the most important rule in the library. A domain class +(`Domain/**`) may depend **only** on: + +- other Domain models / services in this library, +- interfaces it **owns**: `RepositoryContracts`, `ProxyContracts`, +- `Domain/Integration/**` interfaces (the platform boundary), +- pure PHP and `src/Infrastructure` primitives that carry no platform assumption + (e.g. serializer, logger contracts). + +A domain class must **never**: + +- reference a concrete repository, a concrete proxy, or anything in + `DataAccess/`, `SeQuraAPI/`, or an API controller — depend on the **contract**, + not the implementation; +- import or assume a shop platform / framework (no Magento, Woo, Symfony, Laravel, + global `$_GET`/superglobals, sessions, filesystem, `echo`, HTTP specifics); +- perform I/O directly (DB, network, clock, randomness) — that belongs behind a + Proxy/Repository/Integration interface so it can be mocked in a unit test; +- be constructed with `new` from inside another domain service — collaborators are + **injected** (see §4). + +Why: the domain is shared by all platforms and must be unit-testable in isolation +with in-memory test doubles (`.claude/docs/unitTests.md`). If a platform need leaks in, +every other integration inherits the coupling. + +### The platform boundary: `Domain/Integration` + +Anything the library needs **from the shop** is expressed as an interface in +`Domain/Integration/{Feature}/` (e.g. `ProductServiceInterface`, +`OrderCreationInterface`, `WidgetConfiguratorInterface`, +`ExpressCheckoutIntegrationInterface`). The platform repo implements these and +registers them in its own bootstrap. Domain code calls the interface; it has no +idea which platform answers. + +When the domain needs new information from the shop, **add a method to the +appropriate Integration interface** — never reach for platform code. + +## 3. PHP language floor (7.2) + +CORE runs on PHP **7.2 → 8.x**. Do **not** use features newer than 7.2: + +- ❌ typed properties, ❌ union/intersection types, ❌ `readonly`, + ❌ promoted constructor properties, ❌ native `enum`, ❌ typed class constants, + ❌ named arguments, ❌ first-class callable syntax, ❌ `match`, ❌ nullsafe `?->`, + ❌ arrow-fn-only constructs that don't exist in 7.2. +- ✅ Declare properties as `protected $foo;` / `private $foo;` with a `@var` + docblock; initialise in the constructor body. +- ✅ Untyped class constants: `public const FOO = '...';`. +- ✅ Scalar + nullable param/return type hints (`?string`) are fine (7.1+). +- ✅ Express closed value sets with the **value-object (Capability) pattern** + (§5), not native enums. + +Type information that the language can't carry goes in docblocks — see §6. + +## 4. Dependencies & DI + +- **Composition root is `BootstrapComponent`.** Services and repositories are + registered there as factory closures and resolved by class-name constant via + `ServiceRegister::getService(Foo::class)`. +- **Constructor injection only.** A class declares its collaborators as + constructor params (typed by interface where an abstraction exists). No service + locator calls or `new` inside business logic. +- **Depend on interfaces** for anything with more than one implementation or any + platform/IO concern (repositories, proxies, integration services). +- Mirror every new registration in `tests/BusinessLogic/Common/BaseTestCase` + (test composition root) so units resolve in tests too. + +## 5. Domain models, value objects, exceptions + +- **Models** are immutable in spirit: `protected` properties set in the + constructor, state exposed through explicit getters (`getX()`, `isX()`). + `toArray()` is **allowed** on CORE domain models (legacy convention — do not add + a separate Mapper layer). +- **Value objects for closed sets (Capability pattern)** — e.g. + `ExpressCheckoutPage`: `private const` per value, `private __construct`, a + `public static function {value}(): self` factory each, a `get{Concept}()` getter, + and a `parse(string $raw): self` that throws `Invalid{Concept}Exception` on + unknown input. Callers hold the object, not the raw string. +- **Models wrapping collections** validate in the constructor (`assertX()` / + `validateConfigs()`), fail loudly (don't silently filter/dedupe), and drop + setters once invariants are enforced. +- **Exceptions from domain logic extend `BaseTranslatableException`** with a + `TranslatableLabel(human, i18n.key)` and an HTTP-style `protected $code` + (`400/404/409/...`). Do **not** throw built-in `\InvalidArgumentException` from + domain code — wrap contract violations in a translatable domain exception too. + Naming: `Invalid{Concept}Exception` (wrong shape), `Duplicated{Concept}Exception` + (uniqueness), etc. + +## 6. Type safety & docblocks + +- Scalar/nullable type hints on every parameter and return type that 7.2 allows. +- Array shapes **always** documented: `@param string[] $ids`, + `@return array` — never a bare `array` without a docblock type. +- **Docblocks are house style throughout `src/`**: a class docblock (1–3 lines on + purpose), and `@param` / `@return` / `@throws` on methods. `@throws` must list + every exception a method can propagate (PHPStan level 6 + callers rely on it). + `@inheritDoc` is fine on overrides where the parent doc suffices. +- Keep `@throws` honest when you refactor — if a delegated call newly throws + `DeploymentNotFoundException`, surface it on the caller's docblock. + +## 7. API / facade layer + +- Public entry points are the `…API` facades (e.g. + `CheckoutAPI::get()->checkout($storeId)->getInitializationData($request)`), + which wrap controller methods with **Aspects** (`ErrorHandlingAspect`, + `StoreContextAspect`). A thrown exception is converted by `ErrorHandlingAspect` + into an **unsuccessful `Response`** — controllers/services should throw + meaningful exceptions rather than returning error flags. +- Each endpoint takes a **Request** object and returns a **Response** object + (`extends Response`, implements `toArray()`); `isSuccessful()` + `toArray()` are + the contract the platform consumes. +- A **Request** is a plain value object (constructor + getters) unless it genuinely + needs `fromArray()`/`toArray()` deserialization — only extend + `DataTransferObject` when those are actually used. +- Controllers are thin: translate Request → domain call → Response. No business + logic in controllers. + +## 8. Repository pattern + +- Domain owns `{Foo}RepositoryInterface`; `DataAccess/` provides the implementation + backed by `RepositoryRegistry::getRepository({Entity}::getClassName())`. +- Repositories are **store-scoped**: inject `StoreContext`, filter every query by + `storeId`, stamp `setStoreId(...)` on writes. +- `setX()` is **upsert** (select existing by store → update, else create). +- Wire the repository in `BootstrapComponent` **and** `BaseTestCase`; register the + entity against `MemoryRepository` in `BaseTestCase` for tests. + +## 9. General style + +- KISS / DRY / YAGNI. Extract shared logic to a base class or a lower-level service + rather than copy-pasting; don't build for hypothetical needs. +- One class per file. `Interface` suffix for interfaces, `Abstract` prefix for + abstract classes. Prefer composition over inheritance. +- Methods: verb-noun (`getScriptUri`, `solicit`); booleans `is/has/should/can`. + Keep them short, use early returns, ≤ ~4 params (use a value object beyond that). +- Constants for magic strings/numbers, scoped to a class. No global constants. +- Match the surrounding code's formatting; `./bin/phpcbf` settles PSR-12 disputes. + +## 10. Checklist for a new class / feature + +- [ ] Placed in the correct layer (§1 table); domain code obeys §2 (no platform/IO, + depends only on contracts). +- [ ] New shop dependency expressed as a `Domain/Integration` interface, not platform code. +- [ ] PHP 7.2-safe syntax (§3); type hints + docblocks with `@throws` (§6). +- [ ] Registered in `BootstrapComponent` **and** `BaseTestCase`. +- [ ] Isolated unit test added (mock collaborators, in-memory repos) — `.claude/docs/unitTests.md`. +- [ ] `./run-tests.sh` green: phpunit (all PHP versions) + phpcs + phpstan L6. diff --git a/.claude/docs/unitTests.md b/.claude/docs/unitTests.md new file mode 100644 index 00000000..62fd43f8 --- /dev/null +++ b/.claude/docs/unitTests.md @@ -0,0 +1,257 @@ +# Unit Testing Guide + +How we write unit tests in `integration-core`. The rule of thumb: **every unit +(service, repository, controller) gets its own isolated test, and every +collaborator it depends on is replaced by a test double.** A test should fail +only because the unit under test is wrong — never because of a real network +call, a real database, or a bug in a different class. + +## Layout + +Tests live under `tests/` and mirror `src/` one-to-one: + +``` +src/Infrastructure/... -> tests/Infrastructure/... +src/BusinessLogic/Domain/... -> tests/BusinessLogic/Domain/... +src/BusinessLogic/CheckoutAPI/... -> tests/BusinessLogic/CheckoutAPI/... +src/BusinessLogic/DataAccess/... -> tests/BusinessLogic/DataAccess/... +``` + +Two PHPUnit suites are defined in `phpunit.xml`: *Infrastructure Test Suite* +(`tests/Infrastructure`) and *Business Logic Test Suite* (`tests/BusinessLogic`). + +Test class name = unit name + `Test` (e.g. `CheckoutInitializationServiceTest`), +placed in the package that mirrors the unit's namespace. + +## Running + +Tests run inside the project's PHP container; there is no host PHP. + +```bash +# full matrix + static analysis (CI parity) +./run-tests.sh # PHP 7.4, 8.0, 8.1, 8.2, 8.3, 8.4, 8.5 + phpcs + phpstan + +# during development (single version) +vendor/bin/phpunit --configuration ./phpunit.xml +vendor/bin/phpunit --configuration ./phpunit.xml tests/BusinessLogic/Domain/Checkout/Services/CheckoutInitializationServiceTest.php +``` + +> Pass a **single** path to scope a run. PHPUnit does not accept multiple +> arbitrary paths in one invocation here — run them one at a time. + +The library supports PHP 7.4–8.x, so tests must pass on **all** versions in +`run-tests.sh`, not just your local one. + +## Coverage + +`phpunit.xml` whitelists `./src` with `processUncoveredFilesFromWhitelist="true"`, +which means **every file in `src/` counts toward coverage** — a file with no test +shows up as 0%, it is not silently ignored. The expectation is therefore: + +- Every **service** has a `…ServiceTest`. +- Every **repository** has a `…RepositoryTest`. +- Every **CheckoutAPI / AdminAPI / WebhookAPI controller** is covered by an + `…ApiTest` that drives it through its public facade. +- Both the **happy path and the meaningful edge/failure paths** are covered + (e.g. "not found returns null", "unsupported currency returns false", + exception is translated into an unsuccessful response). One behaviour per test + method. + +When you add a class to `src/`, you add its test in the same PR. + +## The test foundation + +### `BaseTestCase` + +Almost every business-logic test extends +`tests/BusinessLogic/Common/BaseTestCase` (which extends PHPUnit's `TestCase`). +Its `setUp()` builds a fully wired **test container** so the unit under test can +resolve its real collaborators — but pointed at in-memory / fake +implementations: + +- `TestServiceRegister` is (re)initialised with a map of service closures (the + test-environment equivalent of `BootstrapComponent`). +- `TestRepositoryRegistry` registers every entity against an in-memory + `MemoryRepository` (see "Repository tests"). +- Infrastructure is faked: `TestHttpClient`, `TestShopLogger`, + `TestQueueService`, `TestEncryptor`, `JsonSerializer`, in-memory `Configuration`. + +`tearDown()` calls `TestRepositoryRegistry::cleanUp()`, so in-memory state never +leaks between tests. + +> **When you add a new service or controller to `src/`, register it in +> `BaseTestCase`** (and, for the production app, in `BootstrapComponent`). If a +> controller is missing from `BaseTestCase`'s map, the facade can't resolve it +> and the API test fails with an unsuccessful response. + +### Overriding a dependency for one test + +To swap a collaborator for a test double, register it on `TestServiceRegister` in +the test's `setUp()` — closures resolve lazily, so anything that depends on it +picks up the double: + +```php +$this->bannerService = new MockBannerService(); +TestServiceRegister::registerService(BannerServiceInterface::class, function () { + return $this->bannerService; +}); +``` + +## Test doubles — which kind to use + +We use three kinds of doubles, in order of preference for a given collaborator: + +### 1. In-memory repositories (don't mock repositories) + +Repositories are **not** mocked. `BaseTestCase` registers each entity against a +real `MemoryRepository` via `TestRepositoryRegistry`, and the production +repository class runs on top of it. This exercises the real repository logic +(serialization, query filters, store scoping) against fast in-memory storage. + +### 2. Hand-written `Mock*` components + +For shop-integration interfaces (`*Interface`) and for domain services with +behaviour worth controlling, we keep hand-written doubles under +`tests/BusinessLogic/Common/MockComponents/` (and a few feature-local +`MockComponents/` folders). They implement the interface / extend the class and +expose `setMock…()` configuration setters (and sometimes capture call history): + +```php +class MockProductService implements ProductServiceInterface +{ + private $productCategories = []; + + public function getProductCategoriesByProductId(string $productId): array + { + return $this->productCategories; + } + + public function setMockProductCategories(array $productCategories): void + { + $this->productCategories = $productCategories; + } + // ... +} +``` + +Prefer a `Mock*` component when several tests need the same configurable +behaviour, or when you want to stub one method of a real service while keeping +the rest (extend the real class — e.g. `MockWidgetSettingsService extends +WidgetSettingsService`). + +### 3. PHPUnit `createMock()` + +For a quick, test-local stub of a concrete collaborator, use `createMock()` and +`->method()->willReturn()/willThrowException()`. This is the right tool for a +focused service test where you only need to script a couple of return values. + +## Patterns by layer + +### Service tests — isolate the service, mock its dependencies + +Instantiate the service directly with mocked dependencies and assert the domain +model / behaviour it returns. Cover the success path, the empty/fallback path, +and the failure path. (Real example: `CheckoutInitializationServiceTest`.) + +```php +protected function setUp(): void +{ + parent::setUp(); + $this->credentialsService = $this->createMock(CredentialsService::class); + $this->checkoutService = $this->createMock(CheckoutService::class); + $this->widgetConfigurator = $this->createMock(WidgetConfiguratorInterface::class); + $this->paymentMethodsService = $this->createMock(PaymentMethodsService::class); +} + +public function testReturnsNullWhenCredentialsNotFound(): void +{ + // Arrange + $this->credentialsService->method('getCredentialsByCountry') + ->willThrowException(new CredentialsNotFoundException()); + + // Act + $data = $this->service()->getInitializationData('ES', 'ES'); + + // Assert + self::assertNull($data); +} +``` + +If the service caches state in static properties, reset them in `setUp()` so +tests don't bleed into each other (e.g. +`CheckoutService::$generalSettingsFetched = false;`). + +### Repository tests — real repository over in-memory storage + +Resolve the real repository from the container, write through it, read back, and +assert the round-trip (and store-scoping / overwrite semantics). No mocks. + +```php +$this->repository = TestServiceRegister::getService(ConnectionDataRepositoryInterface::class); +$this->repository->setConnectionData($connectionData); + +$loaded = $this->repository->getConnectionDataByDeploymentId('sequra'); +$this->assertEquals('test', $loaded->getMerchantId()); +``` + +### Controller / API tests — drive the public facade, mock the domain service + +Controllers are tested through their public entry point +(`CheckoutAPI::get()->group($storeId)->method($request)` / +`AdminAPI::get()->…`), with the underlying domain service replaced by a double +registered on `TestServiceRegister`. Assert on the `Response`: +`isSuccessful()` and `toArray()`. (Real example: `CheckoutApiTest`.) + +```php +protected function setUp(): void +{ + parent::setUp(); + $this->checkoutInitializationService = $this->createMock(CheckoutInitializationService::class); + TestServiceRegister::registerService(CheckoutInitializationService::class, function () { + return $this->checkoutInitializationService; + }); +} + +public function testGetInitializationDataToArray(): void +{ + $this->checkoutInitializationService->method('getInitializationData')->willReturn( + new CheckoutInitializationData('assets1', 'merchant1', ['i1', 'pp3'], 'scriptUri.com', 'es-ES', 'EUR', ',', '.') + ); + + $response = CheckoutAPI::get()->checkout('1') + ->getInitializationData(new CheckoutInitializationRequest('ES', 'ES')); + + self::assertEquals([ + 'assetKey' => 'assets1', 'merchant' => 'merchant1', 'products' => ['i1', 'pp3'], + 'scriptUri' => 'scriptUri.com', 'locale' => 'es-ES', 'currency' => 'EUR', + 'decimalSeparator' => ',', 'thousandSeparator' => '.', + ], $response->toArray()); +} +``` + +API tests must also cover the **error contract**: a domain service returning +`null` (not configured) should yield a successful response with an empty array, +and a thrown exception should be turned into an *unsuccessful* response by the +`ErrorHandlingAspect`. + +## Conventions + +- **Arrange / Act / Assert** — separate the three with comments, as above. +- **One behaviour per test method**; name it after the behaviour + (`testReturnsNullWhenCredentialsNotFound`). +- **No real I/O** — no live HTTP (use `TestHttpClient`), no real DB (use + `MemoryRepository`), no `sleep`, no clock/`rand` dependence. +- **Deterministic** — tests must not depend on order; `tearDown` cleans state. +- **All PHP versions** — keep tests free of version-specific syntax; they run on + 7.4 through 8.5. + +## Checklist for a new unit + +- [ ] Test class added under the mirroring `tests/` package, extends `BaseTestCase`. +- [ ] Dependencies isolated: repos via `MemoryRepository`, collaborators via + `Mock*` components or `createMock()`. +- [ ] New service/controller registered in `BaseTestCase` (and `BootstrapComponent`). +- [ ] Success, edge (empty/fallback), and failure paths covered. +- [ ] For controllers: assert `isSuccessful()` + `toArray()`, including the + not-configured and exception cases. +- [ ] `vendor/bin/phpunit`, `phpcs`, and `phpstan` all green on every PHP version. diff --git a/.claude/skills/scaffold-feature/SKILL.md b/.claude/skills/scaffold-feature/SKILL.md index 9ae2b2f6..979d09e8 100644 --- a/.claude/skills/scaffold-feature/SKILL.md +++ b/.claude/skills/scaffold-feature/SKILL.md @@ -18,7 +18,9 @@ file matches the existing style. **`CountryConfiguration` is the canonical refer `WebhookAPI`, or `ConfigurationWebhookAPI`) + a facade method. - **Talks to SeQura's HTTP API?** → also needs a `ProxyContracts/` interface and a `SeQuraAPI` proxy (see `OrderProxy`). Out of scope for the basic skeleton — flag it. -2. Hard constraints (these are non-negotiable in `src/`): +2. Hard constraints (these are non-negotiable in `src/`) — the binding source of truth + is `.claude/docs/codingStandard.md` (and `.claude/docs/unitTests.md` for tests); the + list below is the summary: - **PHP 7.2 syntax only.** No arrow fns, no `?->`, no named args, no enums, no constructor promotion, no union/typed-property syntax newer than 7.2. - **PSR-12** (`.phpcs.xml.dist`) and **PHPStan level 6** clean. @@ -69,6 +71,7 @@ Add a `ServiceRegister::registerService(...)` lazy-callable block in the matchin ->beforeEachMethodOfService(Controller::class)`. ### 5. Tests (`tests/BusinessLogic//`) +Follow `.claude/docs/unitTests.md` (layout mirrors `src/`, collaborators replaced by test doubles). Mirror an existing controller/service test (see `tests/BusinessLogic/AdminAPI/CountryConfiguration/`). Register the ORM entity in the test bootstrap so the in-memory repository can resolve it. diff --git a/CLAUDE.md b/CLAUDE.md index e08ca8fc..24ebfc0b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,6 +10,13 @@ Architecture follows the Onion model with two top-level namespaces (PSR-4): - `SeQura\Core\Infrastructure\` → `src/Infrastructure` — technical foundation (HTTP, ORM abstraction, logger, task runner, service registry, serializer). - `SeQura\Core\BusinessLogic\` → `src/BusinessLogic` — SeQura domain logic and the public API facades. +## Authoritative standards — read before writing code + +These two documents are binding and are the single source of truth; the summaries elsewhere in this file (and in the agents/skills) defer to them: + +- **Before writing or changing any code in `src/`** → read `.claude/docs/codingStandard.md` (architecture, PHP 7.2 syntax floor, PSR-12, PHPStan level 6, the quality gate). +- **Before writing or changing tests** → read `.claude/docs/unitTests.md` (test layout mirroring `src/`, test doubles, isolation rules). + ## Working principles General working guidelines (adapted from [andrej-karpathy-skills/CLAUDE.md](https://github.com/forrestchang/andrej-karpathy-skills/blob/main/CLAUDE.md)). They bias toward caution over speed; for trivial tasks, use judgment. diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index d4d4ff78..c06d40ba 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -9,6 +9,7 @@ This guide provides comprehensive information for developers working with the Se - [Debugging](#debugging) - [Running Tests](#running-tests) - [Development Workflow](#development-workflow) +- [Code Review & AI Tooling](#code-review--ai-tooling) - [Troubleshooting](#troubleshooting) ## Available Development Tools @@ -505,6 +506,66 @@ When their checks do run, both hooks execute inside / against the Docker `php` s --- +## Code Review & AI Tooling + +This repo ships one Claude Code **skill** (`.claude/skills/scaffold-feature/`) and a +custom **agent** (`.claude/agents/integration-core-reviewer.md`). Both are available +automatically when you open the repo in Claude Code — nothing to install. Claude Code +also provides a number of **built-in skills** (e.g. `/code-review`, `/security-review`) +that work in any project; those are not part of this repo and are not guaranteed to be +present in every Claude Code setup. + +### Using a skill + +Skills are invoked with a **slash command**: type `/` in Claude Code and pick (or type) +the skill name, optionally followed by arguments. + +**Shipped by this repo:** + +| Command | What it does | +|---|---| +| `/scaffold-feature ` | Generates the Domain + DataAccess + BootstrapComponent (+ facade) skeleton for a new feature, following repo conventions. | + +Example: `/scaffold-feature ShippingRules`. + +**Built-in Claude Code skills** (available generally, not repo-specific) — handy here include +`/code-review` (diff review for bugs/cleanups), `/security-review`, `/review` (review a PR), +and `/verify` / `/run` (run the app / confirm a change works). Type `/` to see what your +Claude Code install offers. + +### Code review — two complementary passes + +**1. `/code-review` skill (generic correctness + cleanups)** + +```text +/code-review # default effort: fewer, high-confidence findings +/code-review high # broader coverage, may include lower-confidence findings +/code-review --fix # apply the findings to the working tree +/code-review --comment # post findings as inline PR comments +``` +Reviews the current diff for bugs, simplifications, reuse, and efficiency — language-level +issues that aren't specific to this codebase. + +**2. `integration-core-reviewer` agent (this repo's architecture invariants)** + +This is a sub-agent, not a slash command — you run it by asking Claude to delegate to it: +- "review my changes with the **integration-core-reviewer**" +- type `@agent-` and select **integration-core-reviewer** +- or just "review the current diff against this repo's invariants" + +It enforces what the generic review misses: the PHP 7.2 syntax floor, onion-layer +boundaries, the controllers-return-never-throw facade contract, multistore `StoreContext` +scoping, and `BootstrapComponent` registration. It also cross-checks against +`.claude/docs/codingStandard.md` and `.claude/docs/unitTests.md`. It reports +`file:line` findings only — it does not edit code. + +**When to use which:** run both for a thorough review — `/code-review` for general bugs, +then the `integration-core-reviewer` for the architecture-specific pass. Make sure your +changes exist (committed, staged, or in the working tree) before running either, since +both review the current diff. + +--- + ## Troubleshooting ### Docker Issues From 32906fa1085876e008a84bfa5f1050febac07719 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michel=20Escalante=20=C3=81lvarez?= Date: Wed, 17 Jun 2026 12:15:33 +0200 Subject: [PATCH 14/14] chore: address PR review feedback on Claude config and docs Resolve the blocker and should-fix items from the PAR-805 review: - reviewer agent: drop mandatory graphify step (tool not in repo); orient via the agent's available tools instead - settings.json: narrow broad docker compose allow-rule to `up`/`exec php`, and standardize all Bash perms on the `cmd:*` prefix form - DEVELOPER_GUIDE.md: bump stale phpstan --memory-limit 512M to 1G - CLAUDE.md / scaffold-feature: correct aspect namespace (concrete aspects live in AdminAPI\Aspects, only the runner is in Bootstrap/Aspect); note .phpcs.xml.dist precedence; collapse single-test and run-tests.sh duplication into DEVELOPER_GUIDE.md pointers Co-Authored-By: Claude --- .claude/agents/integration-core-reviewer.md | 6 +++--- .claude/settings.json | 19 ++++++++++--------- .claude/skills/scaffold-feature/SKILL.md | 2 ++ CLAUDE.md | 15 ++++----------- DEVELOPER_GUIDE.md | 2 +- 5 files changed, 20 insertions(+), 24 deletions(-) diff --git a/.claude/agents/integration-core-reviewer.md b/.claude/agents/integration-core-reviewer.md index b694e74e..3272c33b 100644 --- a/.claude/agents/integration-core-reviewer.md +++ b/.claude/agents/integration-core-reviewer.md @@ -18,9 +18,9 @@ the enforcement summary; cite the relevant doc section when flagging a violation ## How to run 1. Scope the diff: `git diff master...HEAD` (or `git diff --staged` / `git diff` as appropriate). -2. Orient before reading source: graphify-out/graph.json exists, so run - `graphify query ""`, `graphify explain ""`, or - `graphify path "" ""` first; only read raw files to inspect the changed lines. +2. Orient before reading source: locate the relevant classes, contracts, and + `BootstrapComponent` registrations, then inspect the changed lines and their + collaborators. 3. Review each changed file against the checklist below. ## Invariants to enforce (in priority order) diff --git a/.claude/settings.json b/.claude/settings.json index 2a3603c0..7ed76b60 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -3,20 +3,21 @@ "permissions": { "allow": [ "Bash(./bin/phpunit)", - "Bash(docker compose *)", + "Bash(docker compose up:*)", + "Bash(docker compose exec php:*)", "Bash(./bin/phpcs)", "Bash(./bin/phpcbf)", "Bash(./bin/phpstan)", - "Bash(./bin/phpstan *)", - "Bash(./bin/composer *)", - "Bash(./bin/php-syntax-check *)", + "Bash(./bin/phpstan:*)", + "Bash(./bin/composer:*)", + "Bash(./bin/php-syntax-check:*)", "Bash(git status)", - "Bash(git status *)", - "Bash(git diff *)", - "Bash(git log *)", - "Bash(git add *)", + "Bash(git status:*)", + "Bash(git diff:*)", + "Bash(git log:*)", + "Bash(git add:*)", "Bash(git --version)", - "Bash(git check-attr *)", + "Bash(git check-attr:*)", "Skill(sq-git:commit)", "Skill(sq-git:open-pr)", "Skill(sq-git:sync-branch)" diff --git a/.claude/skills/scaffold-feature/SKILL.md b/.claude/skills/scaffold-feature/SKILL.md index 979d09e8..33a9bc78 100644 --- a/.claude/skills/scaffold-feature/SKILL.md +++ b/.claude/skills/scaffold-feature/SKILL.md @@ -69,6 +69,8 @@ Add a `ServiceRegister::registerService(...)` lazy-callable block in the matchin - Add a facade method on the facade class (e.g. `AdminAPI::(string $storeId): Aspects`) returning `Aspects::run(new ErrorHandlingAspect())->andRun(new StoreContextAspect($storeId)) ->beforeEachMethodOfService(Controller::class)`. + `ErrorHandlingAspect`/`StoreContextAspect` come from `SeQura\Core\BusinessLogic\AdminAPI\Aspects` + (reused across all facades) — not from `Bootstrap/Aspect`, which only holds the aspect runner. ### 5. Tests (`tests/BusinessLogic//`) Follow `.claude/docs/unitTests.md` (layout mirrors `src/`, collaborators replaced by test doubles). diff --git a/CLAUDE.md b/CLAUDE.md index 24ebfc0b..38679b1a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,22 +33,15 @@ The `bin/` scripts are **Docker wrappers**, not local binaries. `phpunit`, `phpc ```bash ./bin/composer install # install deps (runs in composer:latest image) ./bin/phpunit # full test suite, PHP 7.2 (uses phpunit.xml, --testdox) -./bin/phpcs # PSR-12 style check (config: .phpcs.xml.dist) +./bin/phpcs # PSR-12 style check (config: .phpcs.xml.dist — passed explicitly; the repo's bare .phpcs.xml is ignored by all tooling) ./bin/phpcbf # auto-fix style violations ./bin/phpstan # static analysis, level 6 over src/ (config: phpstan.neon) ./bin/php-syntax-check --php=7.2 # syntax-only check at a target PHP version (php:-cli-alpine) ``` -**Running a single test:** `bin/phpunit` ignores any arguments you pass it and always runs the whole suite. To target one test, invoke PHPUnit inside the container directly: +**Running a single test:** `bin/phpunit` ignores any arguments you pass it and always runs the whole suite. To target one test, invoke PHPUnit inside the container directly — e.g. `docker compose exec php vendor/bin/phpunit --configuration phpunit.xml --filter testSomeMethod` (see `DEVELOPER_GUIDE.md` for more forms). Note `bin/phpstan` and `bin/composer` *do* forward arguments; `bin/phpcs`/`bin/phpcbf`/`bin/phpunit` do not. -```bash -docker compose exec php vendor/bin/phpunit --configuration phpunit.xml --filter testSomeMethod -docker compose exec php vendor/bin/phpunit --configuration phpunit.xml tests/Infrastructure/ServiceRegisterTest.php -``` - -(`bin/phpstan` and `bin/composer` *do* forward arguments; `bin/phpcs`/`bin/phpcbf`/`bin/phpunit` do not.) - -`run-tests.sh` is the CI-equivalent gate: it runs the suite against **local** `/usr/bin/php7.4`–`php8.5` (not Docker) and then phpcs + phpstan. It only works on a machine that has all those PHP versions installed — it will not run on a typical dev box that lacks them. +`run-tests.sh` is the local multi-version CI gate — it runs the suite against **local** `/usr/bin/php7.4`–`php8.5` (not Docker) then phpcs + phpstan, so it only runs on a machine that has all those PHP versions installed, not a typical dev box. See `DEVELOPER_GUIDE.md`. Tests are split into two PHPUnit suites: `tests/Infrastructure` and `tests/BusinessLogic`. The lowest supported version is **PHP 7.2** — `composer.json` pins the platform to 7.2, so do not use syntax newer than 7.2 in `src/`. The container runs XDebug 2.9.8 on port 9003; see `DEVELOPER_GUIDE.md` for IDE setup. @@ -59,7 +52,7 @@ Tests are split into two PHPUnit suites: `tests/Infrastructure` and `tests/Busin **`BootstrapComponent` is the wiring manifest.** `BusinessLogic\BootstrapComponent::init()` (extends `Infrastructure\BootstrapComponent`) registers every repository, service, controller, proxy, event listener, and webhook topic handler. When you add a new service/controller/repository, you must register it here, injecting its collaborators via `ServiceRegister::getService(...)`. The host platform calls `init()` once at startup after registering its own platform-specific implementations. **Four public API facades are the entry points** (`BusinessLogic/AdminAPI`, `CheckoutAPI`, `WebhookAPI`, `ConfigurationWebhookAPI`). Pattern, e.g. `AdminAPI::get()->connection($storeId)->validateConnection($request)`: -- The facade returns controllers wrapped by **Aspects** (`BusinessLogic/Bootstrap/Aspect`), which apply cross-cutting behavior before each method call. +- The facade returns controllers wrapped by **Aspects**, which apply cross-cutting behavior before each method call. The aspect *runner* (`Aspect`/`Aspects`/`CompositeAspect`) lives in `BusinessLogic/Bootstrap/Aspect`, but the concrete aspects below live in `BusinessLogic/AdminAPI/Aspects` (namespace `SeQura\Core\BusinessLogic\AdminAPI\Aspects`) and are reused by all four facades. - `ErrorHandlingAspect` wraps every call so controllers **return Response objects and never throw** to the caller — domain exceptions become `TranslatableErrorResponse`. Keep this contract: controllers return `Request`/`Response` DTOs. - `StoreContextAspect($storeId)` sets the active store for the duration of the call. diff --git a/DEVELOPER_GUIDE.md b/DEVELOPER_GUIDE.md index c06d40ba..0ddaa3d1 100644 --- a/DEVELOPER_GUIDE.md +++ b/DEVELOPER_GUIDE.md @@ -457,7 +457,7 @@ docker compose exec -it php bash ./bin/phpcs # Static analysis - docker compose exec php vendor/bin/phpstan analyse -c phpstan.neon src/ --memory-limit=512M + docker compose exec php vendor/bin/phpstan analyse -c phpstan.neon src/ --memory-limit=1G ``` 4. **Run Tests**