From 18c53cf63ada8da5a264acf6f6992eb13c6804e5 Mon Sep 17 00:00:00 2001 From: Philippe Matray Date: Sat, 5 Sep 2026 00:30:20 +0200 Subject: [PATCH] fix: a deliberate no-op exits 0, not 1 macarchy-auto-appearance and macarchy-bar-contrast each detected that their precondition was absent, said so, correctly declined to act -- and then returned 1. Both are timer-driven oneshots, so that parked them in `systemctl --user --failed` for ever and fired their OnFailure= notifier, on every machine with no coordinates set or no compositor installed. macarchy-install#9 carries the journals; they were two of the three units that had been failing there. This is not a CI artifact. Boot a real laptop before grim is installed, or before coordinates are set, and the same two units go permanently failed. auto-appearance: "macarchy-sun could not compute today's sun; theme left alone" is a decision, and the script ALREADY uses exit 0 for the sibling decision two screens down ("user chose something else; leave it alone"). The sun case now matches its own convention. A genuine failure still reds: the final `exec omarchy theme set` propagates that command's status. bar-contrast: a missing capture tool and an absent bar layer both mean there is nothing on screen to sample. A grim that EXISTS and then breaks, or a sample that comes back malformed, still exits 1 -- that side is unchanged and still pinned by the "bad sample exits 1" case. Three test cases asserted the old behaviour and are updated with the reasoning, because they encoded the bug: the invariant they defend is "it must not GUESS a theme", which is the `does not switch` assertion beside each of them, and that one is untouched. The error stays legible where an error belongs -- `status` still reports `error=sun`, and both messages still reach the journal. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SYBhT1xfp3MQ1w3687F4Mp --- style/macarchy-auto-appearance | 7 ++++++- style/macarchy-bar-contrast | 8 ++++++-- tests/test_auto_appearance.sh | 20 ++++++++++++++++---- tests/test_bar_contrast.sh | 12 ++++++++++++ 4 files changed, 40 insertions(+), 7 deletions(-) diff --git a/style/macarchy-auto-appearance b/style/macarchy-auto-appearance index e8aff39..31d37d0 100755 --- a/style/macarchy-auto-appearance +++ b/style/macarchy-auto-appearance @@ -93,8 +93,13 @@ if [[ $action == status ]]; then fi if [[ -z $want ]]; then + # A sun we cannot compute is a reason to do NOTHING, and doing nothing on + # purpose is not a failure. Exiting 1 here left the unit permanently in + # `systemctl --user --failed` and fired its OnFailure= notifier, on every + # machine with no coordinates yet -- see macarchy-install#9. The message still + # reaches the journal; only the verdict changes. echo "macarchy-auto-appearance: macarchy-sun could not compute today's sun; theme left alone" >&2 - exit 1 + exit 0 fi [[ $want == light ]] && want_theme=$LIGHT_THEME || want_theme=$DARK_THEME diff --git a/style/macarchy-bar-contrast b/style/macarchy-bar-contrast index fb43faa..04e2b13 100755 --- a/style/macarchy-bar-contrast +++ b/style/macarchy-bar-contrast @@ -120,11 +120,15 @@ settled_sample() { if [[ -z $sample ]]; then if [[ -z $CAPTURE_CMD ]]; then + # No capture tool means there is no session to sample -- nothing to tint, so + # nothing failed. Same reasoning as the missing bar layer below: exiting 1 + # for it left a permanently failed unit on every machine without a + # compositor (macarchy-install#9). A grim that EXISTS and breaks still reds. for tool in grim magick hyprctl jq; do - command -v "$tool" >/dev/null || { echo "macarchy-bar-contrast: $tool missing" >&2; exit 1; } + command -v "$tool" >/dev/null || { echo "macarchy-bar-contrast: $tool missing; nothing to sample" >&2; exit 0; } done geom=$(bar_geometry) - [[ $geom =~ ^-?[0-9]+,-?[0-9]+\ [0-9]+x[0-9]+$ ]] || { echo "macarchy-bar-contrast: no omarchy-bar layer" >&2; exit 1; } + [[ $geom =~ ^-?[0-9]+,-?[0-9]+\ [0-9]+x[0-9]+$ ]] || { echo "macarchy-bar-contrast: no omarchy-bar layer; nothing to sample" >&2; exit 0; } wait_for_aquarium fi sample=$(settled_sample) || exit 1 diff --git a/tests/test_auto_appearance.sh b/tests/test_auto_appearance.sh index b7ad9a6..4107ff1 100755 --- a/tests/test_auto_appearance.sh +++ b/tests/test_auto_appearance.sh @@ -69,9 +69,14 @@ check "polar night wants dark" called 'omarchy theme set Apple Glass' reset; theme apple-glass; export FAKE_SUN_FAIL=1 err=$(AUTO_APPEARANCE_NOW=12:00 "$SCRIPT" 2>&1); rc=$? -check "sun failure exits 1" [ "$rc" -eq 1 ] +# A sun it cannot compute means "leave the theme alone", which is a decision and +# not a failure. It used to exit 1, which parked the unit in `systemctl --user +# --failed` for ever and fired its OnFailure= notifier on every machine with no +# coordinates set (macarchy-install#9). Doing nothing still has to be SILENT in +# the exit status and LOUD in the journal, so all three of these matter. +check "a sun it cannot compute is a skip, not a failure" [ "$rc" -eq 0 ] check "sun failure changes nothing" not_called 'omarchy theme set' -check "sun failure says so" grep -q 'macarchy-sun' <<<"$err" +check "sun failure still says so" grep -q 'macarchy-sun' <<<"$err" # --- schedule mode ----------------------------------------------------------- reset; theme apple-glass; conf 'MODE=schedule' 'LIGHT_FROM=07:00' 'LIGHT_UNTIL=20:00' @@ -107,9 +112,13 @@ out=$("$SCRIPT" status) check "status polar omits times" [ "$out" = "mode=solar enabled=no want=light" ] # --- garbled macarchy-sun output does not silently force a theme --------------- +# The invariant these three inputs defend is "it must not GUESS a theme", and +# that is `does not switch`. The exit code was never the point: all three reach +# the same "no answer, changed nothing" branch, which exits 0 like the script's +# own "user chose something else" branch two screens down. reset; theme apple-glass; export FAKE_SUN='{"state":""}' err=$(AUTO_APPEARANCE_NOW=12:00 "$SCRIPT" 2>&1); rc=$? -check "empty state exits 1" [ "$rc" -eq 1 ] +check "empty state is a skip too" [ "$rc" -eq 0 ] check "empty state does not switch" not_called 'omarchy theme set' reset; theme apple-glass; export FAKE_SUN='{"state":""}' @@ -118,7 +127,10 @@ check "empty state status line" [ "$out" = "mode=solar enabled=no error=sun" ] reset; theme apple-glass; export FAKE_SUN='not json' err=$(AUTO_APPEARANCE_NOW=12:00 "$SCRIPT" 2>&1); rc=$? -check "garbled sun json exits 1" [ "$rc" -eq 1 ] +# Same branch as above, reached by a different input, so the same verdict: it +# changed nothing, so it did not fail. The error stays legible where an error +# belongs -- `status` reports `error=sun`, and the message is in the journal. +check "garbled sun json is a skip too" [ "$rc" -eq 0 ] check "garbled sun json does not switch" not_called 'omarchy theme set' reset; theme apple-glass; export FAKE_SUN='not json' diff --git a/tests/test_bar_contrast.sh b/tests/test_bar_contrast.sh index a8d8e2b..e8d8fb8 100644 --- a/tests/test_bar_contrast.sh +++ b/tests/test_bar_contrast.sh @@ -104,4 +104,16 @@ check "no give-up message while the layer is in flight" \ [ -z "$(grep -c 'no omarchy-bar layer' <<<"$out" | grep -v '^0$')" ] unset HCALLS +# --- nothing to sample is not a failure -------------------------------------- +# Same defect as auto-appearance's: a machine with no compositor has nothing to +# tint, and saying so with exit 1 parked the unit in `systemctl --user --failed` +# for ever (macarchy-install#9). A capture tool that EXISTS and then breaks must +# still red -- the "bad sample" case above pins that side. +mkdir -p "$TMP/nobin" +before=$(cat "$CONF") +err=$(PATH="$TMP/nobin" "$SCRIPT" 2>&1); rc=$? +check "no capture tool is a skip, not a failure" [ "$rc" -eq 0 ] +check "and it says why" grep -q 'nothing to sample' <<<"$err" +check "and it leaves the config untouched" [ "$(cat "$CONF")" = "$before" ] + echo; [[ $fails -eq 0 ]] && echo "all passed" || { echo "$fails failed"; exit 1; }