fix(installer): make three resume-fragile steps idempotent and correct

Each of these steps misbehaves on a resume re-run or under unexpected system state, so I pulled the logic into small testable helpers and fixed the behavior.

The log-cleanup cron line was appended unconditionally. essential_services is one resume step, so a failure past that point plus a re-run stacked a second identical cron line every time. crontab_append_once now appends only when no existing line matches.

The weekly ZFS scrub timer used `zpool list | head -1`, which picks an arbitrary pool when several exist and builds the malformed unit `zfs-scrub-weekly@.timer` when none do. zfs_scrub_timer_units emits one timer per pool and nothing for no pools, so every pool is scrubbed and an absent pool warns instead of enabling a broken unit.

gamemode was enabled through `systemctl --user enable`, which the script itself documents fails at install time with no user session bus -- so gamemoded was likely left un-enabled behind a silent warning. It now wires the wants symlink by hand through enable_user_service, the same helper syncthing's block folds into.

2 files changed, 174 insertions, 11 deletions

diff --git a/archsetup b/archsetup
index 646d1f9..28ec2c2 100755
--- a/archsetup
+++ b/archsetup
@@ -1131,6 +1131,40 @@ configure_build_environment() {
 
 }
 
+# Append <line> to the crontab piped in on stdin, but only when no existing
+# line contains <match>. Idempotent so a resume re-run of the step never stacks
+# a duplicate entry. Emits the resulting crontab on stdout.
+crontab_append_once() {
+    local match="$1" line="$2" existing
+    existing="$(cat)"
+    [ -n "$existing" ] && printf '%s\n' "$existing"
+    printf '%s\n' "$existing" | grep -qF "$match" || printf '%s\n' "$line"
+}
+
+# Read ZFS pool names on stdin (one per line) and print one scrub-timer unit
+# per non-empty pool. No pools in -> no output, so the caller warns instead of
+# enabling the malformed unit `zfs-scrub-weekly@.timer`; several pools each get
+# their own timer rather than an arbitrary `head -1` pick.
+zfs_scrub_timer_units() {
+    local pool
+    while read -r pool; do
+        [ -n "$pool" ] && printf 'zfs-scrub-weekly@%s.timer\n' "$pool"
+    done
+}
+
+# Enable a systemd --user service at install time by creating the wants symlink
+# directly. `systemctl --user enable` fails during install (no user session
+# bus), so the link is wired by hand -- the pattern syncthing and gamemode both
+# need. Args: <username> <service-unit> <service-file> [home-dir].
+enable_user_service() {
+    local username="$1" service="$2" service_file="$3"
+    local home="${4:-/home/$username}"
+    local wants_dir="$home/.config/systemd/user/default.target.wants"
+    mkdir -p "$wants_dir"
+    ln -sf "$service_file" "$wants_dir/$service"
+    chown -R "$username:$username" "$home/.config/systemd"
+}
+
 # Replace /etc/sudoers with its .pacnew only after visudo validates it. A
 # malformed pacnew that sudo refuses to parse would lock out privilege
 # escalation, so a validation failure warns (non-fatal) and keeps the working
@@ -1760,8 +1794,9 @@ configure_job_scheduling() {
     run_task "enabling the batch delayed command scheduler" systemctl enable atd
 
     action="installing log cleanup cron job" && display "task" "$action"
-    (sudo -u "$username" crontab -l 2>/dev/null; \
-        echo "0 12 * * * \$HOME/.local/bin/cron/log-cleanup") \
+    sudo -u "$username" crontab -l 2>/dev/null \
+        | crontab_append_once 'cron/log-cleanup' \
+            "0 12 * * * \$HOME/.local/bin/cron/log-cleanup" \
         | sudo -u "$username" crontab - \
         >> "$logfile" 2>&1 || error_warn "$action" "$?"
 }
@@ -1903,9 +1938,16 @@ EOF
         run_task "enabling sanoid timer" systemctl enable sanoid.timer
 
         action="enabling weekly ZFS scrub" && display "task" "$action"
