From cdda1cbe2e254c15230a47c7e476cc05409312fe Mon Sep 17 00:00:00 2001 From: dcovington Date: Tue, 21 Jul 2026 17:11:24 +0000 Subject: [PATCH] Add 'AGENTS.MD' --- AGENTS.MD | 154 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 AGENTS.MD diff --git a/AGENTS.MD b/AGENTS.MD new file mode 100644 index 0000000..c966d3b --- /dev/null +++ b/AGENTS.MD @@ -0,0 +1,154 @@ +# Agent notes for this repo + +> Kept in sync with `CLAUDE.md` (same content, duplicated on purpose so both +> Claude Code and other agent tooling pick it up automatically — update both +> files together). + +Operational gotchas discovered while building the warehouse app on top of the +hand-rolled framework in `framework/`. Read this before re-deriving any of it +from scratch — most of it was expensive to find the first time. See +`docs/framework.md` for how the framework itself works (routing, models, +views, etc.) — this file is about the *environment* and *bugs*, not the +framework's intended API. + +## No local PHP/composer — everything runs through Docker + +There is no PHP or Composer on the host. Use: + +- `docker compose run --rm app ` for one-off commands (migrations, tests, scratch scripts) +- `docker compose exec app ` against the running stack (`docker compose up -d`) + +`.env` is required and gitignored — copy it from `.env.example` before first run. + +`composer install` needs `--ignore-platform-reqs`. The committed `composer.lock` +predates the container's PHP 8.2 image: `phpspec/prophecy` requires PHP `<8.1`, +and `ext-zip` (needed by `dbrekelmans/bdi`/`php-webdriver`) isn't installed. +This is a pre-existing gap, not something to "fix" by upgrading deps unless +asked. + +## Ports are a Windows-specific minefield + +Windows reserves TCP ports dynamically for Hyper-V/WSL2 NAT. A reserved port +fails to bind with `ports are not available: ... forbidden by its access +permissions` — this looks like "something else is using it" but isn't; it's +an OS-level exclusion, not a process conflict. Check before picking a port: + +``` +netsh interface ipv4 show excludedportrange protocol=tcp +``` + +`8081`, `9090`, and others have been seen reserved on this machine already. +Current mapping in `docker-compose.yml`: app → host `9092` (container `80`), +phpLiteAdmin → host `9091` (container `8081`). The **container-side** ports +must not change (`80` = nginx, `8081` = phpliteadmin's nginx vhost) — only +remap the host side, and check the exclusion list first. + +## `docker/start-container.sh` must stay LF + +A Windows checkout previously gave it CRLF line endings, which breaks its +shebang inside the Linux container (`start-container: not found` in +`docker logs pro-php-mvc-app`, crash-looping). `.gitattributes` now forces +`*.sh` to LF on checkout. If this error reappears, run `file +docker/start-container.sh` to confirm, then `sed -i 's/\r$//' +docker/start-container.sh`. + +## Stale root-owned cache files break the real app + +`docker compose run` (one-off containers) executes as root. The real `app` +service's php-fpm workers run as `www-data`. If you compile views or write +files via `docker compose run` and then the running app can't overwrite them +(`Permission denied` in `AdvancedEngine::render()`, `storage/framework/views/`), +it's stale root-owned `.php` cache files. Fix: delete them (gitignored, +regenerable) and let `www-data` recreate them: + +``` +docker exec pro-php-mvc-app find /var/www/html/storage/framework/views -maxdepth 1 -name '*.php' -delete +``` + +## Testing gotchas + +`phpunit.xml` registers `ServerExtension`, which **always** tries to +auto-start `php command.php serve` before the suite (even for non-browser +tests) and stop it after. This container has no `pcntl` extension, so both +the start (`ServeCommand::handleSignals()` → `pcntl_async_signals()`) and the +stop (`SIGTERM` constant) crash with a fatal error *after* the tests +themselves have already run — the crash happens in teardown, so a `phpunit` +run always exits non-zero even when everything passed. + +Workaround: bind a dummy TCP listener on the target `APP_HOST:APP_PORT` +(check `docker exec php -r "echo getenv('APP_HOST').':'.getenv('APP_PORT');"` +— currently `0.0.0.0:80`) before running phpunit, so +`ServerExtension::serverIsRunning()` sees it as already running and skips +starting/stopping the real (broken) one: + +``` +docker compose run --rm app sh -c "php -r 'stream_socket_server(\"tcp://0.0.0.0:80\"); sleep(120);' & sleep 1 && vendor/bin/phpunit; kill %1 2>/dev/null; true" +``` + +`BrowserTest.php` (Symfony Panther/Firefox) will still error — there's no +`geckodriver`/Firefox in this image. That's a pre-existing gap, not a +regression to chase. + +`php command.php serve` is itself broken in this container for the same +`pcntl` reason. Don't use it to "prove the app works" — hit a real running +`docker compose up` stack over HTTP instead (curl, or a browser). + +## Framework bugs found (work around at the app level — don't patch `framework/`) + +1. **Router doesn't disambiguate by HTTP method for dynamic paths, and + matches unanchored.** `Framework\Routing\Route::matches()` — once past the + literal exact-match check, the regex built for a `{param}` route is + checked with `preg_match_all` with no `^`/`$` anchors, and without + checking `$method` at all. Consequences: + - A shorter dynamic route (`/items/view/{item}`) matches as a *prefix* of + any longer path (`/items/view/{item}/receive`) — whichever route is + registered first wins, regardless of specificity or method. + - Two routes on the *identical* dynamic path with different methods (GET + show-form / POST submit) collide the same way — first-registered wins + for both methods, so the POST silently runs the GET handler. + + **Rule of thumb:** every dynamic (`{param}`) route in `app/routes.php` + needs a fully unique literal path. Never nest one dynamic route's path + under another's, and never pair GET+POST on the same dynamic path. Static + (no `{}`) paths are unaffected — they only ever use the exact-match + branch. See the stock-action routes in `app/routes.php` (and the comment + above them) for the working pattern: + `/items/receive-form/{item}` (GET) vs `/items/receive/{item}` (POST), etc. + +2. **PHP's `empty("0")` is `true`.** The framework's `required` rule + (`RequiredRule::validate`) is `!empty($data[$field])`, so a legitimate + `"0"` value fails `required`. Don't use `required` on a field where `0` is + valid (e.g. "set stock to zero") — validate presence manually instead. See + `AdjustStockController::handle()` for the pattern. + +3. **No `ORDER BY` / aggregate support** in `Framework\Database\QueryBuilder`. + Sort/sum in PHP after `->all()`. + +4. **No unique constraints** in the migration DSL. Uniqueness (e.g. + `items.sku`, `locations.code`) is not enforced anywhere — enforce at the + app level if it starts to matter. + +## Email + +Two drivers: `postmark` (API-based) and `smtp` (generic +`Swift_SmtpTransport` — no new dependency, ships with `swiftmailer/swiftmailer` +already). Switch via `EMAIL_DRIVER` in `.env`; `config/email.php` defaults to +`smtp`. + +Emails are **always queued** (`app('queue')->push(...)`), never sent +synchronously. Nothing processes the queue automatically — run +`docker compose exec app php command.php queue:work` (blocks, polls every +second) or jobs just sit in the `jobs` table forever. + +## Domain model + +- `locations`, `items`, `stock` (one row per item+location, found-or-created + via `Stock::forItemAtLocation()`), `stock_movements` (audit ledger: + receive/withdraw/adjustment). +- Auth gating is inline per-controller + (`if (!session()->has('user_id')) { return redirect(...); }`) — the + framework has no middleware concept; don't try to add one. +- Low-stock alerts fire from `WithdrawStockController`/`AdjustStockController` + after a stock change, when `item.reorder_level > 0 && total <= + reorder_level`. Not de-duplicated — repeated withdrawals under threshold + requeue repeated alerts.