Stop the browser policy EXIT trap from reporting a clean run as failed

Author: ryanrhughes

bin/omarchy-theme-set-browser-policy

@@ -76,8 +76,13 @@ require_root "$color"
 
 failed=0
 staged=""
+# Bash 5.3 makes the EXIT trap's last command decide the script's exit status,
+# so this handler must not end on a false test. Every successful run clears
+# staged, and a trailing `[[ -n $staged ]] && ...` would report that as failure.
 cleanup() {
-  [[ -n $staged ]] && rm -f "$staged"
+  if [[ -n $staged ]]; then
+    rm -f "$staged"
+  fi
 }
 trap cleanup EXIT
 

test/shell.d/browser-policy-dir-test.sh

@@ -279,6 +279,21 @@ grep -F 'exit "$failed"' "$ROOT/bin/omarchy-theme-set-browser" >/dev/null ||
   fail "omarchy-theme-set-browser exits non-zero when a policy write fails"
 pass "omarchy-theme-set-browser exits non-zero when a policy write fails"
 
+# Bash 5.3 adopts the EXIT trap's last status as the script's exit status, so a
+# handler ending on a false test turns a clean run into a failure and aborts the
+# migration that calls this through omarchy-theme-set-browser.
+policy_cleanup=$(sed -n '/^cleanup() {/,/^}/p' "$ROOT/bin/omarchy-theme-set-browser-policy")
+[[ -n $policy_cleanup ]] || fail "omarchy-theme-set-browser-policy defines an EXIT cleanup handler"
+eval "$policy_cleanup"
+staged=""
+cleanup || fail "omarchy-theme-set-browser-policy's EXIT trap succeeds with nothing staged"
+staged=$test_tmp/staged-policy
+: >"$staged"
+cleanup || fail "omarchy-theme-set-browser-policy's EXIT trap succeeds with a staged file"
+[[ ! -e $staged ]] || fail "omarchy-theme-set-browser-policy's EXIT trap removes the staged file"
+unset -f cleanup
+pass "omarchy-theme-set-browser-policy's EXIT trap never leaks a failure status"
+
 policy_files=(
   "$ROOT/bin/omarchy-install-browser"
   "$ROOT/bin/omarchy-provision-owner"