fix(plugin-manager): surface plugin update validation failures instead of reporting up-to-date#2666
fix(plugin-manager): surface plugin update validation failures instead of reporting up-to-date#2666elibosley wants to merge 3 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 53 minutes and 10 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughA plugin update validation failure is now persisted as a ChangesPlugin Update Validation Failure Marker
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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}
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/include/ShowPlugins.php`:
- Line 201: The validation-failure status label on line 201 uses a translated
string via _('Update validation failed'), but line 209 appears to match against
a hardcoded English phrase "Update validation failed" for ranking purposes. In
non-English locales this hardcoded string won't match the translated output,
causing incorrect ranking. Instead of matching against the translated label
text, use a locale-independent identifier or flag variable to track when the
validation-failure status is set, and use that identifier in the ranking logic
at line 209 rather than comparing against the hardcoded English string.
🪄 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: cced1315-9ea5-41c1-9e84-ac7cc7cc5b71
📒 Files selected for processing (3)
emhttp/plugins/dynamix.plugin.manager/include/ShowPlugins.phpemhttp/plugins/dynamix.plugin.manager/post-hooks/post_plugin_checksemhttp/plugins/dynamix.plugin.manager/pre-hooks/pre_plugin_checks
…d of reporting up-to-date When a plugin update is available, pre_plugin_checks validates it by re-downloading the update's files and verifying their SHA256/MD5. On any failure it reverted the cached .plg to the installed version and the check returned success, so the Plugins page showed "up-to-date" — hiding the available update entirely. This is most visible when the root filesystem is full: the (often large) txz can't be downloaded for the hash check, validation fails, and the update silently disappears with no error shown to the user. Record the available version and failure reason in a marker file and render an "Update validation failed" state (with the reason as a tooltip) in the Plugins page instead of the silent revert. The marker is cleared once validation passes or the plugin is updated/installed.
… remediation) Classify the validation failure in pre_plugin_checks instead of surfacing the raw txz error: distinguish low disk space (reporting actual free space), a download/network failure, and a genuine bad hash. Store a short summary plus a full actionable message in the marker. ShowPlugins.php shows the short cause as a visible second line under the "update validation failed" status and puts the full guidance in the tooltip, so the status answers "why did this fail and what do I do" instead of raising questions. Wording follows existing GUI conventions: lowercase status label like the other plugin statuses (up-to-date, cannot check, not available) and terse messages in the style of "Not enough free space, need at least %s MB". The hook runs without the gettext setup, so its strings are plain (the prior code also echoed untranslated reasons); only ShowPlugins.php uses _().
…slated text The rank logic matched the status cell against the hardcoded English "update validation failed", but the label is translated via _(). In non-English locales the match failed and the row sorted to the bottom (rank 4) instead of the top with updates (rank 0). Track the state with a locale-independent $validation_failed flag and rank on that instead.
elibosley
force-pushed
the
fix/plugin-update-validation-visible
branch
from
c5dc579 to
665b5ff
Compare
1 participant
Summary
When a plugin update is available, the
pre_plugin_checkshook validates it by re-downloading the update's files and verifying their SHA256/MD5. On any validation failure it silently reverted the cached.plgto the installed version, andplugin checkstill exited successfully — so the Plugins page showed "up-to-date" and the available update vanished with no error.The failure is most visible when the root filesystem is full: the (often large)
.txzcan't be downloaded for the hash check, validation fails, and the update silently disappears. This was diagnosed on a live server where a stale 1.2 GB/tmp/unRAIDServer.zipfilled the rootfs — every plugin update check reported "up-to-date" with no indication that validation was failing for lack of space.Root cause
In
checkflow:plugin checkdownloads the new.plg, then runspre_plugin_checks.scripts/plugin validatere-downloads the txz and checks its hash.false(download/IO/network error and bad-hash are indistinguishable), and the hook reverts the cached.plgto the installed one.plugin checkprints the reverted (old) version and exitsdone(0). The version comparison inShowPlugins.phpsees them equal → renders up-to-date.The failure reason was swallowed twice (PluginHelpers discards it; the hook reverts silently).
Change
pre-hooks/pre_plugin_checks— capture the validation reason (viascripts/plugin validate, which printsplugin: <reason>and exits non-zero) and, on failure, write a/tmp/plugins/<name>.invalidmarker recording the available version + reason. Clears the marker when validation passes or no newer version exists. The existing revert behavior is unchanged (we still don't offer an unvalidated update).post-hooks/post_plugin_checks— clear the marker after a successful install/update.include/ShowPlugins.php— when the marker is present and a newer version exists, render an "Update validation failed" status (with the reason as a tooltip) and the available version, ranked with updates — instead of the silent "up-to-date".Additive and contained: success / no-update paths are unchanged except for marker cleanup. OS (
unRAIDServer) updates are excluded, matching the existing hook guard.Testing
php -lclean on all three files.download failure: File I/O error,bad hash value,zero-length file) and the marker-driven status/version/rank rendering.Notes
A natural follow-up (not in this PR) is to also raise a notification and/or proactively warn on low rootfs free space, since "disk full" is the most common trigger.
Summary by CodeRabbit
New Features
Improvements