From 03431e77067f3767a61d0b06d98cc2211d96fa68 Mon Sep 17 00:00:00 2001 From: Philippe Matray Date: Sat, 5 Sep 2026 01:00:59 +0200 Subject: [PATCH 1/3] chore(#12): scaffold draft PR for the notifier ordering fix From 01d9422804a1e472b96a90ff9628b36c2b6ffc0f Mon Sep 17 00:00:00 2001 From: Philippe Matray Date: Sat, 5 Sep 2026 01:03:05 +0200 Subject: [PATCH 2/3] fix(install): make the failure notifier reachable from the first failure --- doctor.sh | 38 +++++++++++++++++++++-- install.sh | 20 ++++++++++-- tests/test_notifier_order.sh | 60 ++++++++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 5 deletions(-) create mode 100755 tests/test_notifier_order.sh diff --git a/doctor.sh b/doctor.sh index d2ebeb7..8c4f298 100755 --- a/doctor.sh +++ b/doctor.sh @@ -78,9 +78,41 @@ else skip "auto-appearance sun (no omarchy on this machine)" fi -# A muted check is worse than none, so the doctor checks that it can speak. -[[ -e $HOME/.config/systemd/user/macarchy-failed@.service ]] \ - && ok "failure notifier" || bad "failure notifier (macarchy-install/install.sh)" +# A muted check is worse than none, so the doctor checks that it can speak. Testing +# that the template FILE exists is not that test: it passed on every first install +# while the units naming it were being told "Unit macarchy-failed@….service not +# found", because the file arrived after they did (#12). Ask the units instead -- +# whoever declares OnFailure= is who has to be able to reach it. +declare -A _want=() +for u in "$HOME"/.config/systemd/user/*.service; do + [[ -f $u ]] || continue # no glob match + while read -r tmpl; do + [[ -n $tmpl ]] && _want["$tmpl"]="${_want["$tmpl"]:+${_want["$tmpl"]} }$(basename "$u")" + done < <(sed -nE 's/^OnFailure=([^ ]*@)%?[nNiIpP]?\.service.*/\1.service/p' "$u") +done +if (( ${#_want[@]} == 0 )); then + ok "failure notifier (nothing declares OnFailure= yet)" +else + _miss=() + for tmpl in "${!_want[@]}"; do + [[ -e $HOME/.config/systemd/user/$tmpl ]] || _miss+=("$tmpl needed by ${_want[$tmpl]}") + done + if (( ${#_miss[@]} )); then + bad "failure notifier: ${_miss[*]} (macarchy-install/install.sh)" + else + # COUNT THE UNITS, not the templates: all three daemons name the same + # macarchy-failed@.service, so ${#_want[@]} is 1 and saying "1 unit" would + # have under-reported the three this check actually covers. + _n=0; for tmpl in "${!_want[@]}"; do + read -ra _u <<<"${_want[$tmpl]}"; _n=$((_n + ${#_u[@]})) + done + if (( _n == 1 )); then + ok "failure notifier (1 unit declares it, reachable)" + else + ok "failure notifier ($_n units declare it, all reachable)" + fi + fi +fi systemctl --user is-enabled -q macarchy-doctor.service 2>/dev/null \ && ok "login self-check enabled" || bad "login self-check (macarchy-install/install.sh)" b=/sys/class/power_supply/macsmc-battery/charge_control_end_threshold diff --git a/install.sh b/install.sh index 57e6cbb..aab57fd 100755 --- a/install.sh +++ b/install.sh @@ -231,6 +231,23 @@ sudo pacman -S --needed --noconfirm \ # ----------------------------------------------------------------- repos +# Before every component, because three of them name it and none of them ships it: +# macarchy-touchbar.service, macarchy-auto-appearance.service and +# macarchy-bar-contrast.service all declare `OnFailure=macarchy-failed@%n.service`. +# It used to be installed with macarchy-doctor.service in the health section near +# the end -- a tidy grouping and the wrong constraint -- so on a FIRST install +# every failure before that point was dropped with "Failed to enqueue OnFailure= +# job, ignoring: Unit macarchy-failed@….service not found", and the one thing the +# notifier exists to catch (a daemon that dies silently) died silently. #12. +# +# A static file in this repo: no clone, no omarchy, no session. Nothing here has +# to wait for anything. +say "Installing the failure notifier the daemons name" +mkdir -p "$HOME/.config/systemd/user" +install -m644 systemd/macarchy-failed@.service "$HOME/.config/systemd/user/" \ + && note "macarchy-failed@.service in place before anything can name it" \ + || warn "could not install the failure notifier template" + say "Fetching the macarchy repos into $MACARCHY_DIR" mkdir -p "$MACARCHY_DIR" for r in "${REPOS[@]}"; do @@ -422,8 +439,7 @@ say "Wiring the machine's own health report" # it is worth having on PATH by hand anyway. mkdir -p "$HOME/.local/bin" "$HOME/.config/systemd/user" install -m755 doctor.sh "$HOME/.local/bin/macarchy-doctor" -install -m644 systemd/macarchy-doctor.service systemd/macarchy-failed@.service \ - "$HOME/.config/systemd/user/" +install -m644 systemd/macarchy-doctor.service "$HOME/.config/systemd/user/" systemctl --user daemon-reload # enable, not --now: firing it here would grade the half-installed state it is # standing in, and the tail of this script already runs ./doctor.sh once. diff --git a/tests/test_notifier_order.sh b/tests/test_notifier_order.sh new file mode 100755 index 0000000..051fa7c --- /dev/null +++ b/tests/test_notifier_order.sh @@ -0,0 +1,60 @@ +#!/bin/bash +# tests/test_notifier_order.sh — the failure notifier has to exist before anything +# can name it, and the doctor has to be able to tell reachable from merely present. +# +# Three units declare `OnFailure=macarchy-failed@%n.service`: macarchy-touchbar, +# macarchy-auto-appearance and macarchy-bar-contrast. All three are installed by +# their own repos, from the component steps in the middle of install.sh. The +# template that answers them used to be installed in the second-to-last section, +# so on a FIRST install every failure before that point was dropped with +# Failed to enqueue OnFailure= job, ignoring: Unit macarchy-failed@….service not found +# while doctor.sh reported `ok failure notifier` throughout -- it only tested that +# the file existed. #12. +set -uo pipefail +cd "$(dirname "$0")/.." + +fails=0 +check() { local name=$1; shift; if "$@"; then echo "ok $name"; else echo "FAIL $name"; fails=$((fails+1)); fi; } +lineno() { grep -n "$1" install.sh | head -1 | cut -d: -f1; } + +# --- ordering, read off install.sh itself ------------------------------------ +notifier=$(lineno 'systemd/macarchy-failed@\.service') +sanity=$(lineno '^say "Checking this machine"') +repos=$(lineno '^say "Fetching the macarchy repos') +core=$(lineno '^say "Installing the macarchy-core suite') + +check "the notifier install is found" [ -n "$notifier" ] +check "it comes after the sanity checks" [ "$notifier" -gt "$sanity" ] +check "it comes BEFORE the repo clone" [ "$notifier" -lt "$repos" ] +check "and before any component install" [ "$notifier" -lt "$core" ] + +# --- the doctor's check, exercised hermetically ------------------------------ +TMP=$(mktemp -d); trap 'rm -rf "$TMP"' EXIT +export HOME="$TMP/home" XDG_STATE_HOME="$TMP/state" +UNITS="$HOME/.config/systemd/user"; mkdir -p "$UNITS" "$TMP/bin" +export PATH="$TMP/bin:$PATH" +for stub in pgrep omarchy-aquarium-toggle systemctl journalctl omarchy; do + printf '#!/bin/bash\nexit 1\n' > "$TMP/bin/$stub" +done +chmod +x "$TMP/bin"/* + +# A unit that names the notifier, with no template beside it. +printf '[Unit]\nOnFailure=macarchy-failed@%%n.service\n' > "$UNITS/some-daemon.service" +out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) +check "a named-but-absent notifier is a MISS" grep -q 'MISS.*failure notifier' <<<"$out" +check "and it says which unit needs it" grep -q 'some-daemon' <<<"$out" + +# Same unit, template present. +printf '[Unit]\nDescription=notifier\n' > "$UNITS/macarchy-failed@.service" +out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) +check "reachable notifier is ok" grep -q 'ok.*failure notifier' <<<"$out" +check "and it says what it verified" grep -qE 'failure notifier \(1 unit' <<<"$out" + +# Nothing declares OnFailure= at all: say so rather than passing silently. +rm -f "$UNITS/some-daemon.service" +out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) +check "no declarations: says so" grep -q 'failure notifier (nothing declares' <<<"$out" +check "no declarations: not a MISS" bash -c '! grep -q "MISS.*failure notifier" <<<"$1"' _ "$out" + +(( fails == 0 )) && echo "all ok" || echo "$fails failed" +exit $(( fails > 0 )) From 4b1f7b7c7019a95c95b0e883397833757f0e7725 Mon Sep 17 00:00:00 2001 From: Philippe Matray Date: Sat, 5 Sep 2026 01:10:13 +0200 Subject: [PATCH 3/3] fix: address code-review findings --- doctor.sh | 74 ++++++++++++++++++++++-------------- install.sh | 4 ++ tests/test_notifier_order.sh | 50 +++++++++++++++++++----- 3 files changed, 90 insertions(+), 38 deletions(-) diff --git a/doctor.sh b/doctor.sh index 8c4f298..bd2705a 100755 --- a/doctor.sh +++ b/doctor.sh @@ -80,38 +80,54 @@ fi # A muted check is worse than none, so the doctor checks that it can speak. Testing # that the template FILE exists is not that test: it passed on every first install -# while the units naming it were being told "Unit macarchy-failed@….service not -# found", because the file arrived after they did (#12). Ask the units instead -- -# whoever declares OnFailure= is who has to be able to reach it. +# while the units naming it were told "Unit macarchy-failed@….service not found", +# because the file arrived after they did (#12). Ask the units instead. +# +# Three things this deliberately does NOT do, each of which it did in review: +# * scan every unit on the machine. doctor.sh:2 scopes this file to "the pieces +# install.sh puts in place"; jarvis, voxtype and aikit-sync also live in that +# directory, and blaming macarchy-install for a template THEY fail to ship is +# a false MISS that reds CI and points the user at the wrong installer. +# * look only under ~/.config. systemd resolves user units from /etc/systemd/user +# and /usr/lib/systemd/user too -- which is exactly where a PACKAGED notifier +# would land, so a package would otherwise make this check cry wolf at login. +# * read only the first target on the line. `OnFailure=a@%n.service b@%n.service` +# is legal, and stopping at the first is the silent pass this check exists to end. +_unit_dirs=("$HOME/.config/systemd/user" /etc/systemd/user /usr/lib/systemd/user) +_resolves() { # _resolves + local d; for d in "${_unit_dirs[@]}"; do [[ -e $d/$1 ]] && return 0; done; return 1 +} declare -A _want=() -for u in "$HOME"/.config/systemd/user/*.service; do - [[ -f $u ]] || continue # no glob match - while read -r tmpl; do - [[ -n $tmpl ]] && _want["$tmpl"]="${_want["$tmpl"]:+${_want["$tmpl"]} }$(basename "$u")" - done < <(sed -nE 's/^OnFailure=([^ ]*@)%?[nNiIpP]?\.service.*/\1.service/p' "$u") +for _d in "${_unit_dirs[@]}"; do + [[ -d $_d ]] || continue + # macarchy's own units only -- every unit type, and drop-ins, not just *.service. + for _u in "$_d"/macarchy-*.{service,timer,path,socket} "$_d"/macos-dynamic-wallpaper*.{service,timer} \ + "$_d"/macarchy-*.d/*.conf; do + [[ -f $_u ]] || continue # no glob match + while read -r _line; do + for _tgt in $_line; do # unquoted: split the list + # a@%n.service -> a@.service; a plain name stays itself + [[ $_tgt == *@* ]] && _tgt="${_tgt%@*}@.service" + _want["$_tgt"]="${_want["$_tgt"]:+${_want["$_tgt"]} }$(basename "$_u")" + done + done < <(sed -nE 's/^[[:space:]]*OnFailure=[[:space:]]*(.*)/\1/p' "$_u") + done +done +# The floor: install.sh installs the template unconditionally, so its absence is a +# MISS even when nothing happens to name it -- otherwise a run whose clones failed +# leaves no declaring units and the check congratulates itself. +_miss=() +_resolves "macarchy-failed@.service" || _miss+=("macarchy-failed@.service is not installed") +for _tmpl in "${!_want[@]}"; do + _resolves "$_tmpl" || _miss+=("$_tmpl needed by ${_want[$_tmpl]}") done -if (( ${#_want[@]} == 0 )); then - ok "failure notifier (nothing declares OnFailure= yet)" +if (( ${#_miss[@]} )); then + bad "failure notifier: ${_miss[*]} (macarchy-install/install.sh)" +elif (( ${#_want[@]} == 0 )); then + ok "failure notifier installed (no macarchy unit declares OnFailure= yet)" else - _miss=() - for tmpl in "${!_want[@]}"; do - [[ -e $HOME/.config/systemd/user/$tmpl ]] || _miss+=("$tmpl needed by ${_want[$tmpl]}") - done - if (( ${#_miss[@]} )); then - bad "failure notifier: ${_miss[*]} (macarchy-install/install.sh)" - else - # COUNT THE UNITS, not the templates: all three daemons name the same - # macarchy-failed@.service, so ${#_want[@]} is 1 and saying "1 unit" would - # have under-reported the three this check actually covers. - _n=0; for tmpl in "${!_want[@]}"; do - read -ra _u <<<"${_want[$tmpl]}"; _n=$((_n + ${#_u[@]})) - done - if (( _n == 1 )); then - ok "failure notifier (1 unit declares it, reachable)" - else - ok "failure notifier ($_n units declare it, all reachable)" - fi - fi + _n=0; for _tmpl in "${!_want[@]}"; do read -ra _u <<<"${_want[$_tmpl]}"; _n=$((_n + ${#_u[@]})); done + ok "failure notifier ($_n macarchy unit$( ((_n>1)) && printf s) declare$( ((_n==1)) && printf s) it, all reachable)" fi systemctl --user is-enabled -q macarchy-doctor.service 2>/dev/null \ && ok "login self-check enabled" || bad "login self-check (macarchy-install/install.sh)" diff --git a/install.sh b/install.sh index aab57fd..691e858 100755 --- a/install.sh +++ b/install.sh @@ -247,6 +247,10 @@ mkdir -p "$HOME/.config/systemd/user" install -m644 systemd/macarchy-failed@.service "$HOME/.config/systemd/user/" \ && note "macarchy-failed@.service in place before anything can name it" \ || warn "could not install the failure notifier template" +# Not left to systemd's unit-directory mtime rescan: it is reliable in practice, +# but the whole point of this block is an ordering GUARANTEE, and one reload makes +# it hold unconditionally. The doctor step further down does the same after its own. +systemctl --user daemon-reload 2>/dev/null || true say "Fetching the macarchy repos into $MACARCHY_DIR" mkdir -p "$MACARCHY_DIR" diff --git a/tests/test_notifier_order.sh b/tests/test_notifier_order.sh index 051fa7c..69c1d3f 100755 --- a/tests/test_notifier_order.sh +++ b/tests/test_notifier_order.sh @@ -38,23 +38,55 @@ for stub in pgrep omarchy-aquarium-toggle systemctl journalctl omarchy; do done chmod +x "$TMP/bin"/* -# A unit that names the notifier, with no template beside it. -printf '[Unit]\nOnFailure=macarchy-failed@%%n.service\n' > "$UNITS/some-daemon.service" +T="$UNITS/macarchy-failed@.service" + +# A macarchy unit that names the notifier, with no template beside it. +printf '[Unit]\nOnFailure=macarchy-failed@%%n.service\n' > "$UNITS/macarchy-probe.service" out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) check "a named-but-absent notifier is a MISS" grep -q 'MISS.*failure notifier' <<<"$out" -check "and it says which unit needs it" grep -q 'some-daemon' <<<"$out" +check "and it says which unit needs it" grep -q 'macarchy-probe' <<<"$out" # Same unit, template present. -printf '[Unit]\nDescription=notifier\n' > "$UNITS/macarchy-failed@.service" +printf '[Unit]\nDescription=notifier\n' > "$T" out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) check "reachable notifier is ok" grep -q 'ok.*failure notifier' <<<"$out" -check "and it says what it verified" grep -qE 'failure notifier \(1 unit' <<<"$out" +check "and it says what it verified" grep -qE 'failure notifier \(1 macarchy unit' <<<"$out" + +# A SPACE-SEPARATED list: systemd allows it, and stopping at the first target is +# the silent pass this check exists to end. +printf '[Unit]\nOnFailure=macarchy-failed@%%n.service other-notify@%%n.service\n' > "$UNITS/macarchy-probe.service" +out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) +check "the second target is checked too" grep -q 'other-notify@.service needed by' <<<"$out" + +# A THIRD-PARTY unit is none of macarchy's business: jarvis, voxtype and aikit-sync +# all live in this directory, and blaming macarchy-install for a template THEY do +# not ship is a false MISS that reds CI and names the wrong installer. +printf '[Unit]\nOnFailure=macarchy-failed@%%n.service\n' > "$UNITS/macarchy-probe.service" +printf '[Unit]\nOnFailure=somebody-elses@%%n.service\n' > "$UNITS/voxtype.service" +out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) +check "a third-party unit is not our MISS" bash -c '! grep -q "somebody-elses" <<<"$1"' _ "$out" +check "and ours still reports ok" grep -q 'ok.*failure notifier' <<<"$out" +rm -f "$UNITS/voxtype.service" -# Nothing declares OnFailure= at all: say so rather than passing silently. -rm -f "$UNITS/some-daemon.service" +# A TIMER declaring it counts: this suite ships macarchy-*.timer units. +printf '[Unit]\nOnFailure=macarchy-failed@%%n.service\n' > "$UNITS/macarchy-probe.timer" +out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) +check "a .timer is scanned too" grep -qE 'failure notifier \(2 macarchy units' <<<"$out" +rm -f "$UNITS/macarchy-probe.timer" + +# Leading whitespace is legal in a unit file. +printf '[Unit]\n OnFailure=macarchy-failed@%%n.service\n' > "$UNITS/macarchy-probe.service" +out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) +check "indented OnFailure= is parsed" grep -qE 'failure notifier \(1 macarchy unit' <<<"$out" + +# Nothing declares it -- but the template's own absence is STILL a MISS. Otherwise a +# run whose repo clones failed leaves no units and the check congratulates itself. +rm -f "$UNITS/macarchy-probe.service" +out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) +check "no declarations, template present: ok" grep -q 'no macarchy unit declares' <<<"$out" +rm -f "$T" out=$(WAYLAND_DISPLAY= ./doctor.sh 2>&1) -check "no declarations: says so" grep -q 'failure notifier (nothing declares' <<<"$out" -check "no declarations: not a MISS" bash -c '! grep -q "MISS.*failure notifier" <<<"$1"' _ "$out" +check "no declarations, no template: MISS" grep -q 'MISS.*is not installed' <<<"$out" (( fails == 0 )) && echo "all ok" || echo "$fails failed" exit $(( fails > 0 ))