feat: add EnforceCurrentUserAttributeRule

[Goosterhof]

Summary

• New PHPStan rule EnforceCurrentUserAttributeRule flagging Request::user() / Auth::user() / auth()->user() calls inside classes extending Illuminate\Routing\Controller. The canonical fix is Laravel's #[\Illuminate\Container\Attributes\CurrentUser] container attribute on the method parameter — eliminates the implicit-nullable-then-assert dance ($user = $request->user(); assert($user instanceof User);) introduced by emmie PR #263 and recurring across the war-room fleet.
• Scoped to Controller descendants only. FormRequest descendants (where $this->user() is canonical because container-attribute injection does not apply to FormRequest::rules() / toDto() / authorize() invocations), middleware, services, Actions, jobs, and console commands are silent by design.
• Detection branches on three call shapes via CallLike registration (mirrors LogRule v0.3.0 expansion shape): type-based receiver match on Illuminate\Http\Request subtype (ObjectType::isSuperTypeOf()), AST-shape match on FuncCall('auth') receiver, FQCN match via $scope->resolveName() on Auth::class. Containing-class gate via ClassReflection::isSubclassOf(Controller::class).

Origin

War-room cross-territory recon (2026-05-22) — ~50+ violations across codebook (~40+), ublgenie (6), entreezuil (3), emmie (2); kendo already clean with 30 adopted sites. Doctrine source: war-room §Architectural Principles — Explicit over implicit. Identifier: enforceCurrentUserAttribute.useAttributeInsteadOfRequestUser.

Versioning

