[kylerankin]

fix(style): unquote the bluefin prefs glob in 99-flatpaks.sh

The *bluefin*.js glob in the flatpaks Firefox-config hook was trapped
inside the quoted rm target, so rm matched a literal filename that never
existed and left stale prefs behind instead of clearing them
(#489). Move the glob outside the quoted directory so it
expands.

This also bumps the hook's version-script gate from 1 to 2, which is a
fleet-wide behaviour change, not a style change. Machines provisioned under
version 1 recorded the hook as succeeded while the quoted glob made rm -f a
no-op, so the stale prefs are still on disk there and the repaired rm would
never run without the bump. The bump makes every already-provisioned machine
re-execute the hook body once: mkdir -p, the repaired rm -f .../pref/*bluefin*.js, and cp -rf /usr/share/ublue-os/firefox-config/* into
the same defaults/pref directory. All three are idempotent and confined to
/var/lib/flatpak/extension/org.mozilla.firefox.systemconfig/${ARCH}/stable/defaults/pref;
the rm deletes only *bluefin*.js prefs the image itself ships, and the
cp rewrites them from the current image. No user data lives under that path.

Adds a regression test (tests/test_flatpaks_hook.py) that pins the glob
outside the quotes, pins the version at 2, and asserts shellcheck stays clean
(skipped when shellcheck is absent). Updates docs/skills/setup-hooks.md so the
Tests section lists the new hook test and no longer claims there is no
shellcheck assertion anywhere.

Not addressed here: the hook still uses the legacy version-script form that
stamps before the body and runs set -x without set -e, contrary to the
once-only contract in docs/skills/setup-hooks.md. That is pre-existing and
tracked by #259.

Closes #489.

— hive: backend=pi model=lemonade/Ornith-1.5-35B-A3B-GGUF-Q6_K

---

🐝 Hive Agent: contributor | SHA: a6bbac2

[hivecommons-hive]

correctness

• tests/test_flatpaks_hook.py:55 — test_shellcheck_passes calls the shellcheck binary with no shutil.which guard/skipTest. No other suite under tests/ depends on it and it is not declarable in tests/requirements.txt, so on a dev host without shellcheck just test errors with FileNotFoundError instead of reporting the glob regression this file exists for. CI is fine today only because the ubuntu-26.04 runner happens to ship shellcheck.

docs-currency

• tests/test_flatpaks_hook.py:8-9 — docstring says "This is a style-only regression (#489): the common-case behaviour is unchanged". The diff is a behaviour fix: before, rm -f ".../pref/*bluefin*.js" (old L14) matched a literal filename and stale prefs survived; after, they are deleted. The comment contradicts the test's own purpose.

No findings from: security, intent-alignment, style.

Confidence: 3/5 (needs attention) — 2 perspectives requested changes

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

correctness

• (medium, in-scope) 99-flatpaks.sh:6 gates the whole body on version-script flatpaks privileged 1 || exit 0, and this PR leaves the version at 1. Any machine that already ran the hook recorded version 1, so the repaired rm -f at 99-flatpaks.sh:14 never executes there — the stale *bluefin*.js prefs #489 describes stay in place, and only fresh installs get the fix. Same shape as #474 (tests/test_setup_hook_version_contract.py:42-47, 20-home-labels.sh bumped 1→2 so provisioned hosts retry). Bump to flatpaks privileged 2 (mkdir/rm/cp are idempotent) and have the new test pin the bump; otherwise the PR body's "clearing them" claim only holds for new installs.

No findings from: security, intent-alignment, style, docs-currency.

Confidence: 3/5 (needs attention) — 1 medium finding; 1 perspective requested changes

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

docs-currency

• docs/skills/setup-hooks.md:113-126 — the "Tests" section enumerates the per-hook test files (test_setup_hook_version_contract.py, test_bootupctl_adopt_hook.py, test_ghostty_hook.py) and states "there is no shellcheck gate in the Justfile or CI". This PR adds tests/test_flatpaks_hook.py, whose test_shellcheck_passes (tests/test_flatpaks_hook.py:67-78) runs shellcheck under just test whenever it is installed. Add the new file to the list and soften the "no shellcheck gate" sentence (e.g. "only test_flatpaks_hook.py runs shellcheck, and skips when it is absent").

intent-alignment

• 99-flatpaks.sh:6-9 bumps version-script flatpaks privileged 1→2, so every already-provisioned machine re-runs the body on next boot. The bump is justified (and the in-file comment explains it), but the PR body only describes unquoting the glob and adding a test — it never mentions the version bump, and the fix(style) title undersells a change that re-executes rm -f/cp -rf fleet-wide. Please state it in the PR body so the behaviour change is visible in history.

No findings from: correctness, security, style.

Confidence: 3/5 (needs attention) — 1 perspective requested changes

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Hive filed out-of-scope review findings as backlog issues so they do not block this PR:

• Review backlog (style): Hook still on legacy version-script gate without set -e #508 — Review backlog (style): Hook still on legacy version-script gate without set -e (Review backlog (style): Hook still on legacy version-script gate without set -e #508)

Cap: 3 backlog issue(s) per PR.

[hivecommons-hive]

style

• low — tests/test_flatpaks_hook.py:63-67 test_arch_variable_still_quoted asserts self.body.count("${ARCH}") == 3; any legitimate edit that adds/removes a ${ARCH} use (or factors the path into a variable) fails this test without any quoting regression. Sibling hook suites (tests/test_bootupctl_adopt_hook.py, tests/test_ghostty_hook.py) drive the real hook against a fake libsetup.sh; a scratch-dir run that seeds a stale foo-bluefin.js and asserts it is gone would pin the actual behaviour instead of the text. Non-blocking.

No findings from: correctness, security, intent-alignment, docs-currency.

Confidence: 5/5 (safe)

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[castrojo]

Replaced the incidental glob-source assertions with real hook behavior: stale Bluefin defaults are removed, a user's neighboring preference file survives, and an absent Bluefin glob does not prevent the hook from completing. The tests execute the hook in isolated source/destination trees and preserve the actual preference contents rather than re-pinning shell spelling. The main integration preserves both version-script-check and version-script-commit at version 2. A new regression starts with a version-1 stamp under both library contracts. A real disposable container using its actual Common library also migrated stamp 1 to 2, cleaned stale preferences, preserved user files and skipped the second invocation.

[Danathar]

The bug is real: the glob sat inside the double quotes, so rm -f never matched anything. Moving *bluefin*.js outside the quotes fixes it. Bumping the hook to version 2 means machines that already recorded v1 get cleaned up too. The new tests run the real hook in a temp tree (with spaces in the path) and show stale *bluefin*.js files removed while a user's own .js stays. They pass locally (7). Not run: a booted image.

[hivecommons-hive]

intent-alignment

• (low) PR body describes work the diff does not contain: it says tests/test_flatpaks_hook.py "pins the version at 2" and "asserts shellcheck stays clean (skipped when shellcheck is absent)", and that docs/skills/setup-hooks.md "no longer claims there is no shellcheck assertion anywhere". The new test file (tests/test_flatpaks_hook.py:1-61) has no shellcheck or version assertion — the version-2 pin lives only in tests/test_migrated_setup_hook_contract.py:128-129 — and the doc still (correctly) states "there is no shellcheck gate in the Justfile or CI" at docs/skills/setup-hooks.md:133-134. The code is fine; the description is stale and will mislead whoever squash-merges it. Please trim the body to match the diff.

No findings from: correctness, security, style, docs-currency.

Confidence: 5/5 (safe)

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88