-        # Get pool name dynamically (usually zroot)
-        zfs_pool=$(zpool list -H -o name | head -1)
-        systemctl enable "zfs-scrub-weekly@${zfs_pool}.timer" >> "$logfile" 2>&1 || error_warn "$action" "$?"
+        # Enable a scrub timer for every pool (usually just zroot). No pool ->
+        # warn rather than enabling the malformed unit zfs-scrub-weekly@.timer.
+        mapfile -t scrub_units < <(zpool list -H -o name | zfs_scrub_timer_units)
+        if [ "${#scrub_units[@]}" -eq 0 ]; then
+            error_warn "$action (no ZFS pool found)" "1"
+        else
+            for scrub_unit in "${scrub_units[@]}"; do
+                systemctl enable "$scrub_unit" >> "$logfile" 2>&1 || error_warn "$action" "$?"
+            done
+        fi
 
         # Note: zfs-replicate.timer is NOT enabled automatically
         # User must set up SSH key auth to TrueNAS first, then run:
@@ -2383,10 +2425,8 @@ desktop_environment() {
     # Enable syncthing user service by creating symlink directly
     # (systemctl --user fails during install - no user session bus available)
     action="enabling syncthing user service" && display "task" "$action"
-    user_systemd_dir="/home/$username/.config/systemd/user/default.target.wants"
-    mkdir -p "$user_systemd_dir"
-    ln -sf /usr/lib/systemd/user/syncthing.service "$user_systemd_dir/syncthing.service"
-    chown -R "$username:$username" "/home/$username/.config/systemd"
+    enable_user_service "$username" syncthing.service \
+        /usr/lib/systemd/user/syncthing.service || error_warn "$action" "$?"
 
     # Desktop Environment Utilities
 
@@ -2465,8 +2505,12 @@ gaming() {
     action="Steam" && display "subtitle" "$action"
     pacman_install steam
 
-    # Enable gamemode service for user
-    run_task "enabling gamemode for user" sudo -u "$username" systemctl --user enable gamemoded.service
+    # Enable gamemode user service. `systemctl --user enable` fails during
+    # install (no user session bus), so wire the wants symlink directly, the
+    # same way syncthing is enabled.
+    action="enabling gamemode user service" && display "task" "$action"
+    enable_user_service "$username" gamemoded.service \
+        /usr/lib/systemd/user/gamemoded.service || error_warn "$action" "$?"
 }
 
 ### Zig Toolchain Pin
diff --git a/tests/installer-steps/test_idempotency_cluster.py b/tests/installer-steps/test_idempotency_cluster.py
new file mode 100644
index 0000000..ecb279d
--- /dev/null
+++ b/tests/installer-steps/test_idempotency_cluster.py
@@ -0,0 +1,119 @@
+"""Test the resume-idempotency cluster: three extracted installer helpers.
+
+Each fix targets a step that misbehaves on a resume re-run or with unexpected
+system state:
+
+- crontab_append_once: the log-cleanup cron line was appended unconditionally,
+  so a resume re-run of essential_services stacked a duplicate line each time.
+  The helper appends only when no existing line matches.
+- zfs_scrub_timer_units: the scrub timer used `zpool list | head -1`, which
+  picks an arbitrary pool when several exist and yields the malformed unit
+  `zfs-scrub-weekly@.timer` when none do. The helper emits one unit per pool
+  and nothing for no pools.
+- enable_user_service: gamemode was enabled via `systemctl --user enable`,
+  which the script itself documents fails at install time (no user bus). The
+  helper creates the wants symlink directly, the pattern syncthing already uses.
+
+Method: sed-extract each helper from the real `archsetup` and drive it with
+plain stdin/args (the first two are pure) or a temp HOME (the third).
+
+Run from repo root:
+    python3 -m unittest tests.installer-steps.test_idempotency_cluster
+"""
+
+import os
+import subprocess
+import tempfile
+import unittest
+
+REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", ".."))
+ARCHSETUP = os.path.join(REPO_ROOT, "archsetup")
+ME = os.environ.get("USER") or __import__("getpass").getuser()
+
+
+def extract(func):
+    return (
+        "source <(sed -n '/^{f}() {{/,/^}}/p' \"{a}\")".format(f=func, a=ARCHSETUP)
+    )
+
+
+def run(func, body, stdin=None):
+    script = f"{extract(func)}\n{body}\n"
+    return subprocess.run(
+        ["bash", "-c", script], capture_output=True, text=True, timeout=10,
+        input=stdin,
+    )
+
+
+class CrontabAppendOnce(unittest.TestCase):
+    def call(self, existing, match, line):
+        body = f'crontab_append_once {match!r} {line!r}'
+        return run("crontab_append_once", body, stdin=existing).stdout
+
+    def test_appends_when_absent(self):
+        out = self.call("", "log-cleanup", "0 12 * * * run-cleanup")
+        self.assertIn("0 12 * * * run-cleanup", out)
+
+    def test_preserves_existing_and_appends(self):
+        out = self.call("0 0 * * * other-job\n", "log-cleanup", "0 12 * * * cleanup")
+        self.assertIn("other-job", out)
+        self.assertIn("0 12 * * * cleanup", out)
+
+    def test_does_not_duplicate_when_present(self):
+        existing = "0 12 * * * $HOME/.local/bin/cron/log-cleanup\n"
+        out = self.call(existing, "cron/log-cleanup", "0 12 * * * dup")
+        self.assertNotIn("0 12 * * * dup", out)
+        self.assertEqual(out.count("log-cleanup"), 1)
+
+
+class ZfsScrubTimerUnits(unittest.TestCase):
+    def units(self, pools_text):
+        out = run("zfs_scrub_timer_units", "zfs_scrub_timer_units", stdin=pools_text).stdout
+        return [ln for ln in out.splitlines() if ln]
+
+    def test_single_pool(self):
+        self.assertEqual(self.units("zroot\n"),
+                         ["zfs-scrub-weekly@zroot.timer"])
+
+    def test_no_pool_yields_no_unit(self):
+        self.assertEqual(self.units(""), [])
+
+    def test_multiple_pools_each_get_a_unit(self):
+        self.assertEqual(
+            self.units("zroot\ntank\n"),
+            ["zfs-scrub-weekly@zroot.timer", "zfs-scrub-weekly@tank.timer"],
+        )
+
+    def test_blank_lines_are_skipped(self):
+        self.assertEqual(self.units("zroot\n\n"),
+                         ["zfs-scrub-weekly@zroot.timer"])
+
+
+class EnableUserService(unittest.TestCase):
+    def test_creates_wants_symlink_in_user_config(self):
+        with tempfile.TemporaryDirectory() as home:
+            body = (
+                f'enable_user_service {ME!r} gamemoded.service '
+                f'/usr/lib/systemd/user/gamemoded.service {home!r}\n'
+                f'link="{home}/.config/systemd/user/default.target.wants/gamemoded.service"\n'
+                f'[ -L "$link" ] && echo "LINK=yes" || echo "LINK=no"\n'
+                f'echo "TARGET=$(readlink "$link")"'
+            )
+            out = run("enable_user_service", body).stdout
+            self.assertIn("LINK=yes", out)
+            self.assertIn("TARGET=/usr/lib/systemd/user/gamemoded.service", out)
+
+    def test_idempotent_on_rerun(self):
+        with tempfile.TemporaryDirectory() as home:
+            svc = "/usr/lib/systemd/user/gamemoded.service"
+            one = (
+                f'enable_user_service {ME!r} gamemoded.service {svc!r} {home!r}\n'
+                f'enable_user_service {ME!r} gamemoded.service {svc!r} {home!r}\n'
+                f'echo "RC=$?"'
+            )
+            out = run("enable_user_service", one).stdout
+            self.assertIn("RC=0", out)
+
+
+if __name__ == "__main__":
+    unittest.main()