Per ADR-0021 §Versioning, this is a candidate Major bump (new errors in already-clean code wherever consumer territories carry un-migrated controllers). The release PR will decide whether this collapses into the existing v0.3.0 [Unreleased] block (already a Major, currently gated on PR #25 recovery) or cuts as v0.4.0 after v0.3.0 ships. CHANGELOG entry added to [Unreleased] per package convention.

Pre-cascade audit verified at filing time across the 5 Laravel territories (recon counts above). Post-release adoption plan:

• emmie / ublgenie / entreezuil: Medic dispatches to migrate violations (single PR each).
• codebook: PHPStan-baseline staging — safer than mass-edit on lightly-staffed AVG/NEN-7510-downstream territory; violations drain in scheduled passes.
• kendo: constraint bump only (0 violations) — regression protection.

Out of scope v1

• Auth::guard('name')->user() and other guard-specific resolution paths — rare, substitution is more nuanced (#[CurrentUser(guard: 'name')]), do not appear in the recon yield. Track as follow-up issue if a future recon surfaces them.
• Multi-guard ergonomics work as expected: #[CurrentUser] Client $client resolves via client guard, #[CurrentUser] User $user via user guard — verified in emmie's ClientController::me. Laravel dispatches by typed parameter.

Mutation-test notes

• Mutation iteration added a 10th fixture (TopLevelAuthCall.php) to kill the FalseValue mutant on insideControllerMethod()'s null-class-reflection branch.
• Two LogicalOr mutants remain alive on the early-return guards (lines 118 and 140) — equivalent-mutant precedent already accepted on LogRule lines 84/99 (same defensive-guard shape, same family).

Test plan

• composer test — 60 tests pass, 101 assertions (10 new test methods on the new rule)
• composer phpstan — level max self-analysis clean (9 services discovered)
• composer format:check — Pint clean
• composer test:coverage + composer coverage:check — 86.00% line coverage (381/443) ≥ 83% gate
• composer mutation:ci — MSI 82.47%, Covered MSI 82.47% (both ≥ 75% gate)
• CI matrix (8.4 + 8.5) — auto-runs on PR open
• Reviewer scan: detection-scope sanity check (does the Controller-ancestry gate fire where it should and stay silent where it shouldn't?)

Doctrine artifacts

• Deployment order (war-room): orders/phpstan-warroom-rules/enforce-current-user-attribute-rule-engineer-deployment.md
• Execution report (war-room): reports/phpstan-warroom-rules/execution/2026-05-22-engineer-enforce-current-user-attribute-rule.md
• ADR-0021 §Versioning + §Identifier convention + §Doctrine source in docblock — all observed

🤖 Generated with Claude Code

[jasperboerhof]

PR Reviewer · claimed

• Target: phpstan-warroom-rules
• PR: feat: add EnforceCurrentUserAttributeRule #26
• Branch: feat/enforce-current-user-attribute-rule
• AC anchor: none (no linked Kendo issue, no plan dir, no AC heading in PR body)
• Worktree: ~/Code/agent-worktrees/phpstan-warroom-rules/review-26

[jasperboerhof]

PR Reviewer · 10/10 · PASS

• PR: feat: add EnforceCurrentUserAttributeRule #26 · phpstan-warroom-rules
• AC anchor: none
• Reviewers run: Acceptance, Silent failures skipped — no triggers, Efficiency skipped — no triggers
• Acceptance: SKIP

Findings

• none — all reviewers clean

Action

merge-ready

[jasperboerhof]

Auto-approved by /review-open-prs — review verdict is PASS. See the verdict comment for the per-reviewer breakdown.

[Goosterhof]

Two blockers, two concerns, one nit, one praise.

The rule is well-constructed mechanically — clean CallLike registration, three detection shapes that mirror the established LogRule pattern, and a thorough fixture matrix that exercises every out-of-scope context (FormRequest, middleware, Action, job, top-level). The detection logic for the three call shapes is correct. The problem is not the detection — it is the scope gate, and it is load-bearing.

Blockers

1. The Controller-ancestry gate does not match the actual fleet — the rule is silent on most real controllers. insideControllerMethod() (src/Rules/EnforceCurrentUserAttributeRule.php:156-169) fires only when $classReflection->isSubclassOf('Illuminate\Routing\Controller'). But across the consumer territories, the overwhelming majority of controllers do not extend any base class at all — modern Laravel scaffolds controllers without a base Controller:

• kendo: 59 controllers, 0 extend a base. The PR positions kendo as "clean with 30 adopted sites" getting a "constraint bump only — regression protection." There is no regression protection: a future $request->user() added to any of those 59 controllers fires nothing, because the ancestry gate returns false for all of them.
• ublgenie: 12 controllers, 0 extend. Recon claims 6 violations — the rule catches 0.
• entreezuil: 7 controllers, 0 extend. Recon claims 3 violations.
• emmie is the only territory where the gate works: 88 of 105 controllers extend a local abstract class Controller extends Illuminate\Routing\Controller (app/Http/Controllers/Controller.php:9,11). The other 17 escape.

So the rule's claimed cross-territory yield (50+ violations) is almost entirely unreachable as written. The fixtures pass because every controller fixture (RequestUserInController.php:10, AuthFacadeInController.php:10, etc.) was authored with extends Controller — they validate the gate against a controller shape three of the four territories don't actually use. The test suite is green against a strawman, and the kendo "regression protection" the PR explicitly promises does not exist.

2. The sibling rule already solved this, and this rule diverges from it. ForbidEloquentMutationInControllersRule scopes "controller" by namespace prefix (App\Http\Controllers, ForbidEloquentMutationInControllersRule.php:140-146) and its docblock explicitly documents why it avoids ancestry: $scope->getClassReflection() is unreliable, and namespace gating catches sub-namespaces like kendo's App\Http\Controllers\Central\* "naturally." Shipping a second controller-scoped rule in the same package with a contradictory, weaker definition of "controller" is an internal inconsistency that will confuse every consumer — one rule fires on a controller, the sibling doesn't, for the same class. Fix path: replace the ancestry gate with $scope->getNamespace() + str_starts_with($namespace, 'App\Http\Controllers'), copy the controllerNamespacePrefixes parameterization hook the sibling already carries. The FormRequest exclusion survives this unchanged — FormRequests live in App\Http\Requests, verified across all four territories, so a namespace gate keeps them silent without the ancestry crutch.

The two together: the rule as written will pass CI in every territory and protect almost nothing. Re-anchoring the gate to namespace is the correction, and it makes the existing fixture matrix more honest (drop the extends Controller from the controller fixtures so they reflect real scaffolding, and add a fixture for a sub-namespace controller).

Concerns

1. getName() === self::CONTROLLER_BASE_FQCN early-return (:164-166) is dead weight under the current gate and unmotivated. It exists to avoid flagging the base Illuminate\Routing\Controller itself, but that class lives in vendor/ and won't be analyzed, and isSubclassOf is already strict-ancestor in the relevant direction. No fixture exercises it. If the gate moves to namespace this branch disappears anyway; if it stays, justify it with a fixture or drop it.

2. The auth() AST-shape match is namespace-naive (receiverIsAuthHelper, :232-243). It matches any FuncCall named auth regardless of namespace resolution — a user-defined App\Support\auth() in a controller would false-positive. Low probability given the helper's ubiquity, but the sibling rules lean on $scope->resolveName() for exactly this discrimination on the facade branch (:206) while the helper branch skips it. Worth a one-line note in the docblock that this is a deliberate AST-only match accepting that collision, or a function_exists-style guard if you want to be strict.

Nits

The CHANGELOG entry is a single ~1900-character paragraph carrying versioning strategy, pre-cascade audit plan, and per-territory rollout. That's campaign-report and release-PR content, not a changelog line. Trim to the rule's behavior and identifier; the rollout plan already lives in the deployment order and PR body.

Praise

The TopLevelAuthCall.php fixture (:453-468) and its test (:699-712) are a genuine catch — exercising the null-getClassReflection() path to kill the FalseValue mutant on the no-class-reflection guard is the kind of mutation-driven coverage that prevents a real crash on top-level code, not a coverage-percentage chase. Named in the PR body with the specific mutant it kills. That's load-bearing.

Verdict: this is a COMMENT, not an approval. The detection engine is sound and the test discipline is real, but the scope gate is anchored to a controller shape three of the four consumer territories don't use — as written it would ship green and enforce almost nothing, including the kendo regression protection the PR explicitly promises. Re-anchor the gate to the namespace prefix the sibling rule already uses, re-author the controller fixtures to drop the artificial extends Controller, and the rule does what it claims. I'd want to re-review the gate change and the fixture re-authoring before merge.

[Goosterhof]

Restacked for v0.4.0 — re-review requested. Rebased onto the restacked #28; base retargeted to queue-87-forbid-eloquent-mutation-in-controllers so this diff shows only the EnforceCurrentUserAttributeRule. Rule source + tests byte-identical to your prior approval — CHANGELOG repositioned into the fresh v0.4.0 [Unreleased] only. Top of the stack — merge last (order: #27 → #28 → #26).

[Goosterhof]

Two blockers, two concerns, one nit, one praise.

The detection engine is sound. The CallLike registration, the three call-shape branches, and the fixture matrix that exercises every out-of-scope context (FormRequest, middleware, Action, job, top-level) are all correct and mirror the established LogRule shape. The problem is not detection — it is the scope gate, and the gate is load-bearing. I verified the gate claims directly against the consumer territories; both blockers below are reproduced, not asserted.

This PR is stacked on queue-87-forbid-eloquent-mutation-in-controllers (its base ref), so its diff correctly carries only EnforceCurrentUserAttributeRule. The sibling I cite below, ForbidEloquentMutationInControllersRule, is the parent in that stack — not yet on main. The contradiction I flag is therefore between two rules shipping together in the same release train.

Blockers

1. The Controller-ancestry gate is silent on three of the four consumer territories — the rule enforces almost nothing as written. insideControllerMethod() (src/Rules/EnforceCurrentUserAttributeRule.php:155-168) fires only when $classReflection->isSubclassOf('Illuminate\Routing\Controller'). But modern Laravel scaffolds controllers with no base class, and that is what the fleet actually runs. Reproduced counts (controllers declaring a class vs. controllers extending any base):

• kendo: 59 controllers, 0 extend a base. The PR positions kendo as getting "a constraint bump only — regression protection." There is no regression protection — a future $request->user() in any of those 59 controllers fires nothing.
• ublgenie: 12 controllers, 0 extend. The PR's own recon claims 6 violations here. The rule catches 0 of them. I confirmed all 6 are real and unreachable — e.g. final class InvoiceController (no extends) at app/Http/Controllers/InvoiceController.php:32 calls $request->user() at :40 and :116; AuthController.php:52,65,74; BranchController.php:28.
• entreezuil: 7 controllers, 0 extend. Recon claims 3 violations — caught: 0.
• emmie: the only territory where the gate works — 89 of 106 controllers extend a local abstract class Controller extends Illuminate\Routing\Controller (backend/app/Http/Controllers/Controller.php:9,11). The other 17 escape.

The fixtures pass because every controller fixture (RequestUserInController.php, AuthFacadeInController.php, AuthHelperInController.php) was authored with extends Controller — a shape three of the four territories do not use. The suite is green against a strawman, and the kendo regression protection the PR explicitly promises does not exist. The claimed 50+ cross-territory yield is almost entirely unreachable.

2. The sibling rule already solved "what is a controller," and this rule contradicts it. ForbidEloquentMutationInControllersRule (the parent PR in this stack) scopes a controller by namespace prefix — $scope->getNamespace() + str_starts_with($namespace, 'App\Http\Controllers') (src/Rules/ForbidEloquentMutationInControllersRule.php:140-142) — and its docblock explicitly documents avoiding $scope->getClassReflection() because it "can return null" and namespace gating catches sub-namespaces like kendo's App\Http\Controllers\Central\* naturally (:60-64, :313). Shipping a second controller-scoped rule in the same release with a weaker, contradictory definition of "controller" means one rule fires on a controller and its sibling stays silent for the same class. Fix path: replace the ancestry gate with the namespace gate the sibling already carries, and lift the controllerNamespacePrefixes parameter hook the sibling documents. The FormRequest exclusion survives unchanged — FormRequests live in App\Http\Requests (verified across all four territories), so namespace gating keeps them silent without the ancestry crutch. The fixture matrix should then drop the artificial extends Controller from the controller fixtures so they reflect real scaffolding, and add a sub-namespace controller fixture.

The two together: the rule passes CI everywhere and protects almost nothing. Re-anchoring to namespace is the correction, and it makes the fixtures honest.

Concerns

1. The getName() === self::CONTROLLER_BASE_FQCN early-return (:163-165) is dead weight and unmotivated. It guards against flagging the base Illuminate\Routing\Controller itself, but that class lives in vendor/ and won't be analyzed, and isSubclassOf is strict-ancestor anyway. No fixture exercises it. If the gate moves to namespace this branch disappears; if it stays, justify it with a fixture or drop it.

2. The auth() AST-shape match is namespace-naive (receiverIsAuthHelper, :231-242). It matches any FuncCall named auth with no namespace resolution — a user-defined App\Support\auth() in a controller would false-positive. Low probability, but the static-call branch leans on $scope->resolveName() for exactly this discrimination (:205) while the helper branch skips it. The docblock already concedes this is a deliberate AST-only match (:225-230); a one-line note in the error-surface docs, or a function_exists-style guard if you want strictness, would close the asymmetry.

Nits

The CHANGELOG entry for this rule (CHANGELOG.md) is a single ~1900-character paragraph carrying versioning strategy, the pre-cascade audit plan, per-territory rollout counts, and multi-guard ergonomics notes. That is release-PR and campaign-report content, not a changelog line. Trim to the rule's behavior and identifier; the rollout plan already lives in the deployment order and the PR body.

Praise

The TopLevelAuthCall.php fixture (tests/Fixtures/CurrentUserAttribute/TopLevelAuthCall.php) and its test (EnforceCurrentUserAttributeRuleTest.php:698-711) are a genuine catch — exercising the null-getClassReflection() path to kill the FalseValue mutant on the no-class-reflection guard prevents a real crash on top-level code, not a coverage-percentage chase. Named in the PR body with the specific mutant it kills. Load-bearing.

Verdict: COMMENT, not an approval. The detection engine and the test discipline are real, but the scope gate is anchored to a controller shape three of the four consumer territories do not use — verified — so as written it ships green and enforces almost nothing, including the kendo regression protection the PR promises, and it contradicts the namespace gate its own stacked sibling uses. Re-anchor the gate to the namespace prefix, re-author the controller fixtures to drop extends Controller, and the rule does what it claims. I'd re-review the gate change and the fixture re-authoring before merge.

[jasperboerhof]

Approved — reviewed, no blockers.

[Goosterhof]

One blocker, two concerns, one nit, one praise.

This PR now carries both rules in a single diff — EnforceCurrentUserAttributeRule (+197) and ForbidEloquentMutationInControllersRule (+370). That changes the most important finding from the prior reviews: the scope-gate contradiction I flag below is no longer a cross-stack inconsistency to reconcile later — it sits in this one diff, with the correct pattern and the broken one side by side in src/Rules/. I verified every gate claim directly against the checked-out territories; the blocker is reproduced, not asserted.

The detection engine on both rules is sound. EnforceCurrentUserAttributeRule's three call-shape branches (CallLike registration, ObjectType::isSuperTypeOf() on the Request receiver, FuncCall('auth') AST match, $scope->resolveName() on the Auth facade) are correct and mirror the established LogRule shape. ForbidEloquentMutationInControllersRule's 24-method blocklist, type-aware Builder coverage, and recursive method-body walk are correct. The problem is not detection — it is one of the two scope gates, and it is load-bearing.

Blockers

1. EnforceCurrentUserAttributeRule's ancestry gate is silent on three of the four consumer territories, and the correct gate is already in this same PR. insideControllerMethod() (src/Rules/EnforceCurrentUserAttributeRule.php:168-181) fires only when $classReflection->isSubclassOf('Illuminate\Routing\Controller'). Modern Laravel scaffolds controllers with no base class, and that is what the fleet actually runs. Reproduced counts (controllers vs. controllers extending any Controller base):

• kendo: 59 controllers, 0 extend a base, no local Controller.php. Every one is final class XController (e.g. AuthController, BillingController). The PR positions kendo as getting "a constraint bump only — regression protection." There is no regression protection: a future $request->user() added to any of those 59 controllers fires nothing.
• ublgenie: 12 controllers, 0 extend. The PR's own recon claims 6 violations here — the rule catches 0. Confirmed real and unreachable: final class InvoiceController (no extends) calls $request->user() at app/Http/Controllers/InvoiceController.php:40 and :116; AuthController.php:52,65,74; BranchController.php:28.
• entreezuil: 7 controllers, 0 extend. Recon claims 3 violations — caught: 0.
• emmie: the only territory where the gate works — 89 of 106 controllers extend abstract class Controller extends BaseController, and BaseController is use Illuminate\Routing\Controller as BaseController (backend/app/Http/Controllers/Controller.php). The ancestry gate resolves through the alias and fires. The other 17 escape.

The fixtures pass because every controller fixture (RequestUserInController.php, AuthFacadeInController.php, AuthHelperInController.php) was authored with extends Controller — a shape three of the four territories do not use. The suite is green against a strawman, and the kendo regression protection the PR explicitly promises does not exist. The claimed 50+ cross-territory yield is almost entirely unreachable.

The fix is in the diff already. ForbidEloquentMutationInControllersRule — shipping in this same PR — scopes a controller by namespace prefix: $scope->getNamespace() + str_starts_with($namespace, 'App\Http\Controllers') (src/Rules/ForbidEloquentMutationInControllersRule.php:413-417). And its resolveClassFqcn() docblock states the reason out loud: it "Avoids depending on $scope->getClassReflection(), which can return null during fixture-mode analysis" (:585-588). The two rules disagree on what a controller is, within one diff — one keys on reflection ancestry the other deliberately rejects. Re-anchor EnforceCurrentUserAttributeRule to the namespace gate the sibling already carries, lift the controllerNamespacePrefixes parameter hook the sibling documents (:333-337), then re-author the controller fixtures to drop the artificial extends Controller so they reflect real scaffolding, and add a sub-namespace controller fixture mirroring ViolationKendoCentralSubnamespace.php. The FormRequest exclusion survives the change unchanged — FormRequests live in App\Http\Requests (verified across all four territories), so the namespace gate keeps them silent without the ancestry crutch.

Concerns

1. The getName() === self::CONTROLLER_BASE_FQCN early-return (EnforceCurrentUserAttributeRule.php:176-178) is dead weight and unmotivated. It guards against flagging the base Illuminate\Routing\Controller itself, but that class lives in vendor/ and is not analyzed, and isSubclassOf is strict-ancestor anyway. No fixture exercises it. If the gate moves to namespace this branch disappears; if it stays, justify it with a fixture or drop it.

2. The auth() AST-shape match is namespace-naive (receiverIsAuthHelper, :244-255). It matches any FuncCall named auth with no namespace resolution — a user-defined App\Support\auth() called inside a controller would false-positive. Low probability given the helper's ubiquity, but the static-call branch leans on $scope->resolveName() for exactly this discrimination (:218) while the helper branch skips it. The docblock concedes this is a deliberate AST-only match (:238-243); a one-line note in the error-surface docs, or a function_exists-style guard if you want strictness, would close the asymmetry.

Nits

The two CHANGELOG.md entries are single paragraphs of ~1900 and ~2400 characters carrying versioning strategy, pre-cascade audit plans, per-territory rollout counts, multi-guard ergonomics, and supersession inventories. That is release-PR and campaign-report content, not changelog lines. Trim each to the rule's behavior and identifier; the rollout detail already lives in the deployment order and the PR body.

Praise

The TopLevelAuthCall.php fixture (tests/Fixtures/CurrentUserAttribute/TopLevelAuthCall.php) and its test (EnforceCurrentUserAttributeRuleTest.php:1592-1605) are a genuine catch — exercising the null-getClassReflection() path to kill the FalseValue mutant on the no-class-reflection guard prevents a real crash on top-level code, not a coverage-percentage chase. Named in the PR body with the specific mutant it kills. Load-bearing. Worth noting the irony: that fixture exists because EnforceCurrentUserAttributeRule leans on getClassReflection(), which ForbidEloquentMutationInControllersRule in the same PR avoids precisely because it can be null.

Verdict: COMMENT, not an approval. The detection engines and the test discipline are real, but EnforceCurrentUserAttributeRule's scope gate is anchored to a controller shape three of the four consumer territories do not use — verified — so as written it ships green and enforces almost nothing, including the kendo regression protection the PR promises. And it contradicts the namespace gate its own co-shipping sibling uses for the same concept. Re-anchor the gate to the namespace prefix, re-author the controller fixtures, and the rule does what it claims. I'd re-review the gate change and the fixture re-authoring before merge.

[jasperboerhof]

PR Reviewer · claimed

• Target: phpstan-warroom-rules
• PR: feat: add EnforceCurrentUserAttributeRule #26
• Branch: feat/enforce-current-user-attribute-rule
• AC anchor: none (no linked Kendo issue, no plan dir, no AC heading in PR body)
• Worktree: ~/Code/agent-worktrees/phpstan-warroom-rules/review-26

[jasperboerhof]

PR Reviewer · 6/10 · REVISE

• PR: feat: add EnforceCurrentUserAttributeRule #26 · phpstan-warroom-rules
• AC anchor: none
• Reviewers run: Acceptance, Simplicity, Surface, Silent failures skipped — no triggers, Efficiency skipped — no triggers
• Acceptance: SKIP
• Simplicity: NEEDS WORK · 6/10
• Surface: PASS · 9/10

Findings

• MAJOR · src/Rules/ForbidEloquentMutationInControllersRule.php:406 — Over-built node-visit shape. The rule registers on Class_ and hand-rolls collectViolations() + walkNodes() + getMethods() + resolveClassFqcn() (~80 lines) to find call nodes. Its OWN sibling rule in this same PR (EnforceCurrentUserAttributeRule) and the existing LogRule both detect call-shapes inside a scoped class via getNodeType() = CallLike + $scope->getNamespace() / $scope->getClassReflection() — PHPStan visits every MethodCall/StaticCall natively, no manual tree walk. Nothing here needs statement-structure reasoning (unlike the App\Actions* transaction rules that legitimately use Class_ to find transaction(...) closures): the rule only needs 'call inside App\Http\Controllers + Model/Builder receiver + blocklisted method'. Rewriting as a CallLike rule deletes the entire walkNodes/collectViolations/getMethods/resolveClassFqcn apparatus. The docblock even acknowledges the walkNodes duplication (Standing Concern ci: pin symfony/console to ^7 — Infection mutation gate broken by Symfony Console 8 #29) but takes the duplicating path when the leaner one was demonstrably available in the adjacent file.
• MINOR · src/Rules/ForbidEloquentMutationInControllersRule.php:598 — shortName() (mb_strrpos/mb_substr) is a byte-for-byte duplicate of EnforceAuditTransactionScopeRule::shortName() (src/Rules/EnforceAuditTransactionScopeRule.php:505). Standing Concern ci: pin symfony/console to ^7 — Infection mutation gate broken by Symfony Console 8 #29 only shields the walkNodes duplication, not this helper. If the rule moves to CallLike (see MAJOR above) and uses $scope->getType()->getReferencedClasses() like LogRule, the short-name handling can lean on PHPStan's referenced-class output; otherwise lift the shared helper to a trait/Support class. Low urgency.
• MINOR · src/Rules/ForbidEloquentMutationInControllersRule.php + src/Rules/EnforceCurrentUserAttributeRule.php — Row 6 / contract scope: this PR's diff against base carries TWO independent cross-territory contract rules (queue-87 ForbidEloquentMutationInControllersRule + EnforceCurrentUserAttributeRule) because base branch queue-87-forbid-eloquent-mutation-in-controllers has not merged. Each rule independently makes already-green consumer CI (kendo/codebook/emmie/ublgenie/entreezuil/ISMS/brick-inventory) newly fail. Both are correctly disclosed in CHANGELOG with Major-bump + per-territory pre-cascade-audit plans, so this is a reviewability/cascade-bisectability note, not a surface defect — ideally each contract rule lands and cascades independently so a downstream CI break maps to one rule.

Action

revise — address MAJORs

The base branch was changed.

[Goosterhof]

Final PR in the stack. Retargeted to main (#28 merged, queue-87 deleted) and rebased onto fresh main — dropped the now-merged ForbidEloquent commit, leaving only the three EnforceCurrentUser commits (feat + tests + docs), byte-identical to your prior approval. CHANGELOG [Unreleased] cleanly carries all three rules + #29's pin. Re-review requested (force-push dismissed the stale approval on the protected base).

[Goosterhof]

Blocker addressed — ancestry gate replaced with namespace gate (pushed 316b8a9).

The agent-review sweep (review pullrequestreview-4401182606) verified that EnforceCurrentUserAttributeRule's isSubclassOf(Illuminate\Routing\Controller) ancestry gate matched 0 controllers across kendo / ublgenie / entreezuil — every consumer ships base-less final controllers with no extends Controller, so the rule was a silent no-op.

Fix: swapped the ancestry gate for the App\Http\Controllers namespace prefix gate the sibling ForbidEloquentMutationInControllersRule already uses ($scope->getNamespace() + str_starts_with). Matches the canonical "controllers are identified by the App\Http\Controllers namespace" convention.

Regression proof — before/after error count. Added a base-less final controller fixture (RequestUserInBaselessController, no extends) resolving the user via $request->user(), plus a compliant #[CurrentUser] base-less controller (CurrentUserAttributeInBaselessController). Running the new flagging test against each gate:

Gate	Base-less final controller fixture	Result
Ancestry (isSubclassOf) — the bug	$request->user()	0 errors (no-op)
Namespace (App\Http\Controllers) — the fix	$request->user()	1 error (line 18) ✅

This reproduces the exact real-world shape the reviewer showed the rule misses today. The compliant #[CurrentUser] fixture stays clean under the namespace gate.

Gates (local): composer phpstan clean · composer test 92/92 · composer coverage:check 87.67% ≥ 83% · composer mutation:ci MSI +9.92pp over the 75 threshold (the new namespace-prefix-string mutant is killed) · composer format:check clean.

Scope held to the detection gate only — the #[CurrentUser] violation-detection internals are untouched (no fixture proved a second defect). CHANGELOG.md [Unreleased] carries the false-negative-closure note; no new identifier, no consumer ignoreErrors shape change.

Note for reviewer: PR #26's branch was rebased onto current main (post-#28 merge) between the sweep and this fix — the head the orders cited (6939f98) is now e122c00 + this commit (316b8a9). The fix re-derives cleanly on the rebased base; behaviour and proof are identical.

[Goosterhof]

✅ Approve-worthy

0 blockers · 1 concern · 3 nits · 1 praise

This revision adds EnforceCurrentUserAttributeRule and, critically, re-anchors the controller-detection gate from the ancestry check (isSubclassOf(Illuminate\Routing\Controller)) to a namespace-prefix check ($scope->getNamespace() starts with App\Http\Controllers). That closes the silent-no-op blocker from the prior sweeps — the rule now fires on the base-less final controllers the fleet actually ships, and it stops contradicting its sibling ForbidEloquentMutationInControllersRule, which already scoped by namespace. The detection engine and the fixture matrix remain sound. What is left is one carried-over concern and some stale-comment debris the gate change introduced.

Resolved since prior review

• Blocker (ancestry gate silent on the fleet) — fixed. insideControllerMethod() (src/Rules/EnforceCurrentUserAttributeRule.php:173-182) now gates on str_starts_with($namespace, 'App\Http\Controllers'). Regression-proofed by RequestUserInBaselessController (flagged, tests/Rules/EnforceCurrentUserAttributeRuleTest.php:709-725) and CurrentUserAttributeInBaselessController (clean, :727-739) — the exact base-less shape the old gate missed.
• Blocker (contradicts sibling's controller definition) — fixed. Both rules now key on the App\Http\Controllers namespace prefix.
• Concern (getName() === CONTROLLER_BASE_FQCN dead branch) — gone with the ancestry gate.

Concerns

• src/Rules/EnforceCurrentUserAttributeRule.php:245-256 — receiverIsAuthHelper() is namespace-naive: it matches any FuncCall named auth with no name resolution.
  • A user-defined App\Support\auth() called inside a controller would false-positive — the static-call branch leans on $scope->resolveName() for exactly this discrimination (:219) while the helper branch skips it.
  • Low probability given the helper's ubiquity, and the docblock concedes it is a deliberate AST-only match (:239-243). Carried unchanged from the prior sweeps.
  • Fix: a one-line note in the README error-surface row that this is AST-only and accepts the collision, or a \function_exists-anchored resolution if you want strictness.

Nits

• tests/Fixtures/CurrentUserAttribute/TopLevelAuthCall.php:530-533 — comment is stale after the gate change: it says $scope->getClassReflection() returns null and the rule "short-circuits at the no-class-reflection gate," but the gate now checks $scope->getNamespace() === null. The test (EnforceCurrentUserAttributeRuleTest.php:796-809) still passes, but its comment and the fixture comment describe the deleted reflection gate.
• tests/Fixtures/CurrentUserAttribute/_stubs.php:570-571 — the "rule scopes detection on: PHPStan reflection (ancestor traversal to Illuminate\Routing\Controller)" line describes the removed ancestry gate; the rule no longer does ancestor traversal. The Illuminate\Routing\Controller stub is now only needed so the legacy extends Controller fixtures parse.
• CHANGELOG.md — the EnforceCurrentUserAttributeRule Added entry is a single ~2700-character paragraph carrying versioning strategy, the pre-cascade audit plan, per-territory rollout counts, and multi-guard ergonomics. That is release-PR and campaign-report content. Trim to the rule's behavior and identifier; the rollout detail lives in the deployment order and the PR body.

Praise

• The namespace re-anchoring is the right call and is documented in the ### Fixed CHANGELOG block with review provenance — the fix names the exact false-negative it closes and ships the base-less-controller fixture that reproduces it, rather than flipping the gate and trusting the green suite.

Automated war-room agent review — posted because this PR carries the Agent Review Requested label.

[Goosterhof]

⚠️ Concerns

0 blockers · 2 concerns · 3 nits · 1 praise

Adds EnforceCurrentUserAttributeRule flagging Request::user() / Auth::user() / auth()->user() inside App\Http\Controllers\* and steers to the #[CurrentUser] container attribute. The rule logic, the three-shape CallLike detection, and the test matrix are sound. The headline judgment: the ### Fixed entry shows this PR already absorbed a prior agent-review catch — the ancestry-gate silent no-op was corrected to a namespace-prefix gate with a real regression fixture — but the conversion left doc/comment artifacts behind that still describe the dead approach.

Concerns

• src/Rules/EnforceCurrentUserAttributeRule.php:102-126 + tests/Fixtures/CurrentUserAttribute/_stubs.php:6-9 — the docblocks still describe the ancestry-walk gate the PR deliberately removed.

  • src/Rules/EnforceCurrentUserAttributeRule.php:124 — "Implementation note: getNodeType() returns CallLike::class" is correct, but the surrounding prose at :102-121 mixes the live namespace-prefix description with stale framing; the body comment is internally consistent now, but _stubs.php:6 still lists "PHPStan reflection (ancestor traversal to Illuminate\Routing\Controller)" as the scoping mechanism the rule uses.
  • That mechanism no longer exists — insideControllerMethod() at :173-182 is pure $scope->getNamespace() + str_starts_with. The _stubs.php comment will mislead the next maintainer into thinking the Illuminate\Routing\Controller stub at :18 is load-bearing for the gate when it is only there to let the extends Controller fixtures parse.
  • Fix: rewrite the _stubs.php:6-9 scoping list to name the namespace-prefix gate, and confirm the Controller stub's comment says it exists for fixture parsing, not for the gate.

• tests/Fixtures/CurrentUserAttribute/TopLevelAuthCall.php:8 + tests/Rules/EnforceCurrentUserAttributeRuleTest.php:798-800 — the test comment claims this fixture kills "the FalseValue mutant on the null-reflection guard" via $scope->getClassReflection(), but the implementation guards on $scope->getNamespace() === null (:177), not on class reflection.

  • The test still passes and still kills the mutant on the real getNamespace() === null branch, so coverage is intact.
  • But the comment cites a guard the code does not contain. Align the comment to the actual getNamespace() null-check so the mutation-kill rationale stays verifiable.

Nits

• src/Rules/EnforceCurrentUserAttributeRule.php:135 — CONTROLLER_NAMESPACE_PREFIX = 'App\Http\Controllers' with str_starts_with has no separator boundary; a hypothetical App\Http\ControllersHelpers namespace would match. Matches the sibling ForbidEloquentMutationInControllersRule convention, so accept as-is, but the unbounded prefix is worth a one-line note.
• tests/Fixtures/CurrentUserAttribute/AuthInAction.php:13 / AuthInJob.php:15 / AuthInMiddleware.php:13 — these negative fixtures all return from a single method; none asserts the rule stays silent on a controller-namespaced sibling in the same run. Coverage is adequate via the dedicated controller fixtures, so this is polish only.
• CHANGELOG ### Added entry is a single ~40-line paragraph. The detail is welcome, but the versioning + pre-cascade-audit material would read better as sub-bullets. Cosmetic.

Praise

• The RequestUserInBaselessController regression fixture (tests/Fixtures/CurrentUserAttribute/RequestUserInBaselessController.php) reproduces the exact base-less final controller shape that made the old ancestry gate a silent no-op — pinning the real-world failure the prior review surfaced, not just the happy path.

Automated war-room agent review — posted because this PR carries the Agent Review Requested label.