feat(webgui): backend-tracked task queue with multi-task tray + foreground recall - #2665
Open
Eli Bosley (elibosley) wants to merge 23 commits into
Open
feat(webgui): backend-tracked task queue with multi-task tray + foreground recall#2665Eli Bosley (elibosley) wants to merge 23 commits into
Eli Bosley (elibosley) wants to merge 23 commits into
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR replaces the legacy banner-based background operation flow with a persistent multi-task queue system. A new ChangesMulti-task background operation system
Sequence Diagram(s)sequenceDiagram
participant User
participant HeadInlineJS as openPlugin/openDocker/openVMAction
participant TaskCommand.php
participant TaskQueue.php
participant nchanDaemon as nchan/tasks daemon
participant taskChannel as taskChannel (/sub/tasks)
participant BodyInlineJS as onTaskListUpdate / foregroundTask
User->>HeadInlineJS: trigger operation
HeadInlineJS->>TaskCommand.php: POST action=create
TaskCommand.php->>TaskQueue.php: task_create() → task_launch()
TaskQueue.php-->>TaskCommand.php: {id, status}
TaskCommand.php-->>HeadInlineJS: JSON {id, status}
HeadInlineJS->>BodyInlineJS: foregroundTask(id) — open modal
TaskQueue.php->>taskChannel: task_publish() via nchan
taskChannel->>BodyInlineJS: onTaskListUpdate()
BodyInlineJS->>BodyInlineJS: start nchanByType[type] live stream
nchanDaemon->>TaskQueue.php: poll PID liveness
nchanDaemon->>TaskQueue.php: task_write() status=done/error
TaskQueue.php->>taskChannel: task_publish()
taskChannel->>BodyInlineJS: onTaskListUpdate() → fireTaskCallback / trayRender
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 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 |
🔧 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
|
Eli Bosley (elibosley)
marked this pull request as ready for review
June 18, 2026 14:08
![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: 6
🧹 Nitpick comments (5)
emhttp/plugins/dynamix/include/TaskQueue.php (1)
147-177: ⚖️ Poor tradeoffPotential race condition in task creation and launch.
Between the deduplication check (lines 151-156), the
task_running_type()check (line 173), andtask_launch(), another concurrent request could create or launch a task of the same type. This could violate the "at most one running task per type" invariant if two requests race.Consider using file locking around the create-and-launch sequence to ensure atomicity:
♻️ Suggested approach using flock
function task_create($type,$cmd,$title,$plg,$func,$start,$button) { if (!in_array($type, TASK_TYPES)) return null; $lockfile = task_dir() . "/.lock.$type"; $fp = fopen($lockfile, 'c'); if (!flock($fp, LOCK_EX)) { fclose($fp); return null; } try { // dedupe check and task creation logic here... // (existing code) } finally { flock($fp, LOCK_UN); fclose($fp); } }🤖 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 `@emhttp/plugins/dynamix/include/TaskQueue.php` around lines 147 - 177, The task_create function has a race condition where concurrent requests could violate the "at most one running task per type" invariant between the deduplication check (inside the loop checking task_list) and the task_launch call. To fix this, wrap the entire deduplication check, task creation, and task launch logic (from the initial in_array check through the task_launch call and task_publish call) in a file lock using flock on a lock file specific to the task type stored in the task directory. Acquire an exclusive lock before entering the critical section and release it in a finally block to ensure the lock is always released, maintaining atomicity for the create-and-launch sequence.emhttp/plugins/dynamix/include/TaskCommand.php (2)
35-51: 💤 Low valueConsider returning a JSON response from abort/dismiss for consistency.
The
abortanddismissactions calldie()without a response body, whilecreateandlistreturn JSON. While the client code (BodyInlineJS.php) doesn't appear to use the response, returning a consistent JSON acknowledgment (e.g.,{"ok":true}) would improve API consistency and debuggability.🤖 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 `@emhttp/plugins/dynamix/include/TaskCommand.php` around lines 35 - 51, The abort case (and similarly the dismiss case) currently calls die() without returning any response body, while other actions like create and list return JSON responses. Replace the die() call at the end of the abort case with a JSON response using die(json_encode(['ok' => true])) to provide a consistent JSON acknowledgment across all task command actions. Apply the same change to the dismiss case for consistency throughout the TaskCommand.php file.
38-44: 💤 Low valueVerify PID validity more robustly before kill.
The check
$task['pid'] > 1guards against killing PID 0 or 1, but$task['pid']is stored as a string (fromexec()output). The comparison works due to PHP's type juggling, but consider:
- The PID could be empty string or non-numeric if exec failed
- Validating the process still belongs to the expected task (though this may be overkill)
Current implementation is acceptable since
escapeshellarg()handles edge cases safely, but explicit validation would be clearer:♻️ Optional: explicit numeric validation
- if ($task['status']==='running' && $task['pid'] > 1) { + if ($task['status']==='running' && is_numeric($task['pid']) && (int)$task['pid'] > 1) {🤖 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 `@emhttp/plugins/dynamix/include/TaskCommand.php` around lines 38 - 44, The PID validation in the condition `if ($task['status']==='running' && $task['pid'] > 1)` relies on PHP's implicit type juggling since $task['pid'] is a string from exec() output. To make this more explicit and robust, add explicit numeric validation before the kill operation. Replace or enhance the condition to explicitly check that $task['pid'] is numeric using is_numeric() or ctype_digit() before proceeding with the exec('kill '.escapeshellarg($task['pid'])) call, ensuring the PID is a valid numeric value and not an empty string or non-numeric value from a failed exec.emhttp/plugins/dynamix/nchan/tasks (1)
32-37: 💤 Low value
_ERROR_marker detection may have false positives.The error detection searches for the literal string
_ERROR_anywhere in the last 64KB of the log. If a task's legitimate output happens to contain this string (e.g., in a log message or user data), it would incorrectly mark the task as failed.Consider making the marker more unique or checking for it only at specific positions (e.g., as a standalone message in the RS-delimited format).
🤖 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 `@emhttp/plugins/dynamix/nchan/tasks` around lines 32 - 37, The task_log_has_error function uses a simple strpos search for the literal string _ERROR_ which can cause false positives if this string appears in legitimate task output. To fix this, make the error marker detection more robust by either creating a more unique/distinctive error marker string (e.g., wrapping it with special delimiters) or by checking for it only in specific positions or formats such as standalone messages in RS-delimited format. Update the strpos check in task_log_has_error to use this more specific detection logic instead of searching for the bare _ERROR_ string anywhere in the log tail output.emhttp/plugins/dynamix/include/DefaultPageLayout/BodyInlineJS.php (1)
126-177: ⚖️ Poor tradeoffUnescaped HTML in nchan message rendering.
Several locations render nchan message data directly into innerHTML/html() without escaping (Lines 142, 155, 160, 173). While these messages originate from server-side processes, if a process outputs content containing
<script>tags or event handlers, it could execute in the browser.This appears to be preserved behavior from the legacy handlers, but worth noting for a security-conscious codebase. Consider escaping output or using textContent where HTML formatting isn't required.
🤖 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 `@emhttp/plugins/dynamix/include/DefaultPageLayout/BodyInlineJS.php` around lines 126 - 177, The renderMessage function renders nchan message data directly into DOM elements using innerHTML and html() without escaping, which could allow script injection if processes output malicious content. In the 'plugins', 'addLog', and 'addToID' cases within the switch statement, replace direct HTML concatenation with properly escaped content or use textContent for plain text output. Create a helper function to escape HTML entities (replacing &, <, >, ", ') and apply it to all data values (data[1], data[2], label, etc.) before inserting them into the DOM, or use textContent instead of innerHTML where HTML formatting is not required.
🤖 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/include/DefaultPageLayout/BodyInlineJS.php`:
- Around line 200-239: The `foregroundTask` function passes `task.title`
directly into the swal dialog title without escaping, and since `html:true` is
enabled, any HTML content in the title will be rendered as markup. Escape the
`task.title` value using a JavaScript HTML escaping function (such as a utility
function that converts characters like <, >, &, and quotes to their HTML entity
equivalents) before inserting it into the swal title string, or use a DOM
manipulation method that treats the content as text rather than HTML to prevent
malicious scripts from executing.
- Around line 271-296: In the trayRender function, the task ID variable t.id is
being directly embedded into onclick handler attribute strings without escaping
on multiple lines (in foregroundTask, confirmAbortTask, cancelTask, and
dismissTask calls). This creates a potential XSS vulnerability if t.id contains
special characters or quotes. Apply the existing escapeTaskHtml function
(already used for t.title in the same function) to escape t.id wherever it
appears in the onclick handler strings to prevent the ID from breaking the
handler or enabling injection attacks.
In `@emhttp/plugins/dynamix/include/DefaultPageLayout/HeadInlineJS.php`:
- Around line 183-192: The createTask function uses $.post() to submit a request
but lacks error handling for network or server failures. When the POST request
fails, the spinner gets hidden in the success callback but the user receives no
error feedback. Add error handling by chaining a .fail() method to the $.post()
call to display an error notification to the user when the request fails, and
consider whether the spinner should be hidden or kept visible during error
states.
In `@emhttp/plugins/dynamix/include/TaskQueue.php`:
- Around line 119-128: The exec() call in the task execution block contains a
command injection vulnerability where the $args variable from task_resolve() is
interpolated directly into a single-quoted bash command without proper escaping.
Although $name is safely handled and $task['id'] uses escapeshellarg(), the
$args variable can contain shell metacharacters including single quotes that
break out of the quoted context. Apply escapeshellarg() to the $args variable
just like you do for $task['id'] before interpolating it into the bash command
string to ensure all shell metacharacters are properly escaped.
In `@emhttp/plugins/dynamix/nchan/tasks`:
- Around line 27-30: The task_pid_alive function does not validate that the PID
is numeric before checking if the /proc directory exists. When $pid is an empty
string, file_exists("/proc/") returns true because /proc is a directory, causing
tasks to be stuck in running state. Fix this by adding a numeric validation
check for $pid (such as is_numeric() or ensuring it's a positive integer) before
the file_exists() call to ensure only valid numeric PIDs are checked against
/proc/$pid.
In `@emhttp/plugins/dynamix/styles/default-base.css`:
- Around line 1919-1924: Add the CSS property `min-width: 0` to the `.op-tray
.op-title` rule. This property is required for flex items with `overflow:
hidden` and `text-overflow: ellipsis` to properly shrink and enable reliable
ellipsis behavior. Without it, long titles may not shrink appropriately and can
overflow, pushing action icons out of view. Insert this property among the
existing declarations in the `.op-tray .op-title` selector.
---
Nitpick comments:
In `@emhttp/plugins/dynamix/include/DefaultPageLayout/BodyInlineJS.php`:
- Around line 126-177: The renderMessage function renders nchan message data
directly into DOM elements using innerHTML and html() without escaping, which
could allow script injection if processes output malicious content. In the
'plugins', 'addLog', and 'addToID' cases within the switch statement, replace
direct HTML concatenation with properly escaped content or use textContent for
plain text output. Create a helper function to escape HTML entities (replacing
&, <, >, ", ') and apply it to all data values (data[1], data[2], label, etc.)
before inserting them into the DOM, or use textContent instead of innerHTML
where HTML formatting is not required.
In `@emhttp/plugins/dynamix/include/TaskCommand.php`:
- Around line 35-51: The abort case (and similarly the dismiss case) currently
calls die() without returning any response body, while other actions like create
and list return JSON responses. Replace the die() call at the end of the abort
case with a JSON response using die(json_encode(['ok' => true])) to provide a
consistent JSON acknowledgment across all task command actions. Apply the same
change to the dismiss case for consistency throughout the TaskCommand.php file.
- Around line 38-44: The PID validation in the condition `if
($task['status']==='running' && $task['pid'] > 1)` relies on PHP's implicit type
juggling since $task['pid'] is a string from exec() output. To make this more
explicit and robust, add explicit numeric validation before the kill operation.
Replace or enhance the condition to explicitly check that $task['pid'] is
numeric using is_numeric() or ctype_digit() before proceeding with the
exec('kill '.escapeshellarg($task['pid'])) call, ensuring the PID is a valid
numeric value and not an empty string or non-numeric value from a failed exec.
In `@emhttp/plugins/dynamix/include/TaskQueue.php`:
- Around line 147-177: The task_create function has a race condition where
concurrent requests could violate the "at most one running task per type"
invariant between the deduplication check (inside the loop checking task_list)
and the task_launch call. To fix this, wrap the entire deduplication check, task
creation, and task launch logic (from the initial in_array check through the
task_launch call and task_publish call) in a file lock using flock on a lock
file specific to the task type stored in the task directory. Acquire an
exclusive lock before entering the critical section and release it in a finally
block to ensure the lock is always released, maintaining atomicity for the
create-and-launch sequence.
In `@emhttp/plugins/dynamix/nchan/tasks`:
- Around line 32-37: The task_log_has_error function uses a simple strpos search
for the literal string _ERROR_ which can cause false positives if this string
appears in legitimate task output. To fix this, make the error marker detection
more robust by either creating a more unique/distinctive error marker string
(e.g., wrapping it with special delimiters) or by checking for it only in
specific positions or formats such as standalone messages in RS-delimited
format. Update the strpos check in task_log_has_error to use this more specific
detection logic instead of searching for the bare _ERROR_ string anywhere in the
log tail output.
🪄 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: a46bcf23-fb41-4180-a1a2-3ff7d615eee1
📒 Files selected for processing (9)
emhttp/plugins/dynamix/include/DefaultPageLayout.phpemhttp/plugins/dynamix/include/DefaultPageLayout/BodyInlineJS.phpemhttp/plugins/dynamix/include/DefaultPageLayout/HeadInlineJS.phpemhttp/plugins/dynamix/include/DefaultPageLayout/MiscElementsBottom.phpemhttp/plugins/dynamix/include/TaskCommand.phpemhttp/plugins/dynamix/include/TaskQueue.phpemhttp/plugins/dynamix/include/publish.phpemhttp/plugins/dynamix/nchan/tasksemhttp/plugins/dynamix/styles/default-base.css
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/nchan/tasks`:
- Around line 62-67: The active check in the loop counts both running and queued
tasks as work, but the actual task processing only advances `$freed` types,
meaning queued-only tasks never get launched. Add a recovery pass before the `if
(!$active) break;` statement that detects when queued tasks exist without
corresponding running tasks, and triggers processing of those queued tasks to
ensure the daemon does not sleep indefinitely when restarted with pending queue
work.
🪄 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: 742983b2-c4a7-447b-a0e8-cfa64f81f836
📒 Files selected for processing (2)
emhttp/plugins/dynamix/include/TaskQueue.phpemhttp/plugins/dynamix/nchan/tasks
Member
Author
|
CodeRabbit fix loop — two nitpicks intentionally not applied The other 10 review items (6 inline + 1 follow-up thread + 3 nitpicks) are addressed in the incoming commit. Two review-body nitpicks are declined, with reasons: 1. 2. |
This was referenced Jun 18, 2026
Eli Bosley (elibosley)
force-pushed
the
feat/backend-task-queue
branch
from
June 19, 2026 14:37
d54a2d6 to
9d016d8
Compare
Eli Bosley (elibosley)
added a commit
that referenced
this pull request
Jun 19, 2026
Generic progress headings ("Updating the container", "Install Plugin")
don't say what is being acted on, which is ambiguous when several
operations run at once. The target is already in scope at each call
site, so fold it into the title that openDocker/openPlugin render:
- docker update: "Update container: <name>"
- docker update all: "Updating all Containers (N)"
- plugin install: "Install Plugin: <plugin>" (Apps, Tailscale)
- language install: "Install Language: <pack>"
Names that aren't charset-constrained (plugin/language files) are reduced
to a sanitized basename slug before display, since the swal heading is
rendered with html:true. Container names rely on Docker's naming charset;
counts are numeric. Stays out of HeadInlineJS so it does not conflict with
the backend task-queue work in #2665.
OS-460
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Eli Bosley (elibosley)
added a commit
that referenced
this pull request
Jun 19, 2026
The foreground task modal drove its single button off an inverted per-type
`button` flag. For docker/vmaction (default button=0) this hid the Close
button for the entire running phase AND left the confirm button disabled,
which surfaced SweetAlert's `.la-ball-fall` loader (the "bouncing green
dots") with no way to background the operation.
Drive the modal by task status instead, uniformly for all types:
- running -> a top-corner minimize control backgrounds the task (it keeps
running under the daemon and stays in the tray); no disabled
button, so the bouncing-dots loader never appears. The
spinning title icon remains the in-progress indicator.
- done/err -> a single primary "Dismiss" button that fires the reload
callback and clears the task from the tray.
Esc while running now backgrounds (instead of deleting) since the
done-callback checks live status. No caller passes the `button` flag, so
no real behavior contract changes; `task.button` is now inert (follow-up:
drop it from the task record + open* signatures).
Stacked on the backend task queue (#2665).
OS-461
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Eli Bosley (elibosley)
added a commit
that referenced
this pull request
Jun 22, 2026
Generic progress headings ("Updating the container", "Install Plugin")
don't say what is being acted on, which is ambiguous when several
operations run at once. The target is already in scope at each call
site, so fold it into the title that openDocker/openPlugin render:
- docker update: "Update container: <name>"
- docker update all: "Updating all Containers (N)"
- plugin install: "Install Plugin: <plugin>" (Apps, Tailscale)
- language install: "Install Language: <pack>"
Names that aren't charset-constrained (plugin/language files) are reduced
to a sanitized basename slug before display, since the swal heading is
rendered with html:true. Container names rely on Docker's naming charset;
counts are numeric. Stays out of HeadInlineJS so it does not conflict with
the backend task-queue work in #2665.
OS-460
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Eli Bosley (elibosley)
added a commit
that referenced
this pull request
Jul 9, 2026
PR #2665 repurposed .nchan into the full-height sheet that hides the button bar when there's no confirm button. openChanges (Release Notes) and openAlert add .nchan but relied on a confirm button that isn't shown, so they rendered as sheets with no way to close (regression vs master, where .nchan was inert). Introduce decorateNchanSheet(): the single place every .nchan sheet gets its chrome -- the resize grip, the shared remembered width, and a corner close control. Task sheets minimize (keep the op in the tray); changelog/alert sheets close. foregroundTask, openChanges and openAlert all route through it, so the Release Notes/alert sheets are closable again and resizing is shared across all of them (one --nchan-sheet-width preference). Also scope .nchan-close (like .nchan-resize) to .nchan so a control left on the shared swal node never shows on an ordinary dialog. Renamed the width helpers/ var/localStorage key from task-modal-* to nchan-sheet-* to reflect the shared scope.
Eli Bosley (elibosley)
added a commit
that referenced
this pull request
Jul 14, 2026
Generic progress headings ("Updating the container", "Install Plugin")
don't say what is being acted on, which is ambiguous when several
operations run at once. The target is already in scope at each call
site, so fold it into the title that openDocker/openPlugin render:
- docker update: "Update container: <name>"
- docker update all: "Updating all Containers (N)"
- plugin install: "Install Plugin: <plugin>" (Apps, Tailscale)
- language install: "Install Language: <pack>"
Names that aren't charset-constrained (plugin/language files) are reduced
to a sanitized basename slug before display, since the swal heading is
rendered with html:true. Container names rely on Docker's naming charset;
counts are numeric. Stays out of HeadInlineJS so it does not conflict with
the backend task-queue work in #2665.
OS-460
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Eli Bosley (elibosley)
added a commit
that referenced
this pull request
Jul 14, 2026
PR #2665 repurposed .nchan into the full-height sheet that hides the button bar when there's no confirm button. openChanges (Release Notes) and openAlert add .nchan but relied on a confirm button that isn't shown, so they rendered as sheets with no way to close (regression vs master, where .nchan was inert). Introduce decorateNchanSheet(): the single place every .nchan sheet gets its chrome -- the resize grip, the shared remembered width, and a corner close control. Task sheets minimize (keep the op in the tray); changelog/alert sheets close. foregroundTask, openChanges and openAlert all route through it, so the Release Notes/alert sheets are closable again and resizing is shared across all of them (one --nchan-sheet-width preference). Also scope .nchan-close (like .nchan-resize) to .nchan so a control left on the shared swal node never shows on an ordinary dialog. Renamed the width helpers/ var/localStorage key from task-modal-* to nchan-sheet-* to reflect the shared scope.
Eli Bosley (elibosley)
force-pushed
the
feat/backend-task-queue
branch
from
July 14, 2026 19:46
4c77a6a to
c10a6fc
Compare
…round recall
Replace the single-backgrounded-task / single-banner model with a backend-owned
task queue shared across subsystems (plugins/Docker/VM) and all clients.
- New TaskQueue.php store + scheduler: one JSON file per task under
/var/local/emhttp/tasks, one-running-per-type invariant so the shared
/sub/{plugins,docker,vmaction} channels never interleave; extra same-type
ops are queued and auto-advanced.
- New TaskCommand.php endpoint (create/abort/dismiss/log/list); every mutation
rebroadcasts the list on /sub/tasks.
- New nchan/tasks daemon (mirrors nchan/file_manager): watches pids, marks
done/error, advances the queue, exits when idle.
- publish.php: env-var-gated (NCHAN_TASK) per-task log tee for foreground replay;
no-op on every other publish path.
- DefaultPageLayout/Head/BodyInlineJS: /sub/tasks subscriber, router/renderer
split of the live handlers, task tray; openPlugin/openDocker/openVMAction keep
their signatures.
- Tray DOM (#opTray) + theme-aware CSS.
Validated with php -l, node --check, and a sandboxed store-logic test. Not yet
runtime-tested on a live Unraid box (nchan pub/sub, real process launch).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The background-operation task tray was fixed at bottom:1rem with z-index 999, but #footer is position:fixed bottom:0 with z-index 10000, so the tray was painted behind and clipped by the footer in the bottom-right corner. Raise the tray's bottom offset above the footer using the same media queries that make the footer fixed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
createTask() called foregroundTask(res.id) immediately on the AJAX response, but foregroundTask() looks the task up in taskList, which is only populated asynchronously by the /sub/tasks websocket broadcast. When the broadcast lands after the AJAX response, taskById() returns null and foregroundTask() bails, so no modal opens even though the command runs and its output is captured server-side. This made actions like the plugin 'Check For Updates' appear to do nothing. Seed an optimistic taskList entry from the data we already have so the modal foregrounds immediately; onTaskListUpdate() reconciles it when the authoritative broadcast arrives. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
NchanSubscriber.start()/stop() throw if called when already running / not
running ('Can't stop NchanSubscriber, it's not running.'). stopAllTypeChannels()
called .stop() on all three type channels unconditionally, and foregroundTask()
runs it at the top before any channel is started, so the uncaught throw aborted
the function before the modal opened. Wrap start/stop in nchanStart()/nchanStop()
helpers that check the .running flag (with a try/catch safety net) and route the
type-channel transitions through them.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…tion Finished tasks previously only left the tray on manual dismissal or a 1-day prune that ran solely at daemon startup, so completed tasks piled up and users cleared them one by one. - Successful tasks now auto-expire 30s after they finish (TASK_DONE_TTL); the scheduler daemon lingers (1s cadence) while any success is pending expiry so it can prune and re-broadcast, then exits as before. - Failures persist (TASK_ERROR_TTL, ~1 day) so errors aren't swept away before they're noticed. - Add a 'Clear finished' tray header action (TaskCommand 'clear' -> task_clear_finished) shown when more than one finished task is present. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Per review, the manual 'Clear finished' tray action is enough; the 30s server-side auto-expiry and the daemon lingering to prune/re-broadcast add complexity we don't need. Restore the scheduler daemon and the done/error prune TTL to their original behavior (1-day prune on daemon startup) and keep only task_clear_finished() backing the tray button. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The bomb icon read as overly aggressive for aborting a running task; fa-stop-circle conveys 'stop' more calmly while keeping the meaning. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A running task only showed a small spinning icon. Add a CSS-only indeterminate bar sweeping along the bottom of the running card so it clearly reads as still active; respects prefers-reduced-motion. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Drop the indeterminate progress bar in favor of a simple loader: running tasks now use the fa-circle-o-notch spinner (clearer 'loading' read than the refresh arrows). Keeps the running indicator minimal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Apply robot review feedback for the backend task queue: - TaskQueue.php: escape the full bash -c payload with escapeshellarg() to close a single-quote shell-injection vector while preserving multi-arg word splitting; serialize create->launch per type with flock to keep the one-running-task-per-type invariant under concurrent requests. - nchan/tasks: require numeric pid before the /proc liveness check; recover queued-only tasks so the daemon never sleeps with pending work; match _ERROR_ as a discrete RS-delimited record (read tail in PHP). - TaskCommand.php: validate pid is numeric before kill. - BodyInlineJS.php: escape task.title in the swal title and task ids in the tray onclick handlers. - HeadInlineJS.php: surface a createTask() failure via .fail(). - default-base.css: add min-width:0 so .op-title ellipsizes reliably. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Completion was registered only by the `tasks` scheduler daemon noticing the launched PID disappear, which it can miss on PID reuse or a daemon-restart race (TOCTOU in task_daemon_start) — leaving a finished task stuck "in progress" until the next page-load nchan sweep. Have the launched command stamp its own result on exit: task_launch wraps the payload to capture the command's exit code and invoke task_complete, which marks the task done/error, advances the queue and broadcasts. This makes completion authoritative at the source. task_log_has_error moves into TaskQueue.php so the stamp and the daemon share it, and task_advance now takes the per-type lock so two advancers can't double-launch the next queued task. The daemon stays a fallback for hard-killed processes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…lds #2669, OS-461) Squash of feat/task-modal-buttons (PR #2669, 31 iteration commits) into the consolidated task-queue PR. Adds backgroundable running operations with a corrected modal button set, drives the plugin "Upgrading" button state from the server-side task queue, and a round of task-modal/tray styling (state strip, dismiss placement, mobile grouped-notification carousel). Touches only the tray/modal UI (BodyInlineJS, HeadInlineJS, default-base.css) and the Plugins page (Plugins.page, PluginHelpers.php); no changes to the core queue/daemon files. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Generic progress headings ("Updating the container", "Install Plugin")
don't say what is being acted on, which is ambiguous when several
operations run at once. The target is already in scope at each call
site, so fold it into the title that openDocker/openPlugin render:
- docker update: "Update container: <name>"
- docker update all: "Updating all Containers (N)"
- plugin install: "Install Plugin: <plugin>" (Apps, Tailscale)
- language install: "Install Language: <pack>"
Names that aren't charset-constrained (plugin/language files) are reduced
to a sanitized basename slug before display, since the swal heading is
rendered with html:true. Container names rely on Docker's naming charset;
counts are numeric. Stays out of HeadInlineJS so it does not conflict with
the backend task-queue work in #2665.
OS-460
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…one tasks
onTaskListUpdate fired a background task's one-shot completion callback
whenever it saw the task transition to done/error. But taskPrev resets to
{} on every page load, and /sub/tasks is an nchan channel that redelivers
its retained message to each new subscriber, so a task that was already
terminal before the page loaded looked like a fresh transition every load.
For reload-style callbacks (func:'refresh') this is an infinite loop: load
-> subscribe -> see stale done task as new -> refresh() -> reload -> repeat.
An OS-update task left as done with func:'refresh' pinned the entire webGUI
in a ~0.7s full-page reload cycle (screen flashing on every page).
Only fire the callback when we actually observed the task run and then
finish during this page's lifetime (prev !== undefined). A task already
terminal on first sight is stale: its effect is applied server-side and it
still shows in the tray for the user to dismiss.
The foreground task sheet (.sweet-alert.nchan) was a fixed 60rem. Add a right-edge grip to drag it wider; the width is remembered per browser and restored the next time the sheet opens. - Width is driven by a --task-modal-width CSS var scoped to .sweet-alert.nchan, not an inline style, so it never leaks onto the shared .sweet-alert node that other dialogs reuse (min(var, 90vw) keeps the 60rem default and viewport cap). - The sheet is centered (left:50% + translateX(-50%)), so its right edge tracks the pointer at width = 2*(pointerX - viewportCenterX); clamped to [~600, 90vw]. - Saved to localStorage on pointer release; re-applied on open via applyTaskModalWidth(). Grip hidden on mobile (sheet is fullscreen there).
PR #2665 repurposed .nchan into the full-height sheet that hides the button bar when there's no confirm button. openChanges (Release Notes) and openAlert add .nchan but relied on a confirm button that isn't shown, so they rendered as sheets with no way to close (regression vs master, where .nchan was inert). Introduce decorateNchanSheet(): the single place every .nchan sheet gets its chrome -- the resize grip, the shared remembered width, and a corner close control. Task sheets minimize (keep the op in the tray); changelog/alert sheets close. foregroundTask, openChanges and openAlert all route through it, so the Release Notes/alert sheets are closable again and resizing is shared across all of them (one --nchan-sheet-width preference). Also scope .nchan-close (like .nchan-resize) to .nchan so a control left on the shared swal node never shows on an ordinary dialog. Renamed the width helpers/ var/localStorage key from task-modal-* to nchan-sheet-* to reflect the shared scope.
…apper
QA found that aborting a tracked op (e.g. Docker Force Update) marked the task
error but let the underlying operation run to completion. task_launch spawned
the op with 'nohup bash -c', so the wrapper shell stayed in php-fpm's process
group; abort's 'kill <pid>' terminated only that shell and orphaned the real
worker (update_container -> docker pull/run), which finished seconds later.
Launch each task under setsid so it owns a session + process group (pid==pgid),
and have abort signal the whole group ('kill -TERM -<pid>') to stop the command
and every child it spawned. The bare-pid kill remains as a fallback.
…live dedupe Three QA findings against the shared task tray: 1. Completion callbacks with an empty plugin identifier never fired. The legacy modal fired (func||'loadlist')(plg) whenever plg was non-null, so Apps.page openPlugin(...,'','refresh'), CA's ca_openPlugin(...,'','OpenSidebarAndRefreshDisplay'), Tailscale install and Remove Selected Plugins all refreshed after the op. fireTaskCallback required a truthy plg, dropping every one of those: installing Community Apps left /Apps on the pre-install screen until a manual reload. Fire the callback whenever one was requested (plg or func non-empty); ':return' still suppresses it. 2. A running task inherited a stale Finished state. The modal streamed from the shared /sub/<type> channel, which does not identify the originating task and retains its last message forever (generic /pub/ channels use nchan_message_timeout 0): foregrounding a new running task consumed the PREVIOUS task's retained _DONE_ and flipped to Finished mid-run. 3. The replay/live handoff duplicated one record: the retained last message re-delivered on subscribe was already rendered by the log replay (30 server records -> 31 rows). publish.php now mirrors every captured message onto the task's own channel (task-<id>) tagged with the record's byte offset in the task log, taken under the log lock. The modal subscribes to /sub/task-<id> instead of the shared type channel, so cross-task leakage (2) is impossible, and it drops any live message whose offset falls inside the byte range the log replay covered -- TaskCommand's log action reports that range as X-Task-Log-Size, read under the shared lock -- making the handoff exact (3). The shared type channels are still published unchanged for external subscribers, and task_delete drops the mirrored channel so retained buffers don't accumulate in nchan shared memory.
Live QA on DGTest03 showed task_channel_delete was a no-op: the generic /pub/
location sets nchan_message_buffer_length $arg_buffer_length, and nginx errors
out ('missing nchan_message_buffer_length value') before nchan processes ANY
method when the arg is absent -- including DELETE. Pass buffer_length=1; with
it, DELETE returns 200 and the channel's retained messages are dropped
(verified live: message count 9 -> channel 404 after dismiss).
Eli Bosley (elibosley)
force-pushed
the
feat/backend-task-queue
branch
from
August 3, 2026 20:39
bac363c to
e000742
Compare
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.
Summary
This PR adds a server-owned background-task queue for plugin, Docker, and VM operations.
Users can queue operations, minimize them, and recall their live or completed output from any tab. The task tray shows all active and finished operations.
Why
The old WebGUI tracked one background operation in one browser tab. It could not restore output after the modal closed.
The first task-queue version captured output only from the WebGUI publish helper. Community Apps, Unraid Connect, and core plugin removal can publish directly to
/pub/plugins. These operations bypassed task capture and showed a blank foreground modal.Resolution
Reviewer considerations
Behavior changes
Implementation summary
TaskQueue.phpowns task state, locking, process identity, launch, completion, and capture.TaskCommand.phpowns the POST-only task actions and log replay.TaskCapture.phpreceives internal Nginx capture requests.nchan/taskshandles fallback completion, abort escalation, pruning, and queue advancement.rc.nginxroutes legacy and first-party publishers through the same capture path.Verification
Passed locally on head
bac363c074385e388bf113b19ddf87760804739d:bash -n etc/rc.d/rc.nginxgit diff --checkLive Unraid QA is still required:
_DONE_, and_ERROR_records.Risk
Medium. This change touches shared background-task lifecycle and Nginx/Nchan publication paths. The implementation fails closed for lifecycle state, but target integration requires live QA.
Related to OS-377.
Fixes OS-460.