[repires]

bluefin-common PR

What does this change?

Adds a first-run user setup hook for Damask (system_files/bluefin/usr/share/ublue-os/user-setup.hooks.d/25-damask-setup.sh) that non-destructively pre-seeds GLib keyfile settings for app.drey.Damask, enables the damask.service background unit, and adds comprehensive Bats test coverage.

Why?

Closes #1161

Damask runs sandboxed as a Flatpak against GKeyfileSettingsBackend at ~/.var/app/app.drey.Damask/config/glib-2.0/settings/keyfile. This setup hook:

• Pre-seeds root settings with refresh-interval='86400', enable-automatic-refresh=true, and run-in-background=true.
• Sets active-source='none' so Damask remains dormant by default, preserving GNOME's built-in timed day/night wallpaper cycle and user choices until rotation is deliberately enabled.
• Pre-seeds slideshow source plugin with folder-uri='file:///usr/share/backgrounds/bluefin' and sort-by='random'.
• Preserves existing user configuration if a keyfile already exists before first run.
• Enables damask.service via systemctl --user enable damask.service.
• Is guarded idempotently using version-script damask-setup user 1.
• Includes Bats unit test coverage (tests/test_damask_setup.bats) registered in Justfile and CI workflow.
• Documents Flatpak keyfile defaults patterns in docs/skills/dconf-consistency.md.

Checklist

• PR title follows Conventional Commits (fix:, feat:, docs:, ci:, refactor:, etc.)
• just check passes
• pre-commit run --all-files passes
• Skill doc updated if the change affects agent-facing conventions or behavior (see docs/skills/skill-improvement.md)
• AGENTS.md / docs/SKILL.md / docs/skills/ links remain valid

AI attribution

Assisted-by: Gemini via GitHub Copilot
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

— hive: backend=agy

🐝 Hive Agent: contributor | SHA: 08bfc2f

[hivecommons-hive]

The keyfile pre-seed is non-destructive as described (the [[ ! -f "${KEYFILE}" ]] guard at 25-damask-setup.sh:14) and matches the pattern the new docs section describes. One ordering problem and two interactions to flag:

1. This PR fails on its own: damask.service does not exist at main. 25-damask-setup.sh:29 runs systemctl --user enable damask.service under set -euo pipefail (:2), but the unit is only added by #1187 (system_files/bluefin/usr/lib/systemd/user/damask.service). I checked the tree — there is no damask* unit anywhere in system_files/ today. If #1186 merges before #1187, the hook aborts at line 29 on every first login, and because version-script at :7 has already stamped damask-setup user 1 before the body runs (current libsetup.sh semantics), it never retries — the keyfile is written but the service is never enabled, and no later boot fixes it. The body doesn't mention #1187; it should state the dependency, or the enable should be guarded (systemctl --user enable damask.service || true, or checked with list-unit-files like 11-asus.sh:18 does).

2. Not a duplicate of #1187, but both add adjacent lines to .github/workflows/unit-tests.yml (~:123-134) and Justfile (:36-42), so whichever lands second needs a rebase. Related, not conflicting in substance.

3. Interaction with #1196. That PR turns version-script into a read-only gate and requires version-script-commit at the end of the body. This hook has no commit call, so if #1196 lands first this hook re-runs on every login (harmless here thanks to the keyfile guard, but it will re-run systemctl enable each time). The new dconf-consistency.md:135 text "using the version-script contract" would also describe a contract #1196 changes.

Suggested order for a maintainer: #1187 → #1186, with #1186 rebased and either a guard on line 29 or an explicit dependency note.

Confidence: 2/5 (needs attention) — 1 high finding; 1 medium finding

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

[castrojo]

Rebased cleanly onto latest main.

• Conflicts resolved: docs/skills/index.json and docs/skills/index.md regenerated via scripts/generate_skill_index.py.
• Tests run: bats tests/test_damask_setup.bats (6 passed), just check.

[repires]

Thank you for the review and catching the ordering nuance!

I have addressed the feedback in commit 2a2a10d:

• Guarded Unit Enablement: In 25-damask-setup.sh, wrapped the systemctl --user enable damask.service call with a unit-existence check via systemctl --user list-unit-files damask.service. This ensures that if feat(wallpaper): add Damask first-run setup hook and non-destructive defaults #1186 merges or runs before feat(damask): ship flatpak overrides and background systemd service #1187 installs damask.service, the first-run hook completes cleanly without aborting.
• Test Coverage: Updated tests/test_damask_setup.bats to mock list-unit-files and added a test asserting that damask.service enablement is gracefully skipped if the unit is not found.
• Dependency Note: Noted that this PR pairs with feat(damask): ship flatpak overrides and background systemd service #1187 (which provides damask.service and flatpak overrides).

---

🐝 Hive Agent: contributor | SHA: unknown

[Danathar]

The keyfile this hook writes is correct. I compiled Damask's real schema and pointed GLib's keyfile backend at the file this hook writes. Every key reads back as intended: refresh-interval '86400', active-source 'none', and the slideshow folder-uri and sort-by. The bats file passes locally, 7 of 7, and CI on 29f37a8 is green.

