Fix Sanctum cache settlement and restore current parity - #484
Conversation
Pass the registering connection name through after-commit and rollback callback registration so work cannot attach to a newer transaction on another connection. Preserve the existing ambient behavior for direct unnamed manager callbacks and document that boundary precisely. Add unit and integration regressions covering independent connections, nested transactions, rollback, immediate execution, and missing-manager failures.
Replace claims that queued work waits for every open database transaction with the actual contract: it follows the latest applicable transaction and its enclosing stack on that connection. Carry the corrected wording through Bus, Foundation, Queue, Events, Mail, Notifications, Broadcasting, Scout, and Eloquent event surfaces. Document how applications should schedule work that depends on commits across multiple connections without implying cross-connection atomic coordination.
Schedule cached Eloquent user invalidation on the mutated model's own database connection instead of clearing entries before persistence or transaction settlement. Snapshot request-sensitive cache-key inputs at model-event time while discovering provider descriptors at commit time. Cover saves, deletes, nested transactions, rollback, cross-connection ownership, immediate execution, fail-closed missing-manager behavior, and descriptors registered while a transaction remains open.
Invalidate token and tokenable cache entries only after successful persistence and transaction settlement, including creation, updates, soft deletes, restores, force deletes, and last-used writes. Add a dedicated token relation whose cache-enabled bulk delete selects and removes one exact scoped ID set without mutating reusable builders. Honor custom token primary keys, connections, string IDs, and late-static cache namespaces throughout. Make token lookup reject malformed or overflowing identifiers before cache or SQL, and compare abilities strictly without allocating a flipped map. Add counterfactual coverage for transaction outcomes, custom models, relation deletion, cache-disabled query count, lookup boundaries, and stored ability coercion.
Delegate Authorization parsing to the framework request parser and make the configured token model's findToken method the single authority for token format and lookup behavior. Remove the guard's duplicate model allocation, identifier checks, and query/body credential fallback. Preserve configured retrieval callbacks and provider matching, with regressions for standard bearer parsing, header-only defaults, custom lookup models, invalid credentials, and last-used behavior.
Make actingAs preserve the concrete authenticatable, normalize enum abilities strictly, and select the requested guard as the coroutine-local default as well as setting its user. Replace misleading token-model generics with the actual configurable model contract and restore the current-host placeholder from a canonical default during test cleanup. Add focused coverage for custom guard selection, enum abilities, concrete returns, and static placeholder reset.
Move CSRF route registration into the provider so route caching, disablement, and custom prefixes are honored, and restore the protected guard/provider construction seams from current Sanctum. Retain Hypervel's direct coroutine-safe guard and Middleware::statefulApi priority owner. Validate each Sanctum guard's explicit session_guards contract, keep middleware filtering strict, and derive cache-safe provider models from configured auth guards. Cover real CSRF behavior, route names and middleware, cached and malformed configuration, factory overrides, provider validation, and exact middleware filtering.
Resolve the configured personal-access-token model and reject negative, decimal, or nonnumeric --hours values before constructing or executing a destructive query. Preserve zero and positive values, return standard command status codes, and add regressions proving invalid input performs no deletion while configured models and expiration rules remain honored.
Declare the split package's direct runtime dependencies and mirror Sanctum provider discovery in the monorepo metadata. Add executable metadata checks and maximum-level static-analysis coverage for actingAs enum abilities and concrete returns, configurable token models, current-token generics, and trait-only token relationship construction. Keep the public contracts aligned with the runtime without adding wrappers or broad suppressions.
Describe per-guard session trust, provider matching, token-only guards, explicit middleware-priority ownership, and the supported protected relation extension point. Correct cache invalidation and transaction timing guidance, document deliberate eventless and raw-query escape hatches, explain bounded negative-cache and pruning behavior, and state exact route configuration types. Keep the package README limited to public Laravel differences while the Boost guide owns operational detail.
Mark Sanctum complete in the audit routing index and checklist, close carried revalidation items, and route the new Sanctum, Database, and Auth findings to every affected package. Add the final completion ledger entry with ownership, transaction semantics, compatibility, performance, rejected complexity, regression coverage, and validation results. Remove stale references to a future Sanctum audit and record Bus and Foundation as consumers of the corrected after-commit documentation.
Record the evidence, final architecture, accepted findings, implementation boundaries, counterfactual test coverage, performance budget, compatibility decisions, and rejected alternatives for the completed Sanctum work. Keep the core anti-overengineering rules intact and document the final Database and Auth ownership decisions, including connection-local callback settlement and the absence of speculative cache, locking, retry, or cross-connection coordination machinery.
# 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
|
Warning Review limit reached
Next review available in: 9 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 (10)
📝 WalkthroughWalkthroughSanctum now supports transaction-aware token and user-cache invalidation, configurable CSRF routes and guards, stricter token handling, custom token relations, command validation, package metadata, documentation, and expanded tests. ChangesSanctum correctness and parity
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/sanctum/src/Sanctum.php (1)
91-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport
HasAbilitiesfor the PHPDoc type.Add a
use Hypervel\Sanctum\Contracts\HasAbilities;import and useHasAbilities&MockInterfacein this annotation. This keeps the file consistent with the required import style.As per coding guidelines, import classes with
usestatements instead of using fully qualified class names.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sanctum/src/Sanctum.php` at line 91, Update the PHPDoc annotation for $token in Sanctum to reference HasAbilities&MockInterface without its fully qualified namespace, and add the corresponding use Hypervel\Sanctum\Contracts\HasAbilities; import alongside the existing imports.Source: Coding guidelines
tests/Sanctum/PersonalAccessTokenCacheTest.php (1)
1370-1414: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNamespace the new inline fixture models.
These six fixture classes are declared in the global namespace with generic names, for example
SoftDeletingPersonalAccessTokenandEventPersonalAccessToken. Another Sanctum test file that declares the same name causes a fatal redeclaration error. Move them into a capitalizedFixtures/directory, or give them a test-specific namespace whose final segment isPersonalAccessTokenCacheTest.As per coding guidelines: "Put standalone test support files under a capitalized
Fixtures/directory" and "Use test-specific namespaces for collision-prone generic helper classes, with the test class name as the final namespace segment."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Sanctum/PersonalAccessTokenCacheTest.php` around lines 1370 - 1414, Namespace all six inline fixture models—NamespacedPersonalAccessToken, SoftDeletingPersonalAccessToken, TimestampDisabledPersonalAccessToken, CustomTimestampPersonalAccessToken, SecondaryConnectionPersonalAccessToken, and EventPersonalAccessToken—under a test-specific namespace whose final segment is PersonalAccessTokenCacheTest, and update any references in the test accordingly to prevent global-name collisions.Source: Coding guidelines
src/sanctum/src/PersonalAccessTokenRelation.php (1)
47-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOne settlement rule is implemented twice. Both files decide when a cache mutation runs by checking
getTransactionManager() === null && transactionLevel() === 0and otherwise callingafterCommit(). If the rule changes, one copy can be missed.
src/sanctum/src/PersonalAccessTokenRelation.php#L47-L65: call the shared helper instead of repeating the branch, and keep only the ID-loop callback here.src/sanctum/src/PersonalAccessToken.php#L310-L321: promotesettleCacheMutation()to a shared static helper, or a small trait, that accepts the connection and the callback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sanctum/src/PersonalAccessTokenRelation.php` around lines 47 - 65, The cache-settlement rule is duplicated across both sites. In src/sanctum/src/PersonalAccessToken.php lines 310-321, promote settleCacheMutation() to a shared static helper accepting the connection and callback; in src/sanctum/src/PersonalAccessTokenRelation.php lines 47-65, remove the transaction branch and call that helper while retaining only the ID-loop callback.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.md`:
- Line 1891: Update the table cell describing findToken() to escape the literal
pipe in “id|token” or replace it with equivalent wording, preserving the full
malformed- and overflow-identifier rejection requirement within the same table
column.
---
Nitpick comments:
In `@src/sanctum/src/PersonalAccessTokenRelation.php`:
- Around line 47-65: The cache-settlement rule is duplicated across both sites.
In src/sanctum/src/PersonalAccessToken.php lines 310-321, promote
settleCacheMutation() to a shared static helper accepting the connection and
callback; in src/sanctum/src/PersonalAccessTokenRelation.php lines 47-65, remove
the transaction branch and call that helper while retaining only the ID-loop
callback.
In `@src/sanctum/src/Sanctum.php`:
- Line 91: Update the PHPDoc annotation for $token in Sanctum to reference
HasAbilities&MockInterface without its fully qualified namespace, and add the
corresponding use Hypervel\Sanctum\Contracts\HasAbilities; import alongside the
existing imports.
In `@tests/Sanctum/PersonalAccessTokenCacheTest.php`:
- Around line 1370-1414: Namespace all six inline fixture
models—NamespacedPersonalAccessToken, SoftDeletingPersonalAccessToken,
TimestampDisabledPersonalAccessToken, CustomTimestampPersonalAccessToken,
SecondaryConnectionPersonalAccessToken, and EventPersonalAccessToken—under a
test-specific namespace whose final segment is PersonalAccessTokenCacheTest, and
update any references in the test accordingly to prevent global-name collisions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 636aa963-8ab8-4823-bba3-dec69e08a9c4
📒 Files selected for processing (48)
composer.jsondocs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.mddocs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.mddocs/plans/2026-08-07-1302-sanctum-correctness-cache-settlement-and-current-parity.mdsrc/auth/src/EloquentUserProvider.phpsrc/boost/docs/broadcasting.mdsrc/boost/docs/events.mdsrc/boost/docs/mail.mdsrc/boost/docs/notifications.mdsrc/boost/docs/queues.mdsrc/boost/docs/sanctum.mdsrc/bus/src/Queueable.phpsrc/database/src/Concerns/ManagesTransactions.phpsrc/database/src/DatabaseTransactionsManager.phpsrc/database/src/Eloquent/BroadcastableModelEventOccurred.phpsrc/database/src/Eloquent/BroadcastsEvents.phpsrc/database/src/Eloquent/BroadcastsEventsAfterCommit.phpsrc/events/src/Dispatcher.phpsrc/foundation/src/Bus/PendingDispatch.phpsrc/queue/src/Queue.phpsrc/sanctum/README.mdsrc/sanctum/composer.jsonsrc/sanctum/routes/web.phpsrc/sanctum/src/Console/Commands/PruneExpired.phpsrc/sanctum/src/HasApiTokens.phpsrc/sanctum/src/Http/Middleware/EnsureFrontendRequestsAreStateful.phpsrc/sanctum/src/PersonalAccessToken.phpsrc/sanctum/src/PersonalAccessTokenRelation.phpsrc/sanctum/src/Sanctum.phpsrc/sanctum/src/SanctumGuard.phpsrc/sanctum/src/SanctumServiceProvider.phpsrc/scout/config/scout.phpsrc/scout/src/ModelObserver.phptests/Database/DatabaseTransactionsTest.phptests/Integration/Auth/EloquentUserProviderCacheTest.phptests/Integration/Database/DatabaseTransactionsTest.phptests/Sanctum/ActingAsTest.phptests/Sanctum/CurrentApplicationUrlWithPortTest.phptests/Sanctum/EnsureFrontendRequestsAreStatefulTest.phptests/Sanctum/GuardTest.phptests/Sanctum/HasApiTokensTest.phptests/Sanctum/PackageMetadataTest.phptests/Sanctum/PersonalAccessTokenCacheTest.phptests/Sanctum/PersonalAccessTokenTest.phptests/Sanctum/PruneExpiredTest.phptests/Sanctum/SanctumRoutesTest.phptests/Sanctum/SanctumServiceProviderTest.phptypes/Sanctum/Sanctum.php
💤 Files with no reviewable changes (1)
- src/sanctum/routes/web.php
Greptile SummaryThe PR completes Sanctum cache-settlement correctness and current API parity while binding deferred callbacks to their owning database connections.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| src/database/src/DatabaseTransactionsManager.php | Associates transaction callbacks with their owning connection while preserving ambient unnamed-callback behavior. |
| src/database/src/Concerns/ManagesTransactions.php | Routes transaction lifecycle and after-commit registration through connection-aware manager APIs. |
| src/database/src/Connection.php | Exposes connection transaction-manager state and documents manager removal as a tests-only operation that must not survive pooled reuse. |
| src/sanctum/src/PersonalAccessToken.php | Makes token cache reads and invalidation model-aware and settles successful mutations after the owning transaction commits. |
| src/sanctum/src/PersonalAccessTokenRelation.php | Deletes an exact scoped token set and invalidates that same set only after successful transaction settlement. |
| src/auth/src/EloquentUserProvider.php | Defers cached-user invalidation on the mutated model’s database connection. |
| src/sanctum/src/SanctumGuard.php | Restores bearer parsing and token validation parity while retaining coroutine-scoped authentication state. |
| src/sanctum/src/SanctumServiceProvider.php | Restores provider extension points, configurable CSRF routing, and package integration behavior. |
| composer.json | Mirrors split-package Sanctum discovery in aggregate package metadata without adding Sanctum to the default startup-provider boundary. |
| tests/Sanctum/PersonalAccessTokenCacheTest.php | Covers commit, rollback, relationship deletion, and intentional managerless fail-closed cache behavior. |
Reviews (2): Last reviewed commit: "sanctum: pin managerless relation settle..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Add a counterfactual regression for cache-enabled token relation deletion when its owning database connection has an open transaction but no transaction manager. The test proves the operation fails closed, restores the exact connection state, and leaves committed cache entries intact after rollback. Document transaction-manager removal as a tests-only connection seam because the null manager survives pooled release. Import the Sanctum ability contract for the existing intersection annotation without changing runtime behavior. Repair malformed Markdown table cells found during review and record database-28 across the audit dependency index, completion ledger, ownership maps, and Sanctum implementation plan. No production path, Laravel API, query count, cache operation, or coroutine state changes.
|
@coderabbitai @greptile-apps — review follow-up is pushed in f978924. Accepted changes:
No production change was made for managerless transactions. Immediate invalidation before commit can repopulate stale credentials from another connection, and a managerless connection has no safe after-commit hook. The existing exception is intentional fail-closed behavior; all three settlement sites now have counterfactual coverage. The root provider entry also remains. Root Composer metadata mirrors split-package discovery in this aggregate repository. Sanctum remains absent from I did not add a test-fixture namespace, a shared settlement helper, or a Markdown lint subsystem. Those changes would add structure without fixing a supported failure. The authoritative formatting, static-analysis, parallel-test, Testbench, and dogfood checks are green. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --stat --oneline --decorate --no-renames f97892426
printf '\n--- changed files ---\n'
git diff-tree --no-commit-id --name-only -r f97892426
printf '\n--- focused diff ---\n'
git show --format=fuller --find-renames --find-copies -- \
f97892426 -- \
src/sanctum/src/PersonalAccessTokenRelation.php \
tests/Sanctum/PersonalAccessTokenCacheTest.php \
src/database/src/Connection.php \
composer.json \
docs/plans/2026-08-07-1302-sanctum-correctness-cache-settlement-and-current-parity.md \
docs/plans/2026-07-12-0900-framework-coroutine-state-lifecycle-audit.md \
docs/plans/2026-07-12-0915-framework-coroutine-state-lifecycle-audit-ledger.mdLength of output: 38851 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Sanctum typing diff ---'
git show --format= -- f97892426 -- src/sanctum/src/Sanctum.php
printf '%s\n' '--- corrected plan-table diffs ---'
git show --format= -- f97892426 -- \
docs/plans/2026-08-02-1321-http-correctness-json-api-and-current-laravel-parity.md \
docs/plans/2026-08-03-1909-mail-correctness-current-parity-and-package-boundaries.md \
docs/plans/2026-08-06-0916-view-correctness-lifecycle-and-current-parity.md \
docs/plans/2026-08-06-0925-translation-correctness-current-parity-and-worker-lifecycles.md
printf '%s\n' '--- relevant relation settlement implementation ---'
ast-grep outline src/sanctum/src/PersonalAccessTokenRelation.php --items all
rg -n -C 8 'function (delete|settleInvalidation)|afterCommit|transactionLevel|getTransactionManager' \
src/sanctum/src/PersonalAccessTokenRelation.php \
src/sanctum/src/PersonalAccessToken.php \
src/auth/src/EloquentUserProvider.phpLength of output: 15841
The Markdown table fixes are correct. The The new relation-delete regression verifies the intended managerless transaction behavior. It asserts the fail-closed exception, rolls back the secondary connection, restores its transaction manager, and preserves both committed cache entries. The No further findings from this follow-up. ✏️ Learnings added
If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
1 participant
Summary
This change completes the Sanctum correctness audit and updates the package to the current supported Laravel surface while preserving Hypervel's coroutine-safe guard and cache design.
The main change is ownership. Database callbacks now attach to the connection that registered them. Sanctum token cache changes and Auth user cache invalidation now settle only after the owning database transaction commits. Rollbacks leave the previously committed cache state intact.
The package also restores current CSRF route configuration, protected provider extension points, bearer parsing, testing behavior, command validation, package metadata, and public types. Existing Hypervel choices remain intact: explicit per-guard session configuration, header-only credentials, the strict
id|tokenformat, and middleware priority owned by the application middleware configurator.For more details, see:
docs/plans/2026-08-07-1302-sanctum-correctness-cache-settlement-and-current-parity.mdTransaction ownership
afterCommitand rollback callbacks to the connection that registered them.Token and user cache correctness
Relationship revocation
MorphManyrelation with a protected construction seam onHasApiTokens.Sanctum behavior and parity
findToken().actingAs()select the requested guard and preserve the concrete authenticatable type.Documentation
Compatibility and performance
Supported Laravel method names, signatures, named arguments, guard behavior, token-model customization, and protected provider extension points are preserved or restored.
Bearer authentication removes a duplicate validation pass and model allocation. Ability checks avoid
array_flip(). Cache-disabled relation deletion remains one query. Cache-enabled relation deletion adds one scalar ID-selection query only on that write path so the deleted set can be invalidated correctly. Transaction registration adds a local connection-name lookup only when a callback is scheduled. No read path gains a query, cache round trip, lock, retry, yield, or retained worker state.Validation
Summary by CodeRabbit