fix(plugin-manager): treat a dangling plugin symlink as not installed#2696
Open
elibosley wants to merge 2 commits into
Open
fix(plugin-manager): treat a dangling plugin symlink as not installed#2696elibosley wants to merge 2 commits into
elibosley wants to merge 2 commits into
Conversation
The plugin manager tracks installed plugins with a symlink under
/var/log/plugins/<name>.plg pointing at the real /boot/config/plugins/<name>.plg.
It used `@readlink()` alone to decide whether a plugin is installed, so a
*dangling* symlink — one whose target .plg was removed or renamed on the flash
out-of-band — was still read as "installed". The manager then tried to parse the
missing target and aborted with:
plugin: XML file doesn't exist or xml parse error
For `install` this made the plugin permanently un-installable until the stale
symlink was removed by hand; `check` and `update` failed the same way.
Add an `installed_plugin()` helper that resolves the tracking symlink to its
target, treats a dangling link as not installed, and removes the stale link so a
subsequent install/update/check starts clean. Use it in the install, check, and
update handlers in place of the bare `@readlink()`. (`remove` already unlinks the
stale symlink before acting, so it self-heals.)
Verified on a live box: with a dangling tracking symlink, `check` now reports
"plugin: not installed" and clears the stale link, and a fresh install proceeds
normally instead of aborting on the parse error.
Contributor
WalkthroughThe plugin script adds validated symlink resolution and stale-link removal. Install, check, update, checkall, updateall, and checkos now treat missing plugin targets as not installed. ChangesPlugin symlink cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
🔧 PR Test Plugin AvailableA test plugin has been generated for this PR that includes the modified files. Version: 📥 Installation Instructions:Install via Unraid Web UI:
Alternative: Direct Download
|
![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 `@emhttp/plugins/dynamix.plugin.manager/scripts/plugin`:
- Around line 521-535: Update the checkall, updateall, and checkos bulk loops to
call installed_plugin() when resolving tracking symlinks instead of bypassing it
with direct checks. Preserve each loop’s existing handling for valid installed
plugins while ensuring dangling symlinks are removed and treated as not
installed.
🪄 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: 615588bd-ed09-4084-9a2f-057611ff1eae
📒 Files selected for processing (1)
emhttp/plugins/dynamix.plugin.manager/scripts/plugin
unraid-bot
added
the
7.4.0
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
2 participants
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
Plugin Manager now treats dangling plugin tracking symlinks as not installed across both direct and bulk operations, removing the stale registration so the plugin can be checked or installed normally again.
Why This Exists
The manager tracks installed plugins with
/var/log/plugins/<name>.plgsymlinks pointing to/boot/config/plugins/<name>.plg.readlink()returns a path even when the target was removed or renamed out-of-band, so stale registrations were treated as installed and later failed with a misleading XML parse error. The Plugins page uses the bulk handlers, which also left these stale links behind.Resolution
Add one
installed_plugin()helper that resolves a tracking link, verifies its target is a regular file, and deletes a dangling link before reporting the plugin as not installed. Use the same helper ininstall,check,update,checkall,updateall, andcheckosso direct CLI calls and WebGUI-triggered bulk actions share one installed-state contract.Reviewer Considerations
removeremains unchanged because it already unlinks the tracking entry before acting and self-heals this state.Behavior Changes
install,check, andupdatetreat a dangling tracking link as not installed and remove it.checkall,updateall, and built-incheckosnow perform the same cleanup, covering the Plugins page Check For Updates path.Implementation Summary
installed_plugin()as the canonical tracking-link resolver.readlink()checks in the three direct handlers and three bulk handlers.Verification
checkall,updateall, andcheckos; those exact call sites now use the already-validated helper.php -l emhttp/plugins/dynamix.plugin.manager/scripts/plugin— passed.git diff --check— passed.build-pluginfor commit8a9d594— passed.Risk
Low; the follow-up changes only the resolver used by three bulk loops and reuses the helper already validated on the direct paths. Live bulk-path re-verification is still required before QA sign-off.