diff --git a/.ai/guidelines/app.actions.blade.php b/.ai/guidelines/app.actions.blade.php index 87a0c8b64..3a8df6504 100644 --- a/.ai/guidelines/app.actions.blade.php +++ b/.ai/guidelines/app.actions.blade.php @@ -7,8 +7,10 @@ - Inject dependencies via constructor using private properties. - Create new actions with `php artisan make:action "{name}" --no-interaction` - Wrap complex operations in `DB::transaction()` within actions when multiple models are involved. +- Transactions and row locks that guard a write live in actions, never in HTTP middleware: that puts database rules in the HTTP layer and holds the transaction open for the whole request. +- When a concurrent request can pass the same check before the write (a limit, the last admin), the action locks the parent row (the conversation, the organization) in a short `DB::transaction()`, checks the condition again under the lock, and doesn't write if it no longer holds. This is not validation: it never aborts or throws a `ValidationException` or an HTTP exception, and the request keeps its own check and error message. When the caller must tell the user, `throw new` a dedicated exception from `app/Exceptions` (never `throw_if()`), named for the rule it protects, without an `Exception` suffix, and the caller catches it (`OrganizationMustHaveAnAdmin`). `CreateOrganizationAdmin`, which returns `false` instead, predates this. When the request's error is enough, skip the write silently (`UpdateMessagePin`). - Some actions won't require dependencies via `__construct` and they can use just the `handle()` method. -- Actions never authorize or validate anything: that lives in the form request of the endpoint calling them. +- Actions never authorize or validate anything: that lives in the form request of the endpoint calling them. Checking again under a lock, above, is the only exception. ## CRUD by design diff --git a/.ai/guidelines/app.http.requests.blade.php b/.ai/guidelines/app.http.requests.blade.php index c7bb7152d..65df26eb3 100644 --- a/.ai/guidelines/app.http.requests.blade.php +++ b/.ai/guidelines/app.http.requests.blade.php @@ -2,7 +2,7 @@ - Never use policies or gates: no `app/Policies`, no `Gate::define()`, no `can:` middleware, no `$user->can()`, no `$this->authorize()`. - Authorization and validation only ever happen in form requests: authorization in `authorize()`, validation in `rules()`. Never in controllers, actions, or models. This includes `index` and `show` endpoints. -- Controllers and actions never check anything themselves: no `abort()`, `abort_if()`, `abort_unless()`, `throw_if()`, `throw_unless()`, `Validator::make()`, `$request->validate()`, or `ValidationException`. +- Controllers and actions never check anything themselves: no `abort()`, `abort_if()`, `abort_unless()`, `throw_if()`, `throw_unless()`, `Validator::make()`, `$request->validate()`, or `ValidationException`. The only exception is an action checking again under a row lock to close a race, which skips the write or throws its own exception for the caller to catch (see the actions guidelines). - Create a dedicated form request for each endpoint that checks anything, and never share one between endpoints. If the endpoint has none yet, create one. - Name it after the controller method it serves, CRUD by design: `index` → `List…Request`, `show` → `Show…Request`, `store` → `Create…Request`, `update` → `Update…Request`, `destroy` → `Delete…Request`. For example `CommitPinController@update` → `UpdateCommitPinRequest`, `CommitPinController@destroy` → `DeleteCommitPinRequest`, `CommitApprovalController@store` → `CreateCommitApprovalRequest`. - When hiding something should look like it does not exist, return `Response::denyAsNotFound()` from `authorize()` instead of `false`.