From 3c6b3c809869e790594a290f7e5c3960edf0ab57 Mon Sep 17 00:00:00 2001 From: Danathar Date: Sat, 12 Sep 2026 10:49:07 +0000 Subject: [PATCH] fix(brew): preserve user-owned preinstall packages Snapshot installed formulas and casks before bundle reconciliation so declarations already present on the system are not adopted into Bluefin managed state. Preserve prior managed entries and fail closed when inventory cannot be collected. Fixes #915 Assisted-by: GPT-5 via GitHub Copilot Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: Danathar --- docs/skills/brew-lifecycle/SKILL.md | 11 +- .../references/service-mechanics.md | 15 +- docs/skills/index.json | 6 +- docs/skills/index.md | 2 +- .../shared/usr/libexec/brew-preinstall | 40 +++- tests/test_brew_preinstall.bats | 174 +++++++++++++++++- 6 files changed, 231 insertions(+), 17 deletions(-) diff --git a/docs/skills/brew-lifecycle/SKILL.md b/docs/skills/brew-lifecycle/SKILL.md index ebcfecd6..cd12cd97 100644 --- a/docs/skills/brew-lifecycle/SKILL.md +++ b/docs/skills/brew-lifecycle/SKILL.md @@ -1,7 +1,7 @@ --- name: brew-lifecycle -version: "1.4" -last_updated: "2026-08-18" +version: "1.5" +last_updated: "2026-09-12" id: brew-lifecycle one_line_purpose: Manage OS-managed Homebrew packages and RPM/brew placement. entry_point: docs/skills/brew-lifecycle/SKILL.md @@ -62,8 +62,11 @@ pattern, and the rules for what can and cannot move to brew. 2. Open a PR. 3. On next successful login sync after the OS update, packages recorded in the previous managed state get uninstalled. Packages outside that state - are unaffected. State records the desired set, not who originally installed - each package, so a manually installed package can later become managed. + are unaffected. On a hash-changing reconciliation, the service snapshots + installed formulae and casks before `brew bundle`: declarations already + present and absent from prior managed state remain user-owned, while new + declarations installed by the bundle become managed. Existing managed + entries remain authoritative across later reconciliations. Bluefinctl is no longer provisioned by common. Removing its dedicated Brewfile uses this existing lifecycle to uninstall state-tracked copies; no separate diff --git a/docs/skills/brew-lifecycle/references/service-mechanics.md b/docs/skills/brew-lifecycle/references/service-mechanics.md index 97b4c990..a749bb9d 100644 --- a/docs/skills/brew-lifecycle/references/service-mechanics.md +++ b/docs/skills/brew-lifecycle/references/service-mechanics.md @@ -41,13 +41,19 @@ only runs when brew is installed at `/home/linuxbrew/.linuxbrew/bin/brew`. ``` State files created before cask management have no `casks` key. They are read -as an empty cask list and require no migration. +as an empty cask list and require no migration. The `packages` and `casks` +arrays contain declarations Bluefin owns, not every declaration in the current +Brewfiles. ### On every login 1. Hash all `preinstall.d/*.Brewfile` files combined. 2. Compare to stored hash. **Identical → fast exit**, nothing touched. -3. **Different:** run `brew bundle --file=` on each Brewfile (idempotent). +3. **Different:** snapshot installed formulae and casks, then run + `brew bundle --file=` on each Brewfile (idempotent). A declaration already + installed in the snapshot remains user-owned unless it was already present + in the previous managed state. A new declaration absent from the snapshot + becomes managed after the successful bundle pass. Continue through independent Brewfiles, but exit before removals and state writes if any bundle fails. 4. Diff previous formula and cask sets (from state JSON) against the current @@ -63,8 +69,9 @@ triggers re-run automatically. **Safety rule:** the uninstall step only removes packages that were in the *previous managed state file*. If a user independently ran `brew install inxi` -themselves, it is not in their state file's managed list and will never be -touched. +themselves before `inxi` appeared in a managed Brewfile, the pre-bundle +snapshot keeps it out of managed state and a later Brewfile removal will never +touch it. ### What happens to long-time users on a package removal diff --git a/docs/skills/index.json b/docs/skills/index.json index 6bc2915d..e7984d45 100644 --- a/docs/skills/index.json +++ b/docs/skills/index.json @@ -1,5 +1,5 @@ { - "generated_at": "2026-08-18", + "generated_at": "2026-09-12", "schema_version": "1.0", "skills": [ { @@ -67,8 +67,8 @@ "packages" ], "description": "Manage OS-managed Homebrew packages. Use when adding/removing default brew packages, moving tools between RPM and brew, or auditing image-vs-brew placement.", - "version": "1.4", - "last_updated": "2026-08-18", + "version": "1.5", + "last_updated": "2026-09-12", "doc_type": "procedure" }, { diff --git a/docs/skills/index.md b/docs/skills/index.md index 8dd84c2a..a76011cd 100644 --- a/docs/skills/index.md +++ b/docs/skills/index.md @@ -3,7 +3,7 @@ This file is a human-readable mirror of `index.json`. Both are generated by `scripts/generate_skill_index.py` — do not hand-edit either file. -Generated: 2026-08-18 · schema 1.0 · 40 skills +Generated: 2026-09-12 · schema 1.0 · 40 skills | id | category | status | one-line purpose | |---|---|---|---| diff --git a/system_files/shared/usr/libexec/brew-preinstall b/system_files/shared/usr/libexec/brew-preinstall index cdb51061..5a7d6aa7 100755 --- a/system_files/shared/usr/libexec/brew-preinstall +++ b/system_files/shared/usr/libexec/brew-preinstall @@ -96,6 +96,19 @@ fi echo "brew-preinstall: Brewfiles changed (${current_hash:0:12}...), applying..." +# Snapshot installed names before brew bundle. A declaration that was already +# installed at this point belongs to the user unless the previous state file +# already says Bluefin managed it. If either inventory query fails, stop before +# bundling so an incomplete snapshot cannot adopt user-owned software. +installed_packages_before=$(brew list --formula) || { + echo "brew-preinstall: error: could not snapshot installed formulae; state unchanged, will retry" + exit 1 +} +installed_casks_before=$(brew list --cask) || { + echo "brew-preinstall: error: could not snapshot installed casks; state unchanged, will retry" + exit 1 +} + # Install all packages declared in Brewfiles (brew bundle is idempotent). # Continue after an individual failure so independent Brewfiles still install, # but leave state untouched so the complete run is retried. @@ -169,6 +182,29 @@ managed_name_present() { return 1 } +managed_names_for_reconciliation() { + local current_names="$1" + local previous_names="$2" + local installed_before="$3" + local name + + while IFS= read -r name; do + [[ -z "${name}" ]] && continue + if managed_name_present "${name}" "${previous_names}" \ + || ! managed_name_present "${name}" "${installed_before}"; then + printf '%s\n' "${name}" + fi + done <<< "${current_names}" +} + +# A declaration already present before this run is user-owned unless it was +# already in managed state. New declarations absent from the snapshot were +# installed by the successful bundle pass and can become managed. +managed_packages=$(managed_names_for_reconciliation \ + "${current_packages}" "${previous_packages}" "${installed_packages_before}") +managed_casks=$(managed_names_for_reconciliation \ + "${current_casks}" "${previous_casks}" "${installed_casks_before}") + uninstall_failed=0 if [[ -n "${previous_packages}" ]]; then @@ -211,8 +247,8 @@ fi # Persist new state: hash + full managed package and cask lists for next diff. # Write atomically via temp file + rename to avoid corrupt state on SIGKILL. mkdir -p "$(dirname "${STATE_FILE}")" -packages_json=$(echo "${current_packages}" | jq -R . | jq -s 'map(select(length > 0))') -casks_json=$(echo "${current_casks}" | jq -R . | jq -s 'map(select(length > 0))') +packages_json=$(echo "${managed_packages}" | jq -R . | jq -s 'map(select(length > 0))') +casks_json=$(echo "${managed_casks}" | jq -R . | jq -s 'map(select(length > 0))') jq -n \ --arg hash "${current_hash}" \ --argjson packages "${packages_json}" \ diff --git a/tests/test_brew_preinstall.bats b/tests/test_brew_preinstall.bats index 895e3e7c..3a50cfc9 100644 --- a/tests/test_brew_preinstall.bats +++ b/tests/test_brew_preinstall.bats @@ -22,7 +22,7 @@ PATCHED_SCRIPT="" PATCHED_WRAPPER="" setup() { - WORKDIR="$(mktemp -d)" + WORKDIR="$(mktemp -d /var/tmp/bluefin-common-brew-preinstall.XXXXXX)" export WORKDIR mkdir -p "${WORKDIR}/bin" "${WORKDIR}/preinstall.d" @@ -66,6 +66,29 @@ BREWMOCK chmod +x "${PATCHED_WRAPPER}" } +mock_brew_lists() { + local formulas="$1" + local casks="$2" + + cat > "${WORKDIR}/bin/brew" << BREWMOCK +#!/usr/bin/env bash +BREW_LOG="\${BREW_LOG:-/dev/null}" +printf 'brew %s\n' "\$*" >> "\${BREW_LOG}" +case "\$1" in + shellenv) printf 'export PATH="%s:\${PATH}"\n' "${WORKDIR}/bin" ;; + bundle) ;; + list) + case "\$2" in + --formula) printf '%s\n' "${formulas}" ;; + --cask) printf '%s\n' "${casks}" ;; + esac + ;; + uninstall) ;; +esac +BREWMOCK + chmod +x "${WORKDIR}/bin/brew" +} + teardown() { rm -rf "${WORKDIR}" } @@ -292,6 +315,72 @@ EOF [ "${bundle_count}" -eq 2 ] } +# --------------------------------------------------------------------------- +# Ownership — declarations already installed before bundle remain user-owned +# --------------------------------------------------------------------------- + +@test "brew-preinstall: does not adopt a user-owned formula or remove it later" { + echo 'brew "htop"' > "${WORKDIR}/preinstall.d/system-cli.Brewfile" + mock_brew_lists "htop" "" + + BREW_LOG="${WORKDIR}/first.log" run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + jq -e '.packages == []' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" + + echo 'brew "ripgrep"' > "${WORKDIR}/preinstall.d/system-cli.Brewfile" + BREW_LOG="${WORKDIR}/second.log" run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + ! grep -q '^brew uninstall htop ' "${WORKDIR}/second.log" + jq -e '.packages == ["ripgrep"]' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" +} + +@test "brew-preinstall: manages a formula installed by the bundle and removes it later" { + echo 'brew "ripgrep"' > "${WORKDIR}/preinstall.d/system-cli.Brewfile" + + run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + jq -e '.packages == ["ripgrep"]' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" + + echo 'brew "htop"' > "${WORKDIR}/preinstall.d/system-cli.Brewfile" + BREW_LOG="${WORKDIR}/second.log" run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + grep -q '^brew uninstall ripgrep --ignore-dependencies$' \ + "${WORKDIR}/second.log" + jq -e '.packages == ["htop"]' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" +} + +@test "brew-preinstall: preserves a previously managed formula across reconciliation" { + echo 'brew "ripgrep"' > "${WORKDIR}/preinstall.d/system-cli.Brewfile" + + run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + + echo 'brew "fd"' >> "${WORKDIR}/preinstall.d/system-cli.Brewfile" + mock_brew_lists "ripgrep" "" + BREW_LOG="${WORKDIR}/second.log" run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + ! grep -q '^brew uninstall ripgrep ' "${WORKDIR}/second.log" + jq -e '.packages == ["fd", "ripgrep"]' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" +} + +@test "brew-preinstall: preserves legacy managed formula state" { + echo 'brew "ripgrep"' > "${WORKDIR}/preinstall.d/system-cli.Brewfile" + mkdir -p "${WORKDIR}/.local/share/ublue-os" + printf '{"hash":"oldhash","packages":["ripgrep"]}\n' \ + > "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" + mock_brew_lists "ripgrep" "" + + run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + jq -e '.packages == ["ripgrep"] and .casks == []' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" +} + # --------------------------------------------------------------------------- # Package removal — packages dropped from Brewfile are uninstalled # --------------------------------------------------------------------------- @@ -361,7 +450,12 @@ printf 'brew %s\n' "\$*" >> "\${BREW_LOG}" case "\$1" in shellenv) printf 'export PATH="%s:\${PATH}"\n' "${WORKDIR}/bin" ;; bundle) ;; - list) exit 1 ;; + list) + if [[ "\$2" == "--formula" && -n "\$3" ]]; then + exit 1 + fi + exit 0 + ;; uninstall) ;; esac BREWMOCK @@ -488,7 +582,7 @@ case "\$1" in shellenv) printf 'export PATH="%s:\${PATH}"\n' "${WORKDIR}/bin" ;; bundle) ;; list) - if [[ "\$*" == *"--cask"* ]]; then + if [[ "\$2" == "--cask" && -n "\$3" ]]; then exit 1 fi exit 0 @@ -530,6 +624,54 @@ BREWMOCK [[ "${casks}" == *"chairlift"* ]] } +@test "brew-preinstall: does not adopt a user-owned cask or remove it later" { + echo 'cask "chairlift"' > "${WORKDIR}/preinstall.d/chairlift.Brewfile" + mock_brew_lists "" "chairlift" + + BREW_LOG="${WORKDIR}/first.log" run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + jq -e '.casks == []' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" + + echo 'cask "zed"' > "${WORKDIR}/preinstall.d/chairlift.Brewfile" + BREW_LOG="${WORKDIR}/second.log" run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + ! grep -q '^brew uninstall --cask chairlift$' "${WORKDIR}/second.log" + jq -e '.casks == ["zed"]' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" +} + +@test "brew-preinstall: manages a cask installed by the bundle and removes it later" { + echo 'cask "chairlift"' > "${WORKDIR}/preinstall.d/chairlift.Brewfile" + + run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + jq -e '.casks == ["chairlift"]' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" + + echo 'cask "zed"' > "${WORKDIR}/preinstall.d/chairlift.Brewfile" + BREW_LOG="${WORKDIR}/second.log" run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + grep -q '^brew uninstall --cask chairlift$' "${WORKDIR}/second.log" + jq -e '.casks == ["zed"]' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" +} + +@test "brew-preinstall: preserves a previously managed cask across reconciliation" { + echo 'cask "chairlift"' > "${WORKDIR}/preinstall.d/chairlift.Brewfile" + + run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + + echo 'cask "zed"' >> "${WORKDIR}/preinstall.d/chairlift.Brewfile" + mock_brew_lists "" "chairlift" + BREW_LOG="${WORKDIR}/second.log" run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 0 ] + ! grep -q '^brew uninstall --cask chairlift$' "${WORKDIR}/second.log" + jq -e '.casks == ["chairlift", "zed"]' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" +} + @test "brew-preinstall: accepts indented single-quoted cask declarations" { echo " cask 'chairlift'" > "${WORKDIR}/preinstall.d/apps.Brewfile" @@ -714,6 +856,32 @@ BREWMOCK [[ "${pkgs}" == *"fd"* ]] } +@test "brew-preinstall: inventory failure aborts before bundle and state write" { + echo 'brew "ripgrep"' > "${WORKDIR}/preinstall.d/system-cli.Brewfile" + mkdir -p "${WORKDIR}/.local/share/ublue-os" + printf '{"hash":"oldhash","packages":["fd"],"casks":[]}\n' \ + > "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" + + cat > "${WORKDIR}/bin/brew" << BREWMOCK +#!/usr/bin/env bash +BREW_LOG="\${BREW_LOG:-/dev/null}" +printf 'brew %s\n' "\$*" >> "\${BREW_LOG}" +case "\$1" in + shellenv) printf 'export PATH="%s:\${PATH}"\n' "${WORKDIR}/bin" ;; + list) exit 1 ;; + bundle) ;; +esac +BREWMOCK + chmod +x "${WORKDIR}/bin/brew" + + BREW_LOG="${WORKDIR}/brew.log" run bash "${PATCHED_SCRIPT}" + [ "${status}" -eq 1 ] + [[ "${output}" == *"could not snapshot installed formulae"* ]] + ! grep -q '^brew bundle ' "${WORKDIR}/brew.log" + jq -e '.hash == "oldhash" and .packages == ["fd"]' \ + "${WORKDIR}/.local/share/ublue-os/brew-preinstall-state.json" +} + @test "brew-preinstall: taps all Brewfile taps before any bundle runs" { printf 'tap "frostyard/tap", trusted: true\ncask "chairlift"\n' \ > "${WORKDIR}/preinstall.d/chairlift.Brewfile"