-
Notifications
You must be signed in to change notification settings - Fork 19
IT: adding auto code review from claude on PR opening #295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
adumont-payplug
merged 1 commit into
develop
from
feature/implementing_auto_code_review
May 26, 2026
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| # Copilot Instructions | ||
|
|
||
| These instructions apply to code reviews on pull requests in this repository. | ||
|
|
||
| ## Project Context | ||
|
|
||
| This is PayPlug's official Sylius plugin. PayPlug is a Payment Service Provider (PSP). The plugin integrates PayPlug's payment processing into Sylius via multiple payment methods: standard card (PayPlug), Oney financing, Bancontact, Scalapay, Apple Pay, and American Express. | ||
|
|
||
| **Stack**: PHP 8.2+, Sylius 2.0+, Symfony 6.4+, Payum, `payplug/payplug-php ^4.0` | ||
|
|
||
| ## Intentional Patterns — Do Not Flag as Issues | ||
|
|
||
| - **`sleep(10)` in `NotifyAction`** — intentional, prevents a race condition between the IPN webhook and the user redirect. Never suggest removing it. | ||
| - **No direct SDK calls** — `PayPlugApiClient` is the only allowed entry point to the PayPlug PHP SDK. Any direct call to SDK classes bypasses this and is a bug. | ||
| - **Dual architecture** — Sylius 2.1+ uses a command/response provider model alongside legacy Payum actions. Both coexist intentionally. | ||
| - **Card saving condition** — a card is only saved when the PayPlug API response includes `metadata['customer_id']`. Absence of this guard is a bug. | ||
|
adumont-payplug marked this conversation as resolved.
|
||
| - **`payment_context.cart`** — required in the PayPlug API payload for Oney and Scalapay payments. Missing it causes API rejection. | ||
| - **Distributed lock in `NotifyAction`** — Symfony Lock is used on payment ID to prevent concurrent webhook processing. Do not flag as over-engineering. | ||
|
adumont-payplug marked this conversation as resolved.
|
||
|
|
||
| ## Code Review Dimensions | ||
|
|
||
| ### Security | ||
| - SQL injection, XSS, CSRF | ||
| - Authentication and authorization flaws | ||
| - Secrets or credentials committed in code | ||
| - Insecure deserialization, path traversal, SSRF | ||
| - Direct calls to PayPlug PHP SDK classes instead of going through `PayPlugApiClient` | ||
|
|
||
| ### Performance | ||
| - N+1 queries (especially in Sylius entity traversal) | ||
| - Unnecessary memory allocations | ||
| - Algorithmic complexity (O(n²) in hot paths) | ||
| - Missing database indexes | ||
| - Unbounded queries or loops | ||
| - Resource leaks | ||
|
|
||
| ### Correctness | ||
| - Edge cases: empty input, null, overflow | ||
| - Race conditions and concurrency issues | ||
| - Error handling and propagation | ||
| - Off-by-one errors, type safety | ||
| - `declare(strict_types=1)` must be present in every PHP file | ||
| - For new payment gateways (PPRO pattern): verify the full implementation checklist is covered (gateway factory, form type, resolver decorator, refund provider, templates, service definitions, translations) | ||
|
|
||
| ### Maintainability | ||
| - Naming clarity, single responsibility, duplication | ||
| - Test coverage: PHPUnit in `tests/PHPUnit/`, Behat in `features/` | ||
| - Documentation for non-obvious logic only — do not flag missing comments on self-explanatory code | ||
| - PHPStan level max compliance; suppressions must go in `ruleset/phpstan-baseline.neon` | ||
| - ECS coding standard based on `sylius-labs/coding-standard` | ||
|
|
||
| ## Output Format | ||
|
|
||
| Rate each dimension: **Good** / **Needs Attention** / **Critical** | ||
|
|
||
| List findings with file path and line number. Lead with Critical findings. Include positive observations alongside issues. | ||
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.
Uh oh!
There was an error while loading. Please reload this page.