Merge pull request #8835 from basecamp/fix-browser-policy-exit-trap

Fix migration 1787515927 failing on Bash 5.3

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
 

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

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"