feat(drive-health): manage collector with plugin lifecycle (#129)

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

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
 

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()

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

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")
 

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"),

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",