feat(Internal_Boot): add initial boot options#2538
feat(Internal_Boot): add initial boot options#2538SimonFair wants to merge 19 commits intounraid:masterfrom
Conversation
WalkthroughAdds a Boot Device Settings page with backup/cleanup flow; implements GRUB support across BootParameters UI, backend handler, and management script; enriches device↔pool mapping and rendering and block-device helpers; extends CacheDevices boot‑pool UI; and guards DeviceInfo form access. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser
participant UI as Web UI (JS)
participant PHP as emhttp PHP
participant Script as manage_boot_params.sh
participant FS as Filesystem
Browser->>UI: user action (apply params / start backup / create boot pool)
UI->>PHP: POST apply / request backup / create boot pool
PHP->>Script: invoke bootloader-aware operation (read/write/list/restore)
Script->>FS: read/update grub.cfg or syslinux.cfg, create/remove backups, update defaults
FS-->>Script: operation result
Script-->>PHP: result/status (includes bootloader_type, backup path)
PHP-->>UI: response (success/error, download URL)
UI->>Browser: show preview/result or redirect to backup ZIP
Browser->>UI: on backup completion trigger cleanup
UI->>PHP: request unlink backup file
PHP->>FS: remove zip
FS-->>PHP: confirmation
PHP-->>UI: cleanup confirmation
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🔧 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: 6
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/BootInfo.page`:
- Around line 58-60: The external anchor element linking to
"https://unraid.net/contact" with text '_(Contact Support)_' uses
target="_blank" and should include rel="noopener noreferrer" to prevent
tab-nabbing; update that <a> tag in BootInfo.page (the anchor with href
"https://unraid.net/contact" and text '_(Contact Support)_') to add
rel="noopener noreferrer" while keeping target="_blank".
- Around line 47-54: The template currently prints raw device metadata via
$var['flashVendor'], $var['flashProduct'], and $var['flashGUID']; escape these
values before rendering to prevent HTML injection by applying an HTML-escaping
function (e.g., htmlspecialchars with ENT_QUOTES and UTF-8) wherever these
variables are output in BootInfo.page so the rendered text is safe even if
descriptors contain markup or quotes.
In `@emhttp/plugins/dynamix/BootParameters.page`:
- Around line 1917-1919: Normalize the bootloader type string by lowercasing the
value assigned to bootloaderType (from config.bootloader_type) before any
comparisons; update the assignment for bootloaderType and ensure all downstream
checks that use bootloaderType (e.g., any conditionals or switch statements
referencing bootloaderType) operate on the normalized lowercase value so inputs
like "GRUB" or "Syslinux" match correctly against "grub" and "syslinux".
- Around line 1244-1285: The dropdown currently uses deciseconds (timeoutDeci /
timeoutOptions) for option values causing a 10x mismatch on submit; update the
value logic so the <select id="timeout-dropdown"> stores seconds (not
deciseconds) that the backend expects: convert parsedTimeout (from set timeout=)
back to seconds for the selected value, build option .val() using
Math.round(deci/10) (or keep a data attribute like data-deci if you must
preserve precision) and ensure the customOption uses the original parsed
seconds; keep UI labels in seconds via formatSeconds but submit seconds, and
ensure markFormModified() remains on change.
In `@emhttp/plugins/dynamix/scripts/manage_boot_params.sh`:
- Around line 288-290: Replace regex matching of GRUB menuentry labels with
AWK's literal substring matching: wherever the script sets in_section using the
regex operator against "$0" and the quoted label (e.g., the block matching
"menuentry" that assigns in_section = ($0 ~ "\"" label "\"")), change the logic
to use AWK's index() to test for the literal substring of "\"" label "\""
instead of "~" so parentheses in labels are treated literally; apply the same
replacement for the analogous assignments around the other occurrences
referenced (the blocks around the symbols handling menuentry, label, and
in_section at the other two locations).
- Around line 291-295: The AWK uses the GNU-only \s in gsub and should be made
POSIX-compliant: in the block matching linux/linuxefi (the match(...) line and
the subsequent gsub(...)), replace instances of \s with the POSIX whitespace
class [[:space:]] (e.g. change gsub(/^\s+|\s+$/, "", args) to
gsub(/^[[:space:]]+|[[:space:]]+$/, "", args)); keep the existing [ \t] regex
for explicit space/tab matching elsewhere.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
emhttp/plugins/dynamix/nchan/device_list (1)
551-622:⚠️ Potential issue | 🟠 MajorRemove duplicated device cells in the DISK_NP branch.
There are four identical<td>outputs, which breaks table column alignment.🧩 Suggested fix
if (in_array(_var($disk,'name'),$pools) || $fstype=='zfs') { $echo[] = "<td>".device_info($disk,true,$poolName,$poolstatusData,$options)."</td>"; - $echo[] = "<td>".device_info($disk,true,$poolName,$poolstatusData,$options)."</td>"; - $echo[] = "<td>".device_info($disk,true,$poolName,$poolstatusData,$options)."</td>"; - $echo[] = "<td>".device_info($disk,true,$poolName,$poolstatusData,$options)."</td>"; $echo[] = "<td><a class='static'><i class='icon-disk icon'></i><span></span></a><em>".($fstype=='zfs' ? _('Not present') : _('Not installed'))."</em></td>"; $echo[] = "<td colspan='4'></td>"; $echo[] = $showFsInfo ? fs_info($disk,true) : "<td colspan='4'></td>"; }
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/nchan/device_list`:
- Around line 628-637: In the DISK_NP case remove the duplicated device_info()
calls so only a single
device_info($disk,true,$poolName,$poolstatusData,$options) is appended to the
$echo array for the device info column; ensure the remaining line produces
exactly one <td> worth of output and keep the subsequent lines that append the
"Not present/Not installed" <td>, colspan placeholders and fs_info() logic
untouched so the table HTML remains valid.
🧹 Nitpick comments (3)
emhttp/plugins/dynamix/include/Helpers.php (1)
1686-1700: Function only returns SATA/SAS disks, excluding NVMe and other device types.The regex
/^sd[a-z]+$/filters out NVMe devices (nvme*), virtio devices (vd*), and other block device types. Since this is used for duplicate disk detection, systems with NVMe drives won't have duplicates detected for those devices.Consider expanding the pattern to include other device types:
♻️ Proposed fix to include NVMe and other device types
function get_block_devices(): array { $out = []; exec('lsblk -ndo NAME 2>/dev/null', $out); $devs = []; foreach ($out as $l) { - if (preg_match('/^sd[a-z]+$/', trim($l))) { + if (preg_match('/^(sd[a-z]+|nvme\d+n\d+|vd[a-z]+|hd[a-z]+)$/', trim($l))) { $devs[] = trim($l); } } sort($devs); return $devs; }emhttp/plugins/dynamix/nchan/device_list (2)
385-393: Remove commented-out debug code.Line 390 contains commented-out debug code that should be removed before merging.
🧹 Proposed cleanup
if (!$online && isset($duplicate_devices[$fullDevName])) { $duplicate = isset($duplicate_devices[$fullDevName]['other_devices']) ? implode("<br>", $duplicate_devices[$fullDevName]['other_devices'])."<br>" : ""; - #$duplicate = json_encode($duplicate_devices[$fullDevName]); $isduplicate = isset($duplicate_devices[$fullDevName]) ? "true" : "false"; $warndup .= "<a class='info'><i class='fa fa-clone fa-fw orange-text'></i><span>".$duplicate._('Duplicate device identifier detected (WWID or Serial)')." </span></a>"; }
391-391: Unused variable$isduplicate.The variable
$isduplicateis assigned but never used. Consider removing it.🧹 Proposed cleanup
if (!$online && isset($duplicate_devices[$fullDevName])) { $duplicate = isset($duplicate_devices[$fullDevName]['other_devices']) ? implode("<br>", $duplicate_devices[$fullDevName]['other_devices'])."<br>" : ""; - $isduplicate = isset($duplicate_devices[$fullDevName]) ? "true" : "false"; $warndup .= "<a class='info'><i class='fa fa-clone fa-fw orange-text'></i><span>".$duplicate._('Duplicate device identifier detected (WWID or Serial)')." </span></a>"; }
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/CacheDevices.page`:
- Around line 119-130: The "_(Activate and Reboot)_" button handler currently
duplicates the "_(Activate)_" logic (calls validate(), submits the form, closes
the dialog) but never triggers a reboot; update the "_(Activate and Reboot)_"
handler in CacheDevices.page so that after validate() it sets a hidden form
field (e.g., add or set input name="reboot" value="true" on the same form) or
otherwise marks the form payload to indicate a reboot before calling
form.submit() and then closing the dialog; locate the handler by the literal
"_(Activate and Reboot)_" string and the validate() /
$(this).find('form').submit() calls to implement the change.
- Around line 355-369: The device label/value logic can produce a leading-space
label and an empty option value when $devId is empty; update the construction in
the loop (variables $devId, $devName, $devLabel inside the foreach) so that if
$devId is non-empty you use $devId as the primary label and set value to $devId
(append " ($devName)" if $devName present), otherwise set both the option value
and label to $devName (no leading space); ensure you still run htmlspecialchars
on the chosen value and label before echoing.
- Around line 316-321: The page currently renders an "Add Bootable Pool" button
twice and a new variant unconditionally; remove the unconditional duplicate and
gate the "Add Bootable Pool New" button behind the same bootEligible check so
both addBootPoolPopup and addBootPoolPopupNew are only output when
_var($var,'bootEligible') == "yes" (remove the lone input with
onclick="addBootPoolPopup()" and wrap the input with
onclick="addBootPoolPopupNew()" inside the existing <?if
(_var($var,'bootEligible')=="yes"):?> ... <?endif;?> block).
🧹 Nitpick comments (1)
emhttp/plugins/dynamix/CacheDevices.page (1)
146-181: Slot rendering logic is well-structured; minor note on event handler rebinding.The implementation correctly preserves existing selections, clones options from the hidden source, and syncs device availability. One thing to be aware of:
bootPoolSyncDeviceOptionsrebinding.on('change', …)on line 178 will accumulate handlers ifbootPoolRenderSlotsis called multiple times without the DOM elements being removed first. However, since line 155 removes.bootpool-device-rowelements (destroying old selects) before creating new ones, this is safe in practice.Consider using event delegation on
$containerinstead, which would avoid re-binding entirely:Optional: event delegation approach
- $container.find('select[name="poolDevice[]"]').on('change', function(){ - bootPoolSyncDeviceOptions(popup); - }); + // Bind once in bootPoolInit instead: + // popup.find('#bootPoolDeviceSlots').on('change', 'select[name="poolDevice[]"]', function(){ + // bootPoolSyncDeviceOptions(popup); + // });
| "_(Activate)_": function() { | ||
| if (validate($(this).find('input[name="poolName"]').val())) { | ||
| $(this).find('form').submit(); | ||
| $(this).dialog('close'); | ||
| } | ||
| }, | ||
| "_(Activate and Reboot)_": function() { | ||
| if (validate($(this).find('input[name="poolName"]').val())) { | ||
| $(this).find('form').submit(); | ||
| $(this).dialog('close'); | ||
| } | ||
| }, |
There was a problem hiding this comment.
"Activate and Reboot" performs the same action as "Activate" — reboot logic is missing.
Both button handlers execute identical code (validate → submit form → close dialog). The "Activate and Reboot" button never triggers a reboot, which will mislead users into thinking a reboot was initiated when it wasn't.
You likely need to either set a hidden input (e.g., reboot=true) or issue a separate reboot command after the form submission.
Proposed fix (example approach using a hidden field)
"_(Activate and Reboot)_": function() {
if (validate($(this).find('input[name="poolName"]').val())) {
+ $(this).find('form').append('<input type="hidden" name="reboot" value="yes">');
$(this).find('form').submit();
$(this).dialog('close');
}
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "_(Activate)_": function() { | |
| if (validate($(this).find('input[name="poolName"]').val())) { | |
| $(this).find('form').submit(); | |
| $(this).dialog('close'); | |
| } | |
| }, | |
| "_(Activate and Reboot)_": function() { | |
| if (validate($(this).find('input[name="poolName"]').val())) { | |
| $(this).find('form').submit(); | |
| $(this).dialog('close'); | |
| } | |
| }, | |
| "_(Activate)_": function() { | |
| if (validate($(this).find('input[name="poolName"]').val())) { | |
| $(this).find('form').submit(); | |
| $(this).dialog('close'); | |
| } | |
| }, | |
| "_(Activate and Reboot)_": function() { | |
| if (validate($(this).find('input[name="poolName"]').val())) { | |
| $(this).find('form').append('<input type="hidden" name="reboot" value="yes">'); | |
| $(this).find('form').submit(); | |
| $(this).dialog('close'); | |
| } | |
| }, |
🤖 Prompt for AI Agents
In `@emhttp/plugins/dynamix/CacheDevices.page` around lines 119 - 130, The
"_(Activate and Reboot)_" button handler currently duplicates the "_(Activate)_"
logic (calls validate(), submits the form, closes the dialog) but never triggers
a reboot; update the "_(Activate and Reboot)_" handler in CacheDevices.page so
that after validate() it sets a hidden form field (e.g., add or set input
name="reboot" value="true" on the same form) or otherwise marks the form payload
to indicate a reboot before calling form.submit() and then closing the dialog;
locate the handler by the literal "_(Activate and Reboot)_" string and the
validate() / $(this).find('form').submit() calls to implement the change.
| <?if (_var($var,'bootEligible')=="yes"):?> | ||
| <input type="button" value="_(Add Bootable Pool)_" style="margin:0" onclick="addBootPoolPopup()"> | ||
| <?endif;?> | ||
| <input type="button" value="_(Add Bootable Pool)_" style="margin:0" onclick="addBootPoolPopup()"> | ||
| <input type="button" value="_(Add Bootable Pool New)_" style="margin:0" onclick="addBootPoolPopupNew()"> | ||
|
|
There was a problem hiding this comment.
Duplicate / unconditional "Add Bootable Pool" buttons appear to be debug leftovers.
- Line 317 renders "Add Bootable Pool" conditionally (gated by
bootEligible). - Line 319 renders an identical "Add Bootable Pool" button unconditionally, bypassing the eligibility check entirely.
- Line 320 renders "Add Bootable Pool New" also unconditionally.
This means users will always see bootable-pool buttons regardless of whether their system is eligible. If this is intentional for development, it should be removed or gated before merge.
Suggested fix — gate new buttons behind the eligibility check and remove the duplicate
<?if (_var($var,'bootEligible')=="yes"):?>
<input type="button" value="_(Add Bootable Pool)_" style="margin:0" onclick="addBootPoolPopup()">
+<input type="button" value="_(Add Bootable Pool New)_" style="margin:0" onclick="addBootPoolPopupNew()">
<?endif;?>
-<input type="button" value="_(Add Bootable Pool)_" style="margin:0" onclick="addBootPoolPopup()">
-<input type="button" value="_(Add Bootable Pool New)_" style="margin:0" onclick="addBootPoolPopupNew()">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <?if (_var($var,'bootEligible')=="yes"):?> | |
| <input type="button" value="_(Add Bootable Pool)_" style="margin:0" onclick="addBootPoolPopup()"> | |
| <?endif;?> | |
| <input type="button" value="_(Add Bootable Pool)_" style="margin:0" onclick="addBootPoolPopup()"> | |
| <input type="button" value="_(Add Bootable Pool New)_" style="margin:0" onclick="addBootPoolPopupNew()"> | |
| <?if (_var($var,'bootEligible')=="yes"):?> | |
| <input type="button" value="_(Add Bootable Pool)_" style="margin:0" onclick="addBootPoolPopup()"> | |
| <input type="button" value="_(Add Bootable Pool New)_" style="margin:0" onclick="addBootPoolPopupNew()"> | |
| <?endif;?> |
🤖 Prompt for AI Agents
In `@emhttp/plugins/dynamix/CacheDevices.page` around lines 316 - 321, The page
currently renders an "Add Bootable Pool" button twice and a new variant
unconditionally; remove the unconditional duplicate and gate the "Add Bootable
Pool New" button behind the same bootEligible check so both addBootPoolPopup and
addBootPoolPopupNew are only output when _var($var,'bootEligible') == "yes"
(remove the lone input with onclick="addBootPoolPopup()" and wrap the input with
onclick="addBootPoolPopupNew()" inside the existing <?if
(_var($var,'bootEligible')=="yes"):?> ... <?endif;?> block).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/nchan/device_list`:
- Line 395: The tooltip text concatenation currently assigned to $errorLines
(the expression _("Pool name").":".$poolLabel) omits a space after the colon;
update that assignment so the colon is followed by a space (e.g. change the
_("Pool name").":".$poolLabel concatenation to include ": " between the label
and $poolLabel) so tooltips render like "Pool name: Cache".
- Around line 38-44: normalize_device_name currently uses a single regex
'/p?\d+$/' which treats the 'p' as optional and therefore strips the final digit
from NVMe base names like "nvme0n1"; fix by detecting NVMe-style names first
(e.g. preg_match('/^nvme\d+n\d+/', $device) inside normalize_device_name) and
for those only strip a trailing partition token with a required 'p' (use
preg_replace('/p\d+$/', '', $device)), otherwise keep the existing behavior for
non-NVMe devices (use preg_replace('/p?\d+$/', '', $device)).
🧹 Nitpick comments (4)
emhttp/plugins/dynamix/nchan/device_list (4)
64-104: Duplicated pool-resolution logic betweenget_fs_errorsandget_fs_error_tooltip.The device-key normalization →
$devicePoolslookup →find_pools_for_device_in_statusmerge → dedup → member-matching loop is copy-pasted across both functions (Lines 64-101 vs Lines 350-399). Extracting a shared helper (e.g.,resolve_pools_for_device) would keep the two call sites in sync and reduce ~40 lines of duplication.Also,
array_uniqueon Line 85 is redundant —$poolsToCheckwas already deduplicated on Line 80.
300-348: Consider using an options array forpool_function_rowto reduce its 12-parameter signature.The function has 12 positional parameters, most of which are optional with defaults. This makes call sites hard to read and error-prone (see the 12-argument invocations at Lines 1047-1063, 1067-1080, 1163-1179, 1192-1205). An
$optionsassociative array — like you already use fordevice_infoandarray_online— would improve readability and reduce the risk of argument-order bugs.
492-500: Dead code:$isduplicateis assigned but never read; debug line should be removed.Line 498 sets
$isduplicatewhich is unused. Line 497 has a commented-out debug statement.Proposed cleanup
$warndup = ''; $devName = _var($disk,'device',''); $fullDevName = "/dev/" . _var($disk,'device', ''); if (!$online && isset($duplicate_devices[$fullDevName])) { $duplicate = isset($duplicate_devices[$fullDevName]['other_devices']) ? implode("<br>", $duplicate_devices[$fullDevName]['other_devices'])."<br>" : ""; - #$duplicate = json_encode($duplicate_devices[$fullDevName]); - $isduplicate = isset($duplicate_devices[$fullDevName]) ? "true" : "false"; $warndup .= "<a class='info'><i class='fa fa-clone fa-fw orange-text'></i><span>"._('Duplicate device identifier detected (WWID or Serial)')."<br>$duplicate</span></a>"; }
1034-1042: Nit:$bootPoolNameis computed on Line 1034 then identically reassigned on Line 1040.Same pattern occurs in the started-state block at Lines 1140 vs 1144. The second assignment can be removed.
Proposed fix (stopped-state block)
$bootDisk = find_boot_disk_for_pool($pool, $Boot, $Cache); $bootPoolName = prefix(_var($bootDisk,'name','')); $bootMetrics = get_boot_pool_metrics($Boot, $bootPoolName, $bootDisk); $dataMetrics = get_pool_data_metrics($poolDisk, $bootMetrics); $poolLabel = ucfirst($pool); $summaryName = $poolLabel; if (!empty($bootDisk)) { - $bootPoolName = prefix(_var($bootDisk,'name','')); $bootLabel = ucfirst($bootPoolName);
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/BootParameters.page`:
- Around line 1088-1157: The parseGrubConfig function only captures
double-quoted menuentry labels; update the menuentry parsing in parseGrubConfig
to accept both single and double quotes (e.g. match
menuentry\s+["']([^"']+)["']) and strip surrounding quotes from the captured
label so labels like menuentry 'My OS' are parsed correctly; keep the existing
fallback that uses trimmed.substring('menuentry '.length).trim() when no quotes
are present and ensure entry.label is stored unquoted so the defaultValue
comparison (which already strips double quotes) works for single-quoted defaults
as well.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/CacheDevices.page`:
- Around line 388-410: The file contains a duplicated HTML template with id
"templatePopupSubpool" which causes invalid duplicate IDs and ambiguous
selectors; fix by either removing this duplicate template block or giving it a
unique id (e.g., change id on the duplicate and update any JS/CSS selectors that
reference "templatePopupSubpool"); ensure any references to the template
(selectors used to query the DOM, event handlers, or code that clones the
template) are updated to the new id, and confirm the remaining template keeps
the form fields (select[name="subpool"], select[name="poolSlots"], and the
hidden inputs) so behavior is unchanged.
In `@emhttp/plugins/dynamix/scripts/manage_boot_params.sh`:
- Around line 287-302: The GRUB menuentry matching only handles double-quoted
labels; update parse_grub_linux_args, update_grub_entry, and update_grub_default
to accept either single or double quotes by changing the pattern that looks for
"\"" label "\"" to a regex that matches either "'"label"'" or "\""label"\"";
ensure the awk/grep/sed expressions and index/match checks account for both
quote characters (e.g., check for index/$0 ~ /['"]label['"]/ or use a regex
alternation) so menuentry 'Label' and menuentry "Label" are both recognized and
processed.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/scripts/manage_boot_params.sh`:
- Around line 1154-1157: The current conditional lets GRUB configs skip
validation; update the logic around BOOTLOADER_TYPE, validate_config, temp_file
and target_cfg so GRUB still undergoes a basic sanity check before mv: when
BOOTLOADER_TYPE == "grub" run a minimal validator (e.g., non-empty and contains
"menuentry") — similar to write_config_grub's checks — and only proceed to mv
"$temp_file" "$target_cfg" if that basic check passes (otherwise abort/log an
error).
- Around line 529-539: The reserved-directive check in the function that uses
lower_param with BOOTLOADER_TYPE incorrectly rejects parameters that merely
start with reserved words (e.g., "setpci") because the regexes (used in the GRUB
branch and the syslinux branch) match prefixes like "set" or "menu" without
requiring a separator; update those regexes to only match exact directive names
or the directive followed by a separator (e.g., end-of-string or '='), so modify
the patterns used in the if [[ "$lower_param" =~ ... ]] checks to include a word
boundary or explicit separator (or anchor with ($|=)) for the reserved tokens
before calling error_exit, ensuring legitimate kernel params like setpci are
allowed.
- Around line 1108-1116: The write_config_grub function currently
unconditionally moves the temp file into GRUB_CFG; change it to validate the
temp file (temp_file) before mv: ensure the file is non-empty and contains at
least one "menuentry" (or other expected grub stanza) and only then perform mv
"$temp_file" "$GRUB_CFG"; if validation fails, do not overwrite GRUB_CFG, log an
error (echo or process logger) and exit with non-zero status so callers detect
failure—leave the original GRUB_CFG intact and surface the failure to the
caller.
- Around line 287-302: The AWK snippet in manage_boot_params.sh incorrectly
embeds a literal single-quote sequence "'" label "'" which breaks the outer
single-quoted shell string and causes AWK to search for the literal text " label
"; fix each occurrence by replacing the problematic "'" label "'" embedding with
an escaped single-quote byte sequence such as "\x27" label "\x27" so the AWK
expression index($0, "\"" label "\"") || index($0, "\x27" label "\x27")
correctly matches both double- and single-quoted menuentry names (apply this
change to all three occurrences of the pattern in the AWK/menuentry matching
logic).
🧹 Nitpick comments (9)
emhttp/plugins/dynamix/CacheDevices.page (3)
79-83:bootPoolInitis a no-op foraddBootPoolPopup.
templatePopupBootPool(lines 412-431) doesn't contain#bootPoolDeviceSlotsor#bootPoolDevicesSource, sobootPoolInit→bootPoolRenderSlots→bootPoolSyncDeviceOptionsall operate on empty jQuery sets. It also needlessly binds achangelistener onpoolSlotsthat re-runs the no-op on every change.Consider guarding or removing this call for the old flow, keeping
bootPoolInitonly foraddBootPoolPopupNew.
169-171: Attribute selector escaping is incomplete — CSS meta-characters in device values could break the lookup.
val.replace(/"/g, '\\"')only escapes double quotes. If a device ID contains],\, or other characters meaningful in CSS selectors, theoption[value="…"]lookup silently fails and the previously selected value is lost.Consider using
$.escapeSelector()(jQuery 3+) or.filter()for a robust match:Proposed fix
if (existing[i]) { var val = existing[i]; - if ($select.find('option[value="' + val.replace(/"/g, '\\"') + '"]').length) { + if ($select.find('option').filter(function(){ return $(this).attr('value') === val; }).length) { $select.val(val); } }
412-431: Consider consolidatingtemplatePopupBootPoolandtemplatePopupBootPoolNew.These two templates share most of their structure (name input, slot select capped at 2, boot size field, same hidden inputs). The "new" template extends the original with device-slot scaffolding. If the intent is for
addBootPoolPopupNewto eventually replaceaddBootPoolPopup, you could simplify maintenance by having a single template and conditionally showing the device section.This is non-blocking — fine to defer until the flows stabilize.
emhttp/plugins/dynamix/scripts/manage_boot_params.sh (6)
483-506: Minor: Inconsistent JSON indentation.Lines 485 and 504 use 4-space indentation while lines 486–503 use 2-space indentation. This doesn't break functionality but makes the JSON output inconsistent.
722-728: Simplify: remove unnecessaryecho "$(...)".
echo "$(command)"is equivalent to just running the pipeline directly. This avoids an extra subshell and is more idiomatic.♻️ Suggested simplification
build_kernel_args() { - echo "$(build_append_line | sed 's/^initrd=\/bzroot[ ]*//')" + build_append_line | sed 's/^initrd=\/bzroot[ ]*//' } build_kernel_args_gui_safe() { - echo "$(build_append_line_gui_safe | sed 's/^initrd=\/bzroot[ ]*//')" + build_append_line_gui_safe | sed 's/^initrd=\/bzroot[ ]*//' }
1014-1039:BEGINblock placement is unconventional; silent return on missing label may hide config errors.The
BEGIN { idx=0 }on line 1026 is placed after the main rule. AWK executesBEGINfirst regardless of position, so this works, but placing it before the main rule is more conventional and readable.More importantly, if the requested
DEFAULT_BOOT_ENTRYlabel doesn't exist in the GRUB config, the function silently returns (line 1030) without any warning. If a user mistypes a label name, the default won't change and there's no feedback. Consider logging a warning.
1189-1196:lsiteration and unescaped dots in sed are fragile but low-risk.Line 1189: ShellCheck SC2045 flags
for backup in $(ls ...)as fragile. Since backup filenames are tightly controlled (no spaces), this is safe in practice.Line 1196: The
sedpattern uses${backup_prefix}without escaping dots, sosyslinux.cfgmatchessyslinuxXcfgin theory. Again, no practical risk given the controlled naming.Both are fine to defer.
1058-1070: ShellCheck SC2155:localmasks return values from command substitutions.Multiple lines in this function (and throughout the file) use
local var=$(cmd). Underset -e, if the command fails,localswallows the non-zero exit code. The highest-risk instance is line 1058 wherebuild_kernel_argsfailure would silently produce empty args that get written to the GRUB config.The idiomatic fix is to separate declaration and assignment:
local new_args new_args=$(build_kernel_args)This pattern applies to lines 530, 636, 711, 1018, 1058, 1059, 1065, 1070, 1149, 1195, and 1196.
1130-1137: Repeated bootloader-type branching for paths could be consolidated.The pattern of setting
target_cfg,target_backup_dir, andbackup_prefixbased onBOOTLOADER_TYPEis duplicated inwrite_raw_config,list_backups,restore_backup, anddelete_all_backups. Consider extracting this to a helper or setting these as global variables alongsideBOOTLOADER_TYPE(near line 39).💡 Example consolidation
# Near line 39, after BOOTLOADER_TYPE is set: if [[ "$BOOTLOADER_TYPE" == "grub" ]]; then ACTIVE_CFG="$GRUB_CFG" ACTIVE_BACKUP_DIR="$GRUB_BACKUP_DIR" BACKUP_PREFIX="grub.cfg" else ACTIVE_CFG="$SYSLINUX_CFG" ACTIVE_BACKUP_DIR="$BACKUP_DIR" BACKUP_PREFIX="syslinux.cfg" fiAlso applies to: 1174-1179, 1214-1221, 1264-1269
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/scripts/manage_boot_params.sh`:
- Around line 1101-1108: The current conversion of TIMEOUT deciseconds to GRUB
integer seconds in manage_boot_params.sh uses truncation
(timeout_seconds=$((TIMEOUT / 10))) which drops fractional seconds; change this
to round to the nearest second (e.g. compute timeout_seconds with integer
rounding such as timeout_seconds=$(((TIMEOUT + 5) / 10))) before updating
temp_file so a UI value like 15 deciseconds becomes 2 seconds rather than 1;
update the assignment of timeout_seconds and leave the existing sed logic that
writes/updates "set timeout=" unchanged.
🧹 Nitpick comments (5)
emhttp/plugins/dynamix/scripts/manage_boot_params.sh (5)
483-506: Minor: inconsistent JSON indentation.Lines 485 and 504 use 4-space indentation while all other properties use 2-space. This won't affect parsing but is a cosmetic inconsistency.
- "bootloader_type": "$BOOTLOADER_TYPE", + "bootloader_type": "$BOOTLOADER_TYPE",- "timeout": "$timeout" + "timeout": "$timeout"
653-717:build_append_line_gui_safeis nearly identical tobuild_append_line— consider extracting shared logic.The two functions differ only in whether framebuffer parameters are included (lines 574–577). The rest (~60 lines of PCI merging, power management, custom params, and unraiduuid handling) is duplicated verbatim. A single function with a
skip_framebufferflag would eliminate this maintenance burden.
722-728: Uselessecho "$(…)"wrapper — pipe directly.
echo "$(cmd | sed …)"spawns an extra subshell. Simplify:build_kernel_args() { - echo "$(build_append_line | sed 's/^initrd=\/bzroot[ ]*//')" + build_append_line | sed 's/^initrd=\/bzroot[ ]*//' } build_kernel_args_gui_safe() { - echo "$(build_append_line_gui_safe | sed 's/^initrd=\/bzroot[ ]*//')" + build_append_line_gui_safe | sed 's/^initrd=\/bzroot[ ]*//' }
1196-1204: Iterating overlsoutput is fragile (SC2045); also unescaped dots in sed pattern.While backup filenames are tightly controlled,
for backup in $(ls -t ...)will break on whitespace in paths. A safer alternative:- for backup in $(ls -t "$target_backup_dir"/${backup_prefix}.bak.* 2>/dev/null); do + while IFS= read -r -d '' backup; dousing
find ... -print0 | sort -z -rinstead ofls -t.Additionally, on line 1203,
${backup_prefix}(e.g.,grub.cfg) contains literal dots that are unescaped in the sed regex, so they match any character. Low risk here given the naming pattern, but worth noting.
1230-1241: Comments reference only syslinux format, but now GRUB is supported too.The block comment (lines 1231–1239) documents only the
syslinux.cfg.bak.*format/example. Now thatgrub.cfg.bak.*is also valid, the comments should mention both formats or be made generic. Minor documentation nit.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
emhttp/plugins/dynamix/scripts/manage_boot_params.sh (2)
1230-1241:⚠️ Potential issue | 🟡 MinorStale comments — still reference syslinux-only format.
The comment block (lines 1231–1240) and example only describe
syslinux.cfg.bak.*format, but the code now dynamically handles both syslinux and GRUB backup prefixes. Update the comments to reflect the dual-bootloader support.
483-506:⚠️ Potential issue | 🟡 MinorInconsistent JSON indentation —
bootloader_typeandtimeoutuse 4 spaces, rest uses 2.Lines 485 and 504 are indented with 4 spaces while all other JSON fields use 2 spaces. This doesn't break parsing but is a visible inconsistency.
✏️ Fix indentation
- "bootloader_type": "$BOOTLOADER_TYPE", + "bootloader_type": "$BOOTLOADER_TYPE",- "timeout": "$timeout" + "timeout": "$timeout"
🧹 Nitpick comments (4)
emhttp/plugins/dynamix/scripts/manage_boot_params.sh (4)
719-728: Remove unnecessaryecho "$(…)"wrapper and consider reducing duplication.
echo "$(cmd)"is a no-op wrapper — the pipeline output is captured and re-echoed. Also, strippinginitrd=/bzrootvia sed is fragile coupling withbuild_append_line. A shared internal function that builds args without the initrd prefix would be cleaner;build_append_linewould call it and prependinitrd=/bzroot, whilebuild_kernel_argsuses it directly.♻️ Minimal fix (remove UUOC)
build_kernel_args() { - echo "$(build_append_line | sed 's/^initrd=\/bzroot[ ]*//')" + build_append_line | sed 's/^initrd=\/bzroot[ ]*//' } build_kernel_args_gui_safe() { - echo "$(build_append_line_gui_safe | sed 's/^initrd=\/bzroot[ ]*//')" + build_append_line_gui_safe | sed 's/^initrd=\/bzroot[ ]*//' }
1196-1203: Minor: Iterating overlsoutput is fragile (SC2045).Filenames are controlled by this script so spaces are unlikely, but a glob-based approach would be more robust and avoids the ShellCheck warning.
♻️ Proposed glob-based iteration
- for backup in $(ls -t "$target_backup_dir"/${backup_prefix}.bak.* 2>/dev/null); do + local -a backups=() + while IFS= read -r -d '' f; do + backups+=("$f") + done < <(find "$target_backup_dir" -maxdepth 1 -name "${backup_prefix}.bak.*" -print0 | xargs -0 ls -t 2>/dev/null | tr '\n' '\0')Alternatively, if time-sorted order is not critical, a simple glob would suffice:
for backup in "$target_backup_dir"/${backup_prefix}.bak.*; do
653-717: ~75 lines duplicated betweenbuild_append_lineandbuild_append_line_gui_safe.These two functions differ only in whether framebuffer parameters are included (lines 574–577). Every other section is identical. Any future parameter change must be applied in both places — a maintenance hazard.
Consider a single function with a flag:
♻️ Sketch
-build_append_line() { +build_append_line() { + local include_framebuffer="${1:-1}" local params="initrd=/bzroot" ... - # 2. Framebuffers - [[ "${EFIFB_OFF}" == "1" ]] && params="$params video=efifb:off" - ... + # 2. Framebuffers (excluded for GUI-safe mode) + if [[ "$include_framebuffer" == "1" ]]; then + [[ "${EFIFB_OFF}" == "1" ]] && params="$params video=efifb:off" + [[ "${VESAFB_OFF}" == "1" ]] && params="$params video=vesafb:off" + [[ "${SIMPLEFB_OFF}" == "1" ]] && params="$params video=simplefb:off" + [[ "${SYSFB_BLACKLIST}" == "1" ]] && params="$params initcall_blacklist=sysfb_init" + fi ... } -build_append_line_gui_safe() { - ... (entire duplicate function removed) -} +build_append_line_gui_safe() { + build_append_line 0 +}
1058-1070: Optional:local var=$(cmd)masks failures underset -e(SC2155).With
set -eactive,localalways returns 0 and swallows any command-substitution failure. This pattern appears throughout the new GRUB code (lines 1058, 1059, 1065, 1070, and others). Splitting declaration and assignment would letset -ecatch failures:local new_args new_args=$(build_kernel_args)Low risk here since these commands are unlikely to fail silently, but worth noting for correctness under
set -e.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@emhttp/plugins/dynamix/CacheDevices.page`:
- Around line 147-188: In bootPoolSizeInit, remove the no-op conditional that
sets $input.val('') when already empty (the check inside syncBootSize) and
simply ensure the custom input is toggled readonly/visibility only (function:
bootPoolSizeInit / syncBootSize; element: $input), change the validation in the
$input 'input change' handler from allowing zero to require positive values
(replace gb >= 0 with gb > 0; references: $input handler, $hidden), and update
the templates that render the poolBootSizeCustomGb numeric input to use HTML min
and max attributes (and remove maxlength) so the browser enforces a valid
numeric range for that input (targets: the poolBootSizeCustomGb input elements
in both template instances).
- Around line 467-497: templatePopupBootPool is missing the device-source and
device-slot containers that bootPoolInit -> bootPoolRenderSlots expects
(`#bootPoolDevicesSource` and `#bootPoolDeviceSlots`), causing slot rendering to
no-op; fix by adding the same device-selection markup (elements with IDs
`#bootPoolDevicesSource` and `#bootPoolDeviceSlots` used by bootPoolRenderSlots)
into templatePopupBootPool so bootPoolRenderSlots can find them, or
alternatively add a guard at the start of bootPoolRenderSlots to early-return if
popup.find('#bootPoolDevicesSource').length===0 ||
popup.find('#bootPoolDeviceSlots').length===0.
🧹 Nitpick comments (1)
emhttp/plugins/dynamix/CacheDevices.page (1)
426-438: Inconsistent inline style between the two boot-size<select>elements.Line 427 uses
max-width: 180pxwhile the equivalent select intemplatePopupBootPool(line 483) usesmin-width: 180px. This will cause different sizing behavior between the two templates. Unify to one or the other.Pick one — e.g., min-width in both
-: <span><select name="poolBootSizePreset" class="wide bootpool-size-preset" style="max-width: 180px;"> +: <span><select name="poolBootSizePreset" class="wide bootpool-size-preset" style="min-width: 180px;">
| function bootPoolSizeInit(popup) { | ||
| var $select = popup.find('select[name="poolBootSizePreset"]'); | ||
| var $input = popup.find('input[name="poolBootSizeCustomGb"]'); | ||
| var $hidden = popup.find('input[name="poolBootSize"]'); | ||
| var $inputWrap = popup.find('.bootpool-size-custom'); | ||
| if (!$select.length || !$input.length || !$hidden.length) return; | ||
|
|
||
| function syncBootSize() { | ||
| var val = $select.val(); | ||
| if (val === 'custom') { | ||
| $input.prop('readonly', false); | ||
| $inputWrap.removeClass('hidden'); | ||
| if ($input.val() === '' || $input.val() === null) $input.val(''); | ||
| } else { | ||
| $input.val(''); | ||
| $input.prop('readonly', true); | ||
| $inputWrap.addClass('hidden'); | ||
| } | ||
| } | ||
|
|
||
| var currentMiB = $.trim($hidden.val()); | ||
| if (currentMiB && $select.find('option[value="' + currentMiB.replace(/"/g, '\\"') + '"]').length) { | ||
| $select.val(currentMiB); | ||
| } else { | ||
| $select.val('custom'); | ||
| if (currentMiB) { | ||
| $input.val(Math.round(parseInt(currentMiB, 10) / 1024)); | ||
| } | ||
| } | ||
|
|
||
| $input.on('input change', function(){ | ||
| var gb = parseInt($input.val(), 10); | ||
| if (!isNaN(gb) && gb >= 0) { | ||
| $hidden.val(gb * 1024); | ||
| } else { | ||
| $hidden.val(''); | ||
| } | ||
| }); | ||
|
|
||
| $select.on('change', syncBootSize); | ||
| syncBootSize(); | ||
| } |
There was a problem hiding this comment.
Several issues in bootPoolSizeInit.
- Line 159 is a no-op:
if ($input.val() === '' || $input.val() === null) $input.val('');— setting an empty/null input to''does nothing. - Line 179 allows zero:
gb >= 0means a user can submit a boot reserved size of 0 MiB, which is almost certainly invalid. Usegb > 0. - Line 436 / 492:
maxlengthhas no effect on<input type="number">per the HTML spec. Useminandmaxattributes to constrain the value range.
Proposed fixes
For the no-op on line 159:
- if ($input.val() === '' || $input.val() === null) $input.val('');
+ // no need to reset — input is already emptyFor allowing zero on line 179:
- if (!isNaN(gb) && gb >= 0) {
+ if (!isNaN(gb) && gb > 0) {For the number input constraints (applies to both templates at lines 436 and 492):
- <input type="number" name="poolBootSizeCustomGb" maxlength="10" class="narrow"> GB
+ <input type="number" name="poolBootSizeCustomGb" min="1" max="9999999999" class="narrow"> GB🤖 Prompt for AI Agents
In `@emhttp/plugins/dynamix/CacheDevices.page` around lines 147 - 188, In
bootPoolSizeInit, remove the no-op conditional that sets $input.val('') when
already empty (the check inside syncBootSize) and simply ensure the custom input
is toggled readonly/visibility only (function: bootPoolSizeInit / syncBootSize;
element: $input), change the validation in the $input 'input change' handler
from allowing zero to require positive values (replace gb >= 0 with gb > 0;
references: $input handler, $hidden), and update the templates that render the
poolBootSizeCustomGb numeric input to use HTML min and max attributes (and
remove maxlength) so the browser enforces a valid numeric range for that input
(targets: the poolBootSizeCustomGb input elements in both template instances).
| <div id="templatePopupBootPool" class="template"> | ||
| <form markdown="1" method="POST" action="/update.htm" target="progressFrame" onsubmit="return validate(this.poolName.value)"> | ||
| <input type="hidden" name="poolBootable" value="yes"> | ||
| <input type="hidden" name="changeSlots" value="apply"> | ||
|
|
||
| _(Name)_: | ||
| : <input type="text" name="poolName" maxlength="40" value="<?=count($pools)==0?'cache':''?>"> | ||
|
|
||
| _(Slots)_: | ||
| : <select name="poolSlots"> | ||
| <?for ($n=1; $n<=2; $n++):?> | ||
| <?=mk_option(1,$n,$n)?> | ||
| <?endfor;?> | ||
| </select> | ||
|
|
||
| _(Boot reserved size)_ (_(MiB)_): | ||
| : <select name="poolBootSizePreset" class="wide bootpool-size-preset" style="min-width: 180px;"> | ||
| <option value="8192">8 GB</option> | ||
| <option value="16384">16 GB</option> | ||
| <option value="32768">32 GB</option> | ||
| <option value="65536">64 GB</option> | ||
| <option value="131072">128 GB</option> | ||
| <option value="custom">_(User defined)_</option> | ||
| </select> | ||
| <span class="bootpool-size-custom"> | ||
| <input type="number" name="poolBootSizeCustomGb" maxlength="10" class="narrow"> GB | ||
| </span> | ||
| <input type="hidden" name="poolBootSize" value="<?=_var($var,'bootSizeDefault')?>"> | ||
|
|
||
| </form> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for all references to addBootPoolPopup
rg -n 'addBootPoolPopup' --type=php --type=html -g '!node_modules'Repository: unraid/webgui
Length of output: 39
🏁 Script executed:
#!/bin/bash
# Search for templatePopupBootPool references
echo "=== Searching for templatePopupBootPool ==="
rg -n 'templatePopupBootPool'
# Search for bootPoolInit to find active boot pool initialization
echo "=== Searching for bootPoolInit ==="
rg -n 'bootPoolInit'
# Search for related boot pool functions
echo "=== Searching for bootPoolRenderSlots ==="
rg -n 'bootPoolRenderSlots'Repository: unraid/webgui
Length of output: 1008
🏁 Script executed:
#!/bin/bash
# Get context for old flow (around line 82-83)
echo "=== Old flow context (lines 75-95) ==="
sed -n '75,95p' emhttp/plugins/dynamix/CacheDevices.page
# Get context for new flow (lines 100-120)
echo "=== New flow context (lines 100-120) ==="
sed -n '100,120p' emhttp/plugins/dynamix/CacheDevices.page
# Get the new template to see device-slot UI (lines 384-430)
echo "=== New template (lines 384-430) ==="
sed -n '384,430p' emhttp/plugins/dynamix/CacheDevices.page
# Verify old template (already provided, but check around it)
echo "=== Old template vicinity (lines 467-520) ==="
sed -n '467,520p' emhttp/plugins/dynamix/CacheDevices.pageRepository: unraid/webgui
Length of output: 3886
🏁 Script executed:
#!/bin/bash
# Get bootPoolRenderSlots implementation
echo "=== bootPoolRenderSlots function (lines 190-250) ==="
sed -n '190,250p' emhttp/plugins/dynamix/CacheDevices.pageRepository: unraid/webgui
Length of output: 2361
🏁 Script executed:
#!/bin/bash
# Find where addBootPoolPopup and addBootPoolPopupNew are called from
echo "=== Where addBootPoolPopup is called ==="
rg -n 'addBootPoolPopup\(' --type=php --type=html -B 2 -A 1
echo ""
echo "=== Where addBootPoolPopupNew is called ==="
rg -n 'addBootPoolPopupNew\(' --type=php --type=html -B 2 -A 1Repository: unraid/webgui
Length of output: 142
🏁 Script executed:
#!/bin/bash
# Search for onclick attributes or references to boot pool functions
echo "=== Searching for onclick handlers with boot pool ==="
rg -n "onclick.*[Bb]oot[Pp]ool" --type=php --type=html
# Search for button text mentioning "Add Bootable Pool"
echo ""
echo "=== Searching for Add Bootable Pool mentions ==="
rg -n "Add Bootable Pool" --type=php --type=html -B 2 -A 2
# Search for any reference to these function names in quotes or attributes
echo ""
echo "=== Searching for addBootPool in attributes ==="
rg -n 'addBootPool' --type=php --type=htmlRepository: unraid/webgui
Length of output: 209
🏁 Script executed:
#!/bin/bash
# Check file size and general structure
echo "=== File info ==="
wc -l emhttp/plugins/dynamix/CacheDevices.page
# Search for references to CacheDevices.page
echo ""
echo "=== References to CacheDevices.page ==="
rg -n 'CacheDevices.page' --type=php --type=html --type=xml
# Check if boot pool functions are referenced in any other context
echo ""
echo "=== Check for boot pool anywhere in codebase ==="
rg -n 'bootPool' --type=php --type=html | head -20Repository: unraid/webgui
Length of output: 210
🏁 Script executed:
#!/bin/bash
# Check if both templatePopupBootPool and templatePopupBootPoolNew have duplicate IDs
echo "=== Checking for duplicate element IDs in both templates ==="
sed -n '384,450p' emhttp/plugins/dynamix/CacheDevices.page | grep -n 'id='
sed -n '467,497p' emhttp/plugins/dynamix/CacheDevices.page | grep -n 'id='
# Check the entire file for element IDs to see if there are other duplicates
echo ""
echo "=== All element IDs in CacheDevices.page ==="
rg -n 'id="' emhttp/plugins/dynamix/CacheDevices.pageRepository: unraid/webgui
Length of output: 795
templatePopupBootPool lacks the device-slot selection UI that bootPoolInit expects.
The old flow and new flow both call bootPoolInit(popup), which then invokes bootPoolRenderSlots. That function searches for #bootPoolDeviceSlots and #bootPoolDevicesSource inside the popup (lines 192–193). The new template contains these elements (lines 421 and 399 respectively), but templatePopupBootPool (lines 467–497) does not. When bootPoolRenderSlots runs against the old template, it silently produces zero device slots because jQuery's find() returns an empty object, and the appended DOM elements are lost.
Add the same device-source and device-slots markup to templatePopupBootPool to maintain functional parity with the new template, or guard bootPoolRenderSlots to skip rendering when the containers are absent.
🤖 Prompt for AI Agents
In `@emhttp/plugins/dynamix/CacheDevices.page` around lines 467 - 497,
templatePopupBootPool is missing the device-source and device-slot containers
that bootPoolInit -> bootPoolRenderSlots expects (`#bootPoolDevicesSource` and
`#bootPoolDeviceSlots`), causing slot rendering to no-op; fix by adding the same
device-selection markup (elements with IDs `#bootPoolDevicesSource` and
`#bootPoolDeviceSlots` used by bootPoolRenderSlots) into templatePopupBootPool so
bootPoolRenderSlots can find them, or alternatively add a guard at the start of
bootPoolRenderSlots to early-return if
popup.find('#bootPoolDevicesSource').length===0 ||
popup.find('#bootPoolDeviceSlots').length===0.
SimonFair
changed the title
1 participant
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes