fix(plugin): avoid stale Sushi snapshot causing ModelNotFoundException on install#2425
Open
kazaminosuke wants to merge 7 commits into
Open
fix(plugin): avoid stale Sushi snapshot causing ModelNotFoundException on install#2425kazaminosuke wants to merge 7 commits into
kazaminosuke wants to merge 7 commits into
Conversation
…n on install/update/uninstall Plugin is a Sushi model whose getRows() scans the plugins/ directory once per process during boot. Long-running queue workers keep serving that stale snapshot, so install/update/uninstall jobs that hold a serialized Plugin model fail at deserialization (restoreModel -> firstOrFail) with ModelNotFoundException for plugins added after the worker started. - Add Plugin::refreshRows() to clear the booted flag, the whenBooted boot callbacks Sushi registers, and the cached connection, forcing getRows() to re-run on next access. - Make the jobs carry the plugin id (string) instead of the Plugin model and re-fetch it in handle() after refreshRows(), so restoration no longer happens at deserialization time. - Pass $plugin->id when dispatching the jobs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughPlugin install, update, and uninstall jobs now accept plugin IDs instead of serialized ChangesPlugin ID-based job dispatch and Sushi refresh
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/Jobs/Plugin/UpdatePlugin.php`:
- Line 31: The update flow in PluginService::updatePlugin() is using a stale
Plugin instance after the plugin folder is replaced, so refresh/reload the
plugin metadata again after the download completes and before installPlugin()
runs. Update the update path so the Plugin object reflects the new plugin.json
contents before installation, using the existing refresh logic already used in
UpdatePlugin::handle().
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 1bcd111d-df32-4323-9fcc-9d7266193b35
📒 Files selected for processing (5)
app/Filament/Admin/Resources/Plugins/PluginResource.phpapp/Jobs/Plugin/InstallPlugin.phpapp/Jobs/Plugin/UninstallPlugin.phpapp/Jobs/Plugin/UpdatePlugin.phpapp/Models/Plugin.php
kazaminosuke
changed the title
…alling updatePlugin() downloaded the new version (overwriting plugin.json) but then ran installPlugin() with the pre-download $plugin instance and stale Sushi snapshot. As a result installPlugin() read the old version's composer_packages (and name/category), so newly added composer dependencies were not required until the snapshot was rebuilt elsewhere (worker restart / next request). Refresh the snapshot and re-fetch the plugin after the download so both the $plugin instance and the Plugin::get() inside manageComposerPackages() see the new plugin.json. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Fixes the root cause behind #2411 / #2415.
Root cause
App\Models\Pluginis a Sushi model backed by aplugins/directory scan(
getRows()). Sushi only runs this scan once per process (on first boot),then serves every subsequent query from an in-memory SQLite snapshot for the
rest of that process's lifetime.
AppServiceProvider::register()callsPluginService::loadPlugins()->Plugin::orderBy('load_order')->get()during application bootstrap — forevery process, including the long-running
queue:workworker.InstallPlugin/UpdatePlugin/UninstallPluginall took aPlugin $pluginconstructor argument and relied on
SerializesModelsto restore it by idwhen the worker dequeues the job. That restoration (
restoreModel()->firstOrFail()) happens beforehandle()is even called, against theworker's snapshot from whenever it last booted the
Pluginmodel — so anyplugin added to
plugins/after the worker started is invisible to it,causing
ModelNotFoundException.This only affects the queue worker (long-lived process); regular web
requests are unaffected since php-fpm boots a fresh process per request.
Fix
Plugin::refreshRows(), which clears the Eloquent boot guard(
$booted), the Sushi boot-callback registry ($bootedCallbacks), andthe cached Sushi connection for this model — forcing a fresh
getRows()scan the next time the model is touched in the current process.
InstallPlugin/UpdatePlugin/UninstallPluginto take theplugin id (string) instead of a
Pluginmodel instance, avoidingSerializesModels's restoration entirely.handle()now callsPlugin::refreshRows()thenPlugin::findOrFail($this->pluginId).dispatch()call sites inPluginResourceaccordingly.Console commands (e.g.
p:plugin:install) callPluginServicedirectly andsynchronously, so they were never affected by this and didn't need changes.
Testing
php -lon all changed files: no syntax errorsvendor/bin/pint: cleanvendor/bin/phpstan analyse(level 6): no errorsPlugin/these jobs, so none were brokenReproduced and verified on a fresh install with
QUEUE_CONNECTION=redis:jobs drained, downloading a new plugin and attempting install ->
InstallPluginfails in ~15ms withModelNotFoundException, whileInstallEggjobs in the same queue succeed normally.fix-plugin-install-stale-sushi): same conditions ->InstallPlugincompletes successfully in ~8s (DONE).