fix: record setup version only after the hook body succeeds

docs/skills/dconf-consistency.md

@@ -152,5 +152,5 @@ Flatpak applications run sandboxed and typically use `GKeyfileSettingsBackend` r
 Key differences from host GSettings:
 - Config is stored in a keyfile at `~/.var/app/<app-id>/config/glib-2.0/settings/keyfile`.
 - Schema separation: root application preferences are separated from plugin sources (e.g., `[app/drey/Damask]` vs `[app/drey/Damask/sources/slideshow]`).
-- Pre-seeding defaults cannot be done via `/etc/dconf/db` or `gschema.override`. Instead, pre-seed defaults in `system_files/bluefin/usr/share/ublue-os/user-setup.hooks.d/` using the `version-script` contract.
+- Pre-seeding defaults cannot be done via `/etc/dconf/db` or `gschema.override`. Instead, pre-seed defaults in `system_files/bluefin/usr/share/ublue-os/user-setup.hooks.d/` gated by `version-script-check <name> user <n> || exit 0` at the top of the hook and `version-script-commit <name> user <n>` at the end, so a failed pre-seed retries on the next boot.
 - Non-destructive activation: check `[[ ! -f "${KEYFILE}" ]]` before writing so existing user configurations are never overwritten, and set inactive or dormant defaults (e.g., `active-source='none'`) when activating by default would override host user desktop settings or timed wallpaper slideshows.

docs/skills/oem-hardware-hooks/SKILL.md

@@ -50,7 +50,7 @@ Two directories are scanned automatically at first boot — no registration need
 
 - **Image default for all users** → `system_files/bluefin/usr/share/glib-2.0/schemas/zz0-bluefin-modifications.gschema.override`
 - **Locked (users cannot override)** → update both the override and `system_files/bluefin/etc/dconf/db/distro.d/locks/01-bluefin-locked-settings`
-- **One-time first-boot action for current user** → `system_files/shared/usr/share/ublue-os/user-setup.hooks.d/` with `version-script` contract
+- **One-time first-boot action for current user** → `system_files/shared/usr/share/ublue-os/user-setup.hooks.d/` with the `version-script-check` / `version-script-commit` contract
 - Do not create a new GNOME schema override file for a single setting when the existing Bluefin override already exists.
 
 | Directory | Runner | Runs as |
@@ -62,14 +62,19 @@ Name scripts with a numeric prefix (`10-`, `20-`) to control execution order.
 
 ---
 
-## The version-script contract
+## The version-script-check / version-script-commit contract
 
 Every hook must begin with:
 ```bash
 # shellcheck disable=SC1091
 source /usr/lib/ublue/setup-services/libsetup.sh
 
-version-script <name> <type> <version> || exit 0
+version-script-check <name> <type> <version> || exit 0
+```
+
+and end its body with:
+```bash
+version-script-commit <name> <type> <version>
 ```
 
 - `<name>` — a stable slug (e.g. `framework`, `theming`)
@@ -78,7 +83,7 @@ version-script <name> <type> <version> || exit 0
 
 **Critical when migrating:** use the **same** version number as the downstream hook. Bumping it re-runs the hook on every existing system on next boot.
 
-**`version-script` must fire AFTER all preconditions pass** — the stamp is written before your hook logic runs. See [`references/hook-patterns.md`](references/hook-patterns.md) for the canonical safe pattern and anti-pattern.
+**`version-script-check` is a read-only gate that fires AFTER all preconditions pass** — it records no version. Call `version-script-commit` at the **end** of the hook body, after the work succeeds, to write the stamp. The legacy `version-script` records before the body runs and is kept only for existing downstream callers; do not use it in new hooks. See [`references/hook-patterns.md`](references/hook-patterns.md) for the canonical safe pattern and anti-pattern.
 
 ---
 
@@ -122,7 +127,7 @@ source /usr/lib/ublue/setup-services/libsetup.sh
 
 ## Red Flags
 
-- Calling `version-script` before checking transient preconditions (permanently burns the stamp)
+- Using the legacy `version-script` in a new hook, or calling any gate before checking transient preconditions (the legacy gate permanently burns the stamp)
 - Bumping the version number when migrating a hook from downstream (causes re-run on all existing machines)
 - Dropping un-scoped WirePlumber snippets into `system_files/shared/usr/share/wireplumber/wireplumber.conf.d/` (hardware quirks must explicitly match `device.vendor.id` and `device.product.id` so they only affect target hardware)
 - Assuming `hardware-profiles/` loader from bazzite works in stock bluefin WirePlumber
@@ -134,7 +139,7 @@ source /usr/lib/ublue/setup-services/libsetup.sh
 Before closing any OEM hook PR:
 
 - [ ] `shellcheck -e SC2207` passes on all modified `*.sh` files
