fix(installer): eight fixes from an overnight bug-hunt and its review

I squashed these because the per-bug reasoning lives in todo.org, which this commit carries.

Two could cost a machine. configure_initramfs_hook swapped the udev hook for systemd on a LUKS root, leaving a standalone encrypt hook under an init that never runs it. The rebuild succeeds and the installer exits clean, then the root won't unlock at the next boot. trim_firmware ran pacman -Rdd against twelve firmware packages behind a DMI gate reading product_name, where "Framework" never appears. That left it dead on the hardware it targets, and dangerous to fix the obvious way: this machine is a Framework Desktop whose Ryzen iGPU needs the amdgpu firmware. It refuses on PCI modalias evidence now.

wipedisk discarded before it checked. blkdiscard ran with -f, which disables the exclusive open, so picking the wrong disk destroyed a live filesystem and then reported that nothing had happened.

Four more are smaller. The NVIDIA preflight aborted dwm and headless installs over a driver floor they never need. zfs-replicate exited 0 after every dataset failed. Unattended installs blocked on two prompts, and the first fix for that inherited a [Y/n] default into passwordless console login. A fresh install left the dotfiles repo permanently dirty.

The review found a pattern worth more than any single fix. Helpers had thorough tests and none proved they were called. Deleting the call left five suites green, including the guard on that pacman -Rdd. CALL_SITES now pins nine caller/callee pairs.

The suite runs 341 tests at exit 0, with no new shellcheck findings. I proved every guard by deleting it and watching the intended test go red.

2 files changed, 90 insertions, 11 deletions

diff --git a/scripts/wipedisk b/scripts/wipedisk
index b833407..9b1b4ab 100644
--- a/scripts/wipedisk
+++ b/scripts/wipedisk
@@ -1,9 +1,29 @@
 #!/usr/bin/env bash
 # SPDX-License-Identifier: GPL-3.0-or-later
 # Craig Jennings <c@cjennings.net>
-# identify disk and erase 
+# identify disk and erase
+
+# Overridable so the test suite can point at a fixture directory, the way
+# nvidia_preflight_report takes NVIDIA_DRM_GLOB.
+by_id_dir="${WIPEDISK_BY_ID:-/dev/disk/by-id}"
+
+# Whole disks only. /dev/disk/by-id publishes a -partN entry for every
+# partition alongside the disks (18 entries on this machine, 12 of them
+# partitions), and the prompt below promises a disk. Globbed rather than
+# `ls | grep` so a name with whitespace can't split into two menu entries.
+all_disk_ids=()
+for entry in "$by_id_dir"/*; do
+    [ -e "$entry" ] || continue          # unmatched glob stays literal
+    name="${entry##*/}"
+    [[ "$name" =~ -part[0-9]+$ ]] && continue
+    all_disk_ids+=("$name")
+done
+
+if [ "${#all_disk_ids[@]}" -eq 0 ]; then
+    echo "No disks found in $by_id_dir." >&2
+    exit 1
+fi
 
-all_disk_ids=( $(ls /dev/disk/by-id/) )
 echo ""; echo "Select the disk id to use. All data will be erased."
 select disk_id in "${all_disk_ids[@]}"; do
     # ensure valid selection
@@ -16,16 +36,58 @@ select disk_id in "${all_disk_ids[@]}"; do
 done
 
 # Confirm the selected disk
-read -p "Confirm: '$selection' [y/n]? " choice
+read -r -p "Confirm: '$selection' [y/n]? " choice
 if [[ "$choice" != "y" ]]; then
     echo "Exiting..."
     exit 1
 fi
 
-DISK="/dev/disk/by-id/$selection"
+DISK="$by_id_dir/$selection"
 echo ""; echo "### Erasing Disk"
