feat: automatic patch updates - #1
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds automatic update capability to the Unraid Updater plugin: a Versions utility for version parsing/comparison, a Settings class for persisted state and cron generation, a cron.php CLI script for checking/staging/installing patch updates, expanded AJAX endpoints, a reworked UI with scheduling controls, plugin cron wiring, and updated docs. ChangesAutomatic Update Feature
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant CronDaemon
participant CronPHP
participant ReleasesAPI
participant Settings
participant Filesystem
CronDaemon->>CronPHP: run check
CronPHP->>Settings: load()
CronPHP->>ReleasesAPI: fetchReleases()
CronPHP->>CronPHP: Versions.findNewestMatchingPatch()
CronPHP->>Filesystem: stageUpdate() download + verify checksum
CronPHP->>Settings: updateState(staged info)
CronDaemon->>CronPHP: run install
CronPHP->>Filesystem: installStagedFile()
CronPHP->>CronPHP: stopArrayAndReboot() if auto_reboot enabled
sequenceDiagram
participant User
participant UpdaterPage
participant AjaxPHP
participant Settings
User->>UpdaterPage: open Tools > Updater
UpdaterPage->>AjaxPHP: get_settings
AjaxPHP->>Settings: load()
Settings-->>AjaxPHP: settings + state
AjaxPHP-->>UpdaterPage: JSON settings/status
User->>UpdaterPage: click Apply Staged Update
UpdaterPage->>AjaxPHP: apply_staged
AjaxPHP->>Filesystem: run install script via nohup
UpdaterPage->>AjaxPHP: poll log
AjaxPHP-->>UpdaterPage: progress/completion
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/usr/local/emhttp/plugins/updater/include/ajax.php (1)
345-416: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
$zipFileparameter is unused.The body is a nowdoc (
<<<'BASH') that reads the ZIP path from the bash positional arg$1; the actual file is passed at exec time (bash {$scriptEsc} {$fileEsc}on Line 83). The$zipFileargument is therefore dead and misleading — drop it to match the caller's real data flow.♻️ Proposed cleanup
- file_put_contents(SCRIPT_FILE, buildManualInstallScript($file)); + file_put_contents(SCRIPT_FILE, buildManualInstallScript());-function buildManualInstallScript(string $zipFile): string +function buildManualInstallScript(): string🤖 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/usr/local/emhttp/plugins/updater/include/ajax.php` around lines 345 - 416, The buildManualInstallScript helper currently declares a $zipFile parameter that is never used because the generated bash script reads the ZIP path from $1 at runtime. Remove the unused parameter from buildManualInstallScript and update any related call sites or signatures so the function reflects the actual data flow used by the updater flow that builds the script and executes it with the file argument.Source: Linters/SAST tools
🤖 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 `@src/usr/local/emhttp/plugins/updater/include/cron.php`:
- Around line 281-282: isVerified currently returns success when both checksum
values are empty, which lets unverified update files pass through; update the
checksum validation flow in isVerified (and the cached-file reuse path that
relies on it) so that a missing md5 and sha256 causes verification to fail
closed, only allowing files that have at least one valid checksum and a
successful digest match before staging or installation.
- Around line 479-480: `releaseLock()` is removing the lock file after closing
it, which can reopen a race where another process acquires the same inode and
then gets unlinked underneath it. Update `releaseLock()` in `cron.php` to only
close the lock handle and stop calling `@unlink(LOCK_FILE)`. Keep the lock
lifecycle centered on `acquireLock()`/`releaseLock()` so the existing
`fopen(..., 'c')` creation logic remains the single source of file creation.
- Around line 453-456: stopArrayAndReboot() is only returning the launch status
from exec(), so the caller never sees failures from the actual reboot script and
the later error check becomes ineffective. Add a pre-flight validation in
stopArrayAndReboot() before building the nohup/bash command to detect obvious
failures such as a missing emcmd or an unlaunchable script, and return a nonzero
code before backgrounding; keep the existing background execution path and
update the pending_reboot/state handling so it is only set when the pre-check
passes.
In `@src/usr/local/emhttp/plugins/updater/include/Settings.php`:
- Around line 174-186: The cron field validation in Settings::sanitizeCron is
using regex patterns with incorrect alternation precedence, so malformed values
can still pass. Update the $patterns entries to anchor the entire
allowed-expression branch by grouping each alternative in the regex, then keep
the foreach validation against $fields so only fully valid cron field formats
are accepted before ajax.php writes the cron file.
In `@src/usr/local/emhttp/plugins/updater/Updater.page`:
- Around line 196-222: loadSettings currently returns immediately on a failed
get_settings response, which leaves branchLabel and the status area
uninitialized and hides the failure from the user. Update loadSettings in
Updater.page to handle the !data.success path by surfacing the backend error
through the existing settingsMsg UI (or a similar visible message) and avoid
leaving the page in a silent loading state; keep the rest of the settings
population logic unchanged.
---
Nitpick comments:
In `@src/usr/local/emhttp/plugins/updater/include/ajax.php`:
- Around line 345-416: The buildManualInstallScript helper currently declares a
$zipFile parameter that is never used because the generated bash script reads
the ZIP path from $1 at runtime. Remove the unused parameter from
buildManualInstallScript and update any related call sites or signatures so the
function reflects the actual data flow used by the updater flow that builds the
script and executes it with the file argument.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e527e700-8dfe-4f4d-9257-0900b80f41dd
📒 Files selected for processing (8)
README.mdplugin/plugin.j2src/install/slack-descsrc/usr/local/emhttp/plugins/updater/Updater.pagesrc/usr/local/emhttp/plugins/updater/include/Settings.phpsrc/usr/local/emhttp/plugins/updater/include/Versions.phpsrc/usr/local/emhttp/plugins/updater/include/ajax.phpsrc/usr/local/emhttp/plugins/updater/include/cron.php
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.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.
This PR introduces automatic Unraid OS patch updates.
Summary by CodeRabbit
New Features
major.minorbranch.Bug Fixes