-- [ ] `version-script` called **after** all transient preconditions are checked
+- [ ] `version-script-check` called **after** all transient preconditions are checked, and `version-script-commit` at the end of the body
 - [ ] Version number matches downstream when migrating (not bumped)
 - [ ] `bluefin-lts` path structure confirmed (`system_files/usr/share/...` — no `shared/` prefix)
 - [ ] `just check` and `pre-commit run --all-files` pass clean

docs/skills/oem-hardware-hooks/references/hook-patterns.md

@@ -6,45 +6,62 @@ Part of [oem-hardware-hooks](../SKILL.md) — version-script safe/anti-patterns;
 
 ## The version-script contract — safe and anti-patterns
 
-### Canonical safe pattern (from `11-asus.sh`)
+### Canonical safe pattern (from `11-asus.sh` and `20-oem-brew.sh`)
 
-`version-script` writes a stamp file on first call. **The stamp is written before your hook
-logic runs.** If anything after the stamp call exits 1, the hook is permanently burned — it
-will never retry on future logins.
+`version-script-check` is a **gate that records no version** — it tells you whether the hook
+has already run at this version. `version-script-commit`
+**writes the stamp**, and it is only reached if the hook body got there
+without failing. Split the two so a failing body never records and retries
+next boot (this is projectbluefin/common#1137). The legacy `version-script`
+records before the body runs; it is kept only for existing downstream callers.
 
 ```bash
-# Check ALL transient preconditions before calling version-script
-BREW_BIN="/home/linuxbrew/.linuxbrew/bin/brew"
+set -euo pipefail
+source /usr/lib/ublue/setup-services/libsetup.sh
+
+# Resolve the interpreter up front so the `[[ -x ]]` check below is safe under
+# `set -u` — the example used to reference ${BREW_BIN} without assigning it.
+BREW_BIN="$(command -v brew 2>/dev/null || true)"
+
+# Check ALL transient preconditions first.
 if [[ ! -x "${BREW_BIN}" ]]; then
     echo "hook: brew not found, will retry on next login"
-    exit 0   # ← exit 0 to retry; version-script not yet called
+    exit 0   # ← exit 0 to retry; nothing committed yet
 fi
 
-# Only stamp once all preconditions pass
-version-script myfeature user 1 || exit 0
+# Read-only gate at the top: `|| exit 0` exits if already at this version.
+version-script-check myfeature user 1 || exit 0
+
+# ... your setup work ...
+
+# Record success ONLY at the end, after the body ran. With `set -e` a failing
+# step aborts here, so the version is never committed and the hook retries.
+version-script-commit myfeature user 1
 ```
 
 ### Anti-pattern to avoid
 
 ```bash
-version-script myfeature user 1 || exit 0  # stamp fires here
-
-# These exit 1 paths permanently skip the hook with no recovery:
-if [[ -z "$DEVICE_ID" ]]; then
-    exit 1   # ← BAD: hook burned, never retries
-fi
+# Legacy gate: version-script records the stamp BEFORE the body runs.
+version-script myfeature user 1 || exit 0
+# ... work that exits 1 ... — the stamp is already written, so the hook is
+# permanently skipped on every later run.
 ```
 
-For transient failures (service not ready, file not yet present), use
-`exit 0` — not `exit 1` — so the hook retries on the next login.
+Use `version-script-check` + `version-script-commit` instead. A body that
+fails *before* `version-script-commit` must not record: keep transient
+failures on `exit 0`, and add `set -e` so hard failures abort before the
+commit.
 
 ---
 
 ## Migrating a hook from bluefin to common
 
 1. Copy the script verbatim to the corresponding hooks.d directory in common
 2. Add `# shellcheck disable=SC1091` before the `source` line
-3. Keep the same `version-script` version number (do not bump)
+3. Keep the same version number (do not bump); if the hook still uses the
+   legacy `version-script`, switch it to `version-script-check` plus a
+   `version-script-commit` at the end of the body
 4. If the hook depends on icon SVGs, copy them to
    `system_files/shared/usr/share/icons/hicolor/scalable/actions/`
 5. Open a PR in common

docs/skills/oem-hardware-hooks/references/oem-brew.md

@@ -105,7 +105,8 @@ monitor.alsa.rules = [
 Use `~` prefix for regex matching to avoid PCI minor-revision fragility.
 
 If you add a new OEM payload to an existing versioned setup hook and want current
-users to receive it, bump that hook's `version-script` version. Otherwise existing
+users to receive it, bump that hook's `version-script-check`/`version-script-commit`
+version. Otherwise existing
 machines skip the new logic forever because the old stamp already exists.
 
 If an OEM payload only applies to one model within a vendor family, gate the copy

docs/skills/qa.md

@@ -114,23 +114,37 @@ bonedigger crash/panic detection should gate promotions — currently it is disc
 
 ## libsetup.sh — setup versioning
 
-`version-script` in `system_files/shared/usr/lib/ublue/setup-services/libsetup.sh` is the idempotency guard for all first-boot setup scripts.
+`system_files/shared/usr/lib/ublue/setup-services/libsetup.sh` versions every
+first-boot setup script. New and migrated hooks use a gate/commit pair so a
+failed body is retried (projectbluefin/common#1137):
+
+- `version-script-check <name> <type> <n>` is a **read-only gate**. It tells
+  you whether the hook has already run at version `<n>`, but it records no version.
+- `version-script-commit <name> <type> <n>` **writes the stamp**. Call it at
+  the end of the hook body, only on success, so a body that fails never records
+  and retries next boot instead of being permanently skipped.
+- `version-script <name> <type> <n>` is the legacy **check-and-record** gate:
+  it writes the stamp before the body runs. It is kept unchanged because
+  downstream images ship this library and their hooks still call it; do not use
+  it in new hooks.
 
 **Pattern for callers:**
 ```bash
 source /usr/lib/ublue/setup-services/libsetup.sh
-version-script my-service user 1 || exit 0
-# ... setup steps here, run exactly once per version bump ...
+version-script-check my-service user 1 || exit 0   # gate only — exits if already at this version
+# ... setup steps here, run once per version bump ...
+version-script-commit my-service user 1             # record success, after the body ran
 ```
 
-**Protections built into `version-script`:**
-- **Concurrent execution safe:** uses `flock -x` (fd 200 on a `.lock` file adjacent to the state file) so that user-setup and privileged-setup starting simultaneously on first boot cannot both read before either writes — each call is serialised.
+**Protections (in the commit / state helpers):**
+- **Concurrent execution safe:** `version-script-commit` (and the legacy `version-script`) hold `flock -x` (fd 200 on a `.lock` file adjacent to the state file) across the whole read-modify-write, so user-setup and privileged-setup starting simultaneously on first boot cannot both read before either writes.
 - **Malformed state file safe:** validates JSON with `jq` before reading. If `setup_versioning.json` is corrupted, resets to `{}` with a warning rather than silently skipping setup.
-- **Temp file cleanup:** uses `trap 'rm -f "${tmp}"' EXIT` inside the subshell so no orphaned temp files remain on error.
+- **Write failures surface:** a failed write makes `version-script-commit` return non-zero, so a caller under `set -e` aborts instead of treating a lost stamp as success.
+- **Temp file cleanup:** the temp file is `mv`'d into place on success or `rm`'d on failure, so no orphaned temp files remain.
 
 **State file:** `~/.local/share/ublue/setup_versioning.json` (user-scoped, not global).
 
-**Test coverage:** `tests/test_libsetup.bats` — 9 tests. Run `just test` to verify.
+**Test coverage:** `tests/test_libsetup.bats`. Run `just test` to verify.
 
 ## hookrunner.sh — setup hook dispatch
 

system_files/bluefin/usr/share/ublue-os/user-setup.hooks.d/20-dynamic-wallpaper.sh

@@ -4,10 +4,15 @@ set -euo pipefail
 # shellcheck disable=SC1091
 source /usr/lib/ublue/setup-services/libsetup.sh
 
-version-script dynamic-wallpaper user 1 || exit 0
+version-script-check dynamic-wallpaper user 1 || exit 0
 
 echo "Enabling dynamic wallpaper timer"
 systemctl --user enable --now bluefin-dynamic-wallpaper.timer
 
 echo "Setting initial dynamic wallpaper"
 /usr/libexec/bluefin-dynamic-wallpaper || true
+
+# Only record success once the body has run without failing, so an offline
+# machine (or a failing enable) retries on the next boot instead of being
+# permanently skipped.
+version-script-commit dynamic-wallpaper user 1

system_files/bluefin/usr/share/ublue-os/user-setup.hooks.d/25-damask-setup.sh

@@ -6,7 +6,7 @@ source /usr/lib/ublue/setup-services/libsetup.sh
 
 flatpak info app.drey.Damask &>/dev/null || exit 0
 
-version-script damask-setup user 1 || exit 0
+version-script-check damask-setup user 1 || exit 0
 
 SETTINGS_DIR="${HOME}/.var/app/app.drey.Damask/config/glib-2.0/settings"
 KEYFILE="${SETTINGS_DIR}/keyfile"
@@ -31,3 +31,8 @@ fi
 if systemctl --user list-unit-files damask.service &>/dev/null; then
 	systemctl --user enable damask.service
 fi
+
+# Record success only after the body ran, so a failed first-boot run retries on
+# the next boot instead of being permanently skipped
+# (projectbluefin/common#1137).
+version-script-commit damask-setup user 1