Skip to content

Commit fc43123

Browse files
authored
chore: add instructions for coding agents and Copilot review (#10428)
1 parent bfa7a49 commit fc43123

8 files changed

Lines changed: 307 additions & 0 deletions

.gitattributes

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ psalm-baseline.xml export-ignore
3232
psalm.xml export-ignore
3333
psalm_autoload.php export-ignore
3434
rector.php export-ignore
35+
AGENTS.md export-ignore
3536

3637
# source user guide
3738
user_guide_src/ export-ignore

.github/copilot-instructions.md

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
# Copilot Code Review Instructions
2+
3+
Repository-wide conventions, the target-branch compatibility policy, coding
4+
standards, and FrankenPHP Worker Mode requirements are defined in the root
5+
`AGENTS.md`. Apply that policy in every review; this file covers review
6+
behavior only.
7+
8+
## Review priorities
9+
10+
Prioritize actionable findings involving:
11+
12+
- incorrect behavior or an unhandled edge case;
13+
- backward compatibility;
14+
- security, input validation, and context-appropriate output escaping;
15+
- request-specific state leaking through globals, static properties, or
16+
shared services, including under Worker Mode where one process serves
17+
many requests;
18+
- differences between supported database drivers (MySQLi, PostgreSQL,
19+
SQLite3, SQLSRV, OCI8);
20+
- missing regression tests, user-guide changes, changelog entries, or
21+
upgrade instructions.
22+
23+
## Review behavior
24+
25+
- Determine the PR base branch and apply the compatibility policy from
26+
`AGENTS.md`. If the base branch cannot be determined, assume `develop`
27+
and state that assumption.
28+
- Do not report formatting-only issues already enforced by PHP-CS-Fixer.
29+
- Every finding should identify a concrete failure scenario and the
30+
affected code.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
---
2+
applyTo: "system/Database/**/*.php,tests/system/Database/**/*.php"
3+
---
4+
5+
# Database layer
6+
7+
- Consider all supported drivers: MySQLi, PostgreSQL, SQLite3, SQLSRV, and
8+
OCI8.
9+
- Check identifier quoting, value binding, `NULL` behavior, transactions,
10+
affected rows, return types, and platform-specific SQL.
11+
- Do not implement driver-independent behavior in only one driver.
12+
- When changing a base database class, inspect driver overrides and their
13+
method signatures.
14+
- Keep driver-independent tests separate from tests that require a live
15+
database.
16+
- Use the `DatabaseLive` PHPUnit group only when a real database connection is
17+
required.
18+
- Prefer focused tests for the affected base class and drivers. Leave the full
19+
multi-driver matrix to GitHub Actions.
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
applyTo: "system/**/*.php"
3+
---
4+
5+
# Framework core
6+
7+
- Assume public and protected symbols are extension points used by third-party
8+
applications unless they are clearly marked internal.
9+
- Apply the target-branch compatibility policy from the root `AGENTS.md`
10+
before changing any extension point.
11+
- Before changing a signature or behavior, inspect parent classes, implemented
12+
interfaces, traits, service factories, configuration, and corresponding
13+
tests.
14+
- Keep file paths, class names, and namespaces aligned under `CodeIgniter\`.
15+
- Preserve existing extension and dependency-injection points.
16+
- Consider long-running worker environments. Do not retain request-specific
17+
state in static properties, globals, or shared services after a request
18+
finishes.
19+
- Avoid introducing a framework-wide abstraction to solve a single local
20+
problem.
21+
- On `develop`, keep exceptions and observable side effects compatible. On a
22+
`4.*` target, change them only as an explicit, narrow, documented breaking
23+
enhancement with a migration path.
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
applyTo: "tests/**/*.php"
3+
---
4+
5+
# PHPUnit tests
6+
7+
- Mirror the corresponding `system/` component and local test conventions
8+
where practical.
9+
- Add the appropriate PHPUnit `Group` attribute described in
10+
`tests/README.md`.
11+
- Prefer `assertSame()` and dedicated assertions over loose or generic
12+
assertions.
13+
- Cover the regression or behavior through a public API where possible.
14+
Avoid testing private implementation details.
15+
- Restore services, factories, environment variables, superglobals, handlers,
16+
locale, time-related state, and other global state modified by a test.
17+
- Tests must not depend on execution order or state left by another test.
18+
- Do not replace a meaningful assertion with a weaker assertion to make a test
19+
pass.
20+
- After changing behavior, run the smallest relevant test file or component.
21+
Do not run the complete test suite by default.
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
applyTo: "user_guide_src/**/*.rst"
3+
---
4+
5+
# User guide
6+
7+
- Preserve the existing reStructuredText structure, terminology, directives,
8+
anchors, and surrounding writing style.
9+
- Keep PHP examples compatible with PHP 8.2 and verify signatures and behavior
10+
against the current framework code.
11+
- Distinguish new behavior from existing behavior and document defaults
12+
precisely.
13+
- Determine the PR target branch before describing compatibility:
14+
`develop` follows the patch-release policy, while `4.*` follows the
15+
minor-release policy in the root `AGENTS.md`.
16+
- Behavior changes, enhancements, deprecations, and important bug fixes may
17+
require a changelog entry.
18+
- Changes requiring users to modify code or configuration may require an
19+
upgrading-guide entry.
20+
- For an intentional compatibility break targeting `4.*`, document the old
21+
behavior, new behavior, affected users, reason for the change, and the
22+
smallest migration in both the minor changelog and upgrading guide.
23+
- Keep code examples minimal but complete enough to run in a normal
24+
CodeIgniter application.
25+
- Leave the complete Sphinx build and documentation validation to GitHub
26+
Actions unless the task specifically requires local documentation
27+
validation.
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
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"
3+
---
4+
5+
# FrankenPHP Worker Mode
6+
7+
Use `system/Commands/Worker/Views/frankenphp-worker.php.tpl` as the canonical
8+
Worker Mode entry point. `worker:install` publishes this template to
9+
`public/frankenphp-worker.php`.
10+
11+
When changing bootstrap or request-lifecycle code, trace all three Worker Mode
12+
phases:
13+
14+
1. One-time process bootstrap through `Boot::bootWorker()`.
15+
2. Per-request preparation: reconnect database and cache connections, reset
16+
the `CodeIgniter` instance, and replace all request superglobals before
17+
calling `$app->run()`.
18+
3. Post-request cleanup: close the session, clean up unfinished database
19+
transactions, reset factories and non-persistent services, clean up event
20+
listeners and performance logs, and reset the debug toolbar.
21+
22+
- Preserve the phase ordering unless the change explicitly proves a different
23+
order is safe.
24+
- Check every new mutable static property, singleton, shared service, event
25+
listener, connection, handler, or global for cross-request state leakage.
26+
- Do not preserve a service across requests merely for performance. Persistent
27+
services must be safe to reuse and must not retain request, response, user,
28+
route, security token, locale, or error state.
29+
- Ensure exceptions and early exits cannot skip cleanup needed before the next
30+
request.
31+
- For state-isolation fixes, prefer a regression test that exercises two
32+
sequential requests or reset cycles in the same process and proves the
33+
second request cannot observe the first request's state.
34+
- If the template contract changes, update focused assertions in
35+
`tests/system/Commands/Worker/WorkerCommandsTest.php`.
36+
- If existing installations need the new generated entry point, update the
37+
changelog and upgrading guide and instruct users to run
38+
`php spark worker:install --force`.

AGENTS.md

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
# CodeIgniter 4 — Repository Guidelines
2+
3+
CodeIgniter 4 is a mature open-source PHP framework. Framework production
4+
code lives in `system/`; its tests mirror that structure under
5+
`tests/system/`.
6+
7+
## Before making changes
8+
9+
Consult the relevant sections of:
10+
11+
- `contributing/pull_request.md`
12+
- `contributing/internals.md`
13+
- `contributing/styleguide.md`
14+
- `tests/README.md`
15+
16+
Follow existing code and tests in the affected component. Search for an
17+
existing implementation before introducing a new abstraction or convention.
18+
19+
## Target branch and compatibility policy
20+
21+
Compatibility is judged against the pull request's base branch, not the
22+
contributor's source or feature branch. When the base branch is unknown,
23+
apply the stricter `develop` policy.
24+
25+
### Base branch `develop`
26+
27+
`develop` is the next patch-release line. Intentional backward-incompatible
28+
changes are not allowed.
29+
30+
- Preserve public and protected APIs, documented behavior, configuration
31+
defaults, generated project files, exceptions, and observable side effects.
32+
- Do not remove deprecated items or change public/protected signatures,
33+
visibility, or interface/abstract requirements.
34+
- A bug or security fix may correct faulty behavior but must preserve the
35+
documented contract and ordinary valid usage.
36+
- If a fix necessarily breaks compatibility, target the appropriate minor
37+
branch instead.
38+
39+
### Base branch `4.*`
40+
41+
Minor-release lines (for example `4.9`). Small, deliberate compatibility
42+
breaks may be accepted when they materially improve the framework or
43+
complete the documented deprecation lifecycle.
44+
45+
- Keep each break narrow, explicit, and justified; never an incidental
46+
side effect of refactoring.
47+
- Deprecated APIs may be removed no earlier than the second subsequent
48+
minor release (deprecated in 4.6.x means removable in 4.8.0), with a
49+
supported replacement.
50+
- Every accepted break needs tests for the new behavior, a minor-version
51+
changelog entry, and migration instructions in the upgrading guide.
52+
53+
### All base branches
54+
55+
- Production code must run on PHP 8.2. Do not require a newer PHP version,
56+
even when a newer version is installed locally.
57+
- Signatures, properties, constants, default values, exceptions, side
58+
effects, configuration, and generated project files are all
59+
compatibility-sensitive.
60+
- Major-scale redesigns and ecosystem-wide migration requirements belong in
61+
a major release, not a patch or minor release.
62+
- Prefer the minimum useful abstraction and keep framework components as
63+
independent as practical.
64+
- Dependencies should be injectable. When a framework service is used as a
65+
default, preserve the ability for applications to replace it.
66+
- Do not add or update a Composer dependency unless the task explicitly
67+
requires it and the change is justified.
68+
- Do not modify code in `system/ThirdParty/`.
69+
70+
## Coding standards
71+
72+
- Do not add `declare(strict_types=1)` mechanically. Follow nearby code and
73+
check the `DeclareStrictTypesRector` exclusions in `rector.php`; PHP files
74+
use strict types unless excluded there. Treat the existing exclusions as
75+
intentional compatibility constraints. Do not remove an exclusion without
76+
dedicated justification and tests.
77+
- Every new class property must have a native type declaration. Use the
78+
most precise type supported by PHP 8.2; use `mixed` only when the
79+
property intentionally accepts unrelated types.
80+
- Do not add or change a type on an existing public or protected property
81+
mechanically. Property types affect inheritance and must follow the
82+
target-branch compatibility policy.
83+
- Add PHPDoc only when it contributes information that native types cannot
84+
express. Do not duplicate a parent or interface docblock.
85+
- Never suppress a static-analysis error or update a baseline to make a
86+
check pass.
87+
88+
## FrankenPHP Worker Mode
89+
90+
In Worker Mode one process serves many requests. The entry point's source
91+
template is `system/Commands/Worker/Views/frankenphp-worker.php.tpl`; the
92+
`worker:install` command publishes it as `public/frankenphp-worker.php`.
93+
94+
- For every change affecting application bootstrap, request handling,
95+
response sending, shutdown, superglobals, sessions, database or cache
96+
connections, services, factories, events, toolbar state, static state, or
97+
other request-lifecycle behavior, inspect the worker entry-point template
98+
even when it is not part of the diff.
99+
- Compare traditional per-process execution with worker execution, where
100+
the framework boots once and handles multiple requests in the same
101+
process.
102+
- Classify mutable state as process-lifetime, intentionally persistent, or
103+
request-specific. Request-specific state must be refreshed or reset for
104+
every request and must not leak into the next request.
105+
- Preserve the ordering requirements between per-request reconnection,
106+
framework reset, superglobal refresh, application execution, and
107+
post-request cleanup.
108+
- Treat the template as the source of truth. Do not edit a generated
109+
`public/frankenphp-worker.php` in place.
110+
- When a release changes the template in a way existing installations must
111+
receive, add an upgrading instruction telling Worker Mode users to
112+
republish it with `php spark worker:install --force`.
113+
114+
## Tests
115+
116+
- Every bug fix should include a regression test that fails without the
117+
fix.
118+
- Test failure paths, exceptions, boundary conditions, and state cleanup,
119+
not only the happy path.
120+
- Prefer strict and dedicated PHPUnit assertions as described in the
121+
project style guide.
122+
- Do not weaken or remove an existing test unless the documented behavior
123+
or specification changed.
124+
125+
## Documentation
126+
127+
- Public API, behavior, message, or default-value changes may require an
128+
update to the user guide and changelog.
129+
- Changes requiring user action, including configuration changes, may also
130+
require an upgrading-guide entry.
131+
- For a `4.*` target, every intentional compatibility break must be
132+
documented in both the minor-version changelog and its upgrading guide.
133+
134+
## Focused validation
135+
136+
GitHub Actions is the authoritative source for full validation. Do not
137+
reproduce the complete CI matrix locally.
138+
139+
- Run only the narrowest PHPUnit test file or component that covers a code
140+
change, for example
141+
`vendor/bin/phpunit tests/system/<Component>/<Class>Test.php`.
142+
- Run a file-scoped analysis or formatting check only when it is quick and
143+
directly relevant.
144+
- Do not run the complete PHPUnit suite, `composer phpstan:check`,
145+
`composer cs`, Psalm, Structarmed, or a repository-wide Rector analysis
146+
by default.
147+
- Report which focused checks were executed and which validation was left
148+
to CI.

0 commit comments

Comments
 (0)