diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000000..4ad38ee3eadf --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,30 @@ +# Copilot Code Review Instructions + +Repository-wide conventions, the target-branch compatibility policy, coding +standards, and FrankenPHP Worker Mode requirements are defined in the root +`AGENTS.md`. Apply that policy in every review; this file covers review +behavior only. + +## Review priorities + +Prioritize actionable findings involving: + +- incorrect behavior or an unhandled edge case; +- backward compatibility; +- security, input validation, and context-appropriate output escaping; +- request-specific state leaking through globals, static properties, or + shared services, including under Worker Mode where one process serves + many requests; +- differences between supported database drivers (MySQLi, PostgreSQL, + SQLite3, SQLSRV, OCI8); +- missing regression tests, user-guide changes, changelog entries, or + upgrade instructions. + +## Review behavior + +- Determine the PR base branch and apply the compatibility policy from + `AGENTS.md`. If the base branch cannot be determined, assume `develop` + and state that assumption. +- Do not report formatting-only issues already enforced by PHP-CS-Fixer. +- Every finding should identify a concrete failure scenario and the + affected code. diff --git a/.github/instructions/database.instructions.md b/.github/instructions/database.instructions.md new file mode 100644 index 000000000000..b550a4a0573e --- /dev/null +++ b/.github/instructions/database.instructions.md @@ -0,0 +1,19 @@ +--- +applyTo: "system/Database/**/*.php,tests/system/Database/**/*.php" +--- + +# Database layer + +- Consider all supported drivers: MySQLi, PostgreSQL, SQLite3, SQLSRV, and + OCI8. +- Check identifier quoting, value binding, `NULL` behavior, transactions, + affected rows, return types, and platform-specific SQL. +- Do not implement driver-independent behavior in only one driver. +- When changing a base database class, inspect driver overrides and their + method signatures. +- Keep driver-independent tests separate from tests that require a live + database. +- Use the `DatabaseLive` PHPUnit group only when a real database connection is + required. +- Prefer focused tests for the affected base class and drivers. Leave the full + multi-driver matrix to GitHub Actions. diff --git a/.github/instructions/framework-core.instructions.md b/.github/instructions/framework-core.instructions.md new file mode 100644 index 000000000000..0ae53495c513 --- /dev/null +++ b/.github/instructions/framework-core.instructions.md @@ -0,0 +1,23 @@ +--- +applyTo: "system/**/*.php" +--- + +# Framework core + +- Assume public and protected symbols are extension points used by third-party + applications unless they are clearly marked internal. +- Apply the target-branch compatibility policy from the root `AGENTS.md` + before changing any extension point. +- Before changing a signature or behavior, inspect parent classes, implemented + interfaces, traits, service factories, configuration, and corresponding + tests. +- Keep file paths, class names, and namespaces aligned under `CodeIgniter\`. +- Preserve existing extension and dependency-injection points. +- Consider long-running worker environments. Do not retain request-specific + state in static properties, globals, or shared services after a request + finishes. +- Avoid introducing a framework-wide abstraction to solve a single local + problem. +- On `develop`, keep exceptions and observable side effects compatible. On a + `4.*` target, change them only as an explicit, narrow, documented breaking + enhancement with a migration path. diff --git a/.github/instructions/tests.instructions.md b/.github/instructions/tests.instructions.md new file mode 100644 index 000000000000..af2d9a96ba96 --- /dev/null +++ b/.github/instructions/tests.instructions.md @@ -0,0 +1,21 @@ +--- +applyTo: "tests/**/*.php" +--- + +# PHPUnit tests + +- Mirror the corresponding `system/` component and local test conventions + where practical. +- Add the appropriate PHPUnit `Group` attribute described in + `tests/README.md`. +- Prefer `assertSame()` and dedicated assertions over loose or generic + assertions. +- Cover the regression or behavior through a public API where possible. + Avoid testing private implementation details. +- Restore services, factories, environment variables, superglobals, handlers, + locale, time-related state, and other global state modified by a test. +- Tests must not depend on execution order or state left by another test. +- Do not replace a meaningful assertion with a weaker assertion to make a test + pass. +- After changing behavior, run the smallest relevant test file or component. + Do not run the complete test suite by default. diff --git a/.github/instructions/user-guide.instructions.md b/.github/instructions/user-guide.instructions.md new file mode 100644 index 000000000000..9b55a50ba3f1 --- /dev/null +++ b/.github/instructions/user-guide.instructions.md @@ -0,0 +1,27 @@ +--- +applyTo: "user_guide_src/**/*.rst" +--- + +# User guide + +- Preserve the existing reStructuredText structure, terminology, directives, + anchors, and surrounding writing style. +- Keep PHP examples compatible with PHP 8.2 and verify signatures and behavior + against the current framework code. +- Distinguish new behavior from existing behavior and document defaults + precisely. +- Determine the PR target branch before describing compatibility: + `develop` follows the patch-release policy, while `4.*` follows the + minor-release policy in the root `AGENTS.md`. +- Behavior changes, enhancements, deprecations, and important bug fixes may + require a changelog entry. +- Changes requiring users to modify code or configuration may require an + upgrading-guide entry. +- For an intentional compatibility break targeting `4.*`, document the old + behavior, new behavior, affected users, reason for the change, and the + smallest migration in both the minor changelog and upgrading guide. +- Keep code examples minimal but complete enough to run in a normal + CodeIgniter application. +- Leave the complete Sphinx build and documentation validation to GitHub + Actions unless the task specifically requires local documentation + validation. diff --git a/.github/instructions/worker-mode.instructions.md b/.github/instructions/worker-mode.instructions.md new file mode 100644 index 000000000000..ec6e1742ec5a --- /dev/null +++ b/.github/instructions/worker-mode.instructions.md @@ -0,0 +1,38 @@ +--- +applyTo: "system/Boot.php,system/CodeIgniter.php,system/Commands/Worker/**,system/Config/BaseService.php,system/Config/Factories.php,system/Config/Factory.php,system/Config/Services.php,system/Database/Config.php,system/Database/BaseConnection.php,system/Database/ConnectionInterface.php,system/Database/*/Connection.php,system/Events/Events.php,system/Session/**,system/Cache/**,system/HTTP/**,system/Filters/**,system/Router/**,system/Security/**,system/Debug/Toolbar.php,system/Debug/Toolbar/**,app/Config/WorkerMode.php,tests/system/Commands/Worker/**,user_guide_src/source/installation/worker_mode.rst" +--- + +# FrankenPHP Worker Mode + +Use `system/Commands/Worker/Views/frankenphp-worker.php.tpl` as the canonical +Worker Mode entry point. `worker:install` publishes this template to +`public/frankenphp-worker.php`. + +When changing bootstrap or request-lifecycle code, trace all three Worker Mode +phases: + +1. One-time process bootstrap through `Boot::bootWorker()`. +2. Per-request preparation: reconnect database and cache connections, reset + the `CodeIgniter` instance, and replace all request superglobals before + calling `$app->run()`. +3. Post-request cleanup: close the session, clean up unfinished database + transactions, reset factories and non-persistent services, clean up event + listeners and performance logs, and reset the debug toolbar. + +- Preserve the phase ordering unless the change explicitly proves a different + order is safe. +- Check every new mutable static property, singleton, shared service, event + listener, connection, handler, or global for cross-request state leakage. +- Do not preserve a service across requests merely for performance. Persistent + services must be safe to reuse and must not retain request, response, user, + route, security token, locale, or error state. +- Ensure exceptions and early exits cannot skip cleanup needed before the next + request. +- For state-isolation fixes, prefer a regression test that exercises two + sequential requests or reset cycles in the same process and proves the + second request cannot observe the first request's state. +- If the template contract changes, update focused assertions in + `tests/system/Commands/Worker/WorkerCommandsTest.php`. +- If existing installations need the new generated entry point, update the + changelog and upgrading guide and instruct users to run + `php spark worker:install --force`. diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 000000000000..208568025619 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,148 @@ +# CodeIgniter 4 — Repository Guidelines + +CodeIgniter 4 is a mature open-source PHP framework. Framework production +code lives in `system/`; its tests mirror that structure under +`tests/system/`. + +## Before making changes + +Consult the relevant sections of: + +- `contributing/pull_request.md` +- `contributing/internals.md` +- `contributing/styleguide.md` +- `tests/README.md` + +Follow existing code and tests in the affected component. Search for an +existing implementation before introducing a new abstraction or convention. + +## Target branch and compatibility policy + +Compatibility is judged against the pull request's base branch, not the +contributor's source or feature branch. When the base branch is unknown, +apply the stricter `develop` policy. + +### Base branch `develop` + +`develop` is the next patch-release line. Intentional backward-incompatible +changes are not allowed. + +- Preserve public and protected APIs, documented behavior, configuration + defaults, generated project files, exceptions, and observable side effects. +- Do not remove deprecated items or change public/protected signatures, + visibility, or interface/abstract requirements. +- A bug or security fix may correct faulty behavior but must preserve the + documented contract and ordinary valid usage. +- If a fix necessarily breaks compatibility, target the appropriate minor + branch instead. + +### Base branch `4.*` + +Minor-release lines (for example `4.9`). Small, deliberate compatibility +breaks may be accepted when they materially improve the framework or +complete the documented deprecation lifecycle. + +- Keep each break narrow, explicit, and justified; never an incidental + side effect of refactoring. +- Deprecated APIs may be removed no earlier than the second subsequent + minor release (deprecated in 4.6.x means removable in 4.8.0), with a + supported replacement. +- Every accepted break needs tests for the new behavior, a minor-version + changelog entry, and migration instructions in the upgrading guide. + +### All base branches + +- Production code must run on PHP 8.2. Do not require a newer PHP version, + even when a newer version is installed locally. +- Signatures, properties, constants, default values, exceptions, side + effects, configuration, and generated project files are all + compatibility-sensitive. +- Major-scale redesigns and ecosystem-wide migration requirements belong in + a major release, not a patch or minor release. +- Prefer the minimum useful abstraction and keep framework components as + independent as practical. +- Dependencies should be injectable. When a framework service is used as a + default, preserve the ability for applications to replace it. +- Do not add or update a Composer dependency unless the task explicitly + requires it and the change is justified. +- Do not modify code in `system/ThirdParty/`. + +## Coding standards + +- Do not add `declare(strict_types=1)` mechanically. Follow nearby code and + check the `DeclareStrictTypesRector` exclusions in `rector.php`; PHP files + use strict types unless excluded there. Treat the existing exclusions as + intentional compatibility constraints. Do not remove an exclusion without + dedicated justification and tests. +- Every new class property must have a native type declaration. Use the + most precise type supported by PHP 8.2; use `mixed` only when the + property intentionally accepts unrelated types. +- Do not add or change a type on an existing public or protected property + mechanically. Property types affect inheritance and must follow the + target-branch compatibility policy. +- Add PHPDoc only when it contributes information that native types cannot + express. Do not duplicate a parent or interface docblock. +- Never suppress a static-analysis error or update a baseline to make a + check pass. + +## FrankenPHP Worker Mode + +In Worker Mode one process serves many requests. The entry point's source +template is `system/Commands/Worker/Views/frankenphp-worker.php.tpl`; the +`worker:install` command publishes it as `public/frankenphp-worker.php`. + +- For every change affecting application bootstrap, request handling, + response sending, shutdown, superglobals, sessions, database or cache + connections, services, factories, events, toolbar state, static state, or + other request-lifecycle behavior, inspect the worker entry-point template + even when it is not part of the diff. +- Compare traditional per-process execution with worker execution, where + the framework boots once and handles multiple requests in the same + process. +- Classify mutable state as process-lifetime, intentionally persistent, or + request-specific. Request-specific state must be refreshed or reset for + every request and must not leak into the next request. +- Preserve the ordering requirements between per-request reconnection, + framework reset, superglobal refresh, application execution, and + post-request cleanup. +- Treat the template as the source of truth. Do not edit a generated + `public/frankenphp-worker.php` in place. +- When a release changes the template in a way existing installations must + receive, add an upgrading instruction telling Worker Mode users to + republish it with `php spark worker:install --force`. + +## Tests + +- Every bug fix should include a regression test that fails without the + fix. +- Test failure paths, exceptions, boundary conditions, and state cleanup, + not only the happy path. +- Prefer strict and dedicated PHPUnit assertions as described in the + project style guide. +- Do not weaken or remove an existing test unless the documented behavior + or specification changed. + +## Documentation + +- Public API, behavior, message, or default-value changes may require an + update to the user guide and changelog. +- Changes requiring user action, including configuration changes, may also + require an upgrading-guide entry. +- For a `4.*` target, every intentional compatibility break must be + documented in both the minor-version changelog and its upgrading guide. + +## Focused validation + +GitHub Actions is the authoritative source for full validation. Do not +reproduce the complete CI matrix locally. + +- Run only the narrowest PHPUnit test file or component that covers a code + change, for example + `vendor/bin/phpunit tests/system//Test.php`. +- Run a file-scoped analysis or formatting check only when it is quick and + directly relevant. +- Do not run the complete PHPUnit suite, `composer phpstan:check`, + `composer cs`, Psalm, Structarmed, or a repository-wide Rector analysis + by default. +- Report which focused checks were executed and which validation was left + to CI.