Never notify about pending migrations during an update

The retired omarchy-update-user-notify.path stays loaded in sessions that started before the update removing it, and pacman writes the migrations directory mid-transaction, so it fired a critical toast for migrations that omarchy-migrate was about to apply a step later. Migration 1785095882 stops that watcher, but migrations run after pacman, so it lands 11 seconds too late to prevent the toast it exists to retire. Check the lock omarchy-update holds for its whole pipeline instead of trusting that no trigger exists. That covers the stale watcher and anything added later: during an update every pending migration is by definition already being applied. The check repeats after waiting for the notification server, which is long enough for an update to start underneath it. Only this user's runtime directory is read, never the /tmp path the updater falls back to without XDG_RUNTIME_DIR. A shared lock file belongs to whoever created it first, so honouring it would let one user silence another user's notification; a redundant toast is the better failure. The sleep inhibitor now starts with the lock descriptor closed. It outlives the step that starts it, so an update killed before restore_update_inhibitors left it holding the flock indefinitely. That already blocked later updates, and now that the notifier reads the same lock it would have silenced migration notices at every login. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-27 09:41:03 -07:00
parent 4d93ad588f
commit 03902f2460
6 changed files with 159 additions and 2 deletions
@@ -4,6 +4,19 @@

 set -euo pipefail

+update_in_progress() {
+  local lock="${XDG_RUNTIME_DIR:-}/omarchy-update.lock"
+
+  [[ -n ${XDG_RUNTIME_DIR:-} && -f $lock ]] || return 1
+
+  # flock -n only fails here when the lock is held, since the file is ours.
+  ! flock -n "$lock" true 2>/dev/null
+}
+
+if update_in_progress; then
+  exit 0
+fi
+
 pending_migrations=$(omarchy-migrate --pending 2>/dev/null) || exit 0
 pending_count=$(printf '%s\n' "$pending_migrations" | sed '/^[[:space:]]*$/d' | wc -l)

