From 8851ac514c8f3e8397310b4615b0182a3650acf8 Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Fri, 30 Oct 2026 11:26:19 +0100 Subject: [PATCH 1/3] docs: keep transactions and locks in actions, never in middleware --- .ai/guidelines/app.actions.blade.php | 4 +++- .ai/guidelines/app.http.requests.blade.php | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.ai/guidelines/app.actions.blade.php b/.ai/guidelines/app.actions.blade.php index 87a0c8b64..673841869 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 skips the write if it no longer holds. This is not validation: it never aborts or throws, and the request keeps its own check and error message. Return `false` when the write was skipped and the caller must tell the user (`CreateOrganizationAdmin`); return nothing when the request's error is enough (`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..a1988352e 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 instead of failing (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`. From 77e1d9b65ea6ec3deb8c58ec345cab98b51d8fdf Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Fri, 30 Oct 2026 12:04:16 +0100 Subject: [PATCH 2/3] docs: throw a dedicated exception when a re-check under a lock refuses the write --- .ai/guidelines/app.actions.blade.php | 2 +- .ai/guidelines/app.http.requests.blade.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.ai/guidelines/app.actions.blade.php b/.ai/guidelines/app.actions.blade.php index 673841869..5deb06269 100644 --- a/.ai/guidelines/app.actions.blade.php +++ b/.ai/guidelines/app.actions.blade.php @@ -8,7 +8,7 @@ - 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 skips the write if it no longer holds. This is not validation: it never aborts or throws, and the request keeps its own check and error message. Return `false` when the write was skipped and the caller must tell the user (`CreateOrganizationAdmin`); return nothing when the request's error is enough (`UpdateMessagePin`). +- 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 what went wrong without an `Exception` suffix, and the caller catches it (`LastOrganizationAdmin`). `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. Checking again under a lock, above, is the only exception. diff --git a/.ai/guidelines/app.http.requests.blade.php b/.ai/guidelines/app.http.requests.blade.php index a1988352e..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`. The only exception is an action checking again under a row lock to close a race, which skips the write instead of failing (see the actions guidelines). +- 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`. From 288c0a8debcc41a940f64b3cbc40de27af12d74c Mon Sep 17 00:00:00 2001 From: Matthieu Napoli Date: Fri, 30 Oct 2026 15:45:17 +0100 Subject: [PATCH 3/3] docs: cite the exception by its new name, OrganizationMustHaveAnAdmin --- .ai/guidelines/app.actions.blade.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ai/guidelines/app.actions.blade.php b/.ai/guidelines/app.actions.blade.php index 5deb06269..3a8df6504 100644 --- a/.ai/guidelines/app.actions.blade.php +++ b/.ai/guidelines/app.actions.blade.php @@ -8,7 +8,7 @@ - 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 what went wrong without an `Exception` suffix, and the caller catches it (`LastOrganizationAdmin`). `CreateOrganizationAdmin`, which returns `false` instead, predates this. When the request's error is enough, skip the write silently (`UpdateMessagePin`). +- 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. Checking again under a lock, above, is the only exception.