Skip to content

Commit a9cb37f

Browse files
committed
ci: gate pull requests on a worker-mode state audit
Changes - Add `igor-php/igor-php` as a dev dependency and register `IgorPhpBundle` in dev - Configure it in `igor.json`: project scope, dev environment, baseline file - Record the 33 existing findings in `igor-baseline.json`, each with a reason - Add `composer worker-state-check` and `worker-state-baseline`, and a `Worker state audit` job to the review workflow Why The statelessness rules the last few commits established are the kind that decay without enforcement, and they matter whether or not worker mode is ever switched on: `messenger:consume` is already long-running in production. igor-php audits every shared service in the compiled container rather than grepping for patterns, which is why it caught the `AdminUrlGenerator` mutations that reading `src/` for stateful properties had missed. Against that, roughly two thirds of its project findings are noise — mostly Doctrine entities returned from a repository, which it reads as shared services — so it is only usable behind a baseline. Vendor code is out of scope: it reported 341 findings there, none of them ours to fix. Every baseline entry carries a reason rather than the generated TODO, so the file documents why each is safe instead of just silencing it. Verified the gate is live: introducing a stateful property on a service fails the audit, and removing it passes.
1 parent a2c02af commit a9cb37f

9 files changed

Lines changed: 290 additions & 1 deletion

File tree

.github/workflows/pr.yaml

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,27 @@ jobs:
1818
docker compose run --rm frankenphp composer install --no-interaction
1919
docker compose run --rm frankenphp vendor/bin/phpstan analyse
2020
21+
worker-state:
22+
runs-on: ubuntu-latest
23+
name: Worker state audit
24+
steps:
25+
- name: Checkout
26+
uses: actions/checkout@v6
27+
28+
- name: Create docker network
29+
run: docker network create frontend
30+
31+
# igor-php audits every shared service in the compiled container for
32+
# state that would leak between requests in a worker. It reads the
33+
# service map IgorPhpBundle writes during cache:clear, and fails only
34+
# on findings absent from igor-baseline.json — every entry in which
35+
# carries a reason. Regenerate with `composer worker-state-baseline`.
36+
- name: Audit shared services for worker-mode state leaks
37+
run: |
38+
docker compose run --rm frankenphp composer install --no-interaction
39+
docker compose run --rm frankenphp bin/console cache:clear
40+
docker compose run --rm frankenphp composer worker-state-check
41+
2142
phpunit:
2243
runs-on: ubuntu-latest
2344
name: PHP Unit tests

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4747
consumer, which in a worker outlives the request
4848
- Cover all of it with tests: the factories had none, and neither the
4949
dashboard nor the Security Contract CRUD was in the admin smoke test
50+
- Gate pull requests on `igor-php`, which audits every shared service in the
51+
compiled container for state that would leak between requests.
52+
`igor-baseline.json` records the 33 existing findings with a reason each, so
53+
the job fails only on new ones; vendor code is out of scope
5054
- Document worker mode and the statelessness it requires in `README.md` and
5155
`claude.md`. It stays off: measured here it gives roughly 20% more requests
5256
per second on `/admin` and half the median latency, but around 40% fewer on

README.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,27 @@ deliberate, implement `Symfony\Contracts\Service\ResetInterface` –
188188
For an object you do not own, clear it at the call site, the way every
189189
`AdminUrlGenerator` chain here opens with `unsetAll()`.
190190

191+
That rule is enforced. [igor-php](https://github.com/igor-php/igor-php) audits
192+
every shared service in the compiled container for state that would leak between
193+
requests, and runs on every pull request:
194+
195+
```sh
196+
docker compose exec frankenphp composer worker-state-check
197+
```
198+
199+
Existing findings live in `igor-baseline.json`, so the job fails only on new
200+
ones. Every entry there carries a reason – most are Doctrine entities returned
201+
from a repository, which igor reads as shared services, and `AdminUrlGenerator`
202+
chains it cannot see are already cleared by `unsetAll()`. Read the reasons before
203+
adding to them; if a finding is genuine, fix it rather than baseline it. After a
204+
deliberate change, regenerate with `composer worker-state-baseline` and write a
205+
reason for each new entry.
206+
207+
The audit needs the service map that `IgorPhpBundle` writes during
208+
`cache:clear`, so run that first if the cache is cold. Vendor code is out of
209+
scope (`ignore_vendors` in `igor.json`): it reported 341 findings there, none of
210+
them ours to fix.
211+
191212
`FRANKENPHP_RESET_KERNEL=1`, on Symfony 8.1 and later, clones the kernel between
192213
requests instead. It hides this class of bug at the cost of a boot per request,
193214
which is most of what worker mode is for – useful to compare against, not to

claude.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,12 @@ Things that break a worker and have no place here: `exit()`/`die()`, writes to
119119
superglobals, `__destruct()` on a shared service, and mutable `static`
120120
properties.
121121

122+
`composer worker-state-check` audits this with igor-php and runs on every pull
123+
request. `igor-baseline.json` holds the known findings, each with a reason, so
124+
the job fails only on new ones — fix a genuine finding rather than baselining
125+
it, and regenerate with `composer worker-state-baseline` only after a deliberate
126+
change.
127+
122128
`FRANKENPHP_RESET_KERNEL=1` (Symfony 8.1+) clones the kernel between requests
123129
and papers over all of this, at the cost of a boot per request. Treat it as a
124130
measurement baseline, not a fix.

composer.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"ergebnis/composer-normalize": "^2.23",
4848
"friendsofphp/php-cs-fixer": "^3.6",
4949
"hautelook/alice-bundle": "^2.14",
50+
"igor-php/igor-php": "^0.9.5",
5051
"justinrainbow/json-schema": "^6.0",
5152
"phpstan/extension-installer": "^1.4",
5253
"phpstan/phpstan": "^2.1",
@@ -136,6 +137,12 @@
136137
"update-api-spec": [
137138
"bin/console api:openapi:export --output=public/api-spec-v1.yaml --yaml --no-interaction",
138139
"bin/console api:openapi:export --output=public/api-spec-v1.json --no-interaction"
140+
],
141+
"worker-state-baseline": [
142+
"vendor/bin/igor-php --generate-baseline ."
143+
],
144+
"worker-state-check": [
145+
"vendor/bin/igor-php ."
139146
]
140147
}
141148
}

composer.lock

Lines changed: 51 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

config/bundles.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,4 +23,5 @@
2323
Twig\Extra\TwigExtraBundle\TwigExtraBundle::class => ['all' => true],
2424
Symfony\UX\TwigComponent\TwigComponentBundle::class => ['all' => true],
2525
Doctrine\Bundle\MigrationsBundle\DoctrineMigrationsBundle::class => ['all' => true],
26+
IgorPhp\IgorBundle\IgorPhpBundle::class => ['dev' => true],
2627
];

0 commit comments

Comments
 (0)