From 7bfe1af5b119c590c489e3a9c26131d29a1bd6bf Mon Sep 17 00:00:00 2001 From: Bernardo Magri Date: Mon, 13 Jul 2026 14:09:05 +0100 Subject: [PATCH] fix(power): coalesce charge-limit reapply events USB-C power event bursts could repeatedly restart and SIGTERM the settling oneshot until systemd reached its start limit. Let successful runs return inactive and use coalescing start jobs for immediate and delayed reapplication. Verified: V0 nix flake check --no-build, module parse, and git diff --check; V2 focused battery-charge-limit KVM test with a 12-event active-run burst. V3 hardware item #101 remains queued. --- agent/HARDWARE-QUEUE.md | 14 +++++++++ agent/JOURNAL.md | 15 ++++++++++ agent/MEMORY.md | 5 +++- docs/ROADMAP.md | 11 ++++--- flake.nix | 63 +++++++++++++++++++++++++++++++++++------ modules/nixos/power.nix | 13 +++++---- 6 files changed, 102 insertions(+), 19 deletions(-) diff --git a/agent/HARDWARE-QUEUE.md b/agent/HARDWARE-QUEUE.md index e25ad84..ab8d839 100644 --- a/agent/HARDWARE-QUEUE.md +++ b/agent/HARDWARE-QUEUE.md @@ -11,6 +11,20 @@ the **T14s** (webcam case). ## Any machine (dev box is fine) +- [ ] **#101 charge-limit service survives USB-C power-event bursts** — + after updating/rebuilding to the commit carrying this entry, clear the + old failure with `sudo systemctl reset-failed + nomarchy-battery-charge-limit.service`. Plug and unplug the powered + USB-C dock twice, allowing five seconds after the last transition. + **Pass:** `systemctl show nomarchy-battery-charge-limit.service -p + ActiveState -p Result` reports `ActiveState=inactive` and + `Result=success`; `systemctl is-failed + nomarchy-battery-charge-limit.service` is false; BAT0's + `charge_control_end_threshold` still matches the configured/live limit; + and `journalctl -b -u nomarchy-battery-charge-limit.service` contains + no new `SIGTERM`, `start-limit-hit`, or `Failed with result` lines after + the rebuild. Capture that journal plus `udevadm monitor + --subsystem-match=power_supply` if it fails. - [ ] **Docking recovery round 4 (#100, closed-lid BenQ sequence)** — after updating to the commit carrying this entry, run `nomarchy-home` and relogin. With the lid open, put visible windows on at least workspaces diff --git a/agent/JOURNAL.md b/agent/JOURNAL.md index 0f02a12..bba2503 100644 --- a/agent/JOURNAL.md +++ b/agent/JOURNAL.md @@ -19,6 +19,21 @@ Template: --- +## 2026-07-13 — #101 charge-limit AC-event burst recovery (this commit) +- **Task:** Fix `nomarchy-battery-charge-limit.service` failing when USB-C + dock power events arrive while its settling write is still active. +- **Did:** The oneshot now returns inactive after success; immediate and + delayed udev paths use coalescing `start` jobs instead of interrupting + `restart` jobs. Strengthened the focused VM with a 12-event Mains burst + during an extended active run and explicit failed/start-limit assertions. +- **Verified:** V0 `nix flake check --no-build`, module parse, and + `git diff --check`; V2 + `nix build .#checks.x86_64-linux.battery-charge-limit --no-link -L` + under KVM — the 12-event active-run burst finished inactive/success, + changed InvocationID, and produced no failed/start-limit journal result. +- **Pending:** V3 post-rebuild dock-power burst check in HARDWARE-QUEUE. +- **Next suggestion:** run V3 #101 alongside docking round 4, then #91. + ## 2026-07-13 — #100 dock transition recovery (this commit) - **Task:** Fix the real T14s dangling workspace, closed-lid cable-removal suspend/apparent crash, and BenQ audio miss after 7ebfab4. diff --git a/agent/MEMORY.md b/agent/MEMORY.md index 4ecc2e8..f953dbe 100644 --- a/agent/MEMORY.md +++ b/agent/MEMORY.md @@ -77,7 +77,10 @@ iteration would otherwise rediscover. examples to crib from: `distro-id` (boots + `switch-to-configuration dry-activate`), `hardware-toggles` (kernel cmdline/PAM assertions), `battery-charge-limit` (fake Mains adapter via `test_power`, real udev - uevent, `InvocationID` change proves the restart). + event burst while the oneshot is active; clean inactive result plus an + `InvocationID` change proves coalesced AC re-apply). AC udev hooks for a + settling oneshot must use `start`, never `restart`: USB-C docks emit event + bursts and restarts SIGTERM the in-flight pass into `start-limit-hit`. - Themed-desktop screenshots work headlessly: software-GL Hyprland (`LIBGL_ALWAYS_SOFTWARE` on virtio-gpu) + `machine.screenshot()` QMP dump — prototyped 2026-06-19, kept as the fallback for theme previews diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 2c03a5f..572e45e 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -595,15 +595,18 @@ Design/decision records and a running log of shipped work (items marked independent sysfs oneshot writes `charge_control_end_threshold`. Off by default; the installer scaffolds it commented-out on laptops. ✓ the threshold is now **re-applied on AC state changes** (a `services.udev` - rule on `SUBSYSTEM=="power_supply", ATTR{type}=="Mains"` restarts the + rule on `SUBSYSTEM=="power_supply", ATTR{type}=="Mains"` starts the oneshot via `systemctl --no-block`), closing the firmware-resets-on- unplug gap — the match is by adapter *type*, not kernel name - (AC/AC0/ADP1/ACAD vary), and `restart` (not `try-restart`) re-applies - even if the boot run was inactive. Eval-verified both ways (rule present + (AC/AC0/ADP1/ACAD vary). The successful oneshot returns inactive and + repeated `start` requests coalesce while it is applying; this avoids a + USB-C dock's event burst repeatedly terminating `restart` jobs and + tripping systemd's start limit. Eval-verified both ways (rule present with charge limit on / absent off), and **VM-verified the trigger** (`checks.battery-charge-limit`: the `test_power` module fakes a Mains adapter, toggling `ac_online` emits a real `power_supply` uevent, and the - udev rule restarts the oneshot — confirmed by a changed `InvocationID`). + udev rule starts the oneshot — a burst during an intentionally extended + active run stays successful and a changed `InvocationID` proves reapply). The sysfs *write* itself needs a real `charge_control_end_threshold`, so a final on-hardware check (a `sudo nixos-rebuild` + a physical unplug) remains — the dev box has both an `AC` Mains adapter and a `BAT0` diff --git a/flake.nix b/flake.nix index 2c089ca..c3251c9 100644 --- a/flake.nix +++ b/flake.nix @@ -688,7 +688,7 @@ }; # Runtime VM check for the battery-charge-limit re-apply: the udev - # rule must restart the oneshot on an AC state change (firmware can + # rule must start the oneshot on an AC state change (firmware can # clear the threshold on unplug). The `test_power` module fakes a # Mains adapter we can toggle to emit a real power_supply uevent — # so this exercises the trigger, not just the config wiring. (The @@ -697,7 +697,7 @@ # options live) + power.nix, so the VM stays minimal. battery-charge-limit = pkgs.testers.runNixOSTest { name = "nomarchy-battery-charge-limit"; - nodes.machine = { ... }: { + nodes.machine = { pkgs, ... }: { imports = [ ./modules/nixos/options.nix ./modules/nixos/power.nix ]; boot.kernelModules = [ "test_power" ]; # fake Mains + battery nomarchy.system.power = { @@ -705,6 +705,10 @@ laptop = true; batteryChargeLimit = 80; }; + # Keep the oneshot active long enough to prove that a burst of + # Mains events coalesces instead of interrupting it. + systemd.services.nomarchy-battery-charge-limit.serviceConfig.ExecStartPost = + "${pkgs.coreutils}/bin/sleep 3"; }; testScript = '' machine.wait_for_unit("multi-user.target") @@ -714,23 +718,64 @@ machine.succeed("udevadm settle") machine.succeed("grep -lx Mains /sys/class/power_supply/*/type") - # Let any add-event restart settle, then sample the invocation. - machine.succeed("sleep 2") + # Let boot/add-event starts and their delayed passes settle, + # then sample the last successful invocation. + machine.succeed("sleep 5") + machine.wait_until_succeeds( + 'test "$(systemctl show -p ActiveState --value ' + 'nomarchy-battery-charge-limit.service)" = inactive', + timeout=30, + ) before = machine.succeed( "systemctl show -p InvocationID --value " "nomarchy-battery-charge-limit.service" ).strip() - # Simulate an AC unplug → 'change' uevent on the Mains device. + # Simulate an AC unplug, wait until the oneshot is in its + # three-second active window, then emit a dock-like burst of + # real power_supply change events during that same invocation. machine.succeed("echo off > /sys/module/test_power/parameters/ac_online") machine.succeed("udevadm settle") - - # The udev rule must have restarted the oneshot (new InvocationID). machine.wait_until_succeeds( - 'test "$(systemctl show -p InvocationID --value ' - 'nomarchy-battery-charge-limit.service)" != "' + before + '"', + 'test "$(systemctl show -p ActiveState --value ' + 'nomarchy-battery-charge-limit.service)" = activating', + timeout=10, + ) + mains = machine.succeed( + "grep -lx Mains /sys/class/power_supply/*/type" + ).strip().removesuffix("/type") + machine.succeed( + f"for i in $(seq 1 12); do " + f"udevadm trigger --action=change {mains}; done; udevadm settle" + ) + + # The delayed starts have all fired, the service returned to a + # clean inactive state, and at least one new invocation ran. + machine.succeed("sleep 8") + machine.wait_until_succeeds( + 'test "$(systemctl show -p ActiveState --value ' + 'nomarchy-battery-charge-limit.service)" = inactive', timeout=30, ) + machine.fail( + "systemctl is-failed --quiet nomarchy-battery-charge-limit.service" + ) + machine.succeed( + 'test "$(systemctl show -p Result --value ' + 'nomarchy-battery-charge-limit.service)" = success' + ) + after = machine.succeed( + "systemctl show -p InvocationID --value " + "nomarchy-battery-charge-limit.service" + ).strip() + assert after != before, ( + f"battery charge limit was not re-invoked: {before}" + ) + service_log = machine.succeed( + "journalctl -b -u nomarchy-battery-charge-limit.service --no-pager" + ) + assert "start-limit-hit" not in service_log, service_log + assert "Failed with result" not in service_log, service_log ''; }; diff --git a/modules/nixos/power.nix b/modules/nixos/power.nix index 38f5280..67d0861 100644 --- a/modules/nixos/power.nix +++ b/modules/nixos/power.nix @@ -66,7 +66,6 @@ in path = [ pkgs.jq pkgs.coreutils ]; serviceConfig = { Type = "oneshot"; - RemainAfterExit = true; }; script = '' set -euo pipefail @@ -153,10 +152,14 @@ in # Writable threshold for the logged-in user (menu live apply) + AC # re-apply of the oneshot. Laptop only; no-op when the attr is absent. - # Immediate + delayed restart: Dell/Latitude firmware often stomps the + # Immediate + delayed start: Dell/Latitude firmware often stomps the # threshold to 100 right after unplug; a single immediate oneshot # loses the race (hardware: unplug→100, plug→80). systemd-run hands - # the delayed restart off so udev's short RUN budget is not held. + # the delayed start off so udev's short RUN budget is not held. Keep the + # oneshot inactive after success and use `start`, not `restart`: a USB-C + # dock can emit a burst of Mains change events, and restart would SIGTERM + # the in-flight one-second settling pass until systemd hits its start + # limit. Concurrent starts instead coalesce safely. services.udev.extraRules = lib.mkIf cfg.laptop ( let systemctl = "${config.systemd.package}/bin/systemctl"; @@ -166,8 +169,8 @@ in SUBSYSTEM=="power_supply", ATTR{type}=="Battery", TEST=="charge_types", GROUP="users", MODE="0664" SUBSYSTEM=="power_supply", ATTR{type}=="Battery", TEST=="charge_type", GROUP="users", MODE="0664" SUBSYSTEM=="power_supply", ATTR{type}=="Battery", TEST=="charge_control_start_threshold", GROUP="users", MODE="0664" - SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="${systemctl} --no-block restart nomarchy-battery-charge-limit.service" - SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="${systemdRun} --no-block --collect --on-active=2s --timer-property=AccuracySec=200ms ${systemctl} --no-block restart nomarchy-battery-charge-limit.service" + SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="${systemctl} --no-block start nomarchy-battery-charge-limit.service" + SUBSYSTEM=="power_supply", ATTR{type}=="Mains", RUN+="${systemdRun} --no-block --collect --on-active=2s --timer-property=AccuracySec=200ms ${systemctl} --no-block start nomarchy-battery-charge-limit.service" '' );