From c21c569be1a55c1c41c4ad284f71706926679a04 Mon Sep 17 00:00:00 2001 From: Gustavo Rosa Date: Tue, 28 Jul 2026 22:17:07 -0300 Subject: [PATCH] feat(drive-health): manage collector with plugin lifecycle (#129) --- drive-health/Makefile | 1 + drive-health/README.md | 19 ++++- drive-health/collector.luau | 94 ++++++++++++++++++++++++ drive-health/plugin.toml | 4 +- drive-health/tests/collector_harness.lua | 65 ++++++++++++++++ drive-health/tests/panel_harness.lua | 17 +++-- drive-health/translations/en.json | 1 + 7 files changed, 191 insertions(+), 10 deletions(-) diff --git a/drive-health/Makefile b/drive-health/Makefile index 025b4b9..8d5c4ce 100644 --- a/drive-health/Makefile +++ b/drive-health/Makefile @@ -16,6 +16,7 @@ unit: lua tests/collector_harness.lua raw-cache lua tests/collector_harness.lua outdated-raw-cache lua tests/collector_harness.lua collector-disabled + lua tests/collector_harness.lua lifecycle lua tests/alert_harness.lua lua tests/history_harness.lua lua tests/panel_harness.lua diff --git a/drive-health/README.md b/drive-health/README.md index 801b270..1192852 100644 --- a/drive-health/README.md +++ b/drive-health/README.md @@ -51,6 +51,14 @@ use the same dialog. Disabling Full SMART in settings makes Drive Health ignore the collector cache; use **Stop background service** to stop an installed timer as well. +Plugin lifecycle actions also reconcile the installed collector. Enabling or +re-enabling Drive Health starts it when Full SMART is enabled, disabling the +plugin pauses it, and uninstalling an enabled plugin removes it. Each privileged +action uses the desktop authorization dialog. Reloading Drive Health or stopping +Noctalia leaves the system timer unchanged. If authorization is cancelled or +fails, the collector remains in its prior state and Drive Health reports the +failure when its runtime is still available. + Expand a drive for detailed counters, trend history, per-drive preferences, and SMART self-tests. A self-test requires explicit confirmation and a Polkit authorization prompt, then runs in the background while progress and its final @@ -124,9 +132,14 @@ self-tests are separate, explicitly authorized firmware operations. They can take minutes or hours, may increase drive activity, and should not be confused with filesystem repair or data recovery. -To remove the optional collector, use **Remove collector** in its controls and -approve the desktop authorization dialog. Removing the Noctalia entry alone -does not silently remove system files. +To remove the optional collector explicitly, use **Remove collector** in its +controls and approve the desktop authorization dialog. Uninstalling Drive Health +while it is enabled requests the same cleanup. Because the plugin cannot wait +for an authorization dialog after its runtime is destroyed, a cancelled or +failed uninstall authorization leaves the collector installed; reinstall the +plugin and use **Remove collector** to retry. If the plugin was disabled first, +its service is no longer running and cannot receive the uninstall event; use +the collector control before disabling in that sequence. ## Development diff --git a/drive-health/collector.luau b/drive-health/collector.luau index 4e39f53..0764545 100644 --- a/drive-health/collector.luau +++ b/drive-health/collector.luau @@ -1203,6 +1203,100 @@ function onConfigChanged() collect() end +local function lifecycleActionError(result) + if result.timedOut == true then + return noctalia.tr("privileged_action.timeout") + end + if tonumber(result.exitCode) == 126 then + return noctalia.tr("privileged_action.cancelled") + end + local detail = noctalia.string.trim(tostring(result.stderr or "")) + if detail == "" then detail = noctalia.string.trim(tostring(result.stdout or "")) end + if #detail > 240 then detail = detail:sub(1, 237) .. "..." end + return detail ~= "" and detail or noctalia.tr("privileged_action.command_failed") +end + +local function notifyLifecycleFailure(action, detail) + noctalia.log("Drive Health could not " .. action .. ": " .. detail) + noctalia.notifyError(noctalia.tr("collector.title"), noctalia.tr("privileged_action.failed", { + action = action, + error = detail, + })) +end + +local function launchLifecycleAction(command, action, observeResult) + if not noctalia.commandExists("pkexec") then + notifyLifecycleFailure(action, noctalia.tr("collector.authorization_required")) + return + end + + local launched + if observeResult then + launched = noctalia.runAsync(command, function(result) + if result.timedOut == true or tonumber(result.exitCode) ~= 0 then + notifyLifecycleFailure(action, lifecycleActionError(result)) + end + end, 120000) + else + -- onExit destroys this VM as soon as the callback returns. Launch without + -- a completion callback so the authorization request survives teardown. + launched = noctalia.runAsync(command) + end + if not launched then + notifyLifecycleFailure(action, noctalia.tr("privileged_action.launch_failed")) + end +end + +local function legacyUninstallCommand() + -- Older collector installations did not copy an uninstaller into libexec. + -- Keep this fallback independent of plugin files because those are removed + -- immediately after an uninstall hook returns. + local serviceName = SYSTEM_NAMESPACE + local cleanup = "systemctl disable --now " .. serviceName .. ".timer 2>/dev/null || true; " + .. "systemctl stop " .. serviceName .. ".service 2>/dev/null || true; " + .. "rm -f /etc/systemd/system/" .. serviceName .. ".service " + .. "/etc/systemd/system/" .. serviceName .. ".timer; " + .. "rm -rf /etc/systemd/system/" .. serviceName .. ".timer.d " + .. "/usr/local/libexec/" .. serviceName .. " /run/" .. serviceName .. "; " + .. "systemctl daemon-reload; " + .. "systemctl reset-failed " .. serviceName .. ".service 2>/dev/null || true" + return "pkexec /bin/sh -c " .. shellQuote(cleanup) +end + +function onEnable() + local manageScript = SYSTEM_LIBEXEC .. "/manage-collector.sh" + if systemCollectorEnabled() and noctalia.fileExists(manageScript) then + launchLifecycleAction( + "pkexec " .. shellQuote(manageScript) .. " start", + noctalia.tr("collector.action_start"), + true + ) + end +end + +function onExit(_signal, reason) + if reason == "disable" then + local manageScript = SYSTEM_LIBEXEC .. "/manage-collector.sh" + if noctalia.fileExists(manageScript) then + launchLifecycleAction( + "pkexec " .. shellQuote(manageScript) .. " pause", + noctalia.tr("collector.action_pause"), + false + ) + end + elseif reason == "uninstall" then + local installedCollector = SYSTEM_LIBEXEC .. "/collect_raw.sh" + if not noctalia.fileExists(installedCollector) then + return + end + local uninstallScript = SYSTEM_LIBEXEC .. "/uninstall-collector.sh" + local command = noctalia.fileExists(uninstallScript) + and "pkexec " .. shellQuote(uninstallScript) + or legacyUninstallCommand() + launchLifecycleAction(command, noctalia.tr("collector.action_remove"), false) + end +end + function onIpc(event, _payload) if event == "check-dependencies" then forceDependencyProbe() diff --git a/drive-health/plugin.toml b/drive-health/plugin.toml index 9866493..1837f82 100644 --- a/drive-health/plugin.toml +++ b/drive-health/plugin.toml @@ -1,7 +1,7 @@ id = "gustav0ar/drive-health" name = "Drive Health" -version = "1.2.4" -plugin_api = 9 +version = "1.2.5" +plugin_api = 17 author = "Drive Health contributors" license = "MIT" deprecated = false diff --git a/drive-health/tests/collector_harness.lua b/drive-health/tests/collector_harness.lua index 840cb1c..8699e66 100644 --- a/drive-health/tests/collector_harness.lua +++ b/drive-health/tests/collector_harness.lua @@ -13,6 +13,8 @@ local watchers = {} local pendingProbeCallback = nil local probeCalls = 0 local probeAction = nil +local failNextLaunch = false +local nextAsyncResult = nil local collectorEnabled = mode == "raw-cache" or mode == "outdated-raw-cache" local available = { @@ -75,6 +77,7 @@ noctalia = { writeFile = function(path, contents) files[path] = contents return true end, log = function(message) table.insert(logs, message) end, notify = function(title, body) table.insert(notifications, { title = title, body = body }) end, + notifyError = function(title, body) table.insert(notifications, { title = title, body = body, error = true }) end, tr = translate, formatTime = function(_pattern, _epoch) return "22:13:20" end, setUpdateInterval = function(_milliseconds) end, @@ -96,6 +99,19 @@ noctalia = { runAsync = function(command, callback, _timeout) launchedCommand = command table.insert(launchedCommands, command) + if failNextLaunch then + failNextLaunch = false + return false + end + if callback == nil then + return true + end + if nextAsyncResult ~= nil then + local result = nextAsyncResult + nextAsyncResult = nil + callback(result) + return true + end if command:match("lsblk=ok") then probeCalls = probeCalls + 1 if probeAction == "pending" then @@ -147,6 +163,55 @@ source = source:gsub("local function publishError", "function publishError") source = source:gsub("([%a_][%w_]*) %+%= ([^\n]+)", "%1 = %1 + %2") assert(load(source, "@collector.luau"))() +if mode == "lifecycle" then + collectorEnabled = true + files["/usr/local/libexec/noctalia-drive-health/collect_raw.sh"] = "installed" + files["/usr/local/libexec/noctalia-drive-health/manage-collector.sh"] = "installed" + files["/usr/local/libexec/noctalia-drive-health/uninstall-collector.sh"] = "installed" + launchedCommands = {} + + onEnable() + assert(launchedCommands[1] + == "pkexec '/usr/local/libexec/noctalia-drive-health/manage-collector.sh' start", + "plugin enable did not start the installed collector") + + onExit(0, "disable") + assert(launchedCommands[2] + == "pkexec '/usr/local/libexec/noctalia-drive-health/manage-collector.sh' pause", + "plugin disable did not pause the installed collector") + + onExit(0, "uninstall") + assert(launchedCommands[3] + == "pkexec '/usr/local/libexec/noctalia-drive-health/uninstall-collector.sh'", + "plugin uninstall did not remove the installed collector") + + files["/usr/local/libexec/noctalia-drive-health/uninstall-collector.sh"] = nil + onExit(0, "uninstall") + assert(launchedCommands[4]:match("^pkexec /bin/sh %-c ") + and launchedCommands[4]:match("/usr/local/libexec/noctalia%-drive%-health") + and not launchedCommands[4]:match("/mock/plugin"), + "legacy uninstall fallback depends on files removed with the plugin") + + local notificationCount = #notifications + failNextLaunch = true + onExit(0, "disable") + assert(#notifications == notificationCount + 1 and notifications[#notifications].error == true, + "detached lifecycle launch failure was not reported") + + nextAsyncResult = { exitCode = 126, stdout = "", stderr = "", timedOut = false } + onEnable() + assert(#notifications == notificationCount + 2 + and logs[#logs]:match("privileged_action%.cancelled"), + "onEnable authorization cancellation was not reported") + + onExit(0, "reload") + onExit(15, "shutdown") + assert(#launchedCommands == 6, "reload or shutdown changed the collector service") + + print("collector lifecycle test passed") + return +end + local snapshot = assert(state.collector_snapshot, "collector did not publish an initialization snapshot") local dependencies = assert(snapshot.dependencies, "snapshot has no dependency state") diff --git a/drive-health/tests/panel_harness.lua b/drive-health/tests/panel_harness.lua index 37e9eb4..65445cb 100644 --- a/drive-health/tests/panel_harness.lua +++ b/drive-health/tests/panel_harness.lua @@ -167,6 +167,13 @@ local function findNodeWithProp(value, kind, property, expected) return nil end +local function clickNodeWithProp(value, kind, property, expected) + local target = assert(findNodeWithProp(value, kind, property, expected), + "could not find " .. kind .. " with " .. property .. "=" .. tostring(expected)) + assert(type(target.props.onClick) == "function", "matching node has no closure callback") + target.props.onClick() +end + onOpen({}) assert(rendered ~= nil and not containsText(rendered, "collector.title"), "healthy collector consumed panel space") @@ -223,7 +230,7 @@ state.snapshot.system_collector.enabled = true state.snapshot.system_collector.status = "healthy" state.snapshot.system_collector.helper_available = true watchers.snapshot(state.snapshot) -onDrive1Clicked() +clickNodeWithProp(rendered, "button", "tooltip", "panel.expand") assert(containsText(rendered, "self_test.title"), "expanded self-test card did not render") assert(containsText(rendered, "metrics.mounted_at / · /home/example"), "expanded drive card omitted its mounted folders") @@ -241,10 +248,10 @@ assert(testProgress.props.progress == 0.37 and testProgress.props.value == nil, "running self-test used an invalid progress property") assert(containsText(rendered, "history.title"), "expanded history graph did not render") assert(containsText(rendered, "preferences.edit"), "drive preference action did not render") -onDrive1Clicked() +clickNodeWithProp(rendered, "button", "tooltip", "panel.collapse") assert(not containsText(rendered, "self_test.title"), "drive details did not collapse") assert(not containsText(rendered, "history.title"), "drive history remained visible after collapse") -onDrive1Clicked() +clickNodeWithProp(rendered, "button", "tooltip", "panel.expand") state.snapshot.disks[1].self_test_state = "passed" state.snapshot.disks[1].self_test_status = "Previous test passed" state.snapshot.disks[1].self_test_completion_percent = nil @@ -364,7 +371,7 @@ watchers.snapshot(state.snapshot) assert(containsText(rendered, "alerts.dismiss_all"), "dismiss-all alert action did not render") assert(findNodeWithProp(rendered, "button", "tooltip", "alerts.dismiss") ~= nil, "per-alert dismiss action did not render") -onDismissAlert1Clicked() +clickNodeWithProp(rendered, "button", "tooltip", "alerts.dismiss") assert(state.dismiss_alert_request.id == "SERIAL1:temperature", "per-alert dismiss action targeted the wrong issue") onDismissAllAlertsClicked() @@ -377,7 +384,7 @@ assert(not containsText(rendered, "alerts.active_title") and findNodeWithProp(rendered, "button", "tooltip", "alerts.dismiss") == nil, "empty alert state kept an alert card or dismiss controls") -onDrive2Clicked() +clickNodeWithProp(rendered, "button", "tooltip", "panel.expand") assert(containsText(rendered, "metrics.start_stop_count") and containsText(rendered, "metrics.load_cycle_count") and containsText(rendered, "metrics.interface_crc_errors"), diff --git a/drive-health/translations/en.json b/drive-health/translations/en.json index da0782e..249f30d 100644 --- a/drive-health/translations/en.json +++ b/drive-health/translations/en.json @@ -41,6 +41,7 @@ "action_pause": "pause the collector", "action_remove": "remove the collector", "action_start": "start the collector", + "authorization_required": "Polkit (pkexec) is required for the collector lifecycle action. Install your distribution's polkit package.", "basic_features": "Drive discovery, mounted folders, storage use, and temperatures exposed by Linux.", "basic_title": "Basic monitoring — no elevated service", "copy_install": "Copy command",