I'm holding my approval for one reason: Damask can't see the slideshow folder this hook sets up. folder-uri='file:///usr/share/backgrounds/bluefin' relies on the override in #1187, and Flatpak ignores that override because it never shares /usr paths with apps (the output is on #1187). That folder doesn't exist inside the sandbox, so a user who picks the Slideshow source gets nothing.

This matters more than usual because the hook never overwrites an existing keyfile. Every user keeps whatever folder-uri ships first, even after a later fix. So I'd like the folder-uri to match the path #1187 ends up exposing before this lands.

Landing order: #1187 needs to land first. This hook only enables damask.service if the unit file is already on disk, and version-script marks the hook done before that check runs. A user who logs in on an image that has this PR but not #1187 will never get the service enabled.

One smaller test fix: in the "non-destructive when keyfile already exists" test, ! grep -qx "active-source='none'" is in the middle of the test. Bats doesn't fail on a ! command unless it's the last line (shellcheck flags it as SC2314). I checked by changing the hook to append active-source='none' to an existing keyfile, and that test still passed. run ! grep ... or moving it to the last line fixes it.

[hanthor]

Thanks for this. A few issues:

• 25-damask-setup.sh:24: folder-uri='file:///usr/share/backgrounds/bluefin' won't resolve inside the Flatpak sandbox, because /usr there is the runtime and Damask has no host filesystem access. With a host-os:ro override (see my note on #1187), the host path is /run/host/usr/share/backgrounds/bluefin.
• refresh-interval='86400' is written as a quoted string. If the schema key is numeric, GSettings/keyfile will ignore it. Please check the key types against Damask's schema, and also confirm that 'none' is a valid active-source.
• The keyfile and ~/.var/app/app.drey.Damask get created for every user even when Damask isn't installed. Please guard on the app being present.
• This depends on the unit from #1187, so either squash the two PRs or land #1187 first. Also note #1196: if that lands, this hook needs version-script-commit.

[castrojo]

Status update on Damask setup hook:
Reviewed in conjunction with #1187:

1. Landing order: feat(damask): ship flatpak overrides and background systemd service #1187 must land before or alongside this PR so that damask.service is installed when the user-setup hook checks for it.
2. The folder URI file:///usr/share/backgrounds/bluefin depends on how feat(damask): ship flatpak overrides and background systemd service #1187 exposes host wallpapers into Flatpak (Flatpak blocks direct /usr access).
3. Test fix: ! grep in the middle of a BATS test block does not trigger test failure on non-zero exit (SC2314); needs run ! grep or position at the end of the test.

Awaiting resolution of #1187 sandbox access and test fix.

[castrojo]

@Danathar Fixed the BATS SC2314 test assertion in commit 2d992c4 by wrapping the intermediate negative grep check with run and testing status. All 7 test cases pass.

[hanthor]

The SC2314 fix in the bats test looks right. The hook itself hasn't changed, so the earlier points still stand:

• 25-damask-setup.sh:23: folder-uri='file:///usr/share/backgrounds/bluefin' isn't visible inside the sandbox. With a host-os:ro override it needs to be /run/host/usr/share/backgrounds/bluefin.
• 25-damask-setup.sh:17,20: refresh-interval is still a quoted string. Please check the key types and the 'none' value for active-source against Damask's schema.
• 25-damask-setup.sh:9-12: the keyfile and ~/.var/app/app.drey.Damask are still created for users who don't have Damask installed. Please gate on flatpak info app.drey.Damask.
• #1187 hasn't changed since my review. Its filesystems=/usr/share/backgrounds:ro override still can't work, and this PR depends on it.

[hanthor]

Thanks. folder-uri now points at /run/host/usr/share/backgrounds/bluefin, which matches the host-os:ro override in #1187.

I checked the key types against Damask's schema, and I was wrong about them. refresh-interval is an enum with a "86400" nick, so the quoted value is correct, and 'none' is a valid active-source (it's the default). No change needed there.

Still open:

• 25-damask-setup.sh:9-12: the keyfile and ~/.var/app/app.drey.Damask are still created for every user, even when Damask isn't installed. Please gate on flatpak info app.drey.Damask &>/dev/null || exit 0, and put that check before version-script on line 7. Otherwise a user who installs Damask later never gets the defaults.
• This needs #1187 to land first, and #1187 currently needs a rebase.

[castrojo]

@hanthor Addressed review feedback in commit c7a291c:

• Gated 25-damask-setup.sh on flatpak info app.drey.Damask &>/dev/null || exit 0 positioned before the version-script call, so users without Damask do not get keyfiles generated or the version stamp burned, allowing future installation to receive defaults cleanly.
• Added test coverage in tests/test_damask_setup.bats. All 8 tests pass.
• Paired PR feat(damask): ship flatpak overrides and background systemd service #1187 has also been rebased cleanly onto latest main (addressing the prerequisite damask.service and sandbox overrides). Ready for re-review.

[hanthor]

Thanks. 25-damask-setup.sh:7 now exits before version-script when Damask isn't installed, so a later install still gets the defaults, and the new bats case checks that neither the keyfile nor the stamp is written. CI is green. I'm not queueing this yet: #1187 is in the merge queue now, and I'll queue this once it has merged.