From 3bb9867245509a46965f3d82a2179f0b19647540 Mon Sep 17 00:00:00 2001
From: Ryan Hughes <ryan@heyoodle.com>
Date: Fri, 28 Aug 2026 21:24:32 -0400
Subject: [PATCH] Merge pull request #8835 from
 basecamp/fix-browser-policy-exit-trap

Fix migration 1787515927 failing on Bash 5.3

(cherry picked from commit 62eb5182d073191e62bee137bf8e2521414445cc)
---
 bin/omarchy-theme-set-browser-policy    |  7 ++++++-
 migrations/1787515927.sh                |  5 ++++-
 test/shell.d/browser-policy-dir-test.sh | 19 +++++++++++++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/bin/omarchy-theme-set-browser-policy b/bin/omarchy-theme-set-browser-policy
index 6f628f9b..ccd07722 100755
--- a/bin/omarchy-theme-set-browser-policy
+++ b/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
 
diff --git a/migrations/1787515927.sh b/migrations/1787515927.sh
index f1f33869..9adae003 100644
--- a/migrations/1787515927.sh
+++ b/migrations/1787515927.sh
@@ -9,8 +9,11 @@ for dir in "${BROWSER_POLICY_MANAGED_DIRS[@]}"; do
   repaired=1
 done
 
+# Repainting the policy color is cosmetic and the next theme change redoes it.
+# Under bash -euo pipefail a failure here would abort the migration before the
+# Firefox directories below are hardened, and the marker would never be written.
 if (( repaired )); then
-  omarchy-theme-set-browser
+  omarchy-theme-set-browser || true
 fi
 
 for dir in "${BROWSER_POLICY_FIREFOX_DIRS[@]}"; do
diff --git a/test/shell.d/browser-policy-dir-test.sh b/test/shell.d/browser-policy-dir-test.sh
index 20b8067c..0d66d216 100755
--- a/test/shell.d/browser-policy-dir-test.sh
+++ b/test/shell.d/browser-policy-dir-test.sh
@@ -279,6 +279,25 @@ 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"
+
+grep -F 'omarchy-theme-set-browser || true' "$ROOT/migrations/1787515927.sh" >/dev/null ||
+  fail "the policy-directory migration hardens Firefox even when the theme refresh fails"
+pass "the policy-directory migration does not abort on a failed theme refresh"
+
 policy_files=(
   "$ROOT/bin/omarchy-install-browser"
   "$ROOT/bin/omarchy-provision-owner"