-blkdiscard -f "${DISK}" || true    # discard all sectors on flash-based storage
-sgdisk --zap-all "${DISK}"         # clear the disk
+
+# blkdiscard opens the device O_EXCL by default (util-linux >= 2.36) and
+# refuses a disk something is holding -- a mounted filesystem, an md member, an
+# LVM PV. That refusal is this script's safety gate, and it has to run BEFORE
+# anything destructive. Passing -f disables the exclusive open, which turned
+# "refuse the wrong disk" into "discard a live filesystem, then let sgdisk fail
+# and tell the user nothing happened". The gate is the kernel's answer, not a
+# heuristic of ours, so no -f.
+discarded=true
+discard_err=$(blkdiscard "${DISK}" 2>&1) || discarded=false
+
+# A discard can fail for two very different reasons. "Operation not supported"
+# means the hardware has no discard -- the run continues and the closing
+# message says the data is still there. Anything else means the device was
+# refused, and nothing has been written yet, so stop while that is still true.
+if [ "$discarded" = false ] \
+   && ! printf '%s' "$discard_err" | grep -qi 'not supported'; then
+    echo "" >&2
+    echo "REFUSED: '$selection' is in use. Nothing was erased." >&2
+    [ -n "$discard_err" ] && echo "  $discard_err" >&2
+    echo "  Unmount its filesystems and stop any md/LVM/ZFS holder, then run" >&2
+    echo "  this again." >&2
+    exit 1
+fi
+
+# sgdisk refuses a busy device too. Reaching here means blkdiscard already
+# accepted the disk, so a failure now is something else -- report it rather
+# than announcing an erase that did not happen.
+if ! sgdisk --zap-all "${DISK}"; then
+    echo "" >&2
+    echo "FAILED: could not clear the partition table on '$selection'." >&2
+    echo "  The sectors were discarded but the partition table was not" >&2
+    echo "  rewritten. Check the device and run this again." >&2
+    exit 1
+fi
 
 echo ""
-echo "Disk erased." 
\ No newline at end of file
+if [ "$discarded" = true ]; then
+    echo "Disk erased. Sectors discarded and the partition table cleared."
+else
+    # Say what actually happened. sgdisk --zap-all destroys partition tables,
+    # not data, so without a successful discard every byte is still readable.
+    echo "Partition table cleared on '$selection'."
+    echo "NOTE: this device did not support discard, so the data is still"
+    echo "      present and recoverable. For a disposal-grade wipe use the"
+    echo "      drive's own secure erase (nvme format, hdparm) or overwrite it."
+fi
diff --git a/scripts/zfs-replicate b/scripts/zfs-replicate
index 02ffcf5..a4cb15d 100755
--- a/scripts/zfs-replicate
+++ b/scripts/zfs-replicate
@@ -25,9 +25,14 @@ YELLOW='\033[1;33m'
 RED='\033[0;31m'
 NC='\033[0m'
 
-info()  { echo -e "${GREEN}[INFO]${NC} $1"; }
-warn()  { echo -e "${YELLOW}[WARN]${NC} $1"; }
-error() { echo -e "${RED}[ERROR]${NC} $1"; exit 1; }
+# Diagnostics go to stderr, never stdout. determine_host below runs inside a
+# command substitution, so anything these print on stdout would be captured
+# into TRUENAS_HOST instead of reaching the terminal or the journal -- which is
+# exactly how an unreachable TrueNAS used to abort this script with exit 1 and
+# no output at all. stderr also keeps the captured hostname clean.
+info()  { echo -e "${GREEN}[INFO]${NC} $1" >&2; }
+warn()  { echo -e "${YELLOW}[WARN]${NC} $1" >&2; }
+error() { echo -e "${RED}[ERROR]${NC} $1" >&2; exit 1; }
 
 command -v syncoid >/dev/null 2>&1 || error "syncoid not found. Install sanoid package."
 
@@ -58,6 +63,12 @@ fi
 info "Starting ZFS replication to $TRUENAS_HOST"
 echo ""
 
+# A failed dataset must not stop the others -- but it must not be forgotten
+# either. This runs as a systemd oneshot on a nightly timer, so the exit code
+# is the only signal anyone sees; exiting 0 after every dataset failed made a
+# backup that had not run in months look identical to a working one.
+failed=0
+
 for dataset in $DATASETS; do
     dest="$TRUENAS_USER@$TRUENAS_HOST:$TRUENAS_POOL/$BACKUP_PATH/${dataset#zroot/}"
     info "Replicating $dataset -> $dest"
@@ -66,8 +77,14 @@ for dataset in $DATASETS; do
         info "  Success"
     else
         warn "  Failed (will retry next run)"
+        failed=$((failed + 1))
     fi
-    echo ""
+    echo "" >&2
 done
 
+if [ "$failed" -gt 0 ]; then
+    warn "Replication complete with $failed of $(echo "$DATASETS" | wc -w) datasets failed."
+    exit 1
+fi
+
 info "Replication complete."