@@ -21,6 +34,12 @@ notify_command=$(printf 'if [[ -n $(omarchy-notification-send -u critical -g 
 # void and the user never learns about their pending migrations.
 omarchy-notification-wait || true

+# That wait is long enough for an update to start underneath us, and the count
+# above is already stale by then, so re-check before spending the toast.
+if update_in_progress; then
+  exit 0
+fi
+
 unit="omarchy-migrations-notification-$(date +%Y%m%d%H%M%S)"
 systemd-run --user --scope --unit="$unit" bash -lc "$notify_command" >/dev/null 2>&1 && exit 0

@@ -45,7 +45,7 @@ disable_sleep_for_update() {
    --who=omarchy-update \
    --why="Omarchy update in progress" \
    --mode=block \
-    sleep infinity >/dev/null 2>&1 &
+    sleep infinity >/dev/null 2>&1 {OMARCHY_UPDATE_LOCK_FD}>&- &
  sleep_inhibit_pid=$!
 }

@@ -52,6 +52,9 @@ Every graphical login starts `omarchy-migrate-notify.service` after
 omarchy-migrate --pending
 ```

+It stays silent while `omarchy update` holds its lock, since that update applies
+the pending migrations itself.
+
 If that user has pending migrations, it shows a notification that opens a
 terminal for:

@@ -163,6 +163,28 @@ transaction inside a normal `omarchy update`, so it fired notifications for
 migrations that `omarchy-migrate` was about to apply in the visible update
 terminal. The retired unit was `omarchy-update-user-notify.path`.

+Retiring that watcher through a migration cannot come in time for the update
+that retires it: pacman writes the migration directory, the watcher fires, and
+only then does `omarchy-migrate` reach the migration that stops it. So the
+notifier also refuses to run while `omarchy update` holds its
+`$XDG_RUNTIME_DIR/omarchy-update.lock`, which covers the stale watcher and any
+trigger added later — during an update, every pending migration is by
+definition already being applied a step away. It checks again after waiting for
+the notification server, since that wait is long enough for an update to start
+underneath it.
+
+The notifier reads only its own user's runtime directory, never the `/tmp` path
+`omarchy-update` falls back to when `XDG_RUNTIME_DIR` is unset. A shared lock
+file belongs to whoever created it first, so honouring it would let one user
+silence another user's notification. Missing an update and showing a redundant
+toast is the better failure.
+
+Suppression is why `omarchy-update` starts its sleep inhibitor with the lock
+descriptor closed. That inhibitor outlives the step that starts it, so an update
+killed before `restore_update_inhibitors` would otherwise leave it holding the
+flock indefinitely — blocking later updates and, now that the notifier reads the
+same lock, silencing migration notifications at every login.
+
 Fallbacks:

 - `omarchy-first-run` enables `omarchy-migrate-notify.service`, which also
@@ -33,9 +33,29 @@ bash -c "$command"
 SH
 chmod +x "$stub_bin/systemd-run"

+# Waiting for the notification server is the notifier's one long pause, so it is
+# also where an update can start underneath it. Stand one up from inside the
+# wait to prove the notifier re-checks afterwards instead of sending a toast it
+# decided to send before the update existed.
 cat >"$stub_bin/omarchy-notification-wait" <<'SH'
 #!/bin/bash
-exit 0
+[[ ${OMARCHY_TEST_LOCK_DURING_WAIT:-0} == 1 ]] || exit 0
+
+lock="$XDG_RUNTIME_DIR/omarchy-update.lock"
+: >"$lock"
+# Hold the lock through a bash-allocated descriptor rather than `flock <file>
+# <command>`: bash marks those close-on-exec, so the holder owns the lock alone
+# and killing it releases immediately, with no exec'd child to outlive it.
+bash -c 'exec {fd}>"$1"; flock -n $fd || exit 1; sleep 60' _ "$lock" &
+echo "$!" >"$OMARCHY_TEST_LOCK_HOLDER_PID"
+
+for _ in {1..200}; do
+  flock -n "$lock" true 2>/dev/null || exit 0
+  sleep 0.05
+done
+
+echo "stub could not establish the update lock" >&2
+exit 1
 SH
 chmod +x "$stub_bin/omarchy-notification-wait"

@@ -45,12 +65,18 @@ printf '%s\n' "$@" >"$OMARCHY_TEST_NOTIFY_ARGS"
 SH
 chmod +x "$stub_bin/omarchy-notification-send"

+runtime_dir="$test_tmp/runtime"
+mkdir -p "$runtime_dir"
+
 run_notify() {
  HOME="$test_home" \
  PATH="$stub_bin:$ROOT/bin:$PATH" \
+  XDG_RUNTIME_DIR="$runtime_dir" \
  OMARCHY_TEST_PENDING_MIGRATIONS="$1" \
  OMARCHY_TEST_NOTIFY_ARGS="$test_tmp/notify-args" \
  OMARCHY_TEST_SYSTEMD_RUN="${2:-run}" \
+  OMARCHY_TEST_LOCK_DURING_WAIT="${OMARCHY_TEST_LOCK_DURING_WAIT:-0}" \
+  OMARCHY_TEST_LOCK_HOLDER_PID="$test_tmp/lock-holder-pid" \
    "$ROOT/bin/omarchy-migrate-notify"
 }

@@ -69,3 +95,55 @@ grep -Fx 'Pending Omarchy Migrations' "$test_tmp/notify-args" >/dev/null || fail
 grep -Fx 'Click to run 1 pending migration.' "$test_tmp/notify-args" >/dev/null || fail "migration notifier describes the pending migration"
 grep -Fx '' "$test_tmp/notify-args" >/dev/null || fail "migration notifier includes the large-slot glyph"
 pass "migration notifier uses the actionable notification format"
+
+# `omarchy update` applies migrations itself, so nothing may notify about them
+# while it holds its lock -- a stale trigger firing mid-transaction is exactly
+# how the retired omarchy-update-user-notify.path used to interrupt updates.
+rm -f "$test_tmp/notify-args"
+update_lock="$runtime_dir/omarchy-update.lock"
+: >"$update_lock"
+exec {update_lock_fd}>"$update_lock"
+flock -n "$update_lock_fd" || fail "test could not hold the update lock"
+
+run_notify 1 >"$test_tmp/during-update.out" 2>"$test_tmp/during-update.err"
+[[ ! -s $test_tmp/during-update.out ]] || fail "migration notifier stays quiet on stdout during an update"
+[[ ! -s $test_tmp/during-update.err ]] || fail "migration notifier stays quiet on stderr during an update"
+[[ ! -e $test_tmp/notify-args ]] || fail "migration notifier sends no notification during an update"
+pass "migration notifier stays quiet while omarchy update holds its lock"
+
+exec {update_lock_fd}>&-
+
+run_notify 1 >/dev/null 2>&1
+grep -Fx 'Pending Omarchy Migrations' "$test_tmp/notify-args" >/dev/null ||
+  fail "migration notifier resumes notifying once the update lock is released"
+pass "migration notifier resumes notifying after the update releases its lock"
+
+rm -f "$test_tmp/notify-args"
+OMARCHY_TEST_LOCK_DURING_WAIT=1 run_notify 1 >"$test_tmp/raced.out" 2>"$test_tmp/raced.err"
+if [[ -s $test_tmp/lock-holder-pid ]]; then
+  kill "$(<"$test_tmp/lock-holder-pid")" 2>/dev/null || true
+  for _ in {1..200}; do
+    flock -n "$runtime_dir/omarchy-update.lock" true 2>/dev/null && break
+    sleep 0.05
+  done
+fi
+[[ ! -e $test_tmp/notify-args ]] ||
+  fail "migration notifier sends no notification when an update starts while it waits for the notification server"
+pass "migration notifier re-checks for an update after waiting for the notification server"
+
+# The guard must never read a lock outside this user's runtime directory: a
+# shared /tmp path belongs to whoever created it first, so honouring it would
+# let one user silence another user's critical notification.
+rm -f "$test_tmp/notify-args" /tmp/omarchy-update.lock
+foreign_lock="$test_tmp/foreign/omarchy-update.lock"
+mkdir -p "$(dirname "$foreign_lock")"
+: >"$foreign_lock"
+exec {foreign_lock_fd}>"$foreign_lock"
+flock -n "$foreign_lock_fd" || fail "test could not hold the foreign update lock"
+
+run_notify 1 >/dev/null 2>&1
+grep -Fx 'Pending Omarchy Migrations' "$test_tmp/notify-args" >/dev/null ||
+  fail "migration notifier ignores update locks outside its own runtime directory"
+pass "migration notifier ignores update locks outside its own runtime directory"
+
+exec {foreign_lock_fd}>&-
@@ -101,6 +101,41 @@ grep -q "already running" "$test_tmp/perform-second.out" || fail "second omarchy
 [[ ! -f $test_tmp/perform-second-started ]] || fail "second omarchy-update-perform did not snapshot while lock was held"
 pass "omarchy-update-perform compatibility wrapper respects update lock"

+# The sleep inhibitor deliberately outlives the step that starts it, so it must
+# not inherit the update lock. An update killed before restore_update_inhibitors
+# would otherwise leave the inhibitor holding the flock forever, blocking every
+# later update and silencing omarchy-migrate-notify, which reads the same lock.
+inhibit_pid_file="$test_tmp/inhibit-pid"
+keyring_marker="$test_tmp/keyring-started"
+write_stub omarchy-snapshot 'exit 0'
+write_stub systemd-inhibit 'echo "$$" >"$INHIBIT_PID_FILE"; exec sleep 30'
+write_stub omarchy-update-keyring 'echo started >"$TEST_MARKER"; sleep 3; exit 0'
+
+OMARCHY_UPDATE_LOGGED=1 TEST_MARKER="$keyring_marker" INHIBIT_PID_FILE="$inhibit_pid_file" \
+  run_with_lock_env "$ROOT/bin/omarchy-update" -y >"$test_tmp/update-inhibit.out" 2>&1 &
+inhibit_update_pid=$!
+
+for _ in {1..100}; do
+  [[ -s $inhibit_pid_file && -f $keyring_marker ]] && break
+  sleep 0.05
+done
+[[ -s $inhibit_pid_file ]] || fail "update starts its sleep inhibitor"
+
+inhibitor_pid=$(<"$inhibit_pid_file")
+kill -0 "$inhibitor_pid" 2>/dev/null || fail "sleep inhibitor is still running when its descriptors are inspected"
+
+lock_target=$(readlink -f "$runtime_dir/omarchy-update.lock")
+inhibitor_holds_lock=0
+for fd in /proc/"$inhibitor_pid"/fd/*; do
+  [[ -e $fd ]] || continue
+  [[ $(readlink -f "$fd" 2>/dev/null) == "$lock_target" ]] && inhibitor_holds_lock=1
+done
+
+wait "$inhibit_update_pid"
+
+(( inhibitor_holds_lock == 0 )) || fail "update keeps the update lock out of the sleep inhibitor it leaves running"
+pass "omarchy-update keeps the update lock out of its sleep inhibitor"
+
 # Update-owned Stay Awake state must be cleared before the restart helper can
 # reboot the machine, rather than relying on an EXIT trap during shutdown.
 write_stub omarchy-snapshot 'exit 0'