Fix Permission custom pivots and relation scope safety - #2
Closed
binaryfire wants to merge 13 commits into
Closed
Conversation
Retain every relation-owned pivot predicate, including between clauses, and replay them as one grouped condition so boolean-or scopes cannot escape the relation identity during explicit pivot writes. Carry those constraints onto hydrated Pivot and MorphPivot instances, preserve primary-key authority, and route current-pivot hydration through the relation's canonical constructor. Explicit custom-pivot attributes now use coroutine-safe forceFill semantics so casts, mutators, timestamps, and model events run without applying request mass-assignment policy. Add counterfactual coverage for grouped scope replay, range predicates, cross-parent isolation, primary-key behavior, guarded custom attributes, morph identity, and stock/custom pivot events.
Resolve saved writes through the public roles and permissions relations, preserve their captured partition/team context, and retain stock set-based mutations while allowing configured custom pivots to run native casts and lifecycle hooks. Deferred assignments now retain their selected pivot class without storing request models or adding coroutine state. Complete warm direct and role-derived pivot metadata, constraints, and orientation; return fresh public via-role pivots so callers cannot mutate cached authorization edges. Replacement events publish the complete pre-operation payload after successful writes, while no-op permission syncs preserve warm caches and role sync mutates only changed edges so retained custom pivot rows survive. Add coverage for custom and partitioned pivots, event ordering and failure paths, warm-pivot save/delete isolation, catalog alias protection, no-op cache retention, exact role-diff writes, deferred assignments, and stock bulk-update retention.
Treat only null and empty team options as absent so the string zero remains a valid team identifier, restore the caller's prior team context after command execution, and report migration publication failures through the command exit status. Declare Permission's direct split-package dependencies, remove runtime class guards for those installed dependencies, and register Permission, Horizon, and Wayfinder providers in the root manifest. Add a repository-wide split-provider discoverability invariant and exact Permission metadata parity coverage. Expand command regressions for disabled teams, zero identifiers, global-role warnings, existing migrations, copy failures, context restoration, and About output.
Cover guard resolution without a provider, LDAP-backed model discovery, missing Passport clients, model-valued team identifiers, missing role IDs, exact pipe-delimited role checks, current-team reverse assignments, invalid wildcard implementations, and blank wildcard subparts. These are supported current Spatie behaviors that Hypervel already implements; the tests make that compatibility executable without adding production seams or compatibility machinery.
Add the direct Symfony HttpKernel, Carbon, and Composer runtime requirements already used by Broadcasting, Concurrency, Contracts, DI, Notifications, Passkeys, Process, and Telescope. Pin each split manifest to the root constraint with focused metadata tests so subtree packages remain independently installable and dependency drift fails in CI.
Define permissions and roles in Laravel-style terms, correct the revocation example, and document saved replacement event payloads, listener gating, transaction ordering, and no-op cache behavior. Show the supported trait-alias custom-pivot extension pattern, explain which model-returning APIs load the real relation, preserve the compact authorization cache contract, and state the reverse arbitrary-model boundary and exact performance costs.
Route permission-06 through permission-18, database-29 through database-33, and the related discovery and metadata findings through the core dependency index. Add the final ledger assessment covering lifecycle ownership, custom-pivot semantics, replacement events, no-op cache preservation, differential role sync, performance boundaries, cross-package revalidation, rejected machinery, verification, and independent review status while leaving the later fresh Permission audit optional.
Record the agreed Database pivot ownership, Permission custom-pivot and replacement-event contracts, warm-pivot construction, command and metadata corrections, supported upstream coverage, documentation scope, rejected overengineering, and complete verification strategy. The plan includes the core audit's exact anti-overengineering guidance and the final permission-06 through permission-18 design so implementation intent and performance boundaries remain durable across future maintenance.
…ctness-parity-lifecycle # Conflicts: # docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md # tests/Passkeys/PackageMetadataTest.php
Require new root package dependencies to use the latest compatible stable release published on Packagist.\n\nDocument that the root Composer lock is intentionally untracked, that local dependencies should be refreshed after dependency changes, and that stale local lock state is not a repository defect.
Update the role replacement query-count regression for the diff-based synchronization path. Initial assignment now asserts one scoped pivot read followed by one bulk insert, without expecting an unnecessary delete.\n\nClarify that detached-event payloads reuse the mandatory role read and require both role and permission pivot-read regressions to pin the SELECT statement type.
…ctness-parity-lifecycle # Conflicts: # docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md # docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md
…ctness-parity-lifecycle
|
Warning Review limit reached
Next review available in: 25 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (58)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR hardens Permission assignment and Database pivot handling while restoring custom-pivot extension behavior, replacement-event compatibility, package metadata, and command correctness.
Confidence Score: 5/5The PR appears safe to merge; no concrete changed-code defect remains after tracing the affected pivot, permission, event, cache, command, and metadata paths. The changed relation constraints preserve parent and morph identity, custom pivots retain their lifecycle behavior, replacement operations preserve retained edges and invalidate caches only after mutations, and the documented event and deferred-assignment contracts are consistently implemented and tested.
|
| Filename | Overview |
|---|---|
| src/database/src/Eloquent/Relations/Concerns/AsPivot.php | Replays captured relation predicates on composite-key pivot select, save, and delete queries while preserving native primary-key identity. |
| src/database/src/Eloquent/Relations/Concerns/InteractsWithPivotTable.php | Groups destructive pivot constraints, centralizes existing-pivot hydration, and routes explicit custom-pivot attributes through casts and model lifecycle handling. |
| src/database/src/Eloquent/Relations/BelongsToMany.php | Records range predicates so destructive pivot operations retain the same relation scope as reads. |
| src/database/src/Eloquent/Relations/MorphToMany.php | Centralizes morph pivot construction and retains both morph identity and relation-owned constraints. |
| src/permission/src/Traits/HasPermissions.php | Honors public custom-pivot relations across immediate and deferred writes, restores replacement events, and constructs complete scope-safe cached pivots. |
| src/permission/src/Traits/HasRoles.php | Replaces role assignments by exact edge differences while preserving retained custom pivot rows and documented event behavior. |
| src/permission/src/PermissionRegistrar.php | Adds bounded pivot-class memoization and completes cached role-permission pivot orientation without introducing request-state retention. |
| src/permission/src/Commands/UpgradeForTeamsCommand.php | Propagates migration publication failure through the command exit status. |
| src/permission/src/Commands/AssignRoleCommand.php | Preserves exact team identifiers, including string zero, and restores prior team context after execution. |
| src/permission/src/Commands/CreateRoleCommand.php | Preserves exact team identifiers when creating or resolving team-scoped roles. |
| composer.json | Restores root package discovery for Horizon, Permission, and Wayfinder providers. |
Reviews (1): Last reviewed commit: "Merge remote-tracking branch 'origin/0.4..." | Re-trigger Greptile
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This change completes the targeted Permission correctness and extension maintenance while preserving Hypervel's partitioned, denied-permission, and coroutine-safe design.
The main changes are:
using(CustomPivot::class)extension path for permission and role assignments, including deferred assignments, casts, events, and guarded pivot attributes;Design
Permission's warm authorization and permission-name paths remain on the compact assignment cache. They do not construct relations or add database queries.
Saved writes resolve the model's public
roles()orpermissions()relation once and use its captured team and partition context through comparison, mutation, and invalidation. Stock pivots retain set-based writes. Applications that explicitly configure a custom pivot use native per-row Eloquent operations where casts and lifecycle hooks require them.The shared Database relation layer now owns destructive predicate retention. Relation constraints are grouped beneath the parent identity, including range constraints, so an
orpredicate cannot escape to another parent or partition. Explicit custom-pivot attributes bypass mass-assignment filtering while continuing through casts, mutators, timestamps, and events.Role replacement reads the current scoped IDs once, computes the exact difference, and writes only changed edges. The same read supplies detached-event IDs when that event is enabled, avoiding a second payload query. No-op replacements preserve warm caches and perform no writes.
Compatibility and performance
Supported Spatie-facing method names, named arguments, relation overrides, event payloads, command options, pivot events, and custom casts remain compatible or are restored.
The normal authorization path gains no query, relation construction, lock, yield, serialization, or retained worker state. Stock pivots keep bulk operations. Custom pivots pay per-row model work only when an application opts into a custom pivot class. Replacement-event hydration remains listener-gated.
Validation
composer fixFor more details, see:
docs/plans/2026-08-08-0059-permission-correctness-extension-parity-and-relation-scope-safety.md