From 3becfac66dc569e71d0f6085fb56e92cfa6d626a Mon Sep 17 00:00:00 2001 From: Craig Jennings Date: Fri, 24 Jul 2026 12:19:04 -0500 Subject: 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. --- archsetup | 397 +++++++++-- archsetup.conf.example | 7 +- scripts/wipedisk | 76 ++- scripts/zfs-replicate | 25 +- tests/installer-steps/test_configure_autologin.py | 218 ++++++ .../test_framework_firmware_trim.py | 233 +++++++ .../installer-steps/test_mark_volatile_configs.py | 216 ++++++ tests/installer-steps/test_orchestrators.py | 58 ++ tests/installer-steps/test_select_locale.py | 160 +++++ tests/installer-steps/test_switch_udev_hook.py | 178 +++++ .../nvidia-preflight/test_nvidia_preflight_gate.py | 173 +++++ tests/wipedisk/test_wipedisk.py | 273 ++++++++ tests/zfs-replicate/test_zfs_replicate.py | 174 +++++ todo.org | 754 ++++++++++++++++++++- 14 files changed, 2847 insertions(+), 95 deletions(-) create mode 100644 tests/installer-steps/test_configure_autologin.py create mode 100644 tests/installer-steps/test_framework_firmware_trim.py create mode 100644 tests/installer-steps/test_mark_volatile_configs.py create mode 100644 tests/installer-steps/test_select_locale.py create mode 100644 tests/installer-steps/test_switch_udev_hook.py create mode 100644 tests/nvidia-preflight/test_nvidia_preflight_gate.py create mode 100644 tests/wipedisk/test_wipedisk.py create mode 100644 tests/zfs-replicate/test_zfs_replicate.py diff --git a/archsetup b/archsetup index 9d2c946..545edf4 100755 --- a/archsetup +++ b/archsetup @@ -489,6 +489,49 @@ nvidia_preflight_report() { return 11 } +# NVIDIA/Wayland preflight policy: decide whether the check applies, run the +# report, and act on its verdict. Split out of preflight_checks so the policy +# is unit testable the same way nvidia_preflight_report's detection core is. +# +# The check is Wayland-specific. Every line it prints names Wayland/Hyprland, +# and its own failure hint tells the user to install with DESKTOP_ENV=dwm +# instead -- advice the gate itself used to refuse to honor, since it ran +# before any desktop_env branch and aborted an X11 or headless install over a +# driver floor neither of them needs. So it applies only to a hyprland install, +# and only when archsetup is the one installing the driver: --no-gpu-drivers / +# NO_GPU_DRIVERS=yes means the user handles it themselves, which is the ruling +# install_gpu_drivers already makes. +# +# On a detected card whose driver meets the floor (rc 10) the prompt is +# advisory, so an unattended run (--config-file, the documented unattended +# mode) answers it with the prompt's own default rather than blocking on a +# read -- the failure aur_install's --answerdiff/--answerclean comment already +# rules out. A driver below the floor (rc 11) is still a hard stop either way. +nvidia_preflight() { + if [ "$skip_gpu_drivers" = "true" ]; then + return 0 + fi + if [ "$desktop_env" != "hyprland" ]; then + return 0 + fi + + local nvidia_rc=0 + nvidia_preflight_report || nvidia_rc=$? + if [ "$nvidia_rc" = "11" ]; then + exit 1 + elif [ "$nvidia_rc" = "10" ]; then + if [ -n "$config_file" ]; then + echo " [OK] Continuing on NVIDIA (unattended, --config-file)." + return 0 + fi + read -r -p "Continue installing for Wayland on NVIDIA? [Y/n]: " nvidia_go + case "$nvidia_go" in + [nN]*) echo "Aborted at NVIDIA preflight."; exit 1 ;; + *) echo " [OK] Continuing on NVIDIA." ;; + esac + fi +} + check_disk_space() { # df -P forces POSIX single-line output; plain `df` wraps to two lines when # the source device name is long (live ISO / device-mapper root), which @@ -513,6 +556,63 @@ check_disk_space() { echo " [OK] Disk space: $((available_kb / 1024 / 1024))GB available" } +# Resolve the system locale for this install and echo what was chosen. $1 is +# the locale.conf path, defaulting to the system file so the step runs against +# fixtures. Sets the global $locale only when it picks one -- an already +# configured locale.conf stays authoritative and configure_build_environment +# reads LANG straight out of it. +# +# The menu is advisory: it carries its own "[1]" default. LOCALE is optional in +# archsetup.conf.example and archsetup does not require an archangel install, +# so an unattended run (--config-file) can reach this with no locale.conf and +# no LOCALE key -- the state configure_build_environment's own "no LANG=" branch +# anticipates. Unattended takes the default rather than blocking on a read, +# the same ruling nvidia_preflight and configure_autologin make for theirs. +select_locale() { + local conf="${1:-/etc/locale.conf}" + if grep -q "^LANG=" "$conf" 2>/dev/null; then + echo " [OK] Locale: $(grep "^LANG=" "$conf" | cut -d= -f2)" + return 0 + fi + if [[ -n "$locale" ]]; then + echo " [OK] Locale: $locale (from config)" + return 0 + fi + echo "" + echo "Locale not configured. Please select:" + echo " 1) en_US.UTF-8 (US English)" + echo " 2) en_GB.UTF-8 (UK English)" + echo " 3) de_DE.UTF-8 (German)" + echo " 4) es_ES.UTF-8 (Spanish)" + echo " 5) fr_FR.UTF-8 (French)" + echo " 6) pt_BR.UTF-8 (Brazilian Portuguese)" + echo " 7) ja_JP.UTF-8 (Japanese)" + echo " 8) zh_CN.UTF-8 (Chinese)" + echo " 9) Other (enter manually)" + local choice="" + if [ -n "$config_file" ]; then + echo " [OK] Taking choice 1 (unattended, --config-file)." + else + read -r -p "Choice [1]: " choice + fi + case "${choice:-1}" in + 1) locale="en_US.UTF-8" ;; + 2) locale="en_GB.UTF-8" ;; + 3) locale="de_DE.UTF-8" ;; + 4) locale="es_ES.UTF-8" ;; + 5) locale="fr_FR.UTF-8" ;; + 6) locale="pt_BR.UTF-8" ;; + 7) locale="ja_JP.UTF-8" ;; + 8) locale="zh_CN.UTF-8" ;; + 9) + read -r -p "Enter locale (e.g., nl_NL.UTF-8): " locale + [[ -z "$locale" ]] && locale="en_US.UTF-8" + ;; + *) locale="en_US.UTF-8" ;; + esac + echo " [OK] Locale: $locale (selected)" +} + preflight_checks() { echo "Running pre-flight checks..." @@ -546,55 +646,11 @@ preflight_checks() { echo " [OK] System: Arch Linux detected" # NVIDIA + Wayland preflight: abort on a too-old driver, confirm on a - # detected card (the report prints the guidance + env vars). - local nvidia_rc=0 - nvidia_preflight_report || nvidia_rc=$? - if [ "$nvidia_rc" = "11" ]; then - exit 1 - elif [ "$nvidia_rc" = "10" ]; then - read -r -p "Continue installing for Wayland on NVIDIA? [Y/n]: " nvidia_go - case "$nvidia_go" in - [nN]*) echo "Aborted at NVIDIA preflight."; exit 1 ;; - *) echo " [OK] Continuing on NVIDIA." ;; - esac - fi + # detected card (the report prints the guidance + env vars). Scoped to the + # installs it applies to -- see nvidia_preflight. + nvidia_preflight - # Check locale configuration - if grep -q "^LANG=" /etc/locale.conf 2>/dev/null; then - current_locale=$(grep "^LANG=" /etc/locale.conf | cut -d= -f2) - echo " [OK] Locale: $current_locale" - elif [[ -n "$locale" ]]; then - echo " [OK] Locale: $locale (from config)" - else - echo "" - echo "Locale not configured. Please select:" - echo " 1) en_US.UTF-8 (US English)" - echo " 2) en_GB.UTF-8 (UK English)" - echo " 3) de_DE.UTF-8 (German)" - echo " 4) es_ES.UTF-8 (Spanish)" - echo " 5) fr_FR.UTF-8 (French)" - echo " 6) pt_BR.UTF-8 (Brazilian Portuguese)" - echo " 7) ja_JP.UTF-8 (Japanese)" - echo " 8) zh_CN.UTF-8 (Chinese)" - echo " 9) Other (enter manually)" - read -r -p "Choice [1]: " choice - case "${choice:-1}" in - 1) locale="en_US.UTF-8" ;; - 2) locale="en_GB.UTF-8" ;; - 3) locale="de_DE.UTF-8" ;; - 4) locale="es_ES.UTF-8" ;; - 5) locale="fr_FR.UTF-8" ;; - 6) locale="pt_BR.UTF-8" ;; - 7) locale="ja_JP.UTF-8" ;; - 8) locale="zh_CN.UTF-8" ;; - 9) - read -r -p "Enter locale (e.g., nl_NL.UTF-8): " locale - [[ -z "$locale" ]] && locale="en_US.UTF-8" - ;; - *) locale="en_US.UTF-8" ;; - esac - echo " [OK] Locale: $locale (selected)" - fi + select_locale # Prompt for username if not set; validate either way if [[ -z "$username" ]]; then @@ -866,7 +922,34 @@ has_nvme_drives() { ### System Configuration +# The tty1 autologin drop-in. $1 is the drop-in directory, defaulting to the +# system path so the step runs against fixtures. +# +# The auto-detect prompt fires only on an encrypted root. --config-file is the +# documented unattended mode and AUTOLOGIN is optional in +# archsetup.conf.example, so an unattended install of an encrypted machine used +# to reach a bare read with nobody there -- and this step runs from boot_ux, the +# last step, so it blocked at the end of the run. +# +# Where nobody can answer, autologin stays OFF. Inheriting the prompt's [Y/n] +# default looks reasonable and is not: a prompt default is calibrated for +# someone who can override it in the moment, and unattended there is no +# override, so the default stops being advisory and becomes policy. The policy +# here is passwordless console login. "Encrypted root means already +# authenticated" is true at boot and nowhere else -- not for a stolen +# powered-on machine, where tty1 sits one Ctrl-Alt-F1 away and a screen locker +# does not lock the console, and not for an install someone performs on behalf +# of the eventual owner. +# +# The error costs decide it. Wrongly ON is a silent authentication downgrade +# nobody notices. Wrongly OFF is one password prompt at boot, noticed at once +# and fixed with a config key that already exists. An explicit AUTOLOGIN=yes/no +# always wins either way. +# +# Two paths reach "nobody can answer": an unattended run, and a read that fails +# because stdin is closed. Both decline, and both say so. configure_autologin() { + local dropin_dir="${1:-/etc/systemd/system/getty@tty1.service.d}" local do_autologin=false # Determine whether to enable autologin @@ -882,15 +965,30 @@ configure_autologin() { echo "Since the disk is encrypted, you already authenticate at boot." echo "Automatic login skips the redundant login prompt after decryption." echo "" - read -r -p "Enable automatic console login for $username? [Y/n] " response - case "$response" in - [nN][oO]|[nN]) - do_autologin=false - ;; - *) - do_autologin=true - ;; - esac + if [ -n "$config_file" ]; then + echo " [--] AUTOLOGIN is unset and this is an unattended run;" + echo " leaving console autologin DISABLED. Set AUTOLOGIN=yes" + echo " in the config file to enable it." + do_autologin=false + elif read -r -p "Enable automatic console login for $username? [Y/n] " response; then + case "$response" in + [nN][oO]|[nN]) + do_autologin=false + ;; + *) + do_autologin=true + ;; + esac + else + # read failed, so stdin is at EOF and nobody is there to answer. + # An empty response would otherwise fall through to the [Y/n] + # default -- same silent enable as the unattended case above, + # reached a different way. + echo "" + echo " [--] no answer available (stdin closed); leaving console" + echo " autologin DISABLED. Set AUTOLOGIN=yes to enable it." + do_autologin=false + fi else # Not encrypted, skip autologin silently return 0 @@ -899,8 +997,8 @@ configure_autologin() { if [ "$do_autologin" = "true" ]; then action="configuring automatic console login" && display "task" "$action" - mkdir -p /etc/systemd/system/getty@tty1.service.d - cat << EOF > /etc/systemd/system/getty@tty1.service.d/autologin.conf + mkdir -p "$dropin_dir" + cat << EOF > "$dropin_dir/autologin.conf" [Service] ExecStart= ExecStart=-/sbin/agetty -o '-p -f -- \\\\u' --noclear --autologin $username %I \$TERM @@ -1450,6 +1548,29 @@ EOF } +# Set git's skip-worktree bit on the stowed configs their apps rewrite in place +# (btop, qalculate, calibre, waypaper). Args: $1 dotfiles dir, $2 username, both +# defaulting to the globals so the step runs against fixtures. +# +# This calls the dotfiles repo's own `skip-volatile` target rather than +# reimplementing it, because the list of volatile paths lives there and a second +# copy would drift. The dotfiles Makefile's `stow` target ends with the same +# call; archsetup stows inline instead, so without this the bit never gets set +# on a machine archsetup installed. The repo then goes dirty the first time one +# of those apps writes its config, and every later `git pull --ff-only` trips +# over paths the user never touched. +# +# Runs as the user: root writing .git/index would leave it root-owned. +# A checkout with no Makefile (a fork, or one predating the target) is a quiet +# no-op rather than a warning -- there is nothing to delegate to. +mark_volatile_configs() { + local dir="${1:-$dotfiles_dir}" user="${2:-$username}" + [ -f "$dir/Makefile" ] || return 0 + action="marking volatile dotfiles skip-worktree" && display "task" "$action" + sudo -u "$user" make -C "$dir" skip-volatile >> "$logfile" 2>&1 \ + || error_warn "$action" "$?" +} + finalize_dotfiles() { action="marking archsetup dir as safe.directory" && display "task" "$action" git config --global --add safe.directory "$user_archsetup_dir" >> "$logfile" 2>&1 \ @@ -1462,6 +1583,9 @@ finalize_dotfiles() { # files too, so its --adopt needs the same restore. run_task "restoring dotfile versions" git -C "$dotfiles_dir" restore . + # After the restore, so the bit lands on a pristine tree. + mark_volatile_configs + } install_maintenance_config() { @@ -3074,14 +3198,65 @@ add_nvme_early_module() { } +# Does this HOOKS line carry a busybox-init hook a systemd initramfs cannot +# run? mkinitcpio's `systemd` hook symlinks /init to systemd, which never +# executes the ash `run_hook` scripts the busybox init drives, so such a hook +# is installed into the image and then silently never runs. +# +# `encrypt` is the boot-critical member of that set. It is what unlocks a LUKS +# root, and its `cryptdevice=` cmdline parameter -- which merge_grub_cmdline +# already treats as boot-critical -- is read by nothing else. The systemd +# equivalent is `sd-encrypt` driven by `rd.luks.*`, so converting between them +# means rewriting the kernel cmdline against the volume's UUID. That is not a +# thing to do behind a comment about fsck message routing. +# +# Matches a standalone `encrypt` (or `plymouth-encrypt`) token only, so +# `sd-encrypt` -- which IS the systemd hook -- does not trigger a refusal. +# Args: $1 the HOOKS line. Returns 0 when the swap must be refused. +hooks_need_busybox_init() { + printf '%s\n' "$1" | grep -qE '[([:space:]](plymouth-)?encrypt[[:space:])]' +} + +# Swap the `udev` initramfs hook for `systemd` so fsck output routes through +# systemd. Conf path is a positional arg defaulting to the system file so the +# step runs against fixtures, matching ensure_nvme_early_module. +# +# Declines the swap when the conf carries a busybox-only hook the systemd init +# would leave inert (see hooks_need_busybox_init) -- mkinitcpio has no conflict +# check of its own, so without this the rebuild succeeds, archsetup reports +# success, and a LUKS machine fails to unlock its root at the next boot. +# +# Returns 0 only when the conf was rewritten, so the caller rebuilds the +# initramfs exactly when something changed. +switch_udev_hook_to_systemd() { + local conf="${1:-/etc/mkinitcpio.conf}" + local hooks_line + hooks_line=$(grep '^HOOKS=' "$conf" 2>/dev/null) + + if hooks_need_busybox_init "$hooks_line"; then + display "task" "keeping the udev initramfs hook: HOOKS carries encrypt, which a systemd initramfs never runs" + return 1 + fi + case "$hooks_line" in + *udev*) : ;; + *) return 1 ;; + esac + + action="delegating fsck messages from udev to systemd" && display "task" "$action" + backup_system_file "$conf" + sed -i '/^HOOKS=/ s/\budev\b/systemd/' "$conf" 2>> "$logfile" || { + error_warn "$action" "$?" + return 1 + } +} + configure_initramfs_hook() { # Only switch to systemd hook for non-ZFS systems # ZFS initramfs hook is busybox-based and incompatible with systemd hook if ! is_zfs_root; then - action="delegating fsck messages from udev to systemd" && display "task" "$action" - backup_system_file /etc/mkinitcpio.conf - sed -i '/^HOOKS=/ s/\budev\b/systemd/' /etc/mkinitcpio.conf 2>> "$logfile" || error_warn "$action" "$?" - mkinitcpio -P >> "$logfile" 2>&1 || error_warn "running mkinitcpio -P to silence fsck messages" "$?" + if switch_udev_hook_to_systemd; then + mkinitcpio -P >> "$logfile" 2>&1 || error_warn "running mkinitcpio -P to silence fsck messages" "$?" + fi fi action="configuring quiet fsck output" && display "task" "$action" @@ -3152,16 +3327,100 @@ EOF } +# Which GPU vendors does this machine physically have? Prints amd / intel / +# nvidia, one per line, for each vendor found. $1 and $2 are the DRM and PCI +# modalias globs, defaulting to the system paths so the probe runs against +# fixtures -- the same shape nvidia_preflight_report already uses. +# +# This is the read install_gpu_drivers and nvidia_preflight_report each already +# do, made reusable. trim_firmware needs it because a DMI model string is a +# marketing name: it cannot see a swapped card, and it cannot see hardware an +# allowlist has never met. The kernel's modalias can. +# +# Vendor IDs: 1002=AMD, 8086=Intel, 10DE=NVIDIA. The PCI fallback covers early +# boot and chroot, where DRM nodes may not exist yet, and filters to display +# class (bc03) so an NVIDIA audio function on the same card doesn't count. +detect_gpu_vendors() { + local drm_glob="${1:-/sys/class/drm/card*/device/modalias}" + local pci_glob="${2:-/sys/bus/pci/devices/*/modalias}" + local found="" modalias_file modalias + + _note_vendor() { + case " $found " in *" $1 "*) ;; *) found="$found $1" ;; esac + } + + # shellcheck disable=SC2086 # the unquoted vars ARE the globs + for modalias_file in $drm_glob; do + [ -f "$modalias_file" ] && [ -r "$modalias_file" ] || continue + modalias=$(cat "$modalias_file" 2>/dev/null) + case "$modalias" in *v00008086*) _note_vendor intel ;; esac + case "$modalias" in *v00001002*) _note_vendor amd ;; esac + case "$modalias" in *v000010DE*|*v000010de*) _note_vendor nvidia ;; esac + done + + if [ -z "$found" ]; then + # shellcheck disable=SC2086 + for modalias_file in $pci_glob; do + [ -f "$modalias_file" ] && [ -r "$modalias_file" ] || continue + modalias=$(cat "$modalias_file" 2>/dev/null) + case "$modalias" in *bc03*) ;; *) continue ;; esac + case "$modalias" in *v00008086*) _note_vendor intel ;; esac + case "$modalias" in *v00001002*) _note_vendor amd ;; esac + case "$modalias" in *v000010DE*|*v000010de*) _note_vendor nvidia ;; esac + done + fi + + unset -f _note_vendor + for modalias in $found; do printf '%s\n' "$modalias"; done +} + +# Is this a Framework Intel laptop -- the one hardware set trim_firmware knows? +# $1 is the DMI directory, defaulting to the system path so the gate runs +# against fixtures. +# +# Three DMI facts, not one. "Framework" appears in sys_vendor and NEVER in +# product_name, so the older gate (grep framework product_name) matched no +# Framework machine at all and the trim was dead code -- velox reads +# product_name "Laptop (13th Gen Intel Core)". Vendor alone is not enough +# either: ratio is a Framework Desktop reading "Desktop (AMD Ryzen AI Max 300 +# Series)", and trimming there removes linux-firmware-amdgpu from the machine +# whose iGPU needs it to bring up a display. So the model must say both Laptop +# and Intel. +is_framework_intel_laptop() { + local dmi="${1:-/sys/class/dmi/id}" + local vendor model + vendor="$(cat "$dmi/sys_vendor" 2>/dev/null)" + model="$(cat "$dmi/product_name" 2>/dev/null)" + printf '%s' "$vendor" | grep -qi "framework" || return 1 + printf '%s' "$model" | grep -qi "laptop" || return 1 + printf '%s' "$model" | grep -qi "intel" || return 1 + return 0 +} + trim_firmware() { - # Firmware trim — Framework 13 Intel only (matched by DMI), where the - # hardware set is known: i915 graphics (linux-firmware-intel), ath9k wifi - # (linux-firmware-atheros, firmware-free driver but kept for safety), and - # the RTL8156 Ethernet expansion card (linux-firmware-realtek — its r8152 - # firmware load fails without it; found on velox's first post-trim boot). + # Firmware trim — Framework Intel laptops only (matched by DMI, see + # is_framework_intel_laptop), where the hardware set is known: i915 + # graphics (linux-firmware-intel), ath9k wifi (linux-firmware-atheros, + # firmware-free driver but kept for safety), and the RTL8156 Ethernet + # expansion card (linux-firmware-realtek — its r8152 firmware load fails + # without it; found on velox's first post-trim boot). # Saves ~550MB. The meta package needs -Rdd because mkinitcpio-firmware # declares a dependency on it; the dangling dep is cosmetic and the # subsequent mkinitcpio warnings cover hardware this machine doesn't have. - if grep -qi "framework" /sys/class/dmi/id/product_name 2>/dev/null && \ + # + # Three gates, and the GPU one is the load-bearing one. The DMI check says + # "this is a hardware set we know"; the cpuinfo check is a second opinion on + # the CPU. Neither can see the actual graphics device, and this step deletes + # graphics firmware, so refuse outright when the kernel reports an AMD or + # NVIDIA GPU present -- that is evidence, where a model string is a name. + local gpus + gpus="$(detect_gpu_vendors)" + if printf '%s\n' "$gpus" | grep -qE '^(amd|nvidia)$'; then + display "task" "skipping firmware trim: a non-Intel GPU is present" + return 0 + fi + + if is_framework_intel_laptop && \ grep -qi "intel" /proc/cpuinfo 2>/dev/null; then action="trimming firmware packages to Framework 13 hardware" && display "task" "$action" pacman -Rdd --noconfirm \ diff --git a/archsetup.conf.example b/archsetup.conf.example index 82f6898..723c035 100644 --- a/archsetup.conf.example +++ b/archsetup.conf.example @@ -28,6 +28,10 @@ # Automatic console login after disk decryption (default: prompt on encrypted systems) # Options: yes, no # Only relevant for systems with encrypted root (LUKS or ZFS native encryption) +# Unattended runs (--config-file) cannot answer a prompt, so leaving this unset +# leaves console autologin DISABLED and says so. Passwordless console login is +# not something to inherit from a prompt default nobody saw. Set AUTOLOGIN=yes +# to opt in. #AUTOLOGIN=yes # Skip GPU driver auto-detection and installation (default: no) @@ -44,7 +48,8 @@ # specific hardware; set to "no" if your machine doesn't have those devices. #INSTALL_DEVICE_UDEV_RULES=yes -# System locale (default: prompt if not already configured) +# System locale (default: prompt if not already configured; an unattended run +# with no locale.conf takes en_US.UTF-8, the menu's own default) # Common options: en_US.UTF-8, en_GB.UTF-8, de_DE.UTF-8, es_ES.UTF-8, # fr_FR.UTF-8, pt_BR.UTF-8, ja_JP.UTF-8, zh_CN.UTF-8 #LOCALE=en_US.UTF-8 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 -# 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." diff --git a/tests/installer-steps/test_configure_autologin.py b/tests/installer-steps/test_configure_autologin.py new file mode 100644 index 0000000..b4bc1c4 --- /dev/null +++ b/tests/installer-steps/test_configure_autologin.py @@ -0,0 +1,218 @@ +"""Tests for configure_autologin in the archsetup installer. + +configure_autologin decides whether tty1 gets an agetty --autologin drop-in. +Three inputs decide it: --autologin / --no-autologin (or AUTOLOGIN=yes/no in +the config file), which are explicit; and, when neither is set, an auto-detect +branch that prompts -- but only on an encrypted root, where the user has +already authenticated at boot. + +That prompt is the reason these tests exist. --config-file is the documented +unattended mode and AUTOLOGIN is optional in archsetup.conf.example, so an +unattended install of an encrypted machine reaches an interactive read with +nobody there to answer it. The step runs from boot_ux, the last step in STEPS, +so it blocks at the end of a 40-60 minute run. The prompt is advisory (it +carries its own [Y/n] default), so unattended must answer it rather than block +-- the same ruling nvidia_preflight already makes for its rc-10 prompt. + +These tests exercise the REAL function body, extracted from the `archsetup` +script at run time (not a copy), with is_encrypted_root stubbed to a chosen +answer and the drop-in written under a temp dir. + +Run from repo root: + python3 -m unittest tests.installer-steps.test_configure_autologin +""" + +import os +import shutil +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") + + +class ConfigureAutologinHarness(unittest.TestCase): + """Source configure_autologin out of the real archsetup script.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="autologin-test-") + self.dropin_dir = os.path.join(self.tmp, "getty@tty1.service.d") + self.wrapper = os.path.join(self.tmp, "run.sh") + with open(self.wrapper, "w") as f: + f.write( + "#!/bin/bash\n" + 'ARCHSETUP="$1"; shift\n' + "source <(sed -n " + "'/^configure_autologin() {/,/^}/p' \"$ARCHSETUP\")\n" + # The step's only other collaborators: a display helper and the + # encryption probe. Both stubbed so the body is what runs. + 'display() { echo "DISPLAY:$2"; }\n' + "is_encrypted_root() { return \"${STUB_ENCRYPTED_RC:-1}\"; }\n" + 'configure_autologin "$AUTOLOGIN_DIR"\n' + 'echo "RETURNED=$?"\n' + # Whatever the prompt did not consume is still on stdin. + 'if IFS= read -r leftover; then echo "LEFTOVER:$leftover"; ' + 'else echo "LEFTOVER:"; fi\n' + ) + os.chmod(self.wrapper, 0o755) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def run_step(self, enable_autologin="", encrypted=False, config_file="", + stdin="", username="testuser"): + env = dict(os.environ) + env["enable_autologin"] = enable_autologin + env["config_file"] = config_file + env["username"] = username + env["AUTOLOGIN_DIR"] = self.dropin_dir + env["STUB_ENCRYPTED_RC"] = "0" if encrypted else "1" + return subprocess.run( + ["bash", self.wrapper, ARCHSETUP], + input=stdin, capture_output=True, text=True, env=env, + ) + + def dropin(self): + """The written drop-in's contents, or None when it wasn't written.""" + path = os.path.join(self.dropin_dir, "autologin.conf") + if not os.path.exists(path): + return None + with open(path) as f: + return f.read() + + def assertReturnedZero(self, r): + self.assertIn("RETURNED=0", r.stdout, + "step did not return 0; stdout=%r stderr=%r" + % (r.stdout, r.stderr)) + + # ---------------------------------------------------------- normal ---- + def test_explicit_true_writes_dropin_without_prompting(self): + r = self.run_step(enable_autologin="true", stdin="sentinel\n") + self.assertReturnedZero(r) + self.assertIn("--autologin testuser", self.dropin() or "") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_explicit_false_writes_nothing_without_prompting(self): + r = self.run_step(enable_autologin="false", encrypted=True, + stdin="sentinel\n") + self.assertReturnedZero(r) + self.assertIsNone(self.dropin()) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_autodetect_unencrypted_root_skips_silently(self): + r = self.run_step(encrypted=False, stdin="sentinel\n") + self.assertReturnedZero(r) + self.assertIsNone(self.dropin()) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + # -------------------------------------------------------- boundary ---- + def test_autodetect_encrypted_interactive_yes_writes_dropin(self): + # bash emits `read -p`'s prompt only when stdin is a terminal, so the + # prompt text is unassertable here. What is assertable is that the read + # consumed exactly the answer line and left the rest of stdin alone. + r = self.run_step(encrypted=True, stdin="y\nsentinel\n") + self.assertReturnedZero(r) + self.assertIn("--autologin testuser", self.dropin() or "") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_autodetect_encrypted_bare_enter_takes_the_yes_default(self): + r = self.run_step(encrypted=True, stdin="\nsentinel\n") + self.assertReturnedZero(r) + self.assertIn("--autologin testuser", self.dropin() or "") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_unattended_config_file_does_not_block_on_the_prompt(self): + # --config-file is the documented unattended mode and AUTOLOGIN is + # optional in archsetup.conf.example, so this is the ordinary + # unattended install of an encrypted machine. It must not read stdin. + r = self.run_step(encrypted=True, config_file="/etc/archsetup.conf", + stdin="sentinel\n") + self.assertReturnedZero(r) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_unattended_leaves_autologin_off(self): + # A prompt default is calibrated for someone who can override it in the + # moment. Unattended there is no override, so the default stops being + # advisory and becomes policy -- and this policy is passwordless console + # login. "Encrypted root means already authenticated" holds only at + # boot: it does not hold for a stolen powered-on machine, where tty1 is + # one Ctrl-Alt-F1 away and a screen locker does not lock the console. + # + # The error costs are asymmetric. Wrongly ON is a silent authentication + # downgrade nobody notices. Wrongly OFF is one password prompt, noticed + # immediately, fixed with a config key that already exists. + r = self.run_step(encrypted=True, config_file="/etc/archsetup.conf") + self.assertReturnedZero(r) + self.assertIsNone(self.dropin()) + + def test_unattended_says_why_it_left_autologin_off(self): + # Silently declining is its own trap: the user gets a login prompt they + # did not expect and no idea which knob controls it. + r = self.run_step(encrypted=True, config_file="/etc/archsetup.conf") + self.assertIn("AUTOLOGIN", r.stdout) + + def test_unattended_explicit_yes_still_enables_it(self): + # Declining by default must not make the feature unreachable + # unattended. AUTOLOGIN=yes is the opt-in and it still works. + r = self.run_step(enable_autologin="true", encrypted=True, + config_file="/etc/archsetup.conf", stdin="sentinel\n") + self.assertReturnedZero(r) + self.assertIn("--autologin testuser", self.dropin() or "") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_unattended_explicit_no_still_wins_over_the_default(self): + # Unattended does not mean "always autologin" -- AUTOLOGIN=no is an + # answer, and the auto-detect default must not override it. + r = self.run_step(enable_autologin="false", encrypted=True, + config_file="/etc/archsetup.conf", stdin="sentinel\n") + self.assertReturnedZero(r) + self.assertIsNone(self.dropin()) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_unattended_unencrypted_root_still_writes_nothing(self): + r = self.run_step(encrypted=False, config_file="/etc/archsetup.conf", + stdin="sentinel\n") + self.assertReturnedZero(r) + self.assertIsNone(self.dropin()) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + # ----------------------------------------------------------- error ---- + def test_autodetect_encrypted_interactive_no_writes_nothing(self): + r = self.run_step(encrypted=True, stdin="n\nsentinel\n") + self.assertReturnedZero(r) + self.assertIsNone(self.dropin()) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_autodetect_encrypted_interactive_capital_no_writes_nothing(self): + r = self.run_step(encrypted=True, stdin="No\nsentinel\n") + self.assertReturnedZero(r) + self.assertIsNone(self.dropin()) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_dropin_overrides_the_packaged_execstart(self): + # A drop-in that adds a second ExecStart without clearing the first is + # a unit systemd refuses to start -- tty1 would have no getty at all. + r = self.run_step(enable_autologin="true") + self.assertReturnedZero(r) + body = self.dropin() or "" + self.assertIn("ExecStart=\n", body) + self.assertLess(body.index("ExecStart=\n"), + body.index("ExecStart=-/sbin/agetty")) + + def test_closed_stdin_leaves_autologin_off(self): + # The same "nobody can answer" state reached without --config-file: a + # piped or redirected stdin at EOF. Pre-fix the read failed, response + # was empty, and `case *` enabled autologin silently. + r = self.run_step(encrypted=True, stdin="") + self.assertReturnedZero(r) + self.assertIsNone(self.dropin()) + + def test_closed_stdin_says_why(self): + r = self.run_step(encrypted=True, stdin="") + self.assertIn("stdin closed", r.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/installer-steps/test_framework_firmware_trim.py b/tests/installer-steps/test_framework_firmware_trim.py new file mode 100644 index 0000000..cdb6a60 --- /dev/null +++ b/tests/installer-steps/test_framework_firmware_trim.py @@ -0,0 +1,233 @@ +"""Tests for is_framework_intel_laptop, the gate on trim_firmware. + +trim_firmware runs `pacman -Rdd` against twelve linux-firmware packages, +keeping only the three a Framework 13 Intel needs (intel, atheros, realtek). +It is the most destructive conditional in the installer, so the condition is +what these tests pin. + +The gate has to be exact in both directions. Too narrow and the step is dead +code: "framework" lives in DMI's sys_vendor, never in product_name, so a gate +reading product_name alone matches no Framework machine at all. Too wide and it +removes linux-firmware-amdgpu from a Framework Desktop, whose Ryzen AI Max iGPU +needs it to bring up a display. Both machines are real: velox is +Framework/"Laptop (13th Gen Intel Core)" and must trim; ratio is +Framework/"Desktop (AMD Ryzen AI Max 300 Series)" and must not. + +These tests exercise the REAL function body, extracted from the `archsetup` +script at run time (not a copy), against a fixture DMI directory. + +Run from repo root: + python3 -m unittest tests.installer-steps.test_framework_firmware_trim +""" + +import os +import shutil +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") + + +class FrameworkIntelGateHarness(unittest.TestCase): + """Source is_framework_intel_laptop out of the real archsetup script.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="fw-trim-test-") + self.dmi = os.path.join(self.tmp, "dmi") + os.mkdir(self.dmi) + self.wrapper = os.path.join(self.tmp, "run.sh") + with open(self.wrapper, "w") as f: + f.write( + "#!/bin/bash\n" + 'ARCHSETUP="$1"; shift\n' + "source <(sed -n " + "'/^is_framework_intel_laptop() {/,/^}/p' \"$ARCHSETUP\")\n" + 'if is_framework_intel_laptop "$DMI_DIR"; then echo "TRIM=yes"; ' + 'else echo "TRIM=no"; fi\n' + ) + os.chmod(self.wrapper, 0o755) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def dmi_write(self, **fields): + for name, value in fields.items(): + with open(os.path.join(self.dmi, name), "w") as f: + f.write(value + "\n") + + def trims(self, dmi_dir=None): + env = dict(os.environ) + env["DMI_DIR"] = self.dmi if dmi_dir is None else dmi_dir + r = subprocess.run(["bash", self.wrapper, ARCHSETUP], + capture_output=True, text=True, env=env) + if "TRIM=yes" in r.stdout: + return True + if "TRIM=no" in r.stdout: + return False + self.fail("no TRIM line; stdout=%r stderr=%r" % (r.stdout, r.stderr)) + + # ---------------------------------------------------------- normal ---- + def test_velox_framework_13_intel_trims(self): + # The exact strings read off velox on 2026-07-24. + self.dmi_write(sys_vendor="Framework", + product_name="Laptop (13th Gen Intel Core)") + self.assertTrue(self.trims()) + + def test_framework_12_intel_trims(self): + self.dmi_write(sys_vendor="Framework", + product_name="Laptop 12 (13th Gen Intel Core)") + self.assertTrue(self.trims()) + + # -------------------------------------------------------- boundary ---- + def test_ratio_framework_desktop_amd_does_not_trim(self): + # The exact strings read off ratio on 2026-07-24. Trimming here would + # remove linux-firmware-amdgpu from the machine whose iGPU needs it. + self.dmi_write(sys_vendor="Framework", + product_name="Desktop (AMD Ryzen AI Max 300 Series)") + self.assertFalse(self.trims()) + + def test_framework_16_amd_laptop_does_not_trim(self): + self.dmi_write(sys_vendor="Framework", + product_name="Laptop 16 (AMD Ryzen 7040 Series)") + self.assertFalse(self.trims()) + + def test_non_framework_intel_laptop_does_not_trim(self): + # The firmware set is only known for Framework hardware. The model + # deliberately carries both "Laptop" and "Intel", so the vendor check + # is the only condition that can reject it -- a fixture that failed the + # model checks too would pass no matter what the vendor check did. + self.dmi_write(sys_vendor="Dell Inc.", + product_name="Laptop 15 (Intel Core i7)") + self.assertFalse(self.trims()) + + def test_framework_desktop_naming_an_intel_cpu_does_not_trim(self): + # No such machine ships today -- the Framework Desktop is AMD -- but + # "an Intel Framework desktop will never exist" is exactly the kind of + # assumption that made the old gate wrong. The model must say Laptop, + # so a desktop is refused on the model rather than on its CPU vendor. + self.dmi_write(sys_vendor="Framework", + product_name="Desktop (Intel Core Ultra)") + self.assertFalse(self.trims()) + + def test_vendor_match_is_case_insensitive(self): + self.dmi_write(sys_vendor="framework", + product_name="Laptop (13th Gen intel Core)") + self.assertTrue(self.trims()) + + def test_vendor_with_surrounding_whitespace_still_matches(self): + # DMI strings are vendor-supplied and often padded. + self.dmi_write(sys_vendor=" Framework ", + product_name="Laptop (13th Gen Intel Core)") + self.assertTrue(self.trims()) + + # ----------------------------------------------------------- error ---- + def test_missing_dmi_directory_does_not_trim(self): + self.assertFalse(self.trims(dmi_dir=os.path.join(self.tmp, "nope"))) + + def test_missing_product_name_does_not_trim(self): + self.dmi_write(sys_vendor="Framework") + self.assertFalse(self.trims()) + + def test_missing_sys_vendor_does_not_trim(self): + self.dmi_write(product_name="Laptop (13th Gen Intel Core)") + self.assertFalse(self.trims()) + + def test_empty_dmi_values_do_not_trim(self): + self.dmi_write(sys_vendor="", product_name="") + self.assertFalse(self.trims()) + +class GpuVendorEvidence(unittest.TestCase): + """trim_firmware deletes GPU firmware, so what it needs to know is which + GPU is physically present -- and the kernel already answers that. + + The DMI model string cannot. It is a marketing name: it does not know a + swapped card, and it cannot see hardware the allowlist has never met. The + same file already reads PCI modalias twice (install_gpu_drivers, and + nvidia_preflight_report, which parameterises its globs for exactly this + kind of test). detect_gpu_vendors is that read, made reusable, so the trim + can refuse on evidence rather than on a name. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="gpu-vendor-test-") + self.drm = os.path.join(self.tmp, "drm") + self.pci = os.path.join(self.tmp, "pci") + os.makedirs(self.drm) + os.makedirs(self.pci) + self.wrapper = os.path.join(self.tmp, "run.sh") + with open(self.wrapper, "w") as f: + f.write( + "#!/bin/bash\n" + 'ARCHSETUP="$1"; shift\n' + "source <(sed -n " + "'/^detect_gpu_vendors() {/,/^}/p' \"$ARCHSETUP\")\n" + 'detect_gpu_vendors "$DRM_GLOB" "$PCI_GLOB"\n' + ) + os.chmod(self.wrapper, 0o755) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def card(self, root, name, modalias): + d = os.path.join(root, name, "device") + os.makedirs(d, exist_ok=True) + with open(os.path.join(d, "modalias"), "w") as f: + f.write(modalias + "\n") + + def vendors(self): + env = dict(os.environ) + env["DRM_GLOB"] = os.path.join(self.drm, "card*", "device", "modalias") + env["PCI_GLOB"] = os.path.join(self.pci, "*", "device", "modalias") + r = subprocess.run(["bash", self.wrapper, ARCHSETUP], + capture_output=True, text=True, env=env) + return sorted(v for v in r.stdout.split() if v) + + # ---------------------------------------------------------- normal ---- + def test_intel_igpu_detected(self): + self.card(self.drm, "card0", "pci:v00008086d00007D55sv00001414bc03sc00i00") + self.assertEqual(self.vendors(), ["intel"]) + + def test_amd_igpu_detected(self): + self.card(self.drm, "card0", "pci:v00001002d0000150Esv00001414bc03sc00i00") + self.assertEqual(self.vendors(), ["amd"]) + + # -------------------------------------------------------- boundary ---- + def test_nvidia_detected_in_either_hex_case(self): + self.card(self.drm, "card0", "pci:v000010DEd00002684bc03sc00i00") + self.assertEqual(self.vendors(), ["nvidia"]) + shutil.rmtree(os.path.join(self.drm, "card0")) + self.card(self.drm, "card0", "pci:v000010ded00002684bc03sc00i00") + self.assertEqual(self.vendors(), ["nvidia"]) + + def test_hybrid_machine_reports_both(self): + self.card(self.drm, "card0", "pci:v00008086d00007D55bc03sc00i00") + self.card(self.drm, "card1", "pci:v000010DEd00002684bc03sc00i00") + self.assertEqual(self.vendors(), ["intel", "nvidia"]) + + def test_falls_back_to_pci_when_drm_is_empty(self): + # Early boot / chroot: no DRM nodes yet, same ruling install_gpu_drivers + # already makes. + self.card(self.pci, "0000:00:02.0", "pci:v00001002d0000150Ebc03sc00i00") + self.assertEqual(self.vendors(), ["amd"]) + + def test_pci_fallback_ignores_non_display_devices(self): + # An NVIDIA audio function on the same card is bc04, not bc03. + self.card(self.pci, "0000:01:00.1", "pci:v000010DEd000022BAbc04sc03i00") + self.assertEqual(self.vendors(), []) + + # ----------------------------------------------------------- error ---- + def test_no_gpu_anywhere_reports_nothing(self): + self.assertEqual(self.vendors(), []) + + def test_unreadable_modalias_is_skipped_not_fatal(self): + d = os.path.join(self.drm, "card0", "device") + os.makedirs(d) + os.mkdir(os.path.join(d, "modalias")) # a directory, not a file + self.assertEqual(self.vendors(), []) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/installer-steps/test_mark_volatile_configs.py b/tests/installer-steps/test_mark_volatile_configs.py new file mode 100644 index 0000000..27eac61 --- /dev/null +++ b/tests/installer-steps/test_mark_volatile_configs.py @@ -0,0 +1,216 @@ +"""Tests for mark_volatile_configs, the tail of the dotfiles install. + +Four stowed configs get rewritten in place by the apps that own them (btop, +qalculate, calibre, waypaper). The dotfiles repo handles that with git's +skip-worktree bit, applied by its own `skip-volatile` make target, which +`make stow` runs as its last step. + +archsetup stows inline rather than through that Makefile, and it never ran the +step -- so a machine installed by archsetup alone starts with a dotfiles repo +that goes dirty as soon as those apps run, and every later `git pull --ff-only` +trips over paths the user never edited. Both daily drivers carry the bits +today, but archsetup contains no code that sets them: they were applied by a +hand-run `make stow`. + +The fix calls the dotfiles Makefile's target rather than reimplementing the +logic here, so the volatile list stays in one place. That is what these tests +pin: that archsetup delegates, and that it degrades quietly when the checkout +it was handed cannot answer. + +These tests exercise the REAL function body, extracted from the `archsetup` +script at run time (not a copy), against a fixture git repo, with sudo stubbed +on PATH. + +Run from repo root: + python3 -m unittest tests.installer-steps.test_mark_volatile_configs +""" + +import os +import shutil +import subprocess +import tempfile +import textwrap +import unittest + + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +ARCHSETUP = os.path.join(REPO_ROOT, "archsetup") + +# The real target, copied from the dotfiles Makefile. The point of the fixture +# is that archsetup calls *a* skip-volatile target rather than doing the work +# itself; the body here mirrors the real one so the assertions are meaningful. +MAKEFILE = textwrap.dedent("""\ + DOTFILES := $(shell pwd) + VOLATILE := $(shell sed 's/#.*//' $(DOTFILES)/volatile-configs 2>/dev/null) + + skip-volatile: + \t@for f in $(VOLATILE); do \\ + \t\tif git -C $(DOTFILES) ls-files --error-unmatch "$$f" >/dev/null 2>&1; then \\ + \t\t\tgit -C $(DOTFILES) update-index --skip-worktree "$$f" && echo " skip-worktree: $$f"; \\ + \t\telse \\ + \t\t\techo " WARN: not tracked, skipping: $$f"; \\ + \t\tfi; \\ + \tdone +""") + +# sudo isn't available (and isn't wanted) in a unit test. Drop `-u ` and +# exec the rest, so the function's real command line is what runs. +SUDO_STUB = textwrap.dedent("""\ + #!/bin/bash + # Record the argv archsetup handed sudo, then drop `-u ` and exec the + # rest, so the function's real command line is what runs. + printf '%s\\n' "$*" >> "$SUDO_CALLS" + if [ "$1" = "-u" ]; then shift 2; fi + exec "$@" +""") + + +class MarkVolatileConfigsHarness(unittest.TestCase): + """Source mark_volatile_configs out of the real archsetup script.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="volatile-test-") + self.repo = os.path.join(self.tmp, "dotfiles") + os.mkdir(self.repo) + self.bindir = os.path.join(self.tmp, "bin") + os.mkdir(self.bindir) + sudo = os.path.join(self.bindir, "sudo") + with open(sudo, "w") as f: + f.write(SUDO_STUB) + os.chmod(sudo, 0o755) + self.logfile = os.path.join(self.tmp, "install.log") + self.sudo_calls = os.path.join(self.tmp, "sudo-calls") + self.wrapper = os.path.join(self.tmp, "run.sh") + with open(self.wrapper, "w") as f: + f.write( + "#!/bin/bash\n" + 'ARCHSETUP="$1"; shift\n' + "source <(sed -n " + "'/^mark_volatile_configs() {/,/^}/p' \"$ARCHSETUP\")\n" + 'display() { echo "DISPLAY:$2"; }\n' + 'error_warn() { echo "WARN:$1"; return 1; }\n' + 'mark_volatile_configs "$REPO" testuser\n' + 'echo "RETURNED=$?"\n' + ) + os.chmod(self.wrapper, 0o755) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def git(self, *args): + return subprocess.run(["git", "-C", self.repo, *args], + capture_output=True, text=True) + + def make_repo(self, tracked=("common/.config/btop/btop.conf",), + volatile=None, makefile: "str | None" = MAKEFILE): + self.git("init", "-q") + self.git("config", "user.email", "t@example.com") + self.git("config", "user.name", "t") + for rel in tracked: + path = os.path.join(self.repo, rel) + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(path, "w") as f: + f.write("original\n") + listed = tracked if volatile is None else volatile + with open(os.path.join(self.repo, "volatile-configs"), "w") as f: + f.write("# apps rewrite these in place\n") + f.write("".join(rel + "\n" for rel in listed)) + if makefile is not None: + with open(os.path.join(self.repo, "Makefile"), "w") as f: + f.write(makefile) + self.git("add", "-A") + self.git("commit", "-q", "-m", "fixture") + + def run_step(self): + env = dict(os.environ) + env["PATH"] = self.bindir + os.pathsep + env["PATH"] + env["REPO"] = self.repo + env["logfile"] = self.logfile + env["SUDO_CALLS"] = self.sudo_calls + return subprocess.run(["bash", self.wrapper, ARCHSETUP], + capture_output=True, text=True, env=env) + + def sudo_argv(self): + """The argument lines sudo was called with, one per invocation.""" + if not os.path.exists(self.sudo_calls): + return [] + with open(self.sudo_calls) as f: + return [line.rstrip("\n") for line in f if line.strip()] + + def skipped(self): + """Paths carrying the skip-worktree bit.""" + r = self.git("ls-files", "-v") + return sorted(line[2:] for line in r.stdout.splitlines() + if line.startswith("S ")) + + # ---------------------------------------------------------- normal ---- + def test_sets_skip_worktree_on_every_volatile_path(self): + paths = ["common/.config/btop/btop.conf", + "hyprland/.config/waypaper/config.ini"] + self.make_repo(tracked=paths) + self.assertEqual(self.skipped(), []) # the bit is not set by default + r = self.run_step() + self.assertIn("RETURNED=0", r.stdout, + "stdout=%r stderr=%r" % (r.stdout, r.stderr)) + self.assertEqual(self.skipped(), sorted(paths)) + + def test_runs_make_as_the_user_not_as_root(self): + # archsetup runs as root against a user-owned checkout. Letting root + # write .git/index leaves it root-owned, and the user's next git + # command then cannot update the index at all. + self.make_repo() + self.run_step() + self.assertEqual(self.sudo_argv(), + ["-u testuser make -C %s skip-volatile" % self.repo]) + + def test_leaves_non_volatile_tracked_files_alone(self): + self.make_repo(tracked=["common/.config/btop/btop.conf", + "common/.bashrc"], + volatile=["common/.config/btop/btop.conf"]) + self.run_step() + self.assertEqual(self.skipped(), ["common/.config/btop/btop.conf"]) + + # -------------------------------------------------------- boundary ---- + def test_no_makefile_is_a_quiet_no_op(self): + # DOTFILES_DIR can point at a checkout that predates the target, or at + # a fork. Nothing to delegate to is not an error. + self.make_repo(makefile=None) + r = self.run_step() + self.assertIn("RETURNED=0", r.stdout) + self.assertNotIn("WARN:", r.stdout) + self.assertEqual(self.skipped(), []) + + def test_empty_volatile_list_sets_nothing_and_does_not_warn(self): + self.make_repo(tracked=["common/.config/btop/btop.conf"], volatile=[]) + r = self.run_step() + self.assertIn("RETURNED=0", r.stdout) + self.assertNotIn("WARN:", r.stdout) + self.assertEqual(self.skipped(), []) + + def test_untracked_path_in_the_list_does_not_fail_the_step(self): + # The Makefile warns and carries on; a stale entry must not take the + # install down, and the tracked entries still get their bit. + self.make_repo(tracked=["common/.config/btop/btop.conf"], + volatile=["common/.config/btop/btop.conf", + "common/.config/gone/removed.conf"]) + r = self.run_step() + self.assertIn("RETURNED=0", r.stdout) + self.assertEqual(self.skipped(), ["common/.config/btop/btop.conf"]) + + # ----------------------------------------------------------- error ---- + def test_makefile_without_the_target_warns_and_returns(self): + self.make_repo(makefile="all:\n\t@true\n") + r = self.run_step() + self.assertIn("WARN:", r.stdout) + self.assertEqual(self.skipped(), []) + + def test_make_output_goes_to_the_logfile_not_the_console(self): + self.make_repo(tracked=["common/.config/btop/btop.conf"]) + r = self.run_step() + self.assertNotIn("skip-worktree:", r.stdout) + with open(self.logfile) as f: + self.assertIn("skip-worktree:", f.read()) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/installer-steps/test_orchestrators.py b/tests/installer-steps/test_orchestrators.py index 2a771ba..8de75cc 100644 --- a/tests/installer-steps/test_orchestrators.py +++ b/tests/installer-steps/test_orchestrators.py @@ -147,3 +147,61 @@ class MaintenanceConfigDispatch(unittest.TestCase): if __name__ == "__main__": unittest.main() + + +# Helper -> the function that must call it. Every one of these helpers has its +# own suite proving it behaves correctly; none of those suites proves it is +# wired in. Deleting the call left every test green in five separate suites, +# including the one guarding a `pacman -Rdd` of twelve firmware packages. +# +# This is a static check on purpose. The behavioural harness above stubs only +# the names it is given and runs the rest of the body for real, which is safe +# for an orchestrator whose body is nothing but calls, and emphatically not +# safe for a leaf like trim_firmware. Cheaper and machine-independent too: the +# alternative depends on what /proc/cpuinfo says on the box running the tests. +CALL_SITES = { + "trim_firmware": ["detect_gpu_vendors", "is_framework_intel_laptop"], + "finalize_dotfiles": ["mark_volatile_configs"], + "preflight_checks": ["select_locale", "nvidia_preflight", "check_disk_space"], + "configure_encrypted_autologin": ["configure_autologin"], + "configure_initramfs_hook": ["switch_udev_hook_to_systemd"], + "add_nvme_early_module": ["ensure_nvme_early_module"], + "configure_snapshots": ["configure_zfs_snapshots", "configure_btrfs_snapshots"], + "configure_zfs_snapshots": ["zfs_scrub_timer_units"], + "install_maintenance_config": ["install_maintenance_thresholds"], +} + + +def function_body(name): + """The body of `name` from archsetup, comments and blank lines stripped, so + a match is a real invocation rather than a mention in a comment.""" + with open(ARCHSETUP) as f: + src = f.read() + start = src.index(f"\n{name}() {{\n") + end = src.index("\n}\n", start) + lines = src[start:end].splitlines()[1:] + keep = [] + for line in lines: + stripped = line.strip() + if not stripped or stripped.startswith("#"): + continue + keep.append(line.split(" #", 1)[0]) + return "\n".join(keep) + + +class CallSiteWiring(unittest.TestCase): + def test_every_guarded_helper_is_actually_called(self): + for caller, callees in CALL_SITES.items(): + body = function_body(caller) + for callee in callees: + with self.subTest(caller=caller, callee=callee): + self.assertRegex( + body, rf"(^|[\s;&|(]){callee}\b", + f"{caller} no longer calls {callee}; its own suite " + f"cannot see this") + + def test_the_check_would_notice_a_missing_call(self): + # The check is only worth having if it fails when the call goes away. + body = function_body("trim_firmware").replace("detect_gpu_vendors", "") + with self.assertRaises(AssertionError): + self.assertRegex(body, r"(^|[\s;&|(])detect_gpu_vendors\b") diff --git a/tests/installer-steps/test_select_locale.py b/tests/installer-steps/test_select_locale.py new file mode 100644 index 0000000..b9b9ce7 --- /dev/null +++ b/tests/installer-steps/test_select_locale.py @@ -0,0 +1,160 @@ +"""Tests for select_locale in the archsetup installer. + +select_locale resolves the system locale during pre-flight. Three inputs, in +precedence order: an existing LANG= in locale.conf wins outright, then LOCALE +from the config file, then a numbered menu. + +That menu is why these tests exist. It carries its own "[1]" default, LOCALE is +optional in archsetup.conf.example, and archsetup does not require an archangel +install -- so an unattended run (--config-file) can reach the menu's bare read +with nobody there to answer it. configure_build_environment's own "no LANG= in +locale.conf" branch shows archsetup expects that state to occur. Unattended +therefore takes the menu's default, the same ruling nvidia_preflight and +configure_autologin make for their prompts. + +These tests exercise the REAL function body, extracted from the `archsetup` +script at run time (not a copy), against a fixture locale.conf. + +Run from repo root: + python3 -m unittest tests.installer-steps.test_select_locale +""" + +import os +import shutil +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") + + +class SelectLocaleHarness(unittest.TestCase): + """Source select_locale out of the real archsetup script.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="select-locale-test-") + self.conf = os.path.join(self.tmp, "locale.conf") + self.wrapper = os.path.join(self.tmp, "run.sh") + with open(self.wrapper, "w") as f: + f.write( + "#!/bin/bash\n" + 'ARCHSETUP="$1"; shift\n' + "source <(sed -n " + "'/^select_locale() {/,/^}/p' \"$ARCHSETUP\")\n" + 'select_locale "$LOCALE_CONF"\n' + 'echo "RESOLVED=$locale"\n' + # Whatever the prompt did not consume is still on stdin. + 'if IFS= read -r leftover; then echo "LEFTOVER:$leftover"; ' + 'else echo "LEFTOVER:"; fi\n' + ) + os.chmod(self.wrapper, 0o755) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def write_conf(self, body): + with open(self.conf, "w") as f: + f.write(body) + + def run_select(self, locale="", config_file="", stdin=""): + env = dict(os.environ) + env["locale"] = locale + env["config_file"] = config_file + env["LOCALE_CONF"] = self.conf + return subprocess.run( + ["bash", self.wrapper, ARCHSETUP], + input=stdin, capture_output=True, text=True, env=env, + ) + + def resolved(self, r): + for line in r.stdout.splitlines(): + if line.startswith("RESOLVED="): + return line.split("=", 1)[1] + self.fail("no RESOLVED line; stdout=%r stderr=%r" % (r.stdout, r.stderr)) + + # ---------------------------------------------------------- normal ---- + def test_existing_locale_conf_wins_and_does_not_prompt(self): + self.write_conf("LANG=de_DE.UTF-8\n") + r = self.run_select(stdin="sentinel\n") + self.assertIn("[OK] Locale: de_DE.UTF-8", r.stdout) + # locale.conf is authoritative, so the global is deliberately left + # unset -- configure_build_environment reads LANG out of the file. + self.assertEqual(self.resolved(r), "") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_config_file_locale_used_when_conf_has_no_lang(self): + self.write_conf("") + r = self.run_select(locale="fr_FR.UTF-8", stdin="sentinel\n") + self.assertIn("(from config)", r.stdout) + self.assertEqual(self.resolved(r), "fr_FR.UTF-8") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_menu_choice_selects_that_locale(self): + self.write_conf("") + r = self.run_select(stdin="3\nsentinel\n") + self.assertEqual(self.resolved(r), "de_DE.UTF-8") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + # -------------------------------------------------------- boundary ---- + def test_missing_locale_conf_falls_through_to_the_menu(self): + # No file at all, not just no LANG= line. + r = self.run_select(stdin="2\nsentinel\n") + self.assertEqual(self.resolved(r), "en_GB.UTF-8") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_bare_enter_takes_choice_one(self): + self.write_conf("") + r = self.run_select(stdin="\nsentinel\n") + self.assertEqual(self.resolved(r), "en_US.UTF-8") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_unattended_config_file_takes_the_default_without_reading(self): + # THE BUG. --config-file is the documented unattended mode and LOCALE + # is optional, so this is an ordinary unattended install on a machine + # whose locale.conf carries no LANG=. + self.write_conf("") + r = self.run_select(config_file="/etc/archsetup.conf", + stdin="sentinel\n") + self.assertEqual(self.resolved(r), "en_US.UTF-8") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_unattended_still_prefers_an_existing_locale_conf(self): + self.write_conf("LANG=ja_JP.UTF-8\n") + r = self.run_select(config_file="/etc/archsetup.conf", + stdin="sentinel\n") + self.assertIn("[OK] Locale: ja_JP.UTF-8", r.stdout) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_unattended_still_prefers_the_configured_locale(self): + # Unattended does not mean "always en_US" -- LOCALE is an answer, and + # the menu default must not override it. + self.write_conf("") + r = self.run_select(locale="pt_BR.UTF-8", + config_file="/etc/archsetup.conf", stdin="sentinel\n") + self.assertEqual(self.resolved(r), "pt_BR.UTF-8") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + # ----------------------------------------------------------- error ---- + def test_unrecognised_choice_falls_back_to_en_us(self): + self.write_conf("") + r = self.run_select(stdin="42\nsentinel\n") + self.assertEqual(self.resolved(r), "en_US.UTF-8") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_manual_entry_reads_a_second_line(self): + self.write_conf("") + r = self.run_select(stdin="9\nnl_NL.UTF-8\nsentinel\n") + self.assertEqual(self.resolved(r), "nl_NL.UTF-8") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_manual_entry_left_empty_falls_back_to_en_us(self): + self.write_conf("") + r = self.run_select(stdin="9\n\nsentinel\n") + self.assertEqual(self.resolved(r), "en_US.UTF-8") + self.assertIn("LEFTOVER:sentinel", r.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/installer-steps/test_switch_udev_hook.py b/tests/installer-steps/test_switch_udev_hook.py new file mode 100644 index 0000000..54969e6 --- /dev/null +++ b/tests/installer-steps/test_switch_udev_hook.py @@ -0,0 +1,178 @@ +"""Test the udev -> systemd initramfs hook swap. + +configure_initramfs_hook swaps the `udev` initramfs hook for `systemd` so fsck +output routes through systemd. mkinitcpio's `systemd` hook symlinks /init to +systemd, which never executes the ash `run_hook` scripts the busybox init +drives -- so any busybox-only hook left in HOOKS is installed into the image +and then never runs. + +`encrypt` is the boot-critical member of that set: it is what unlocks a LUKS +root, and its `cryptdevice=` cmdline parameter (which merge_grub_cmdline +already treats as boot-critical) is read by nothing else. mkinitcpio has no +conflict check, so `mkinitcpio -P` succeeds, archsetup reports success, and the +machine fails to unlock its root at the next boot. The systemd equivalent is +`sd-encrypt` driven by `rd.luks.*`, and converting between the two means +rewriting the kernel cmdline against the volume's UUID -- so the step refuses +the swap rather than attempting a migration. + +Method: sed-extract switch_udev_hook_to_systemd + hooks_need_busybox_init from +the real `archsetup`, point them at a temp mkinitcpio.conf, and fake +backup_system_file/display/error_warn. + +Run from repo root: + python3 -m unittest tests.installer-steps.test_switch_udev_hook +""" + +import os +import re +import subprocess +import tempfile +import textwrap +import unittest + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +ARCHSETUP = os.path.join(REPO_ROOT, "archsetup") + +# The two HOOKS lines Arch's own mkinitcpio.conf documents for an encrypted +# root -- the busybox pairing and the systemd pairing. They are the fixtures +# that matter, because the bug is turning the first into neither. +BUSYBOX_ENCRYPT = ( + "HOOKS=(base udev microcode modconf keyboard keymap consolefont block " + "mdadm_udev encrypt filesystems fsck)\n" +) +SYSTEMD_ENCRYPT = ( + "HOOKS=(base systemd autodetect microcode modconf kms keyboard sd-vconsole " + "sd-encrypt block filesystems fsck)\n" +) +PLAIN_UDEV = "HOOKS=(base udev autodetect microcode modconf block filesystems fsck)\n" + + +def run(conf_body, missing_conf=False): + with tempfile.TemporaryDirectory() as d: + conf = os.path.join(d, "mkinitcpio.conf") + if not missing_conf: + with open(conf, "w") as f: + f.write(conf_body) + script = textwrap.dedent(f"""\ + logfile=/dev/null + action="" + display() {{ echo "DISPLAY: $2"; }} + backup_system_file() {{ echo "BACKUP: $1" >> "{d}/backup.log"; }} + error_warn() {{ echo "WARN: $1"; return 1; }} + source <(sed -n '/^hooks_need_busybox_init() {{/,/^}}/p;/^switch_udev_hook_to_systemd() {{/,/^}}/p' "{ARCHSETUP}") + switch_udev_hook_to_systemd "{conf}" + echo "RC=$?" + echo "CONF:[$(cat "{conf}" 2>/dev/null)]" + [ -f "{d}/backup.log" ] && cat "{d}/backup.log" + exit 0 + """) + return subprocess.run( + ["bash", "-c", script], capture_output=True, text=True, timeout=10, + ) + + +def rc_of(r): + """The step's exact return code. Substring-matching "RC=1" in stdout would + also match "RC=127" (the code bash gives a missing function), so a broken + extraction would read as a passing test.""" + m = re.search(r"^RC=(\d+)$", r.stdout, re.M) + assert m, "no RC line in output: %r / %r" % (r.stdout, r.stderr) + return int(m.group(1)) + + +class SwitchUdevHookToSystemd(unittest.TestCase): + # ------------------------------------------------------------ normal ---- + def test_plain_udev_is_swapped_and_reports_change(self): + r = run(PLAIN_UDEV) + self.assertIn("HOOKS=(base systemd autodetect", r.stdout) + self.assertNotIn("udev", r.stdout.split("CONF:[")[1]) + self.assertEqual(rc_of(r), 0, "a conf that changed must report success") + self.assertIn("BACKUP:", r.stdout, "the conf must be backed up before the edit") + + def test_already_systemd_is_left_alone_and_reports_no_change(self): + body = "HOOKS=(base systemd autodetect block filesystems fsck)\n" + r = run(body) + self.assertIn(body.strip(), r.stdout) + self.assertEqual(rc_of(r), 1, + "an unchanged conf must not report a change (no needless rebuild)") + + # ---------------------------------------------------------- boundary ---- + def test_busybox_encrypt_hook_blocks_the_swap(self): + # THE BUG: swapping here leaves `encrypt` under a systemd init that + # never runs it, so the LUKS root never unlocks and the machine will + # not boot. + r = run(BUSYBOX_ENCRYPT) + self.assertIn(" udev ", r.stdout, "udev must survive next to a busybox encrypt hook") + self.assertIn(" encrypt ", r.stdout) + self.assertNotIn("systemd", r.stdout.split("CONF:[")[1]) + self.assertEqual(rc_of(r), 1) + self.assertNotIn("BACKUP:", r.stdout, "a declined swap must not touch the file") + + def test_tab_separated_encrypt_hook_blocks_the_swap(self): + # The token class was [( ] / [ )], which is a literal space and does + # not include a tab. mkinitcpio's HOOKS is a bash array literal, and + # bash word-splits on IFS -- tabs included -- so a tab-separated HOOKS + # line is valid, loads correctly, and slipped straight past the guard + # into the exact unbootable state it exists to prevent. + r = run("HOOKS=(base udev block\tencrypt\tfilesystems fsck)\n") + self.assertEqual(rc_of(r), 1, "a tab-separated encrypt must block the swap") + self.assertIn("encrypt", r.stdout) + self.assertNotIn("systemd", r.stdout.split("CONF:[")[1]) + self.assertNotIn("BACKUP:", r.stdout) + + def test_tab_separated_sd_encrypt_is_still_not_the_busybox_hook(self): + # Widening the class must not widen what counts as the busybox hook: + # sd-encrypt IS the systemd one and has to keep swapping. + r = run("HOOKS=(base\tsystemd\tsd-encrypt\tfilesystems)\n") + self.assertNotIn("BACKUP:", r.stdout) # already systemd, nothing to do + self.assertEqual(rc_of(r), 1) + + def test_tab_separated_udev_without_encrypt_still_swaps(self): + r = run("HOOKS=(base\tudev\tblock\tfilesystems\tfsck)\n") + self.assertEqual(rc_of(r), 0, "a tab-separated conf with no encrypt must swap") + self.assertIn("systemd", r.stdout.split("CONF:[")[1]) + + def test_plymouth_encrypt_hook_blocks_the_swap(self): + r = run("HOOKS=(base udev block plymouth-encrypt filesystems)\n") + self.assertIn(" udev ", r.stdout) + self.assertEqual(rc_of(r), 1) + + def test_sd_encrypt_is_not_mistaken_for_the_busybox_hook(self): + # sd-encrypt IS the systemd hook -- it must not be read as a reason to + # refuse. (No udev here, so the swap is a no-op either way; what this + # pins is that the substring match does not over-trigger.) + r = run(SYSTEMD_ENCRYPT) + self.assertIn("sd-encrypt", r.stdout) + self.assertNotIn("HOOKS carries encrypt", r.stdout) + + def test_encrypt_as_last_token_is_detected(self): + r = run("HOOKS=(base udev block encrypt)\n") + self.assertIn(" udev ", r.stdout) + self.assertEqual(rc_of(r), 1) + + def test_encrypt_as_first_token_is_detected(self): + r = run("HOOKS=(encrypt udev block)\n") + self.assertIn("(encrypt udev block)", r.stdout) + self.assertEqual(rc_of(r), 1) + + def test_only_the_hooks_line_is_rewritten(self): + # A commented example line mentioning udev must survive untouched. + body = "# HOOKS=(base udev block filesystems fsck)\n" + PLAIN_UDEV + r = run(body) + self.assertIn("# HOOKS=(base udev block filesystems fsck)", r.stdout) + self.assertIn("HOOKS=(base systemd autodetect", r.stdout) + + # ------------------------------------------------------------- error ---- + def test_conf_without_a_hooks_line_is_a_no_op(self): + r = run("MODULES=(nvme)\n") + self.assertEqual(rc_of(r), 1) + self.assertIn("MODULES=(nvme)", r.stdout) + + def test_missing_conf_is_a_no_op_not_a_crash(self): + r = run("", missing_conf=True) + self.assertEqual(rc_of(r), 1) + self.assertEqual(r.returncode, 0) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/nvidia-preflight/test_nvidia_preflight_gate.py b/tests/nvidia-preflight/test_nvidia_preflight_gate.py new file mode 100644 index 0000000..1d76689 --- /dev/null +++ b/tests/nvidia-preflight/test_nvidia_preflight_gate.py @@ -0,0 +1,173 @@ +"""Tests for the nvidia_preflight policy wrapper in the archsetup installer. + +nvidia_preflight_report (covered by test_nvidia_preflight.py) is the pure +detection core. nvidia_preflight is the policy around it: it decides whether +the check applies at all, runs the report, aborts on a too-old driver, and +confirms an otherwise-supported card with the user. + +The check is Wayland-specific -- every line it prints names Wayland/Hyprland, +and its own failure hint tells the user to install with DESKTOP_ENV=dwm +instead. So it must not fire on a dwm (X11) or headless (none) install, and it +must not fire when --no-gpu-drivers / NO_GPU_DRIVERS=yes says the user handles +the driver themselves -- the same ruling install_gpu_drivers already makes. + +These tests exercise the REAL function body, extracted from the `archsetup` +script at run time (not a copy), with nvidia_preflight_report stubbed to a +chosen verdict. + +Run from repo root: + python3 -m unittest tests.nvidia-preflight.test_nvidia_preflight_gate +""" + +import os +import shutil +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") + +# nvidia_preflight_report's documented return codes. +RC_NO_NVIDIA = 0 +RC_NVIDIA_OK = 10 +RC_NVIDIA_TOO_OLD = 11 + + +class NvidiaPreflightGateHarness(unittest.TestCase): + """Source nvidia_preflight out of the real archsetup script.""" + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="nvidia-gate-test-") + self.marker = os.path.join(self.tmp, "report-calls") + self.wrapper = os.path.join(self.tmp, "run.sh") + with open(self.wrapper, "w") as f: + f.write( + "#!/bin/bash\n" + 'ARCHSETUP="$1"; shift\n' + "source <(sed -n " + "'/^nvidia_preflight() {/,/^}/p' \"$ARCHSETUP\")\n" + # Stub the detection core: record the call, answer with $STUB_RC. + "nvidia_preflight_report() {\n" + ' echo "called" >> "$MARKER"\n' + ' return "${STUB_RC:-0}"\n' + "}\n" + "nvidia_preflight\n" + 'echo "PREFLIGHT_RETURNED"\n' + # Whatever the prompt did not consume is still on stdin. + 'if IFS= read -r leftover; then echo "LEFTOVER:$leftover"; ' + 'else echo "LEFTOVER:"; fi\n' + ) + os.chmod(self.wrapper, 0o755) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def run_gate(self, rc, desktop_env="hyprland", skip_gpu_drivers="false", + config_file="", stdin=""): + env = dict(os.environ) + env["MARKER"] = self.marker + env["STUB_RC"] = str(rc) + env["desktop_env"] = desktop_env + env["skip_gpu_drivers"] = skip_gpu_drivers + env["config_file"] = config_file + return subprocess.run( + ["bash", self.wrapper, ARCHSETUP], + input=stdin, capture_output=True, text=True, env=env, + ) + + def report_calls(self): + if not os.path.exists(self.marker): + return 0 + with open(self.marker) as f: + return len([line for line in f if line.strip()]) + + def assertContinued(self, r): + self.assertIn("PREFLIGHT_RETURNED", r.stdout, + "preflight aborted; stdout=%r stderr=%r" % (r.stdout, r.stderr)) + self.assertEqual(r.returncode, 0) + + def assertAborted(self, r): + self.assertNotIn("PREFLIGHT_RETURNED", r.stdout) + self.assertEqual(r.returncode, 1) + + # ---------------------------------------------------------- normal ---- + def test_hyprland_no_nvidia_continues_without_prompting(self): + r = self.run_gate(RC_NO_NVIDIA, stdin="sentinel\n") + self.assertContinued(r) + self.assertEqual(self.report_calls(), 1) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_hyprland_supported_driver_prompts_and_continues_on_yes(self): + # bash emits `read -p`'s prompt only when stdin is a terminal, so the + # prompt text is unassertable here. What is assertable -- and is the + # thing that matters -- is that the read consumed exactly the answer + # line and left the rest of stdin alone. + r = self.run_gate(RC_NVIDIA_OK, stdin="y\nsentinel\n") + self.assertContinued(r) + self.assertIn("Continuing on NVIDIA", r.stdout) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_hyprland_old_driver_still_aborts(self): + # The check must keep working where it actually applies. + r = self.run_gate(RC_NVIDIA_TOO_OLD) + self.assertAborted(r) + self.assertEqual(self.report_calls(), 1) + + # -------------------------------------------------------- boundary ---- + def test_dwm_old_driver_does_not_abort(self): + # An X11 install does not care about the Wayland driver floor -- and + # the abort's own hint tells the user to install with DESKTOP_ENV=dwm. + r = self.run_gate(RC_NVIDIA_TOO_OLD, desktop_env="dwm") + self.assertContinued(r) + self.assertEqual(self.report_calls(), 0) + + def test_none_old_driver_does_not_abort(self): + # A headless install never runs a compositor at all. + r = self.run_gate(RC_NVIDIA_TOO_OLD, desktop_env="none") + self.assertContinued(r) + self.assertEqual(self.report_calls(), 0) + + def test_skip_gpu_drivers_old_driver_does_not_abort(self): + # --no-gpu-drivers means the user installs the driver themselves, so + # the repo's candidate version is not archsetup's call to veto. + r = self.run_gate(RC_NVIDIA_TOO_OLD, skip_gpu_drivers="true") + self.assertContinued(r) + self.assertEqual(self.report_calls(), 0) + + def test_dwm_supported_driver_does_not_prompt(self): + r = self.run_gate(RC_NVIDIA_OK, desktop_env="dwm", stdin="sentinel\n") + self.assertContinued(r) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + def test_unattended_config_file_answers_the_prompt_itself(self): + # --config-file is the documented unattended mode. The rc-10 prompt is + # advisory (the driver requirement is met), so it must not block on a + # read -- the same failure aur_install's --answerdiff/--answerclean + # comment already rules out. + r = self.run_gate(RC_NVIDIA_OK, config_file="/etc/archsetup.conf", + stdin="sentinel\n") + self.assertContinued(r) + self.assertIn("Continuing on NVIDIA", r.stdout) + self.assertIn("LEFTOVER:sentinel", r.stdout) + + # ----------------------------------------------------------- error ---- + def test_hyprland_supported_driver_aborts_on_no(self): + r = self.run_gate(RC_NVIDIA_OK, stdin="n\n") + self.assertAborted(r) + self.assertIn("Aborted at NVIDIA preflight.", r.stdout) + + def test_hyprland_supported_driver_aborts_on_capital_no(self): + r = self.run_gate(RC_NVIDIA_OK, stdin="No\n") + self.assertAborted(r) + + def test_unattended_old_driver_still_aborts(self): + # Unattended does not mean "never fail" -- a driver below the floor is + # still a hard stop on a hyprland install. + r = self.run_gate(RC_NVIDIA_TOO_OLD, config_file="/etc/archsetup.conf") + self.assertAborted(r) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/wipedisk/test_wipedisk.py b/tests/wipedisk/test_wipedisk.py new file mode 100644 index 0000000..95c7df8 --- /dev/null +++ b/tests/wipedisk/test_wipedisk.py @@ -0,0 +1,273 @@ +"""Test scripts/wipedisk, the manual disk-erase helper. + +Three defects, all of which make the script's final word untrue: + +1. `sgdisk --zap-all` had its result discarded and the script printed + "Disk erased." and exited 0 unconditionally. sgdisk fails on a busy device + -- a disk with a mounted filesystem or an active md/LVM/ZFS holder, which is + exactly the case a user hits when they pick the wrong disk -- so the tool + claimed an erase it had not performed. + +2. The prompt says "Select the disk id to use", and then listed every entry in + /dev/disk/by-id, two thirds of which are partitions (18 entries on this + machine, 12 of them -partN). The menu promised disks and offered partitions. + +3. "Disk erased." overstates what the tool does. `sgdisk --zap-all` destroys + partition tables, not data, and `blkdiscard -f || true` deliberately + tolerates a device that cannot discard. On a disk without discard support + the script erased the partition table and every byte of data remained + readable -- while telling the user the disk was erased. + +Method: run the REAL script with WIPEDISK_BY_ID pointed at a fixture directory +and fake blkdiscard/sgdisk on PATH, feeding the select menu and the confirm +prompt on stdin. The env-overridable path follows nvidia_preflight_report's +NVIDIA_DRM_GLOB. + +Run from repo root: + python3 -m unittest tests.wipedisk.test_wipedisk +""" + +import os +import shutil +import stat +import subprocess +import tempfile +import unittest + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +SCRIPT = os.path.join(REPO_ROOT, "scripts", "wipedisk") + +# A realistic by-id listing: two whole disks, each with two partitions, plus +# the eui alias the kernel also publishes. +# Names that cannot collide with a real device. The first cut of this suite +# copied this machine's actual by-id entries, so a test that lost the +# WIPEDISK_BY_ID seam would have driven a menu of real disks and still passed. +DISKS = [ + "wipedisk-fixture-disk-A", + "wipedisk-fixture-disk-B", +] +PARTS = [d + p for d in DISKS for p in ("-part1", "-part2")] + + +class Wipedisk(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="wipedisk-test-") + self.bin = os.path.join(self.tmp, "bin") + self.byid = os.path.join(self.tmp, "by-id") + os.makedirs(self.bin) + os.makedirs(self.byid) + for name in DISKS + PARTS: + open(os.path.join(self.byid, name), "w").close() + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def fake(self, name, body): + path = os.path.join(self.bin, name) + with open(path, "w") as f: + f.write("#!/bin/bash\n" + body) + os.chmod(path, os.stat(path).st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) + + def install_fakes(self, discard_ok=True, sgdisk_ok=True, + discard_err="blkdiscard: BLKDISCARD ioctl failed: " + "Operation not supported"): + log = os.path.join(self.tmp, "calls.log") + self.fake("blkdiscard", + 'echo "blkdiscard $*" >> "%s"\n' + '[ %d -eq 0 ] || echo "%s" >&2\nexit %d\n' + % (log, 0 if discard_ok else 1, discard_err, + 0 if discard_ok else 1)) + self.fake("sgdisk", 'echo "sgdisk $*" >> "%s"\nexit %d\n' + % (log, 0 if sgdisk_ok else 1)) + self.calls_log = log + + def calls(self): + if not os.path.exists(self.calls_log): + return "" + with open(self.calls_log) as f: + return f.read() + + def run_script(self, stdin): + env = dict(os.environ) + env["PATH"] = self.bin + os.pathsep + env["PATH"] + env["WIPEDISK_BY_ID"] = self.byid + return subprocess.run( + ["bash", SCRIPT], input=stdin, capture_output=True, text=True, + env=env, timeout=20, + ) + + # ---------------------------------------------------------- normal ---- + def test_successful_wipe_exits_zero_and_calls_both_tools(self): + self.install_fakes() + r = self.run_script("1\ny\n") + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + # Assert the fixture PATH, not just the tool name: this is what + # catches a lost WIPEDISK_BY_ID seam driving real devices. + self.assertIn("blkdiscard %s/%s" % (self.byid, DISKS[0]), self.calls()) + self.assertIn("sgdisk --zap-all %s/%s" % (self.byid, DISKS[0]), + self.calls()) + + def test_declining_the_confirmation_touches_nothing(self): + self.install_fakes() + r = self.run_script("1\nn\n") + self.assertNotEqual(r.returncode, 0) + self.assertEqual(self.calls(), "", + "a declined confirmation must not run either tool") + + # -------------------------------------------------------- boundary ---- + def test_menu_offers_whole_disks_only(self): + # The prompt says "Select the disk id"; partitions are not disks. + self.install_fakes() + r = self.run_script("1\nn\n") + menu = r.stdout + r.stderr + for part in PARTS: + self.assertNotIn(part, menu, + "the disk menu must not offer partition %s" % part) + for disk in DISKS: + self.assertIn(disk, menu) + + def test_discard_unsupported_does_not_claim_the_data_is_gone(self): + # blkdiscard failing is tolerated by design (not every device can + # discard) -- but then only the partition table went, and the message + # must not tell the user the disk was erased. + self.install_fakes(discard_ok=False) + r = self.run_script("1\ny\n") + out = r.stdout + r.stderr + self.assertIn("sgdisk", self.calls(), + "a failed discard must not stop the partition-table zap") + self.assertNotIn("Disk erased.", out, + "data was not erased -- only the partition table was") + + # ----------------------------------------------------------- error ---- + def test_failed_zap_exits_non_zero(self): + # sgdisk fails on a busy device. Claiming success here is the defect. + self.install_fakes(sgdisk_ok=False) + r = self.run_script("1\ny\n") + self.assertNotEqual(r.returncode, 0, + "a failed zap must not report success") + self.assertNotIn("Disk erased.", r.stdout + r.stderr) + + def test_empty_by_id_directory_says_so(self): + # Without the guard this still exits non-zero and runs nothing -- the + # empty select menu falls through to the confirm prompt, which reads + # EOF and declines. So asserting only on the exit code and the call log + # is not a gate; it passes with the guard deleted. What the guard + # actually buys is saying why, instead of an empty menu the user has to + # break out of. + self.install_fakes() + for name in os.listdir(self.byid): + os.unlink(os.path.join(self.byid, name)) + r = self.run_script("1\ny\n") + self.assertNotEqual(r.returncode, 0) + self.assertEqual(self.calls(), "", + "no disks means nothing to erase, not an unguarded run") + self.assertIn("No disks found", r.stdout + r.stderr, + "an empty device list must be reported, not shown as an empty menu") + + +if __name__ == "__main__": + unittest.main() + + +class WipediskBusyDisk(unittest.TestCase): + """The wrong-disk case, which is the whole reason this tool checks anything. + + 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 the safety gate, so it has to come first. Forcing + past it with -f destroyed a live filesystem and only then let sgdisk fail, + leaving the user told that nothing had happened and to try again. + + A discard that fails because the hardware cannot discard is a different + answer entirely, and must not stop the run. + """ + + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="wipedisk-busy-test-") + self.bin = os.path.join(self.tmp, "bin") + self.byid = os.path.join(self.tmp, "by-id") + os.makedirs(self.bin) + os.makedirs(self.byid) + for name in DISKS + PARTS: + open(os.path.join(self.byid, name), "w").close() + self.log = os.path.join(self.tmp, "calls.log") + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def fake(self, name, body): + path = os.path.join(self.bin, name) + with open(path, "w") as f: + f.write("#!/bin/bash\n" + body) + os.chmod(path, 0o755) + + def install(self, discard_rc, discard_err, sgdisk_rc=0): + self.fake("blkdiscard", + 'echo "blkdiscard $*" >> "%s"\n' + '[ %d -eq 0 ] || echo "%s" >&2\nexit %d\n' + % (self.log, discard_rc, discard_err, discard_rc)) + self.fake("sgdisk", 'echo "sgdisk $*" >> "%s"\nexit %d\n' + % (self.log, sgdisk_rc)) + + def calls(self): + if not os.path.exists(self.log): + return "" + with open(self.log) as f: + return f.read() + + def run_script(self, stdin="1\ny\n"): + env = dict(os.environ) + env["PATH"] = self.bin + os.pathsep + env["PATH"] + env["WIPEDISK_BY_ID"] = self.byid + return subprocess.run(["bash", SCRIPT], input=stdin, + capture_output=True, text=True, env=env, + timeout=20) + + # ---------------------------------------------------------- normal ---- + def test_blkdiscard_is_not_forced_past_the_exclusive_open(self): + # -f is what turned "refuse a busy disk" into "destroy it anyway". + self.install(0, "") + self.run_script() + lines = [l for l in self.calls().splitlines() + if l.startswith("blkdiscard ")] + self.assertEqual(len(lines), 1, self.calls()) + # Exact argv tokens: "-f" is a substring of the fixture disk name, so + # assertNotIn would pass on the fixture path and prove nothing. + args = lines[0].split()[1:] + self.assertNotIn("-f", args) + self.assertNotIn("--force", args) + + # -------------------------------------------------------- boundary ---- + def test_a_device_that_cannot_discard_still_gets_its_table_cleared(self): + self.install(1, "blkdiscard: BLKDISCARD ioctl failed: " + "Operation not supported") + r = self.run_script() + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + self.assertIn("sgdisk", self.calls()) + self.assertIn("still", r.stdout) # the honest data-still-present note + + # ----------------------------------------------------------- error ---- + def test_a_busy_disk_stops_before_sgdisk_is_ever_called(self): + self.install(1, "blkdiscard: /dev/sda: BLKDISCARD ioctl failed: " + "Device or resource busy") + r = self.run_script() + self.assertNotEqual(r.returncode, 0) + self.assertNotIn("sgdisk", self.calls(), + "a refused discard must not fall through to sgdisk") + + def test_a_busy_disk_says_nothing_was_erased(self): + self.install(1, "blkdiscard: /dev/sda: BLKDISCARD ioctl failed: " + "Device or resource busy") + r = self.run_script() + out = r.stdout + r.stderr + self.assertIn("Nothing was erased", out) + self.assertNotIn("Disk erased", out) + + def test_a_busy_disk_never_reports_a_cleared_partition_table(self): + # The defect this class exists for: the old order discarded first and + # then said "could not clear the partition table ... run this again", + # which reads as "nothing happened" after data was already gone. + self.install(1, "blkdiscard: /dev/sda: BLKDISCARD ioctl failed: " + "Device or resource busy") + r = self.run_script() + self.assertNotIn("Partition table cleared", r.stdout + r.stderr) diff --git a/tests/zfs-replicate/test_zfs_replicate.py b/tests/zfs-replicate/test_zfs_replicate.py new file mode 100644 index 0000000..f316ce6 --- /dev/null +++ b/tests/zfs-replicate/test_zfs_replicate.py @@ -0,0 +1,174 @@ +"""Test scripts/zfs-replicate, the nightly ZFS replication to TrueNAS. + +Two defects the script had, both of which make a failing backup look like a +working one. It runs as a systemd oneshot (zfs-replicate.service) on a nightly +timer, so its exit code and its journal output are the only signals anyone ever +sees. + +1. The full-replication loop caught each syncoid failure, warned, and carried + on -- then printed "Replication complete." and exited 0 regardless. With + every dataset failing, systemd recorded a clean success. A backup that had + not run for months was indistinguishable from one that had. + +2. determine_host runs inside a command substitution, and its error() wrote to + stdout. So on an unreachable TrueNAS the message was captured into + TRUENAS_HOST and discarded, and `set -e` killed the script with exit 1 and + no output whatsoever -- a nightly service failing silently with nothing in + the journal to say why. + +Both are fixed by sending every diagnostic to stderr (which also keeps the +command substitution's stdout clean, so only the hostname can land in +TRUENAS_HOST) and by exiting non-zero when any dataset failed. + +Method: run the REAL script with a fake syncoid and a fake ping on PATH, in the +style of tests/zfs-pre-snapshot's fake-zfs. + +Run from repo root: + python3 -m unittest tests.zfs-replicate.test_zfs_replicate +""" + +import os +import shutil +import stat +import subprocess +import tempfile +import unittest + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +SCRIPT = os.path.join(REPO_ROOT, "scripts", "zfs-replicate") + +# The four datasets the script replicates, in order. +DATASETS = ["zroot/ROOT/default", "zroot/home", "zroot/media", "zroot/vms"] + + +class ZfsReplicate(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.mkdtemp(prefix="zfs-replicate-test-") + self.bin = os.path.join(self.tmp, "bin") + os.makedirs(self.bin) + + def tearDown(self): + shutil.rmtree(self.tmp, ignore_errors=True) + + def fake(self, name, body): + path = os.path.join(self.bin, name) + with open(path, "w") as f: + f.write("#!/bin/bash\n" + body) + os.chmod(path, os.stat(path).st_mode | stat.S_IEXEC | stat.S_IXGRP | stat.S_IXOTH) + + def install_fakes(self, reachable="local", fail_datasets=(), no_syncoid=False): + """ping answers for the named host only; syncoid fails for the listed + datasets (the last argument-but-one is the source dataset).""" + if reachable == "local": + ping = 'case "$3" in truenas.local) exit 0 ;; *) exit 1 ;; esac\n' + elif reachable == "tailscale": + ping = 'case "$3" in truenas) exit 0 ;; *) exit 1 ;; esac\n' + else: + ping = "exit 1\n" + # `ping -c 1 -W 2 ` puts the host in $5, but be permissive and + # scan every argument so an option-order change does not silently make + # every host unreachable (which would make these tests pass for the + # wrong reason). + ping = 'for a in "$@"; do case "$a" in\n' + \ + (" truenas.local) exit 0 ;;\n" if reachable == "local" else "") + \ + (" truenas) exit 0 ;;\n" if reachable == "tailscale" else "") + \ + "esac; done\nexit 1\n" + self.fake("ping", ping) + if not no_syncoid: + arms = "".join( + ' %s) echo "fake syncoid failing for %s" >&2; exit 1 ;;\n' % (d, d) + for d in fail_datasets + ) + self.fake("syncoid", ( + 'for a in "$@"; do case "$a" in\n' + arms + "esac; done\nexit 0\n" + )) + + def run_script(self, *args): + env = dict(os.environ) + # Front of PATH so the fakes win; keep the rest so bash/coreutils work. + env["PATH"] = self.bin + os.pathsep + env["PATH"] + return subprocess.run( + ["bash", SCRIPT, *args], + capture_output=True, text=True, env=env, timeout=20, + ) + + # ---------------------------------------------------------- normal ---- + def test_all_datasets_succeed_exits_zero(self): + self.install_fakes() + r = self.run_script() + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + self.assertIn("Replication complete", r.stdout + r.stderr) + + def test_single_dataset_mode_succeeds(self): + self.install_fakes() + r = self.run_script("zroot/home") + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + + def test_falls_back_to_tailscale_when_local_is_unreachable(self): + self.install_fakes(reachable="tailscale") + r = self.run_script() + self.assertEqual(r.returncode, 0, r.stdout + r.stderr) + out = r.stdout + r.stderr + self.assertIn("Using TrueNAS host: truenas", out) + self.assertNotIn("truenas.local", out) + + # -------------------------------------------------------- boundary ---- + def test_every_dataset_failing_exits_non_zero(self): + # THE BUG: this used to print "Replication complete." and exit 0, so a + # nightly oneshot recorded success while backing nothing up. + self.install_fakes(fail_datasets=DATASETS) + r = self.run_script() + self.assertNotEqual(r.returncode, 0, + "a run where every dataset failed must not report success") + + def test_one_dataset_failing_exits_non_zero_and_still_tries_the_rest(self): + self.install_fakes(fail_datasets=["zroot/media"]) + r = self.run_script() + self.assertNotEqual(r.returncode, 0) + out = r.stdout + r.stderr + # The failure must not abort the run -- the dataset after it still goes. + self.assertIn("zroot/vms", out, + "a mid-loop failure must not stop the remaining datasets") + + def test_failure_count_is_reported(self): + self.install_fakes(fail_datasets=["zroot/home", "zroot/vms"]) + r = self.run_script() + self.assertIn("2", (r.stdout + r.stderr).split("complete")[-1] or "", + "the summary must say how many datasets failed") + + # ----------------------------------------------------------- error ---- + def test_unreachable_truenas_reports_why(self): + # THE OTHER BUG: error() wrote to stdout inside a command substitution, + # so this exited 1 with completely empty output. + self.install_fakes(reachable="none") + r = self.run_script() + self.assertNotEqual(r.returncode, 0) + self.assertIn("Cannot reach TrueNAS", r.stdout + r.stderr, + "the reason must reach the journal, not the command substitution") + + def test_missing_syncoid_reports_why(self): + self.install_fakes(no_syncoid=True) + # PATH is the fake bin only, so `command -v syncoid` fails whether or + # not this machine has a real syncoid. bash must then be invoked by + # absolute path -- a PATH that cannot find syncoid cannot find bash + # either, and subprocess would fail to launch instead of testing + # anything. The script's syncoid check runs before any PATH lookup it + # actually needs, so nothing else breaks. + env = dict(os.environ) + env["PATH"] = self.bin + r = subprocess.run([shutil.which("bash") or "/usr/bin/bash", SCRIPT], + capture_output=True, text=True, env=env, timeout=20) + self.assertNotEqual(r.returncode, 0) + self.assertIn("syncoid not found", r.stdout + r.stderr) + + def test_diagnostics_do_not_land_on_stdout_of_the_host_probe(self): + # Whatever determine_host prints on stdout becomes TRUENAS_HOST, so a + # stray diagnostic there corrupts every destination path. + self.install_fakes() + r = self.run_script() + self.assertIn("Using TrueNAS host: truenas.local", r.stdout + r.stderr) + self.assertNotIn("[INFO] Using TrueNAS host: [", r.stdout + r.stderr) + + +if __name__ == "__main__": + unittest.main() diff --git a/todo.org b/todo.org index bfe9e2e..4deca17 100644 --- a/todo.org +++ b/todo.org @@ -45,19 +45,687 @@ below): input-side-spec.org (DRAFT, four decisions open). * Archsetup Open Work -** TODO [#C] Timer module hero hierarchy :feature:waybar:timer: +** DONE [#B] Adversarial review of the sentry run — six fixes reworked :bug:test:tooling:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Craig asked for a skeptical review of every sentry change. Eight agents covered all 23 code commits, each told to disbelieve by default and to answer three questions per commit: does the problem exist and is it reachable, is the fix correct or is there a better one, would each test fail with the fix reverted. Every finding below was re-verified by hand before acting on it. + +SIX COMMITS NEEDED WORK, now fixed: archsetup =1207ca5= (wipedisk), =96e12b5= (firmware trim), =560e1dd= (autologin), =3c2155d= (initramfs tabs); dotfiles =ec7229b= (tunnel import), =a81aa0e= (thumbnail sweep), =56807e5= (three residual guards), =c90ee34= (event-log isolation). Both suites green: archsetup 341, dotfiles 3687 on both gates. + +THE ONE THAT MATTERED MOST. =wipedisk= ran =blkdiscard -f= BEFORE the busy check. =-f= disables the exclusive open util-linux has used since 2.36, so on the exact case the round-11 commit reasoned about — the user picked the wrong disk — it discarded a live filesystem and only then let sgdisk fail, printing "could not clear the partition table ... run this again". Data gone, user told nothing happened. The ordering predates the sentry commit, but round 11 wrote reasoning about the busy-disk case into the comment and error text while leaving the discard first, which made the misreport worse in the one direction that costs something. Dropping =-f= makes the kernel's own O_EXCL the gate. + +THREE PATTERNS WORTH MORE THAN THE INDIVIDUAL FIXES: + +1. CALL SITES WENT UNTESTED IN FIVE SUITES. Every helper had thorough tests; not one proved it was called. Deleting the call left everything green — including the guard on a =pacman -Rdd= of twelve firmware packages, whose removal would have run the trim on ratio. Closed with =CALL_SITES= in =test_orchestrators= (nine pairs, static) and a wiring assertion in the settings suite. Static on purpose: the behavioural harness runs un-stubbed bodies for real, which is fine for an orchestrator and not for a leaf that removes packages. + +2. A NEW OUTCOME VALUE NEEDS EVERY CONSUMER WALKED, EVERY TIME. Done for the portal enum in round 3, skipped for the tunnel-import one in round 4 — where =import_configs= folded a disarm failure into "none imported (N failed)", the opposite of what happened, in the multi-select flow the GUI actually uses. + +3. MY FIXTURES TWICE CLAIMED A FIDELITY THEY DID NOT HAVE. The wipedisk fixture used this machine's real disk names, so five of six tests passed with the seam removed. The mkplaylist fake does a full =cat > /dev/null= drain while its docstring says it "drains stdin exactly when the real one would" — which is what let the wrong failure mode survive. + +AND ONE FINDING WAS DISPROVED OUTRIGHT: round 1's =a57c443= claimed ffmpeg drains the read loop so only the first track is processed. Measured under strace and driven end to end with real ffmpeg (three runs of three, four 120s mp3s), the loop never truncates. The hazard is real and =-nostdin= is right; the symptom was reasoned from shellcheck SC2095 and never run. Corrected in =a30741a=, along with the OpenVPN autoconnect claim and the "four consumers" undercount. + +STILL OPEN, deliberately: sweeping thumbnails per-readable-source instead of all-or-nothing (deferring is safe but a permanently-missing source defers forever and the cache grows — the tick now says so on stderr rather than being silent); the =noop= branch of =_restore_dot= still reports "DNS-over-TLS restored" on a machine where DoT was never configured; =_disable_dot= checks its move but not its restart. None is destructive; each is its own task. + +** DONE [#D] Repair tiers call an unverifiable service restart a failed one :bug:network:bluetooth:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as dotfiles =041d6b9= (committed locally, deliberately NOT pushed — held for Craig's morning review of the sentry run). 7 new tests across =tests/bt/test_bt.py= and =tests/net/test_net.py=; dotfiles suite 3665 -> 3672, =make test= exit 0 on both gates. Each of the three guards proven a real gate by deleting it and watching the suite go red. + +Found in the 2026-07-24 sentry bug-hunt, round 14, on the cross-package =repair.py= diff that rounds 5-13 had left unspent. + +=cmd.service_active= is tri-state in both the net and bt packages, and its docstring says so outright: True, False, or None when systemctl itself can't answer (absent binary, or a timeout). Six callers. Three rule on it correctly — =bt/doctor._service_step= branches on None with "systemctl unavailable — can't check the service", and =net/diag= compares =is False= at both its call sites. Three tested it with plain truthiness: + +- =bt/repair.py= =repair_service_restart= +- =net/repair.py= =_service_restart= (the nm-restart and resolved-restart tiers) +- =net/repair.py= =repair_unmask_nm= + +So an unanswerable systemctl was reported as "bluetooth.service is still not active" / "NetworkManager still isn't running after a restart" — a statement about the service made on no evidence at all. Each then pointed the user at =journalctl -u =, which is the same systemd client stack that had just failed to answer. That last part is round 10's read again: an error message advertising a remedy it cannot honour. + +All three now report =warn= on None, with evidence naming the verification rather than the service, and a next action of checking systemd is reachable and re-running the doctor. Control flow is unchanged: =warn= was already a status both packages emit, both CLIs already exit non-zero on anything but =pass=, and =net/doctor= only inspects a repair step's status for the =dns-test= tier — every consumer was checked before the change, not after. (An adversarial re-review counted twelve, not four; all twelve handle =warn= correctly, so the conclusion held while the claim understated the work.) + +THE SEAM FOR THE TESTS, worth reusing: both suites already carry an exec-failure harness that plants a non-executable file on an emptied PATH, which is exactly what makes =cmd.run= return None. So the None case is reachable through the real code path with no mocking at all. Each test class asserts that premise first (=service_active= really is None in the sandbox) rather than assuming it. + +Grading: Minor severity (the claim is wrong but errs pessimistic — it says a repair failed when it may have worked, rather than falsely reassuring; nothing is damaged) x rare edge case = P4 = [#D]. Fixed rather than filed because the change is three branches and it completes a class — leaving two of three sites collapsed is the failure mode the round-6 =c2eb3e1= commit exists to remember. + +NOT PART OF THIS CLASS, checked and left alone: =settings/toggles.dim_state= is the only other genuine True/False/None helper in the tree, and both its callers pass the value through to the viewmodel rather than collapsing it. Every other "or None" in the packages is two-state (a value or nothing), where falsy handling is correct. + +** DONE [#B] Firmware trim gated on a DMI field that never carries the vendor :bug:tooling:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as archsetup =2e228f7= (committed locally, deliberately NOT pushed — held for Craig's morning review of the sentry run). New =tests/installer-steps/test_framework_firmware_trim.py=, 12 tests carrying the real DMI strings off both daily drivers. Each of the three conditions proven load-bearing by deleting it and watching the suite go red, and the old gate proven wrong by restoring it (4 failures). + +Found in the 2026-07-24 sentry bug-hunt, round 13, reading archsetup's remaining state-mutating steps. =trim_firmware= gated on =grep -qi "framework" /sys/class/dmi/id/product_name= and no Framework machine has "framework" in =product_name= — it lives in =sys_vendor=. Read live: velox is =Framework= / ="Laptop (13th Gen Intel Core)"=, ratio is =Framework= / ="Desktop (AMD Ryzen AI Max 300 Series)"=. The gate returns false on both, so the step has been a silent no-op on the exact hardware it was written for. velox IS trimmed today (=linux-firmware-{atheros,intel,realtek,whence}= and nothing else) but not by this code path. + +THE REPAIR IS WHERE THE DANGER IS, which is why this is worth reading twice. Swapping =product_name= for =sys_vendor= is the obvious one-word fix and it is wrong: ratio is a Framework Desktop, and =trim_firmware= runs =pacman -Rdd linux-firmware-amdgpu=, which takes the firmware its Ryzen AI Max iGPU needs to bring up a display. Today only the =grep -qi intel /proc/cpuinfo= second gate stands between ratio and that. So =is_framework_intel_laptop= wants three DMI facts — vendor Framework, and a model naming both Laptop and Intel — and the cpuinfo read stays as an independent second gate rather than the only one. + +Verified live after the change: velox TRIM=yes, ratio TRIM=no, where the old gate said no to both. + +Grading: Minor severity (the trim never happens; nothing breaks, the machine just carries ~550MB it was meant to shed) x every user, every time (every Framework Intel install, which is the whole population the step targets) = P2 = [#B]. The AMD-firmware removal is not graded separately because it never shipped — it is the hazard the fix is shaped to avoid. + +** DONE [#B] Fresh install leaves the dotfiles repo permanently dirty :bug:tooling:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as archsetup =c3b3617= (committed locally, deliberately NOT pushed — held for Craig's morning review of the sentry run). New =tests/installer-steps/test_mark_volatile_configs.py=, 8 tests against a fixture git repo with =sudo= stubbed on PATH. Every guard proven a real gate by deletion. A note went to =~/.dotfiles/inbox/= because =skip-volatile= now has an outside caller. + +Found in the 2026-07-24 sentry bug-hunt, round 13, diffing archsetup's =stow_dotfiles= against the dotfiles Makefile's =stow= target — two implementations of one operation, which is round 5's read applied across repos rather than across packages. + +The Makefile's =stow= target ends with =$(MAKE) skip-volatile=, setting git's skip-worktree bit on the four configs their apps rewrite in place (=btop=, =qalculate=, =calibre=, =waypaper=; the list is =volatile-configs=). archsetup stows inline with raw =stow= calls and never ran that step. So a machine archsetup installed goes dirty the first time one of those apps writes its config, and every later =git pull --ff-only= trips over paths the user never edited. Confirmed by grep: archsetup contains no =skip-volatile=, no =volatile=, and no =make stow= — yet both daily drivers carry the bits, so they came from a hand-run =make stow=, not the installer. ratio in fact carries seven, three more than =volatile-configs= lists, which is evidence the churn is real and ongoing. + +The fix calls the dotfiles target rather than copying its logic, so the volatile list stays single-source. Two details that are load-bearing: it runs *after* =git restore .= so the bit lands on a pristine tree, and it runs as the user, because root writing =.git/index= leaves it root-owned and the user's next git command then cannot update the index at all. A checkout with no Makefile is a quiet no-op — nothing to delegate to is not an error. + +DELIBERATELY NOT DONE: replacing the whole inline stow with =make -C "$dotfiles_dir" stow "$desktop_env"=. The Makefile stows =--target=$(HOME)=, which during an install is root's home, and it carries interactive conflict handling; archsetup stows =--target=/home/$username --adopt= as root on purpose. =skip-volatile= is the one target with no such coupling — it works on the repo through =git -C= and never reads HOME. + +Grading: Minor severity (a repo that reads dirty forever and pulls that need a stash; the workaround is one command) x every user, every time (every fresh install that stows dotfiles) = P2 = [#B]. + +** DONE [#C] Unattended install blocks on an interactive prompt :bug:tooling:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as archsetup =cbcb53f= (committed locally, deliberately NOT pushed — held for Craig's morning review of the sentry run). New =tests/installer-steps/test_configure_autologin.py= (11) and =tests/installer-steps/test_select_locale.py= (11). Every guard proven a real gate by breaking it and watching the suite go red: dropping the autologin unattended branch fails 1 (on the leftover-stdin assertion, which is the real gate — the drop-in still gets written because the read swallows the sentinel and treats it as "yes"); dropping the locale unattended branch fails 1; breaking either precedence rule fails 2. + +Found in the 2026-07-24 sentry bug-hunt, round 12, continuing through archsetup's own installer. Two members of one class, which is the point: round 10 fixed the third member and left these. + +THE CLASS: an advisory prompt — one that carries its own default — still reading stdin under =--config-file=, the documented unattended mode. Round 10 ruled on it for =nvidia_preflight='s rc-10 prompt. Two sites never got the ruling. + +1. =configure_autologin=. When =enable_autologin= is unset (=AUTOLOGIN= is optional, and =archsetup.conf.example= line 31 ships it commented out) and the root is encrypted, it prompted =Enable automatic console login for $username? [Y/n]= on a bare =read=. It runs from =configure_encrypted_autologin=, inside =boot_ux=, the last entry in =STEPS= — so an unattended install of an encrypted machine works for 40-60 minutes and then sits at a prompt nobody is watching. Under =curl | bash= it is worse: stdin is the script itself, so the read eats a line of source. + +2. =select_locale= (extracted from =preflight_checks= by this commit). The =Choice [1]:= menu fired whenever =/etc/locale.conf= carried no =LANG== and =LOCALE= was unset — also commented out in the example config. archsetup does not require an archangel install, and =configure_build_environment='s own "no LANG=" branch is proof it expects that state. + +Both now take the prompt's own default under =--config-file= and print an =[OK] ... (unattended, --config-file)= line saying so. An explicit =AUTOLOGIN=yes/no= or =LOCALE== still wins; the default only answers a question nobody can. + +WHAT MADE THEM TESTABLE, which is round 10's read (d) applied again: =configure_autologin= hardcoded =/etc/systemd/system/getty@tty1.service.d= and =select_locale= hardcoded =/etc/locale.conf=, so neither could run against a fixture — while their siblings =replace_sudoers_pacnew= and =ensure_nvme_early_module= both take a defaulted path argument for exactly that reason. Both now do. Zero shellcheck delta against HEAD; =make test-unit= 276 -> 298, exit 0. + +Grading: Major severity (unattended installation, a documented feature, does not complete; recoverable by pressing a key, no data loss) x some users, sometimes (needs unattended mode plus an omitted key) = P3 = [#C]. + +THE PROMPTS DELIBERATELY LEFT ALONE, because the class is "prompts with a default", not "all prompts": username (line 636) and password (648/650) have no default to take — there is no sane fallback for either, and =archsetup.conf.example= documents both as "If not set, you will be prompted". They also fire in =preflight_checks=, in the first second of the run, where a blocked prompt is visible rather than silent. The "Enter locale" sub-prompt is reachable only from menu choice 9, which unattended never picks. + +** TODO [#D] net-scenarios harness times out under back-to-back suite runs :test:tooling: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +=tests/net-scenarios/test_run_net_scenarios.py= errored on all 5 tests twice during round 12, each time =subprocess.TimeoutExpired= after its 20s budget on =scripts/testing/run-net-scenarios.sh --target root@fake-vm=. Both occurrences were in =make test-unit= runs launched immediately after a previous full run. It then passed 6 runs in a row (3 on a pristine tree, 3 with the round-12 change), and standalone it finishes in 0.09s, so this is not a regression from any code change. + +The harness stubs =ssh=, =rsync= and =jq= onto =PATH=, so nothing should touch the network at all — which is what makes a 20s timeout suspicious rather than merely slow. Worth reproducing under load before deciding whether the fix is a larger timeout or a real hang in the script. Evidence logs from the round: =/tmp/tu.log= and =/tmp/tu2.log= (tmpfs, gone after reboot). + +Not graded on the bug matrix: it is test infrastructure, not the shipped codebase. + +** DONE [#C] wipedisk says "Disk erased." when it erased nothing :bug:tooling:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as archsetup =HEAD= (committed locally, deliberately NOT pushed — held for Craig's morning review of the sentry run). New =tests/wipedisk/test_wipedisk.py=, 6 tests running the real script against a fixture device directory with fake blkdiscard/sgdisk on PATH. All four guards proven real by deleting each and watching the suite go red. + +Found in the 2026-07-24 sentry bug-hunt, round 11, reading =scripts/= — 30 lines, no tests, and the most destructive script in the repo. Not installed by the installer; it is run by hand from the checkout, which is why the frequency axis stays low. + +Three defects, all of which make the script's final word untrue: + +1. =sgdisk --zap-all= had its result discarded, and "Disk erased." printed unconditionally. sgdisk refuses a busy device — a mounted filesystem or a live md/LVM/ZFS holder — which is exactly what a user hits after picking the wrong disk. So the tool announced an erase it had not performed and exited 0. + +2. "Disk erased." overstates what the tool does even on success. =sgdisk --zap-all= destroys partition tables, not data, and =blkdiscard -f ... || true= deliberately tolerates a device that cannot discard. On a disk without discard support the script cleared the partition table and left every byte readable, while telling the user the disk was erased. That is the one path where the wrong belief has a privacy consequence — someone trusting the message before disposing of a drive. + +3. The prompt says "Select the disk id to use" and then listed every entry in =/dev/disk/by-id=. On this machine that is 18 entries of which 12 are =-partN= partitions (verified by listing it). The menu promised disks and offered partitions. + +Fix: whole disks only (globbed rather than =ls | grep=, so a name with whitespace cannot split into two menu entries); the zap's result is checked and a failure exits 1 naming the busy-device cause; the closing message reports what actually happened, and when discard was unsupported it says the data is still recoverable and points at =nvme format= / =hdparm= for a disposal-grade wipe. + +Grading: Major severity (the tool reports an outcome it did not achieve; in the disposal case that is a data-exposure consequence) × rare edge case (a hand-run helper the installer does not install, and defect 1 additionally needs sgdisk to fail) = P3 = [#C]. + +Worth recording about the tests rather than the code: two of the six passed against the unmodified script for the wrong reason. Without the =WIPEDISK_BY_ID= override the script read the real =/dev/disk/by-id=, so the harness was driving a menu of this machine's actual disks (harmless — the fake blkdiscard/sgdisk shadowed the real ones on PATH — but it was not testing the fixture). And =test_empty_by_id_directory= was not a gate at first: with the guard deleted the empty select menu still falls through to the confirm prompt, reads EOF and declines, so exit code and call log alone pass either way. It now asserts the message. +** DONE [#B] zfs-replicate reports success when every backup failed :bug:backup:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as archsetup =HEAD= (committed locally, deliberately NOT pushed — held for Craig's morning review of the sentry run). Diagnostics moved to stderr; the loop counts failures and exits 1 when any dataset failed. New =tests/zfs-replicate/test_zfs_replicate.py=, 9 tests driving the real script with a fake syncoid and a fake ping on PATH (the =tests/zfs-pre-snapshot/fake-zfs= pattern). Both fixes proven real gates by reverting them: dropping the counter fails 3, putting =error()= back on stdout fails 1. + +Found in the 2026-07-24 sentry bug-hunt, round 11, reading =scripts/= — 73 lines with no test file, installed by =configure_zfs_snapshots= as =/usr/local/bin/zfs-replicate= and run by =zfs-replicate.service=, a =Type=oneshot= on a nightly timer. Its exit code and its journal output are the only signals anyone ever sees. + +Two defects, both verified by running the script rather than argued: + +1. The full-replication loop caught each =syncoid= failure, warned, carried on, then printed "Replication complete." and exited 0 regardless. Driven with a fake syncoid failing all four datasets: four =[WARN] Failed= lines, then "Replication complete.", exit code 0. systemd records =Result=success=. A backup that has not run for months is indistinguishable from a working one — and the whole point of the tool is to have a copy when the primary is gone. + +2. =determine_host= runs inside a command substitution (=TRUENAS_HOST=$(determine_host)=) and its =error()= wrote to stdout. On an unreachable TrueNAS the message was captured into =TRUENAS_HOST= and discarded, and =set -e= then killed the script. Driven with both hosts unreachable: exit 1 and completely empty output. A nightly service failing with nothing in the journal to say why. + +Same class as three bugs already fixed this session — =_restore_dot= claiming "DNS-over-TLS restored" without checking, =portal_restore_watch= discarding its outcome, =import_config= returning ok on an unchecked modify. A mutating operation that reports a success it did not get. + +Grading: Critical severity (a backup system that reports success while backing nothing up; the failure surfaces only when the backup is needed — graded on the harm once in the failure state, not on how rarely it is entered) × rare edge case (needs a ZFS root, a reachable TrueNAS, and the user enabling the timer by hand — archsetup deliberately does not enable it, and =findmnt -n -o FSTYPE /= on this machine says btrfs, so it is latent here) = P2 = [#B]. + +Left alone: =BACKUP_PATH="backups" # TODO: Configure actual path= is still an unresolved TODO in the destination, and single-dataset mode relies on =set -e= to propagate a syncoid failure rather than reporting it. Neither is a defect in the sense above; the TODO is Craig's call. +** TODO [#D] Wireless regdom is silently unset for a three-letter-language locale :bug:installer:solo: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +=configure_networking= derives the wireless regulatory domain by fixed offset: =wireless_region="${current_lang:3:2}"=, with a comment reading "extract country code (positions 3-4)". That is correct only for a two-letter language code. + +=validate_config= accepts =^[a-z]{2,3}(_[A-Z]{2})?...=, so a three-letter language is a legal =LOCALE=, and glibc ships 75 of them (=agr_PE=, =ast_ES=, =ber_DZ=, =ayc_PE=, ...). Verified by running the expansion: =ber_DZ.UTF-8= yields =_D=, =ayc_PE.UTF-8= yields =_P=, =C= yields the empty string, =POSIX= yields =IX=. + +The sed that follows only uncomments an existing =#WIRELESS_REGDOM="XX"= line in =/etc/conf.d/wireless-regdom= (176 of them, owned by wireless-regdb). A garbage region matches nothing, sed exits 0, and the =|| error_warn= never fires — so the regdom is never set and nothing says so. The task line does print the garbage region ("configuring wireless regulatory domain (_D)"), so it is visible in the log rather than fully silent. + +Confirmed the mechanism itself works for the normal case: line 168 of this machine's =/etc/conf.d/wireless-regdom= reads =WIRELESS_REGDOM="US"= uncommented, which is archsetup's own edit. + +Grading: Minor severity (WiFi falls back to the conservative "00" regdomain — fewer channels and lower tx power, but WiFi works) × rare edge case (one of 75 three-letter-language locales, or a =LOCALE= with no country) = P4 = [#D]. + +Fix when it comes up: derive the country from the =_CC= group by pattern rather than by offset, and warn when it cannot be derived or when the sed changed nothing. Worth doing together with the sibling gap — nothing in the installer verifies that a =sed -i= uncomment actually matched, so a distro reshuffling one of these config files would fail the same silent way. All 22 =sed -i= sites share that stance, so it is a uniform design choice rather than an odd one out. +** DONE [#B] Initramfs hook swap can leave a LUKS machine unbootable :bug:installer:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as archsetup =HEAD= (committed locally, deliberately NOT pushed — held for Craig's morning review of the sentry run). The swap moved into =switch_udev_hook_to_systemd=, which declines when =hooks_need_busybox_init= sees a standalone =encrypt= token, and the caller now rebuilds the initramfs only when the conf actually changed. New =tests/installer-steps/test_switch_udev_hook.py=, 10 tests; both guards proven real by breaking them (removing the refusal: 4 failures; loosening the token match to a bare =encrypt= substring: 1 failure). + +Found in the 2026-07-24 sentry bug-hunt, round 10. =configure_initramfs_hook= ran =sed -i '/^HOOKS=/ s/\budev\b/systemd/'= on any non-ZFS root, then =mkinitcpio -P=. Its only guard was =is_zfs_root=. + +Why that breaks a LUKS machine, verified against the installed mkinitcpio rather than argued: +- =/usr/lib/initcpio/install/systemd= line 70 is =add_symlink /init usr/lib/systemd/systemd=, so the systemd hook replaces the busybox init outright. +- =/usr/lib/initcpio/hooks/encrypt= is an =#!/usr/bin/ash= script whose entire body is a =run_hook()= function — the busybox init's mechanism. Under systemd init nothing calls it. +- =mkinitcpio= carries no conflict check for the pairing (grepped; nothing), so the rebuild succeeds and archsetup reports success. +- This machine's own =/etc/mkinitcpio.conf= documents the two valid pairings as separate examples: =udev= + =encrypt= (line 45) and =systemd= + =sd-encrypt= (line 51). The sed converted half of the first pairing and produced neither. + +Effect: on a LUKS root using the standard busybox =encrypt= hook, archsetup rewrites HOOKS to =systemd= while leaving =encrypt= behind, rebuilds the initramfs, and exits cleanly. At the next boot the root is never unlocked. The machine needs live media and manual mkinitcpio surgery to recover. + +The sibling asymmetry: =is_encrypted_root()= already exists in this script and =configure_autologin= uses it to branch on exactly this condition. The initramfs step consulted neither it nor HOOKS. =merge_grub_cmdline='s own comment names =cryptdevice== as a boot-critical parameter to preserve — and =cryptdevice== is read only by the =encrypt= hook, so archsetup explicitly anticipates the configuration that another of its steps then breaks. + +Grading: Critical severity (the machine will not boot and recovery needs external media — graded on the harm once in the failure state, not on how rarely it is entered) × some users, sometimes (LUKS-encrypted non-ZFS root using the busybox =encrypt= hook; deterministic for those machines, absent everywhere else) = P2 = [#B]. + +Deliberately not attempted: migrating =encrypt= to =sd-encrypt=. That means rewriting the kernel cmdline from =cryptdevice== to =rd.luks.name== against the volume's UUID, which is a real migration and not a mechanical edit. Refusing the cosmetic swap keeps a working machine working, which is the right trade against quieter fsck output. +** TODO [#D] keymap and consolefont hooks are inert under the systemd initramfs :bug:installer:solo: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Same class as the =encrypt= bug above, but cosmetic rather than boot-critical, so it was filed rather than bundled into that fix. + +Enumerating the busybox-only hooks on this machine (every hook under =/usr/lib/initcpio/hooks/= defining =run_hook=/=run_earlyhook=/=run_latehook=) gives: btrfs, consolefont, encrypt, grub-btrfs-overlayfs, keymap, memdisk, resume, sleep, udev, usr. All go inert once =/init= is systemd. Of those, =encrypt= is the only boot-critical one — =resume= is handled natively by systemd's hibernate-resume generator, and =btrfs= by udev rules (this machine runs =btrfs= alongside =systemd= and boots fine). + +=keymap= and =consolefont= are the live leftovers. Run =grep '^HOOKS=' /etc/mkinitcpio.conf= on this machine: the line carries =systemd= plus =keymap consolefont= and no =udev=, so archsetup's swap has already run here and both hooks are installed into the image and never executed. The systemd equivalent is the single =sd-vconsole= hook, which is what the distro's own systemd example on line 51 of =/etc/mkinitcpio.conf= uses. + +Effect: the early-boot console keeps the default font and keymap until =systemd-vconsole-setup= runs in the real root. =add_nvme_early_module= sets =FONT=ter-132n= in =/etc/vconsole.conf= expecting it to apply at that stage, so the configured font is briefly not what archsetup asked for. + +Grading: Cosmetic severity (a few seconds of default console font on a machine that boots normally) × some users, sometimes = P4 = [#D]. + +Fix when it comes up: have =switch_udev_hook_to_systemd= also rewrite =keymap consolefont= to =sd-vconsole= when it performs the swap, and add the fixture cases to =tests/installer-steps/test_switch_udev_hook.py=. Worth confirming first whether a non-US keymap is ever needed at the initramfs prompt on a machine that reaches this path. +** DONE [#B] NVIDIA Wayland preflight blocks dwm and headless installs :bug:installer:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as archsetup =HEAD= (committed locally, deliberately NOT pushed — held for Craig's morning review of the sentry run). The NVIDIA block moved out of =preflight_checks= into a new =nvidia_preflight= function that returns early unless =desktop_env= is =hyprland= and archsetup is the one installing drivers. New =tests/nvidia-preflight/test_nvidia_preflight_gate.py=, 11 tests; each of the three guards was proven a real gate by deleting it and watching the suite go red (3, 1, and 1 failures respectively). + +Found in the 2026-07-24 sentry bug-hunt, round 10, reading archsetup's own installer. =preflight_checks= called =nvidia_preflight_report= unconditionally and exited 1 on rc 11 (repo driver below the 535 Wayland floor, or =pacman -Si nvidia-utils= unable to answer). The check is Wayland-specific — every line it prints names Wayland/Hyprland — but it ran before any =desktop_env= branch and consulted neither =desktop_env= nor =skip_gpu_drivers=. + +Effect, proven empirically rather than argued (three scenarios driven against the extracted block): =DESKTOP_ENV=dwm= plus =--no-gpu-drivers= on an NVIDIA machine with an old repo driver aborts the install; so does =DESKTOP_ENV=none=. Neither install ever runs a compositor, and =--no-gpu-drivers= means the user installs the driver themselves. Worse, the abort's own fix hint reads "install with DESKTOP_ENV=dwm (X11) instead" — the one remedy it prints is the one it refuses to honor, so the user has no working workaround short of editing the script. + +The sibling asymmetry that makes it an oversight rather than a decision: =install_gpu_drivers= returns early on =skip_gpu_drivers=, and =display_server= / =window_manager= both branch on =desktop_env= with a =none= arm that skips outright. The preflight gate applied neither ruling. + +Second defect at the same site, fixed in the same commit: the rc-10 path (card detected, driver fine) prompts with a bare =read=. =--config-file= is documented as "unattended installation", and =aur_install= already rules that a prompt not covered by =--noconfirm= "blocks forever waiting for input" on a headless install. The rc-10 prompt is advisory, so it now answers itself with its own =[Y/n]= default when a config file was supplied. rc 11 stays a hard stop either way. + +Grading: Critical severity (archsetup cannot be run at all on that machine, and the printed workaround does not work — graded on the harm once in the failure state, not on how rarely it is entered) × rare edge case (needs an NVIDIA card, a repo driver below the floor or an unsynced pacman db, and a non-hyprland =desktop_env=; hyprland is the default and Craig's own machines are AMD and Intel) = P2 = [#B]. + +Noted, not fixed: =display_server= and =window_manager= both point their unknown-value hint at a =--desktop-env= flag that the argument parser does not implement. Both arms are unreachable today (=validate_config= rejects a bad =DESKTOP_ENV=, and without a config file the value is always the default), so it is a stale string rather than a live defect. +** DONE [#B] mkplaylist retags only the first file :bug:music:quick:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as dotfiles =a57c443= (committed locally, deliberately NOT pushed — held for Craig's morning review of the sentry run). =ffmpeg -nostdin= on the conversion call. New =tests/mkplaylist= suite, 12 tests; removing the flag turns the suite red (verified by reverting: 5 failures, green on restore). NOTE: the fake ffmpeg does a full =cat > /dev/null= drain, which the real one does not do — so the suite gates the flag's presence, not the production failure mode. The docstring claiming the fake "drains stdin exactly when the real one would" is false and should be corrected. +Found in the 2026-07-24 sentry bug-hunt (shellcheck SC2095). =common/.local/bin/mkplaylist=: =generate_music_m3u= pipes the file list into =tag_music_file= (line 130), which consumes it with =while IFS= read -r file=. Inside that loop, =ffmpeg -i "$file" -vn -c:a flac "$outputfile"= (line 46) reads stdin by default for its interactive keyboard controls, so it consumes bytes the loop is relying on. + +CORRECTION (2026-07-24, from an adversarial re-review): the failure mode stated above — "the loop sees EOF and exits after the first file" — is WRONG, and this task originally asserted it. Measured under strace, ffmpeg polls fd 0 and reads roughly one byte per half-second of transcode wall time; flac encoding runs about 2000x realtime, so a ten-minute mp3 converts in ~0.28s and yields zero or one stolen byte, never a drain. Driven end to end with real ffmpeg against four 120s mp3s, three runs of three: all four were converted and retagged every time. The loop never truncated. + +What is real is the hazard, not the observed symptom: one stolen byte mangles a path, which makes mid3v2/metaflac fail and =set -e= abort the run loudly. =-nostdin= is still the right fix and the commit still stands. The original finding came from shellcheck SC2095 plus reasoning, and was never run — which is exactly what "verify before filing" exists to prevent. + +Effect: on a directory of non-flac audio, only the first file is converted and retagged. Files 2..N are silently skipped — no error, no output, and the playlist itself still generates (a separate =find=), so nothing signals that the retagging stopped. + +Grading: Major severity (the retagging feature is broken past the first file, and it fails silently) × most users frequently (the script exists to batch-process a directory, so more than one non-flac file is the normal case) = P2 = [#B]. + +Fix: =ffmpeg -nostdin= (or =< /dev/null= on the call). Verifiable with a fake =ffmpeg= on PATH asserting it is invoked once per input file. +** DONE [#C] timezone-change prints command-not-found instead of its help :bug:tooling:quick:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as dotfiles =15d2b63= (committed locally, deliberately NOT pushed — held for Craig's morning review), together with the Portugal-zone defect below. New =tests/timezone-change= suite, 12 tests. +Found in the 2026-07-24 sentry bug-hunt (shellcheck SC2288). =common/.local/bin/timezone-change=, default =*)= case (lines 63-67): =echo= sits alone on its own line, so the following quoted string runs as a *command* rather than as its argument. + +#+begin_src sh +*) + echo + "Invalid option chosen." + echo + "Some valid options are: eastern, central, pacific, rome, london, st_lucia, italy, france, spain ." + ;; +#+end_src + +The user gets two blank lines and two =command not found= errors; the list of valid options never prints. The timezone is correctly left unchanged, so this is an output defect only. + +Grading: Minor severity (wrong output on an error path, nothing corrupted) × some users sometimes (only on an unrecognized option) = P3 = [#C]. + +Fix: fold each string into its =echo=. Verifiable by running the script with a bogus argument and asserting the option list appears on stdout. +** DONE [#C] Thumbnail sweep wipes the whole cache when a wallpaper source is unreadable :bug:settings:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as dotfiles =0bd8c67= (committed locally, deliberately NOT pushed — held for Craig's morning review). 8 new tests. +Found in the 2026-07-24 sentry bug-hunt, reviewing the orphan sweep shipped the night before (dotfiles =e752a16=). =os.walk= stays silent about a directory it cannot enter, so =wallpaper.scan_sources= returns =[]= for a source that is missing, renamed, or permission-denied — the same answer it gives for a gallery the user emptied on purpose. =settings/cli.py= tick then hands that empty list to =thumbstore.sweep_orphans=, =live_names= comes back empty, and every cache-shaped file is classified an orphan. + +Proven empirically rather than reasoned: seeding three well-formed thumbnails plus a stray README, then sweeping against a nonexistent source directory, deleted all three (the README survived, so the cache-name regex guard works — it just doesn't help here). + +Effect once entered: the entire persistent thumbnail cache is deleted, so the next wallpaper-view open pays the cold-decode cost the cache was built to remove (measured at 3.7s for a viewport of Craig's largest 8, which is what tripped the compositor's kill prompt), and the tick needs roughly ten idle beats — about twenty minutes — to rewarm at =WARM_PER_BEAT= 8. + +Grading: Major severity (grading the being-in-it, per the don't-double-count-rarity rule: the cache is gone, the original freeze returns, and recovery is unattended and slow) × rare edge case (both configured sources — =~/videos/wallpaper= and =~/pictures/wallpaper= — are local directories, so this needs one deleted, renamed, or made unreadable while a beat fires; a removable or network source would hit it routinely) = P3 = [#C]. + +Fixed in this session: new =wallpaper.sources_available(sources)= tells "readable and empty" apart from "could not read", and =sweep_orphans= grew a =sources_ok= parameter that declines to sweep when it is False. Deferring a sweep costs only some stale files; sweeping wrongly costs the whole cache. +** DONE [#C] timezone-change sets a nonexistent zone for Portugal :bug:tooling:quick:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Fixed as dotfiles =15d2b63= (committed locally, deliberately NOT pushed — held for Craig's morning review). =Europe/Lisbon=. The suite also pins the general invariant: every zone the script can emit must exist in tzdata, so a future bad entry fails at test time rather than in Craig's hands. +Found in the 2026-07-24 sentry bug-hunt, validating every zone the script sets against =/usr/share/zoneinfo=. =common/.local/bin/timezone-change= line 39 maps =portugal= / =lisbon= to =Europe/Portugal=, which is not a tzdata identifier — the real one is =Europe/Lisbon= (a bare =Portugal= legacy alias also exists at the top level, but not under =Europe/=). =timedatectl set-timezone "Europe/Portugal"= fails, so the timezone is never changed. + +The other 17 zones the script sets all resolve correctly, so this is the single bad entry. + +Grading: Major severity (the option is wholly broken — the zone is not set and the command errors) × rare edge case (one option of eighteen, hit only when actually switching to Portugal) = P3 = [#C]. + +Fix: =Europe/Lisbon=. Verifiable by asserting the argument handed to a fake =timedatectl=, plus a suite-wide check that every zone the script names exists in the tzdata database. +** DONE [#C] settings-project stop() can SIGTERM an unrelated process :bug:settings:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 2, reviewing =settings/src/settings/project.py=. =stop()= read a pid out of =$XDG_RUNTIME_DIR/settings-project.pid= and SIGTERMed it with no check that the pid still belonged to the projection. A projection that dies without running =stop()= (crash, OOM, a failed =execvpe= on the clock path — that last one was already noted as tolerated residue) leaves the file behind, so once the kernel wraps its pid counter that pid can name something else entirely, and the next =start= or =stop= kills it. + +This is a hazard the codebase had already ruled on elsewhere and simply hadn't applied here: =maint/src/maint/doctor.py= revalidates =/proc//comm= against the expected name before its KILL remedy fires, explicitly to refuse recycled pids. + +Grading: Major severity (grading the being-in-it — an arbitrary user process takes a SIGTERM, and an editor with unsaved work is a plausible victim) × rare edge case (needs an unclean exit *and* pid reuse; =pid_max= here is 4194304, so wrap-around takes a very long time) = P3 = [#C]. + +Fixed as dotfiles =722994e= (committed locally, deliberately NOT pushed — held for Craig's morning review). The pidfile now records the process start time from =/proc//stat= next to the pid, and =stop()= fires only when the recorded value still matches the live process. Start time is the right token rather than =comm=: it is mode-independent (the clock channel execs into =python3=, so comm changes while comm-matching would have needed per-mode knowledge) and it is exec-stable, verified directly — pid and start time were identical either side of an =execvpe=. A recycled pid cannot reproduce it. Legacy bare-pid pidfiles keep the old unconditional behavior so the upgrade never strands a live projection. +** DONE [#C] wtimer alarms fire an hour off on the eve of a DST change :bug:timer:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 3, reading =timer/src/timer/engine.py=. =parse_alarm= resolves a bare wall-clock time ("07:00") to its next occurrence: it builds today's instant, and when that is already past it rolled forward with =epoch += 86400=. A DST day is 23 or 25 hours long, so a fixed 86400 lands on the wrong wall time whenever tomorrow crosses a transition. + +Reproduced against America/Chicago and the two 2026 US transitions. Asking for =07:00= at 08:00 on Sat 2026-03-07 (spring forward that Sunday) gave 08:00 Sunday — an hour late. Asking for =07:00= at 08:00 on Sat 2026-10-31 (fall back that Sunday) gave 06:00 Sunday — an hour early. + +The recurring path was never affected, which is what makes this an oversight rather than a design choice: =next_alarm= walks candidate days and rebuilds =datetime(y, m, d, hh, mm)= per day, so it is already DST-correct. Only the one-shot rollover took the shortcut. Both were pinned by the new tests. + +Grading: Major severity (grading the being-in-it — an alarm that fires an hour off has wholly failed at the one thing an alarm does, and the fall-back direction wakes you early while the spring-forward direction lets you oversleep) × rare edge case (two nights a year, and only when the requested wall time has already passed today) = P3 = [#C]. + +Fixed as dotfiles =9b6c2c9= (committed locally, deliberately NOT pushed — held for Craig's morning review). The rollover now rebuilds the local time on tomorrow's calendar date, the same construction =next_alarm= uses. Eight tests pin =TZ=America/Chicago= (saved and restored around each case), covering both transitions, the twelve-hour form, an ordinary-day control, a DST eve where the requested time is still ahead, and two characterization cases asserting the recurring path stays DST-safe. +** DONE [#B] net portal-restore claims encrypted DNS is back without checking :bug:net:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 3, reading =net/src/net/repair.py=. A captive-portal login moves the DNS-over-TLS drop-in aside so plain DNS can reach the venue's login page, and =_restore_dot()= moves it back afterwards. It fired both privileged steps — the =mv= and the =systemctl restart systemd-resolved= — and returned ="restored"= without reading either result. =repair_portal_restore()= then rendered a pass step reading "DNS-over-TLS restored". + +So a declined or failed =sudo -n mv= left DNS-over-TLS off while the tool told the user it was back on. The same for a resolved restart that fails: the drop-in is on disk but the running resolver is still serving plain DNS. + +The asymmetry is what makes it an oversight rather than a decision. The sibling =_disable_dot()=, twenty lines up, checks its own move with =_ok()= and returns False rather than claiming a success it did not get. The restore half simply never got the same treatment, and it is the half where the failure is silent — the disable path's failure is visible immediately because the portal page won't load. + +Grading: graded on severity alone under the privacy carve-out. DNS queries continue in cleartext to the venue resolver on an untrusted network, and the affirmative "restored" message is what removes the user's reason to check. Bounded by =net diagnose='s =encrypted-dns= step, which exists precisely to catch a portal run that never restored, so the exposure ends at the next diagnose rather than persisting unseen forever. Major severity = P2 = [#B]. + +Fixed as dotfiles =018c0c5= (committed locally, deliberately NOT pushed — held for Craig's morning review). Both privileged steps are now checked, with two new outcomes: ="failed"= when the move back fails (encrypted DNS still off, rendered as a fail step) and ="unapplied"= when the drop-in is back but resolved would not restart (rendered as a warn step). Each names the command to run by hand. Four tests cover both failures at the =_restore_dot()= and step levels, mirroring the existing declined-move test on the disable side. +** DONE [#B] the portal restore watcher fails silently, so DNS stays in the clear :bug:net:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 4, reading the rest of =net/src/net/repair.py= after the round-3 fix above. =portal_restore_watch()= polls until the link comes back online, calls =_restore_dot()=, and discards the outcome entirely. + +Three things compound into a silent failure. The watcher is spawned detached with =stdin=, =stdout=, and =stderr= all on =/dev/null=, so nothing it could print reaches anyone. It runs outside the =repair()= dispatch, so unlike every other mutating tier it never wrote an event-log line either. And =repair_portal_login= tells the user "encrypted DNS restores itself once you're online", which is precisely what removes their reason to check. A ="failed"=, ="unapplied"=, or ="ambiguous"= restore therefore left the machine on plain DNS on a venue network with no signal at any level. + +This is the round-3 finding one layer out, and the asymmetry is the tell: =018c0c5= taught =repair_portal_restore()= — the *manual fallback* — to stop claiming a success it did not get, while the *automatic* path, the one that actually runs in the normal flow, kept dropping the same result on the floor. Fixing the fallback and leaving the primary silent is a worse split than the original bug. + +Grading: graded on severity alone under the privacy carve-out, exactly as the round-3 sibling. Same exposure (cleartext DNS to an untrusted venue resolver), same bound (=net diagnose='s =encrypted-dns= step catches the stranded state), and the same affirmative promise removing the reason to look. Major severity = P2 = [#B]. + +Fixed as dotfiles =601c5b4= (committed locally, deliberately NOT pushed — held for Craig's morning review). The watcher now returns the outcome, appends a =portal-restore-watch= event with it, and fires a persistent =notify security= alert on each of the three failing outcomes, each naming the command to run by hand. A clean restore stays silent. Five tests: one per failing outcome, one pinning the silence on a clean restore, and one on the event-log line. The whole =TestPortalLogin= class now shadows =notify= with a logging fake, so no future watcher test can fire a real desktop notification mid-suite. +** TODO [#D] dns-override failure path says "reverted" without checking :bug:net:quick:solo: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 3, sweeping for siblings of the portal-restore finding above. =net/src/net/repair.py=, =repair_dns_override()= failure path: when the 1.1.1.1 override doesn't restore resolution, it calls =priv.run("dns-revert", iface)=, discards the result, and returns evidence reading "override didn't restore resolution — reverted". A failed revert leaves 1.1.1.1 set on the link while the step says it was removed. + +Same defect class as the portal-restore bug, three hundred lines up in the same file, and it survived the sweep only because the consequence is much smaller. Every other mutating repair in this file verifies by re-measuring afterwards rather than by reading an exit code, which is the stronger pattern and is why the sweep otherwise came back dry. + +Grading: Minor severity (a stale per-link override sends DNS to Cloudflare instead of the venue resolver, it dies on the next reconnect, and =net diagnose='s =dns-override-present= step exists specifically to catch it) × rare edge case (needs the override to fail *and* the revert to fail) = P4 = [#D]. + +Fix: the same idiom the portal-restore fix now uses. Wrap the revert in =_ok()= and drop the "— reverted" claim (or say the revert failed and name =resolvectl revert =) when it returns False. The existing =RepairHarness= makes the privileged call fail with =NET_SUDO="false"=, so the test is a near-copy of =test_restore_reports_failure_when_the_move_back_is_declined=. +** DONE [#B] a timezone-less Date header crashes the whole net diagnose run :bug:net:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 4, reading =net/src/net/diag.py=. =_clock_skew_s()= fetches the probe server's =Date= header with =curl -sI=, parses it with =parsedate_to_datetime=, and subtracts it from a timezone-aware =datetime.now(timezone.utc)=. RFC 5322 allows a =Date= to carry =-0000=, which means UTC while explicitly claiming no local zone, and a =Date= with no zone at all parses leniently as well. Both come back *naive*, and subtracting a naive datetime from an aware one raises =TypeError=. + +The =try= wraps only the =parsedate_to_datetime= call, so the =TypeError= from the line below it is uncaught. It escapes =_clock_skew_s=, escapes =_steps_egress_edges=, and takes down the entire =diagnose()= run — no report, no steps, a Python traceback. =net doctor= runs diagnose first, so the panel's doctor button dies with it. + +Verified against Python 3.14.6 before writing the fix: =parsedate_to_datetime("Thu, 01 Jan 2020 00:00:00 -0000")= returns =tzinfo=None=, and the subtraction raises. The zoneless form behaves the same. Only the =GMT= form (which the well-behaved probe host sends) comes back aware, which is why this never showed up in normal use. + +What makes it more than a curiosity is *when* the code runs. =_steps_egress_edges= fires only after the http-probe has already failed, so the server answering that =HEAD= is frequently a captive portal's interception appliance rather than the real probe host — and a minimal embedded HTTP stack is exactly the kind that emits a non-GMT =Date=. The one path guaranteed to be talking to a non-standard server is the one that can't survive a non-standard header. + +Grading: Major severity (grading the being-in-it — the diagnostic tool produces no report at all, and =net doctor= goes with it, on precisely the broken network it exists to diagnose) × rare edge case (needs a failing probe *and* a portal appliance that omits a numeric offset) = P2 = [#B]. + +Fixed as dotfiles =8933500= (committed locally, deliberately NOT pushed — held for Craig's morning review). A naive parse is now read as UTC, which is what =-0000= means. Two tests, and the second is the one that matters: it drives a *current* =-0000= timestamp and asserts no clock row, so a lazy "catch =TypeError= and return None" fix would fail it while the correct reading passes. +** DONE [#C] a tunnel import that can't be disarmed still reports success :bug:net:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 4, reading =net/src/net/manage.py=. =import_config()= imports a WireGuard or OpenVPN config as an NM profile, then fires =nmcli connection modify connection.id connection.autoconnect no= — and discarded the result, returning =ok=True= regardless. + +That modify is the whole safety of the feature, and the module's own docstring says so: =nmcli connection import= *auto-activates* the profile it creates, "which nobody asked for by picking a file", so "every import here ends with the profile deactivated and autoconnect off". A failed modify inverts that. For WireGuard — a device-type connection — autoconnect stays on, so the tunnel re-arms itself at the next boot and takes the default route with it, and the profile keeps the transient staged interface name (=wgpvpn=) while the envelope reports the config's real name, so the panel names a profile that isn't there. + +CORRECTION (2026-07-24, from an adversarial re-review): the blanket claim originally written here — that a failed disarm re-arms the tunnel at boot — is wrong for OpenVPN. =man 5 nm-settings-nmcli= states autoconnect is not implemented for VPN profiles, and an OpenVPN import is an NM VPN profile, so the modify is near-cosmetic on that half. The bug is real and security-relevant for WireGuard, which is the primary case; the severity as stated overreached to cover both. + +The asymmetry, again the tell: =_nmcli_import()=, twenty lines up in the same file, checks its own =returncode= and raises rather than return a UUID it did not get. The modify below it never got the same treatment. + +Grading: Major severity (grading the being-in-it — a full-tunnel VPN the user never asked to connect arms on every boot and carries all their egress, it persists across reboots rather than self-healing, and the affirmative "imported X" is what removes the reason to check) × rare edge case (needs the modify to fail after the import succeeded) = P3 = [#C]. + +Fixed as dotfiles =e0d4d8a= (committed locally, deliberately NOT pushed — held for Craig's morning review). New =_disarm()= returns whether the modify took. On failure the profile is still deactivated first — the import already brought it up, and the verdict shouldn't decide whether it keeps running — and then a =disarm-failed= envelope names the UUID and the exact command to finish the job. Three tests: the failing verdict, =import_configs= counting it as failed rather than imported, and a characterization test pinning that the deactivate still runs on the failure path. +** DONE [#C] a binary that can't be exec'd crashes the panels instead of degrading :bug:net:bluetooth:audio:maint:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 5, comparing the four panel packages' subprocess wrappers against each other. + +Every wrapper in the panels states the same contract: an unusable tool becomes a degraded result, never an exception. =cmd.run= returns None; =nmcli.run=, =btctl.run= and =pactl.run= raise their own domain error, which every caller already guards on; =speedtest.run_speedtest= returns an error envelope. All of them caught only =FileNotFoundError=, so they kept the contract for a tool that is *absent* and broke it for a tool that is *present but unusable*. + +Verified against Python 3.14.6 rather than argued. =subprocess.run= raises =PermissionError= for a file without its execute bit, =OSError= (ENOEXEC, "Exec format error") for an executable file that is neither a binary nor a script with a shebang, =NotADirectoryError= when a path component is a plain file, and =OSError= when a fork is refused under memory or PID pressure. None of the four is =FileNotFoundError=, so each escapes the guard: waybar's net/bt/audio modules die rather than dimming, and a maint probe takes the whole envelope with it — in exactly the machine state maint exists to report on. + +The asymmetry, and this codebase had already ruled on it three separate times: =net/iw.py='s =signal_dbm= and =settings/spawn.py='s =detached= both catch =(OSError, subprocess.TimeoutExpired)=, and =audio/cmd.py='s doctor-tier =probe()= enumerates =FileNotFoundError=, =NotADirectoryError= and =PermissionError= as "absent" under a docstring promising it never raises. Its sibling =run()=, twenty lines up in the same file, kept the narrow catch — as did all five copies of =run()= and all three tool wrappers. =audio/status.py='s docstring records that this same class already bit once ("the bar's audio module died rather than dimming"); that fix widened the guard's *scope* and left its *exception set* alone. + +Grading: Major severity (grading the being-in-it — the status surface is dead while the condition holds, and for maint the tool that reports the fault is the one that dies of it; no data loss, and it clears when the tool or the pressure does) × rare edge case (needs a binary with wrong permissions, a lost shebang, or a fork refused under pressure) = P3 = [#C]. + +Fixed as dotfiles =44fdae1= (committed locally, deliberately NOT pushed — held for Craig's morning review). Widened to =OSError= across net, bt, audio, maint and panelkit — five =cmd.run= helpers, the three tool wrappers, =probe._curl= and =speedtest.run_speedtest=. The domain-error wrappers keep their " not found" message for a genuinely absent binary and add a second arm naming the errno for an unusable one, so the report can still tell the two apart. 28 tests, one class per package, driving all three exec failures against real files on a temp PATH; each was watched failing against unmodified production code first (27 red). Audio's class carries a characterization case pinning =cmd.probe='s existing behavior, so the sibling that got this right can't regress into the one that didn't. +** DONE [#C] a failed pty-backed spawn strands both ends of the pty :bug:net:bluetooth:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 6, auditing the =subprocess.Popen= sites the round-5 fix didn't reach. + +Two spawns open a pty before launching and catch only =FileNotFoundError= around the =Popen=: =bt/pairing.py='s =pair_interactive= (bluetoothctl under a pty so the passkey agent is interactive) and =net/speedtest.py='s =run_speedtest_stream= (speedtest-go under a pty because it buffers everything to exit when piped). Both are the same exec-failure class as =44fdae1= — a binary present but not executable raises =PermissionError=, a lost shebang raises =OSError= — and neither is =FileNotFoundError=. + +What makes these worse than the =run= wrappers is where the cleanup lives. =os.close(master)= and =os.close(slave)= sit *inside* the =FileNotFoundError= arm, so an escaping =OSError= skips them: every failed attempt strands two descriptors. Both call sites are buttons in a long-lived panel process — the pairing flow and the console's SPEED key — and a user who gets no feedback presses again, so the leak accumulates under exactly the conditions that caused it. + +Grading: Major severity (grading the being-in-it — a descriptor leak in a process meant to run for days, on a path the user retries, plus the exception escaping a documented "(ok, detail)" / error-envelope contract) × rare edge case (needs an unusable bluetoothctl or speedtest-go) = P3 = [#C]. + +Fixed as dotfiles =c2eb3e1= (committed locally, deliberately NOT pushed — held for Craig's morning review). An =OSError= arm on each closes both ends and returns the module's own failure shape, naming the errno. Four tests: two pin the return contract, two count =/proc/self/fd= across three attempts — the fd count is what actually fails against unmodified code, and it was watched failing before the fix. + +The wider sweep this came from is recorded so it isn't repeated: every =except FileNotFoundError= in production was enumerated. The other exec sites were already correct (=maint/gui.py= x3, =net/kick.py=, =timer/engine.py= x2, =timer/gui.py=, =net/repair.py= x2, =audio/peak.py= all catch =OSError=), and the remaining hits are file-open catches, not exec. =clock/__main__.py='s =toggle()= has no guard at all but spawns =sys.executable=, which is by definition runnable; not filed. +** DONE [#C] one impatient client kills the clock panel's toggle listener for good :bug:clock:waybar:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 6, sweeping every acquired resource (pty, socket, mkstemp, tempdir) for cleanup that isn't in a =finally=. + +=clock/src/clock/app.py='s =_listen()= guards =accept()= with =except OSError: return= and leaves the request body — =recv=, =runtime_log=, =sendall= — outside any guard. =send_toggle()= in =__main__.py= gives the panel 0.25s to acknowledge, then closes. An ack later than that hits a dead peer and raises =BrokenPipeError=, which escapes the =while= loop and ends the listener thread. + +Verified empirically, not argued: a client that connects, sends, and gives up after 250ms makes the server's =sendall= raise =BrokenPipeError= (errno 32) and the listener thread exits. + +What makes it Major rather than a nuisance is that it neither self-heals nor announces itself. The socket file stays bound, so every later =clock toggle= still *connects* — then stalls the full 250ms, gets no reply, and falls through to spawning =clock serve=. GTK's single-instance forwarding turns that into =do_activate= on the running service, and =do_activate= calls =show_clock()=, not =toggle()=. So from the first bad client onward, clicking the waybar time module opens the panel every time and never closes it; the only ways out are the right-click dismiss inside the panel or restarting the service. Nothing logs it. + +Grading: Major severity (grading the being-in-it — the toggle is one-way from then on, it persists for the life of the service, and there is no signal it happened) × rare edge case (needs a reply to miss the 250ms budget: a busy main loop mid-redraw, a slow runtime-log write, or an interrupted =clock toggle=) = P3 = [#C]. + +Fixed as dotfiles =7c02614= (committed locally, deliberately NOT pushed — held for Craig's morning review). An =OSError= arm around the request body scopes a dead peer to its own request, mirroring the guard =accept()= already had. =GLib.idle_add= runs before the ack, so the user's click still takes effect — only the acknowledgement is lost. New =tests/clock/test_socket.py=, 3 tests driving the real =_listen= against a stand-in owner (it touches only =self._socket= and =self.toggle=, so no Gtk.Application is needed). The gate is the second toggle after an impatient first: it times out on unmodified code because no listener is left. The other two pin what the fix must preserve — the toggle fires even when the ack can't be delivered, and an unknown command is still answered without toggling. + +Left alone deliberately: =do_activate= calling =show_clock()= rather than =toggle()=. Changing it would alter what a cold =clock toggle= does on first launch, which is a design call for Craig rather than part of this defect. Worth raising if he ever wants the spawn path to toggle too. +** DONE [#B] fuzzel breaks the pinentry protocol loop on every passphrase :bug:security:gpg:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 7 — from the live journal rather than from reading. Grepping this boot for tracebacks turned up four instances of =pinentry-fuzzel: line 36: read: 0: read error: Resource temporarily unavailable=, and every one sits 4-7 seconds after a =GETPIN= (the time it takes to type a passphrase). The =BYE= handler's log line never appears once. + +=hyprland/.local/bin/pinentry-fuzzel= speaks the Assuan pinentry protocol on a pipe gpg-agent keeps open, reading one command per iteration of =while read cmd rest=. The =GETPIN= arm shells out to fuzzel, which *inherits that pipe as its stdin*. fuzzel runs an event loop over its own input, so it sets =O_NONBLOCK= on fd 0 — and =--dmenu= would read the pipe as menu items besides. The flag lands on the shared open file description and outlives fuzzel, so the shell's next =read= fails with =EAGAIN= and the loop ends mid-protocol. + +Grading: Minor severity (the passphrase is delivered *before* the break, so decrypts still succeed and nothing is corrupted — what's lost is everything after: =BYE= is never acknowledged, and gpg-agent's same-connection retry after a wrong passphrase, =SETERROR= then =GETPIN= again, can't be served; that retry is what the script's "reenter" label exists for, and it has never once been reachable) × every user, every time (four for four in the journal, and the test reproduces it deterministically) = P2 = [#B]. + +Fixed as dotfiles =e727dcd= (committed locally, deliberately NOT pushed — held for Craig's morning review). =< /dev/null= on the fuzzel call, so the non-blocking flag lands somewhere harmless; =--lines 0= was already there, so no menu input was ever wanted. =ENABLE_LOGGING= became env-overridable as a test seam — the script logs through an absolute =/usr/bin/logger= that PATH can't shadow, so without it every test run would write ten lines into the real journal. + +New =tests/pinentry-fuzzel/=, 8 tests driving the real script over a live pipe the way gpg-agent does. The fake fuzzel sets =O_NONBLOCK= on whatever fd 0 it is handed, exactly as the real one does, which is what makes them a gate rather than a restatement of the fix. Four fail against unmodified code — one reproducing the journal's message verbatim — and one records the fd fuzzel was given, pinning the cause rather than the symptom. + +THE CALIBRATION NOTE, and it is about my own earlier sweep. This is the same shape as round 1's =a57c443= (ffmpeg draining the pipe a =while read= loop was consuming). Round 1 swept both repos for siblings of that bug and came back empty — because it searched for the *mechanism* (a child that drains stdin) rather than the *shape* (a child that inherits stdin at all inside a read loop). Two different mechanisms, one shape, and the narrower search missed a live daily-use instance. Scope a class sweep by shape, not by the mechanism of the first instance found. +** DONE [#C] a truncated webcam record strands every camera off :bug:settings:privacy:solo: +CLOSED: [2026-07-24 Fri] +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 8, sweeping production for non-atomic file writes. + +=settings/src/settings/webcam.py='s =_record()= wrote =~/.local/state/settings/webcam.json= with a plain truncate-in-place =open(path, "w")=. That record is the only route back on, and the module docstring says so: deauthorizing a camera removes its video4linux nodes, so =usb_devices()= returns nothing afterward and =_recorded()= becomes the sole source of the paths to re-authorize. A write that truncated and then failed left an empty file; =_recorded()= caught the resulting =JSONDecodeError= and returned =[]=; =_known_devices()= then had nothing; and =set_power(True)= returned None without re-authorizing anything. Every camera stranded off, with no way back through the panel until a replug or a reboot. + +The asymmetry, seventh instance of this read: six other state writers in the tree already write through a temp file and a rename — =maint/cache=, =net/cache=, =audio/ptt=, =timer/engine=, =settings/store=, =maint/curation=. The one whose loss is most expensive was the one that didn't. + +Grading: Major severity (grading the being-in-it — the privacy switch becomes one-way, the panel offers no route back, and the user has to know to replug the camera or write sysfs by hand; bounded by the fact that a reboot re-enumerates USB and restores authorized=1) × rare edge case (needs a crash or ENOSPC inside a microsecond-wide write window) = P3 = [#C]. + +Fixed as dotfiles =8b40b79= (committed locally, deliberately NOT pushed — held for Craig's morning review). =_record= now mirrors =store.save=: =mkstemp= in the target directory, write, =os.replace=, unlink the temp on any failure. Four tests; the gate is a =_record= whose =json.dump= raises, after which the previous record must still be readable — it isn't on the old code. The other three pin what the fix must preserve: no temp-file residue, the =_recorded()= round trip, and the end-to-end power-off/power-on with the class symlinks removed, which is the scenario the record exists for. + +HOW IT WAS FOUND, and it confirms round 7's lesson twice over. Round 4 ran an atomic-write sweep and reported "nine sites, six unique-per-writer, three sharing a fixed =.tmp=" — it enumerated the writers that *were* atomic and compared their temp-file naming, and never asked which state writers aren't atomic at all. Same narrowing that made round 1's stdin sweep miss the pinentry bug: the sweep was scoped to a property of the instances already found rather than to the shape of the hazard. +** VERIFY Should coredump entries group as one journal-digest row per binary? :maint: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Noticed in the 2026-07-24 sentry bug-hunt, round 8, while verifying the =journal_errors= probe against the live journal. Not filed as a bug — it needs your call on what "the same error" means here. + +The probe's counting is correct, and I checked it rather than assumed: it reports 1674 real + 16 noise = 1690, and =journalctl -p 3 -b -o json | wc -l= returns exactly 1690 entries this boot. (A =wc -l= on =-o cat= says 23037, but that splits multi-line messages like coredump stack traces across lines — the probe counts entries, which is right.) + +What's off is the *grouping* for multi-line messages. Thirteen =systemd-coredump= entries on ratio right now — nine usbredirect, three telega-server, one python3 — land as four separate digest rows (counts 4, 3, 3, 3) instead of one row per binary. =_signature()= blanks hex addresses and long integer runs, which handles the pid, but two dumps of the same binary still differ in frame count and thread layout, so their signatures diverge. + +Consequence is modest: the digest's top-N rows get eaten by near-duplicates, so genuinely distinct errors fall off the evidence list sooner. It doesn't affect the metric's value or severity. + +The question is what you'd want: group coredumps by the binary named in the first line (a special case for =systemd-coredump=), signature only the *first line* of any multi-line message (a general rule, and arguably the right one — the first line is the error, the rest is context), or leave it alone. The middle option is the smallest general change and I'd lean that way, but it changes grouping for every multi-line error, so it's yours to call. +** TODO [#D] a failed wallpaper apply reports "nothing to apply" :bug:settings:quick:solo: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 7, sweeping the settings panel's worker callbacks. + +=settings/gui.py='s =_async= passes an exception through as the *result* rather than as a separate error argument, so every =done= callback has to test =isinstance(res, Exception)=. Five do — =_mx_pin=, =_mx_letter=, =_after_matrix=, =_set_pointer=, the drum/dial/gallery/refresh callbacks. =_wp_apply= is the one that doesn't: + +#+begin_src python +def _wp_apply(self, note="Wallpaper set"): + self._async(lambda: panel.wallpaper_apply(self.state), + lambda ok: self._toast( + note if ok is True else "nothing to apply", + good=ok is True)) +#+end_src + +=panel.wallpaper_apply= calls =store.save=, which can raise =OSError= (disk full, a permissions change on the config dir). The exception then arrives as =ok=, =ok is True= is False, and the toast reads "nothing to apply" — describing a no-op when the apply actually failed. The toast is at least marked =good=False= (red), so the user gets a negative signal; what's lost is the reason, which every sibling callback surfaces via =str(res)=. + +Grading: Minor severity (wrong text on an error path, correctly marked as a failure, nothing corrupted) × rare edge case (needs =store.save= or =wallpaper.apply= to raise rather than return False) = P4 = [#D]. + +Fix: give it the same =isinstance(res, Exception)= arm its five siblings have — toast =str(res)= on an exception, keep the current two-way message otherwise. One callback, three lines. +** TODO [#D] two manage.py nmcli reads sit outside their own error conversion :bug:net:quick:solo: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 4, reading =net/src/net/manage.py=. =nmcli.run()= raises =NmcliTimeout= on timeout and =NmcliError= on a missing binary, and every mutation in this module is written to convert both into a result envelope. Two calls escape that conversion because they run through =_key_mgmt()=, which wraps =nmcli.get_value= and catches nothing: + +- =edit()= line 243 calls =_key_mgmt(uuid)= for the enterprise-profile refusal *before* its own =try=, while the next four lines catch exactly those two exceptions around =nmcli.run=. +- =_classify_up_failure()= calls it on =up()='s failure path, so a slow =connection show= turns a classifiable activation failure into an exception. + +Consequence is a leaked exception where the caller expected an envelope. The panel absorbs it — =gui.bg()= catches =Exception= and renders =str(e)= — so there it degrades to a worse message rather than a crash. =net edit= from the CLI has no such catch and prints a traceback. + +Grading: Minor severity (the operation fails either way; what's lost is the classified message, and only the CLI path shows a traceback) × rare edge case (=connection show= has a 2s timeout and nmcli's presence is already established by the time either site runs) = P4 = [#D]. + +Fix: give =_key_mgmt= the same conversion its callers use — catch =(nmcli.NmcliError, nmcli.NmcliTimeout)= and return "", which both call sites already handle correctly (neither "wpa-eap" nor "sae"). One =try= in one helper covers both sites. +** TODO [#D] three atomic writers share one fixed .tmp name :bug:quick:solo: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt, round 4, sweeping both repos for the temp-file half of the atomic-write idiom. The tree writes state atomically in nine places, and six of them make the temp path unique per writer: =net/cache.py= and =timer/engine.py= both use =f"{path}.tmp.{os.getpid()}"=, and =settings/store.py=, =settings/idle.py=, =bt/repair.py=, =net/probe.py= all use =tempfile.mkstemp=/=NamedTemporaryFile=. Three use a bare =path + ".tmp"=: + +- =audio/src/audio/ptt.py= =write_state= (the lead carried over from round 3's Next Steps) +- =maint/src/maint/cache.py= =put= +- =maint/src/maint/curation.py= =_write_user= + +=os.replace= makes the *rename* atomic, but a shared temp name is not: two writers open the same path, the second truncates under the first, and the file that gets renamed into place is a blend of both. The loser's own =os.replace= then raises =FileNotFoundError=, because the winner already renamed the name out from under it. + +Real concurrent-writer pairs exist for two of the three. =maint/cache.py= =updates_repo= is written by =maint-net-scan.timer= hourly and again by =doctor._fresh_pending()= at UPDATE fire time. =audio/ptt.py= has three writers by design (the CLI toggle bound to a key, the waybar right-click, and the GTK panel) — its module docstring says so. =curation.py= is written by panel key presses and CLI verbs. + +Grading: Minor severity (every reader degrades rather than crashes — =cache.get= catches =ValueError= and reports no data, =read_state= reads a torn file as disarmed, and both recover on the next write; the sharpest edge is the loser's =FileNotFoundError= aborting the rest of =scan_net=, which the next hourly run repairs) × rare edge case (the write window is a millisecond or two, and the overlapping writers are an hourly timer against a human keypress) = P4 = [#D]. + +Fix: give all three the =f"{path}.tmp.{os.getpid()}"= form the two careful siblings already use. It is three one-line changes and needs no new abstraction. Note this closes the torn-file half only — the read-modify-write in =ptt.toggle_plan= and =curation.set_preference= can still lose an update between two writers, which wants a lock rather than a temp-name change and should stay a separate decision. +** TODO [#C] dmenuexitmenu word-splits its menu so no entry matches :bug:dwm:solo: +:PROPERTIES: +:LAST_REVIEWED: 2026-07-24 +:END: +Found in the 2026-07-24 sentry bug-hunt (shellcheck SC2128). =dwm/.local/bin/dmenuexitmenu= line 4 expands the menu unquoted: =choice=$(echo -e $menuitems | dmenu ...)=. Word-splitting collapses the runs of spaces the labels carry, so dmenu shows =Lock= where the =case= arm expects =Lock = (two spaces) and =Logout 󰩈= where the arm expects a trailing space. No arm matches, so choosing an entry does nothing at all. + +Grading: Major severity (every menu action is inert) × rare edge case (dwm is not stowed on this machine — =~/.local/bin/dmenuexitmenu= does not exist, so the script is currently dead code) = P3 = [#C]. + +Fix: quote the expansion (=echo -e "$menuitems"=) and index the array element explicitly. Left unfixed for now because dwm is inactive; raise to [#B] if the dwm tier is ever restowed. +** VERIFY [#B] Two agent sessions sharing one git repo :chore:tooling: +SCHEDULED: <2026-07-27 Mon> +Twice on 2026-07-22/23 a session swept the other's uncommitted work into its commit while both worked the dotfiles tree. The dotfiles session pathspec'd around mine once; my 2bd7d56 then carried their settings-panel window-rule revert (100%-854 → 100%-584) under a message describing only hypridle. Nothing needed undoing — the change was wanted — but the commit misdescribes itself, and the next collision may not be benign. + +Root of it: =git commit -- = commits the *working tree* state of that path, so a pathspec guards against other files but not against another session's edits to the same file. The shared =.git/index= is the deeper half — one session's =git add= disturbs the other's staging even when their files don't overlap. + +Worktrees are the standard answer but fight this repo: stow symlinks point at =~/.dotfiles=, so only that clone is live and a worktree edit isn't real until it lands back there. A file-ownership split needs renegotiating whenever work crosses a boundary, which is most interesting work. + +The unintuitive part, worth stating plainly because both sessions built habits on the wrong model: a pathspec commit reads the *working tree*, not the index. The dotfiles session's pathspec habit carried the same exposure the whole time and only held because I never edited a file on one of their lists. Neither side had a mechanism, just different luck. + +*Options, after their skeptical review (their read is better than my first two):* + +1. =GIT_INDEX_FILE= per session — real but heavier than it looks. It only helps if both sessions also stop pathspec-committing (otherwise the working tree still wins, so it isn't coordination-free), it needs seeding from =.git/index= or git sees every tracked file as new, and it confuses hooks and anything reading default-index state. +2. "Read the staged hunks, not the stat" — *already on the books*. The publish flow requires =/review-code --staged= before a commit, and that reviews the diff. So this is an existing rule that didn't run, not a missing rule. I skipped it on both speedrun commits (2bd7d56, f9b6404) after Craig waived approvals — waiving the approval gate is not the same as waiving the review, and I collapsed the two. The question worth answering is why it was skipped, not what new habit to add. + + *Both sessions agree this is the finding that matters more than the lock shape*, and that the durable version should state it in one line: *an approval waiver never waives the review, because the review is the gate that reads hunks.* Two gates that both stand between an edit and a commit collapse into one the moment either is waived. +3. *Their proposal, and the best of the three:* a =dotfiles-commit= lock via the existing =.ai/scripts/agent-lock= (already used for sentry's runner and roam-write), held across stage → review → commit. It serializes the only window where sweeping happens, needs no change to how either session commits, and self-clears on a crashed holder. It doesn't stop a concurrent edit mid-window, but that window is seconds rather than minutes. + +Craig's call: whether this becomes a durable rule in the shared rules layer (it's cross-project — any repo two sessions touch), which shape, and which session implements it. Both sessions have offered to own it. + +** TODO [#C] Timer module hero hierarchy :feature:waybar:timer:quick:solo: From the roam inbox (Craig, claimed 2026-07-22). Which display ("hero") wins the waybar timer module when several timer modes run simultaneously: pomodoro wins over everything (the user is actively working; it's likely their main focus). The rest rank in chronological order of when they would ring. Worked example: with a just-started 15-min timer, a 1-hr timer at 10 minutes left, a pomodoro, and an alarm ringing in 12 minutes — show the pomodoro; when it completes, the 1-hr timer (rings first), then the alarm, then the 15-min timer. Feeds the timer-panel spec (docs/specs/2026-07-02-timer-panel-spec.org). -** TODO [#C] Timer module: drop RING message, persistent notifications :bug:waybar:timer: +** TODO [#C] Timer module: drop RING message, persistent notifications :bug:waybar:timer:quick:solo: From the roam inbox (Craig, claimed 2026-07-22). Remove the RING message from the timer module display; verify all timer and alarm notifications are persistent; the icon returns to normal once the notification has fired. Rationale: keeps timers and pomodoros from interfering with one another's displays (pairs with the hero-hierarchy task above). -** TODO [#C] PTT icon outline removal :bug:waybar: +** TODO [#C] PTT icon outline removal :bug:waybar:quick:solo: From the roam inbox (Craig, claimed 2026-07-22): the waybar PTT icon should not have an outline. Cosmetic × every-glance = P3 = [#C]. -** TODO [#B] Weather tooltip caching :feature:waybar:weather: +** TODO [#B] Weather tooltip caching :feature:waybar:weather:solo: From the roam inbox (Craig, claimed 2026-07-22): retrieve the weather tooltip data once per hour and cache it. If the network is unavailable, display the cached tooltip with explanatory text saying so. Dotfiles-side work (archsetup owns the lifecycle); touches common/.local/bin/weather. -** TODO [#C] WiFi tooltip signal strength :feature:waybar:network: +** DONE [#B] Dotfiles tests leak state across files :bug:test:dotfiles:solo: +CLOSED: [2026-07-23 Thu] +Resolved 2026-07-23 as dotfiles =c333598=. The polluter was =tests/weather/test_weather.py=, and it accounted for all 38 failures on its own. + +The mechanism was not the env leak the body below guessed at — tests/weather never writes =os.environ=. Its whereami fake did =weather.subprocess.run = ...= on a freshly-loaded module object. The fresh module isolated the weather code, but =weather.subprocess= is the one shared stdlib module object every module in the process holds, so the assignment replaced =subprocess.run= process-wide and never restored it. Every later test file got weather's fake result back from =subprocess.run=; the tell was wtimer asserting on =r.returncode= and getting "'R' object has no attribute 'returncode'", where =R= is weather's fake result class. + +Triage: TEST HYGIENE, not production global state. The weather script reads env at import and never writes, so no long-lived-process caching defect sits behind it. A scan for the same pattern (patching a stdlib module attribute reached through another module's namespace) finds exactly one instance in the suite — the three other =setattr= sites all snapshot and restore. So the planned shared env helper across 28 files was aimed at the wrong target and wasn't needed. + +Fix: rebind the loaded module's own =subprocess= name to a stub namespace, so nothing outside that module changes and there is nothing to restore. + +Gate: =make test= now runs two gates per the add-don't-replace decision — =test-forked= (one process per file, catches order dependence) and the new =test-shared= (every suite in one process, catches leakage). Built on stdlib unittest rather than pytest, since pytest was only the diagnostic tool and isn't a project dependency. Verified as a real gate, not just green today: with the defect deliberately reintroduced it goes red, and green once restored. A focused test in tests/weather pins the invariant on the culprit as well, because the shared gate alone blames the three victim files. + +Verification: 3500 tests, both gates, exit 0. + +Original finding follows. + +Found 2026-07-23 during the speedrun. =make test= is green, but it runs each test file in its own =python3 -m unittest= process, which hides cross-file state leakage. A single-process whole-tree run (=python3 -m pytest tests/ -p no:randomly=) fails 38: 22 in =tests/wtimer/test_wtimer.py=, 10 in =tests/zoom-web/test_zoom_web.py=, 6 in =tests/wlogout-menu/test_wlogout_menu.py=. + +Not a regression — a worktree at the pre-speedrun commit produces the identical 22/10/6 profile, so this predates tonight's work. Those three files also pass cleanly when run together (170 passed), so the polluter is a fourth file somewhere in the tree that mutates global state (env var, cwd, or a module-level patch) without restoring it. 28 test files write =os.environ= directly. + +Why it matters: the green gate can't see this class of bug, so a real isolation defect — or a genuine failure that only appears under a different order — passes CI silently. Bisect by running the tree with subsets until the polluter is identified (pytest's =-p no:randomly= keeps the order stable while bisecting), fix its cleanup, then decide whether =make test= should gain a single-process pass so the gate covers it. + +** TODO [#B] Settings gear becomes four device toggles :feature:waybar:dotfiles:solo: +From the roam inbox (Craig, claimed 2026-07-23): the waybar gear should become four icons — touchpad, mouse, webcam, and a notification bubble. Clicking each toggles that setting directly. The first three turn red when disabled; the bubble turns red when DND is enabled. + +Today =custom/settings= (=hyprland/.config/waybar/config=) is one gear glyph (󰒓) whose only job is =on-click: settings-panel=. The toggles themselves already exist and are tested — the settings package owns touchpad, mouse, and webcam (=webcam.py= is the USB-authorized kill switch from 2026-07-22), so this is a bar-side surface over existing backends rather than new capability. + +Note the state-polarity split when wiring the colors: three read "red = off" and DND reads "red = on". That asymmetry is deliberate (red means "something is disabled that normally isn't, or suppressed that normally isn't"), so encode it per-icon rather than deriving one rule. + +Decided 2026-07-23 (Craig): the gear STAYS alongside the four toggles as the panel launcher. So the bar's right side grows from 12 modules to 16 — the four toggles are net-new, the gear keeps its =on-click: settings-panel=. Open sub-question for build time, not blocking: whether the four toggles are four separate waybar modules or one custom module rendering four glyphs (fewer layout entries, one exec). Pick at build; the four-module shape is simplest and matches how mic/net already sit as individual modules. + +** DONE [#B] Wallpaper view freezes the panel — thumbnail decode :bug:dotfiles:solo: +CLOSED: [2026-07-23 Thu] +Craig reported 2026-07-23: selecting the wallpaper button freezes the module and the compositor asks whether to kill it. Root cause proven: =_Thumb._draw= decoded each source image with =new_from_file_at_scale= on the GTK main thread. Measured on Craig's 78 wallpapers — a viewport of the 8 largest takes 3.7s, the whole set 13s. That block trips Hyprland's "not responding" watchdog. + +Grading: Critical severity (panel unusable, watchdog kill) × every user every time the wallpaper view opens = P1 = [#A] by the matrix. Held at [#B] because step 1 already shipped and removes the user-visible freeze; the remainder is a latency enhancement, not a showstopper. + +*** 2026-07-23 Thu @ 15:40 Step 1 — async decode (dotfiles f45f321) +Moved the decode to a worker thread via a new =settings/thumbcache.py= (pure, injected decode/scheduler/thread; 6 tests). The thumb shows its dark ground until the pixbuf lands, then redraws. Verified live on a headless output: worst main-loop stall opening the pair view dropped from multi-second to 68ms; the cache filled with 81 decoded pixbufs (the one miss is a .webm, correctly falling back to the ▶ glyph). Full suite 3512, both gates, smoke OK. This alone fixes the reported freeze. + +*** 2026-07-23 Thu @ 16:30 Step 2 — persistent on-disk cache (dotfiles 463cc4f) +Built the persistent layer: =settings/thumbstore.py= decodes each source once to a 512px PNG under =~/.cache/settings/thumbs=, keyed by path + mtime so an edited wallpaper self-invalidates. The hot-path decode reads that PNG and scales in-memory. Warming rides the existing =settings tick= CLI verb (the 2-min timer already runs it), building up to =WARM_PER_BEAT=8= missing thumbnails per beat — best-effort, journals a line on failure, never blocks the wallpaper flip. thumbstore is pure (stat/decode/load/save injected); 10 tests. + +Went with incremental warming (8/beat, ~10 beats to full) as the safe default rather than full-warm-on-change — the per-beat cap is a one-line flip if Craig wants it faster. Measured: hot-path decode of a viewport dropped from 3.7s cold to 47ms warm. No installer change (the tick service already runs =settings tick=); cache lives outside the repo. Full suite 3522, both gates, smoke OK, live panel verified (81 pixbufs render, 48ms worst stall warm). + +** DONE [#C] Panel scrollbars too short :bug:dotfiles:quick:solo: +CLOSED: [2026-07-23 Thu] +Shipped 2026-07-23 as dotfiles =0d64837= (22px scrollbar, 16px trough, 14px slider thickness with a 48px floor along the travel axis). Left open by oversight during the speedrun; closing now. + +Follow-on, and my own regression: enlarging the bar to 22px is what made it start covering the thumbnails, because nothing grew the tray to match. Craig reported it the same day ("scrollbars that obscure the images") and it's fixed in =c0ddf57= — the tray now reserves a 22px lane for the bar as a margin on the scrolled box, so the bar sits below the images instead of across them. Measured before: tray 68px, content 68px, a visible 14px bar inside the same 68px. After: tray 90, content 68, bar clear. The lane is a constant under the scrollbar CSS with a note to keep the two in step, since the coupling between bar thickness and tray height is exactly what broke. + +From the roam inbox (Craig, claimed 2026-07-23): all scrollbars need to be much taller than before. The always-visible scrollbars shipped in 7e8eb4a set =min-height: 10px; min-width: 10px= on the slider (=settings/src/settings/gui.py=, the =.dupre-panel scrollbar slider= rule) — that's the floor for a short slider, and the trough itself is thin. Raise both the slider floor and the trough thickness so the bar is comfortably grabbable. Cosmetic × every glance at the wallpaper trays = P3 = [#C]. +** TODO [#C] Video wallpapers don't fit the desktop :bug:dotfiles:solo: +From the roam inbox (Craig, claimed 2026-07-23): videos don't fit the desktop in desktop-settings. The video channel drives mpvpaper (=settings/src/settings/wallpaper.py=); mpvpaper passes options through to mpv, so the fit is a =--panscan=/=--video-unscaled=/keepaspect question rather than a layout one. Reproduce with a video whose aspect differs from the output, pick the mode that fills without distorting (cover, matching how the image channels behave), and cover it in the wallpaper tests. Minor severity × whenever the video channel is selected = P3 = [#C]. +** DONE [#C] World-clock wallpaper arrangement :feature:dotfiles: +CLOSED: [2026-07-24 Fri] +Shipped 2026-07-24 as dotfiles =6afbe09=, iterated live with Craig. The grid of boxed mini-clocks became a centered vertical clock line: cities down a spine, west (Honolulu) top to east (Wellington) bottom, labels alternating both sides, no boxes. Each shows city / time (12h) / day+date / timezone region name ("US Central"). Day/night dimming + amber home carried over, title dropped, cursor restored over the desktop. Prototypes archived in archsetup 40216e7. The face is parameterized (=?layout=vertical|horizontal=, =?hour12=1|0=) so the panel pickers below can drive it. + +** TODO [#B] World face: orientation + hour-format pickers in the settings panel :feature:dotfiles: +The world face (=settings/faces/world.html=, shipped 6afbe09) already supports both orientations and 12/24-hour via =?layout= and =?hour12= query params, defaulting vertical/12h. What's missing is letting Craig CHOOSE them from the desktop-settings panel. Build: (1) two new fields in the wallpaper state (world_layout, world_hour12) with the vertical/12h defaults; (2) =project.py build_uri("world")= appends =&layout=&hour12== read from state; (3) panel controls in the world channel's config section (=gui.py _conf_projected=, currently just a preview) — an orientation toggle and a 12/24 toggle; (4) TDD the URI-building and state round-trip. Solo — buildable, agent-verifiable (URI + state tests, headless render), no open design question (the two faces already exist and are approved). Requested by Craig 2026-07-23; deferred so the vertical face could ship first. +** TODO [#C] Wallpaper channel: timed transitions as an alternative to sunrise/sunset :feature:dotfiles: +From the roam inbox (Craig, claimed 2026-07-23): the wallpaper channel switches on sunrise/sunset today (the sun-pair mode, =settings/src/settings/wallpaper.py=, location read live via whereami with a state.json cache). Add a timed-schedule mode as an alternative: fixed clock times drive the transitions rather than the solar calc. + +Not :solo: — the capture itself flags the missing inputs ("we'll need to know the transition times, and how many of them there are"). The count and the times are a design decision Craig owes: is it a two-image day/night flip at fixed hours, an N-way ring across the day, per-image dwell vs shared interval? The =set= channel already does fixed-interval cycling through a set, so the new part is specifically clock-anchored transition points, not just "a timer". Ask for the schedule shape at pickup, then build against the existing wallpaper.apply presenter vocabulary. +** TODO [#C] Floating layout — should we? :feature:hyprland: +From the roam inbox (Craig, claimed 2026-07-23): consider whether Hyprland should offer a floating layout — how it would work, the benefits, and the complexity. A brainstorm/spike, not a build: the deliverable is an assessment Craig reads and decides on, not a shipped layout. Not :solo:. When picked up, run it as a brainstorm — how a floating mode coexists with the current tiling binds (toggle keybind, per-workspace vs global, window-rule interactions), what it buys over the existing =togglefloating=, and the config/muscle-memory cost — then bring Craig the recommendation. +** TODO [#B] Panel family: unify the look across net/bt/maint/audio and desktop-settings :feature:design:dotfiles: +From the roam inbox (Craig, claimed 2026-07-24): the network, bt, maint, and audio waybar panels look alike, but the desktop-settings panel looks quite different. He wants them to read as one family. Deliverable: enumerate every difference (chrome, header layout, typography, spacing, control styling, color roles, close-button placement, section dividers) between the two groups and a plan to converge them on one look. Not :solo: — it needs a design pass and Craig's taste calls on which direction each group moves. When picked up, catalogue the deltas from live captures of all five, propose the shared design language (likely the Dupre instrument-console the settings panel uses, since that's the newest and most deliberate), then bring Craig the change list before touching code. +** TODO [#C] Panel text cut off — needs a few px more space :bug:dotfiles:solo: +From the roam inbox (Craig, claimed 2026-07-24): panel labels look cut off; a few more pixels of space fixes it. He named the audio and bt panels, but his "before" capture is the networking panel (=~/pictures/screenshots/2026-07-23_202419.png=; "after" resizing =~/pictures/screenshots/2026-07-23_202458.png=), so the whole panel family likely shares the tight spacing. Confirm which panels clip at pickup, then add the padding/width. Grade: cosmetic × every glance at the affected panels = P3 = [#C]. Solo — buildable (CSS/size tweak) and screenshot-verifiable, no design call once the clipping panels are identified. +** DONE [#C] Velox refresh sweep :chore:maint: +CLOSED: [2026-07-23 Thu] +From the roam inbox (Craig, claimed 2026-07-23): velox needs bringing up to date, the mouse/touchpad module is still there, investigate what else didn't move over. + +Resolved 2026-07-23 by a full sweep over tailscale. The touchpad module was already gone — velox's running waybar (started 01:05, after the reboot) and its tracked config both carry zero =custom/touchpad= entries; what Craig saw was the pre-restow waybar process from before the reboot, and the reboot cleared it. Sweep results: both machines at dotfiles f9b6404 (all three hyprland lock/exit fixes live on velox, config errors clean, =allow_session_lock_restore= reads true); stow restow clean, only the expected skip-worktree files; rulesets pulled to 50fc7ca and =make install= run (agent-text verified working by invoking it — an earlier "MISSING" reading was a PATH artifact of the non-interactive ssh shell, not a real gap); desktop-settings tick timer active; mpvpaper, power-profiles-daemon, gtk4-layer-shell, webkit2gtk all present. + +Genuine remaining differences, all per-machine installs rather than sync failures: =cmail-action=, =gcalcli=, and =playwright= aren't installed on velox, and =obsbot-wb-guard.service= isn't enabled there (the OBSBOT lives on ratio). None block anything; file separately if velox should send mail or drive browser tests. + +** DONE [#C] Weather tooltip sunrise and sunset :feature:waybar:weather:quick:solo: +CLOSED: [2026-07-23 Thu] +Shipped 2026-07-23 as dotfiles =de62e9d=. The two rows sit directly below Humidity in the current-conditions block, rendered in the footer's 12-hour format (=%-I:%M %p=) so the tooltip reads one way throughout. + +Confirmed the no-extra-round-trip premise held: =sunrise,sunset= joined the existing =&daily== block. Split =forecast_url= and =reading_from= out of =fetch= so both the request and the reading are testable without network — that's what let the new cases cover a payload missing the fields. Six tests (Normal/Boundary/Error): row placement and format, a pre-change cache with no sun fields, an unparseable stamp, today's pair picked out of the six-day arrays, and the API omitting them. Reused the existing =_at= helper rather than adding a near-duplicate =_first=. + +Live-verified against the real API: sunrise 6:14 AM, sunset 7:59 PM for today in New Orleans, rendering in the actual tooltip. Full suite 3506 tests, both gates, exit 0. + +Open, not blocking: every other header row carries a glyph (thermometer, droplet, wind arrow) and the sun rows are plain text. The file's glyphs are marked font-confirmed codepoints, and I haven't verified a sunrise/sunset glyph renders rather than showing tofu, so I left them bare. Craig's call. + +From the roam inbox (Craig, claimed 2026-07-23): in the weather module's hover text, the section immediately after the location ends with the current humidity. Add the sunrise and sunset times for the current location directly below it. + +Cheap to source: the module already calls Open-Meteo with a =&daily== block (=common/.local/bin/weather=, the forecast URL around line 336), so =sunrise,sunset= joins that same request with no extra round trip — normalise_daily already parses the daily arrays. Times arrive as local ISO strings; render in Craig's canonical clock format rather than re-deriving one. The settings package's =suntimes.py= (pure NOAA math, no network) stays the offline fallback path if the API field is ever absent — don't duplicate its math here. +** DONE [#C] Maint doctor-row copy button :refactor:maint:quick:solo: +CLOSED: [2026-07-23 Thu] +Shipped 2026-07-23 as dotfiles =761fa5c=, "fix(maint): drop the COPY key from the doctor row" — the key and its orphaned handler removed from =maint/src/maint/gui.py=. =viewmodel.status_copy_text= stays: it's a tested pure serializer and the obvious source if a copy surface returns somewhere better placed. + +Correction to the body below: it describes a per-row button and a separate global one. There is only one COPY key, and it IS the global one Craig added in 8bc79ba two days earlier. He tried it and wanted it gone, so the row now reads DOCTOR · CLEAN UP · REVIEW & FIX. + +From the roam inbox (Craig, claimed 2026-07-23): remove the per-doctor-row copy button (next to REVIEW and FIX) from the maint status wall. The global COPY key (dotfiles 8bc79ba, "one global button copying rendered text") stays the one copy surface — the per-row button turned out to be clutter next to it. +** TODO [#C] Net tooltip IPs and line order :feature:waybar:network:solo: +From the roam inbox (Craig, claimed 2026-07-23): in the wifi hover, add the internal IP, external IP, and gateway IP just below the Interface line; move the Signal line to just above the keyboard-shortcuts line. Design constraint: the bar's hot path does no network I/O (status.py deliberately skips _address_facts on the 2s beat) — internal IP + gateway can ride cheap local reads, but the external IP must come from a cache the connectivity probe refreshes, never a live lookup in waybar-net. +** DONE [#C] WiFi tooltip signal strength :feature:waybar:network: +CLOSED: [2026-07-22 Wed] From the roam inbox (Craig, claimed 2026-07-22): add signal strength to the WiFi tooltip. + +Resolved 2026-07-22: the tooltip's signal line existed but never fired on ratio — the mt7925 driver leaves /proc/net/wireless empty (legacy WEXT procfs unimplemented), so the dBm read returned None and the bar glyph fell to the weakest tier. Fix in dotfiles net/: an iw-dev-link nl80211 fallback (only spawns when procfs is empty), a signal_percent mapping, and an enriched line — Signal: ▂▄▆█ 100% · -32 dBm (excellent) — bars by band, percent, raw dBm, band word. The bar icon tier fixed itself as a side effect. ** TODO [#C] Night-watch live telemetry :feature:maint: Craig, 2026-07-21 ("mind. blown."): drive the Dupre Night Watch screensaver (docs/prototypes/2026-07-21-night-watch-screensaver-prototype-1.html, the idle-pipeline eye-candy stage in the desktop-settings spec) with real system data instead of synthetic signals — a passive status wall while the machine idles. Candidate mappings: scope = CPU load trace, drift chart = memory pressure history, spectrum = per-core utilization, VU pair = net throughput up/down, blinkenlights = disk I/O, tape counter = uptime, systems lamps / annunciator = maint status verdicts, engine-order telegraph = current power profile, VFD wire = maint status one-liner (temps, battery, pending updates). Browser prototype can poll a small local JSON endpoint; the production shape belongs to the idle-stage build. Depends on the spec's idle-pipeline implementation landing first. -** TODO [#B] Dupre Kit merge — casting additions :feature:tooling: +** TODO [#B] Dupre Kit merge — casting additions :feature:tooling:solo: Fold docs/prototypes/dupre-kit-additions.js back into the kit proper: detentFader (NEW — multi-detent slide attenuator with speedbump drag physics: magnet + escape hysteresis, parked tick glow) and the drumRoller redefinition (UPGRADE — 1..N channels and min/max range; stock hardcodes two drums and throws on one, defaults reproduce stock exactly) and the guardedToggle redefinition (UPGRADE — lever throws with rotateX so it flips toward the viewer instead of the stock 180° planar spin that sweeps sideways mid-transition; contract unchanged). Merge means: builders into widgets.js, the additions CSS into DUPRE_CSS, additions-scoped gradients into the shared defs plate, gallery cards for both in panel-widget-gallery.html, and POLICY entries. Origin: the desktop-settings casting sitting 2026-07-21 — Craig's direction is that components get finished by being needed ("the ones needed most will have had the most attention"), so more additions may accrue here before the merge; batch them. ** TODO [#C] Wlogout screen review :bug:hyprland:dotfiles: Craig, 2026-07-21: the wlogout window (Super+Shift+Q — lock/reboot/shutdown/logout/suspend/hibernate) "isn't great and has bugs." Review it end to end: catalogue the specific bugs, then assess the design against the Dupre instrument-console family (it predates the panel aesthetic). Config lives in dotfiles; the bind is hyprland.conf:428 (=pgrep -x wlogout || wlogout-menu=). Context: the desktop-settings panel spec withdrew lock/suspend in favor of this screen (2026-07-21 amendment), so it's now the sole owner of session-exit actions — worth being good. Grade each bug found via the severity×frequency matrix; this parent stays a [#C] review until specifics emerge. @@ -535,10 +1203,15 @@ Build handed off to the dotfiles project 2026-07-04 (=~/.dotfiles/inbox/2026-07- *** TODO Phase 5 — VPN / WireGuard CLI fold (vNext) :network: Rescoped 2026-07-04 (audit): the tunnels track already shipped most of the original Phase 5. Panel tunnel bring-up/down and detection landed (dotfiles 2d9d060 probes tailscale/NM-wireguard/Proton; 21db05a brings overlays up/down from the panel's Tunnels sub-view; 31ba056 diagnose/doctor understand tunnel routes; archsetup 2e40781 wireguard config import; the net-panel-other-interfaces spec is IMPLEMENTED). What remains for Phase 5 is only the =net vpn ...= CLI subcommand — cli.py still has no vpn/tunnel parser. Fold the panel's existing tunnel operations into a CLI surface; spec separately when picked up. -** TODO [#B] Desktop-settings dropdown panel :feature:waybar: +** DONE [#B] Desktop-settings dropdown panel :feature:waybar: +CLOSED: [2026-07-22 Wed] :PROPERTIES: -:LAST_REVIEWED: 2026-07-09 +:LAST_REVIEWED: 2026-07-22 :END: +Resolved 2026-07-22: shipped end to end via the "Build: desktop-settings panel" task (dotfiles 7a15237 → 9038eee; spec IMPLEMENTED, 85 suites + smoke 13/13 + e2e 17/17). Every open question below got settled in the spec: bar consolidation landed (74f723e), the wallpaper manager became the in-panel sub-view, and the format pickers split into their own sibling spec ([[file:docs/specs/2026-07-19-display-format-single-source-of-truth-spec.org]], DRAFT stub). Remaining human-eye checks live under "Manual testing and validation". + +Original body follows as the record. + Initial spec written 2026-07-02: [[file:docs/specs/2026-07-02-desktop-settings-panel-spec.org]] (DRAFT — four decisions await Craig's review before build; architecture updated to the net panel's Blueprint/GTK4 stack). One waybar dropdown gathering the desktop toggles and sliders into a single settings panel, opened from a gear/settings glyph on the bar. Incorporate: @@ -890,6 +1563,39 @@ What we're verifying: the program matrix's pin and wheel hit-tests (Cairo; the e - Move a control that belongs to no program (e.g. flip TOUCHPAD). Expected: seating the pin starts the night light immediately (screen warms) and FOCUS stays active; unseating stops it; each wheel click steps P→B→S and applies live; the manual TOUCHPAD change deactivates the scene (no head stays lit). +*** Hypridle is reaped when its compositor goes away +What we're verifying: the orphan that wedged velox on 2026-07-22 can't accumulate — a compositor exit leaves no hypridle behind, and a fresh session starts with exactly one. Agent-untestable: proving it requires ending a live session, which is the thing that costs work. +- Note the current daemon count before doing anything. +#+begin_src sh :results output +pgrep -c hypridle; pgrep -a hypridle +#+end_src +- Exit the session deliberately (Super+Shift+Backspace twice, now that it confirms), landing back at the console. +- From the TTY, before logging back in, check what survived. +#+begin_src sh :results output +pgrep -a hypridle || echo "no hypridle survived — exec-shutdown reaped it" +#+end_src +- Log back in and let the desktop settle. +#+begin_src sh :results output +pgrep -c hypridle +#+end_src +Expected: zero hypridle processes at the TTY between sessions, and exactly one after logging back in — never two. If caffeine is engaged the count is legitimately zero after login too (caffeine works by stopping the daemon); release caffeine and re-check in that case. + +*** Exit-confirm submap guards the session-kill chord +What we're verifying: mod+Shift+Backspace no longer ends the session on one press — it arms a confirm submap where a second Backspace exits and anything else backs out. Agent-untestable by design: exercising it on a live session risks the session loss it prevents, so the chord press is yours. Save your work first; the confirm path really does exit. +- Press Super+Shift+Backspace. Nothing should visibly happen (no logout, no dialog). +#+begin_src sh :results output +hyprctl submap +#+end_src +- Press Escape. +#+begin_src sh :results output +hyprctl submap +#+end_src +- Press Super+Shift+Backspace again, then press an unrelated key (say =a=) instead of Escape. +#+begin_src sh :results output +hyprctl submap +#+end_src +Expected: after the first chord the submap reads =exitconfirm=; after Escape it reads =default=; after the stray key it reads =default= again (the catchall backs out). The session survives all three. Only Backspace-then-Backspace exits — confirm that separately when you're ready to end a session anyway. + *** Settings panel: locked-path night-watch swap What we're verifying: the WATCH stage under a locked session — the kiosk face reveals only after its window maps (no desktop flash), and first input restores hyprlock. The e2e/live checks so far only exercised the unlocked lifecycle. Note: caffeine was found engaged on 2026-07-22 (hypridle deliberately left stopped); release caffeine first or this never fires. - Release caffeine so hypridle runs with the five-stage conf. @@ -1567,7 +2273,18 @@ while ordinary WiFi stays white. Extend auto-dim with an explicit “dim everything” setting for bright non-dark-mode contexts, with a security/usability review of its scope. -** TODO [#C] Gallery probe: the fader-drag check is flaky :bug:test:design: +** DONE [#C] Gallery probe: the fader-drag check is flaky :bug:test:design:quick:solo: +CLOSED: [2026-07-23 Thu] +Fixed 2026-07-23. Root cause confirmed rather than suspected: =panel-widget-gallery.html= line 74 sets =html{scroll-behavior:smooth}=, so =scrollIntoView= animates and the fixed 200ms sleep sometimes read =getBoundingClientRect= mid-scroll. The drag then dispatched at stale coordinates, the press missed the fader, and the check reported a dead widget. + +Fix: scroll with =behavior:'instant'=. The probe never needed the animation, so this removes the race instead of waiting it out. Also added a =settledRect= guard (rect stable across two reads AND on-screen) for zoom/column relayout, and a =hits()= assertion that the press actually lands on the fader before the drag goes out. + +Applied to the toggle-click check too — it shares the same fixed-sleep shape, and it failed for this exact reason during the diagnosis, so fixing only the fader would have left half the defect. + +Worth recording: my FIRST fix was wrong and made it worse. Polling until the rect stopped changing returned pre-scroll coordinates every time, because two identical samples are also what you get before the animation starts — an intermittent failure became a consistent one. The new hit-test assertion is what caught it, printing the press point at y=1326 against a 1200px window. That's the argument for asserting the press landed rather than only asserting the readout moved. + +Verified against the measured 1-in-6 failure rate: 8 consecutive runs, all three checks passing, with the press point identical every run (429,480) — deterministic, not lucky. Full probe 96 PASS, 0 FAIL, exit 0. + =probe.mjs= check 3 ("fader drag tracks at 3x") intermittently reports =level 68 -> level 68=, i.e. the synthetic drag never registers. It has presumably been doing this all along unnoticed, since the suite is normally run once per batch. Grading: *Minor* severity (a false FAIL costs a re-run and a few minutes, and never ships a defect) x *most users, frequently* = P3 = =[#C]=. @@ -1582,7 +2299,7 @@ Why it matters beyond the annoyance: a gate that cries wolf gets its real failur Recurrences: 2026-07-18 batch-6 gate (first run, passed 3 reruns); 2026-07-18 batch-9 gate (first cold run, =level 68 -> level 68=, passed 2 reruns); 2026-07-21 double-speedrun run (flashed one RED mid-run, passed on rerun). All were a session's first/early probe run — consistent with the stale-rect theory (cold-start relayout settling slower than the fixed 200ms sleep). -** TODO [#C] Maint live-refresh hairline replacement :feature:maint: +** TODO [#C] Maint live-refresh hairline replacement :feature:maint:solo: :PROPERTIES: :LAST_REVIEWED: 2026-07-14 :END: @@ -1604,10 +2321,16 @@ From the roam inbox (routed 2026-07-13): the networking panel should track speed *** 2026-07-14 Tue @ 01:40:48 -0500 Diagnosed: OpenZFS/kernel version skew, blocked on archzfs Reproduced in ~1 minute of install: =dkms install zfs/2.3.3 -k 6.18.38-2-lts= exits 1 during pacstrap. Root cause confirmed: OpenZFS 2.3.3's META declares Linux-Maximum 6.15, and the VM installs linux-lts 6.18.38. The first release supporting 6.18 is 2.4.0 (2.4.1 covers 6.19), and archzfs currently serves only zfs-dkms 2.3.3-1 — nothing on our side to fix. Unblock condition: archzfs publishes zfs-dkms ≥2.4.0; recheck with =curl -s https://archzfs.com/archzfs/x86_64/ | grep -o 'zfs-dkms-[0-9.]*'=, then rerun =FS_PROFILE=zfs make test-vm-base=. -** TODO [#C] Dotfiles stow conflicts: first-launch risk + restow directory handling :bug:dotfiles: +** DONE [#C] Dotfiles stow conflicts: first-launch risk + restow directory handling :bug:dotfiles:quick:solo: +CLOSED: [2026-07-23 Thu] :PROPERTIES: :LAST_REVIEWED: 2026-07-14 :END: +Closed 2026-07-23. The last open item was the velox check, and velox is reachable again. Pulled it from =02df01a= to =de62e9d= (clean tree, fast-forward), then ran =make conflicts hyprland=, which dry-runs every tier: "No stow conflicts", exit 0. Its old conflict copy had already cleared against the updated repo, so there was nothing for =make reset= to do. + +Verified the pull is live through the symlinks rather than just present in the repo: =~/.local/bin/weather= resolves into the dotfiles tree and returns today's sun times on velox. + +Note for the record: =make conflicts common= is not a valid invocation — =check-de= rejects it, because common and the host tier are auto-included in the DE-scoped run. =make conflicts hyprland= is the whole check. *** 2026-07-14 Tue @ 00:51:51 -0500 Ratio calibre check passed; waypaper canonical decided (dark-lion) Ratio's ~/.config/calibre is a directory symlink into the dotfiles repo (stow folded the whole dir), so the first-launch gap never existed there — check closed. Craig decided dark-lion.jpg is the canonical waypaper wallpaper; the repo config.ini updated from the that-one-up-there.jpg placeholder (the file is skip-worktree volatile, unskipped for the commit and re-flagged). Remaining: when velox is back online, run make conflicts / make reset there so its old conflict copy clears against the updated repo. *** 2026-07-02 Thu @ 17:30:00 -0400 Shipped the Makefile hardening + first-launch guard (dotfiles 42a82d2) @@ -1631,7 +2354,7 @@ From the 2026-07-04 roam capture. The waybar collapse mechanism (click the trian :END: Follow-up from the 2026-07-04 net-panel hardening speedrun (Craig's cj question on the no-WiFi item). The shipped no-wifi-hardware verdict covers "no adapter at all." This tier covers "adapter present but the driver is wedged": read-only health signals — =ip link= (device present but no-carrier / down), =dmesg= / =journalctl -k= for firmware-load failures, =rfkill= for a hard block, =modinfo= / =lsmod= for the driver module — classified before a generic reset. Remedy actions: a privileged =modprobe -r && modprobe = reload of the wifi driver, and a firmware-package pointer when the failure is a missing/failed firmware load. Dotfiles net-package work (handled per the archsetup-owns-dotfiles rule). Design pass first to decide whether it's worth a repair tier vs a needs-user-action pointer. -** TODO [#C] Voice dictation / speech-to-text input :feature:tooling: +** TODO [#C] Voice dictation / speech-to-text input :feature:tooling:solo: :PROPERTIES: :LAST_REVIEWED: 2026-07-06 :END: @@ -1679,6 +2402,9 @@ Craig: the camera works (bought for being Linux-friendly), it just needs configu :END: archsetup installs =python-lyricsgenius= with =--mflags --skipinteg=, skipping makepkg integrity + PGP checks — a workaround originally for an expired-signature issue upstream (surfaced by the 2026-06-23 --noconfirm audit). Periodically test whether the cause has cleared: if a plain =aur_install python-lyricsgenius= builds without complaint, drop the =--skipinteg= workaround. Removal needs a real AUR build to confirm, so it isn't a blind change. +*** 2026-07-23 Thu @ 02:20:00 -0500 Rechecked: still needed, unchanged +Fresh AUR clone, =makepkg --verifysource= on 3.7.0-1 (PKGBUILD still unchanged since the last check): the PyPI tarball passes, =LICENSE.txt= still FAILS its b2sum. Same structural cause — the source pins the license at github master, so its checksum drifts whenever upstream touches the file. =--skipinteg= stays. Nothing to change in the installer. + *** 2026-07-14 Tue @ 01:40:48 -0500 Rechecked: still needed, same cause Fresh AUR clone, =makepkg --verifysource= on 3.7.0-1 (PKGBUILD unchanged): tarball passes, =LICENSE.txt= still FAILS its b2sum — the github-master pin, structural as diagnosed 2026-06-24. =--skipinteg= stays. @@ -1712,13 +2438,13 @@ The theme system went single-theme — Hudson was retired (dotfiles e03436e; doc :END: Once-yearly systematic inventory of known deficiencies and friction points in current toolset -** TODO [#D] Installer + scripts refactor opportunities :refactor: +** TODO [#D] Installer + scripts refactor opportunities :refactor:solo: Grading: no behavior change; parking lot. 10 refactors remain from the sentry audit — duplicated GPU-modalias scan, triple hand-rolled retry loop, stow x4, display_server/window_manager dispatch dup, Maia ELO range x3, per-script log helpers, GRUB/snapper/fsck sed clusters, waybar-battery positional sed. Full list with line numbers in [[file:docs/design/2026-07-19-sentry-code-findings.org][sentry code findings]] (High/Medium/Low tagged). Pull individual ones out as their own tasks when tackled. The system-mutation sed clusters (snapper/fsck/GRUB/waybar) want characterization coverage before any rewrite. *** 2026-07-20 Mon @ 16:35:00 -0500 Extracted validate_yesno and the NVIDIA_MIN_DRIVER constant In 67d0b6e: the four yes/no config-validation blocks collapse into validate_yesno (TDD), and the driver-floor literal 535 becomes NVIDIA_MIN_DRIVER. The safe, purely-testable slice; the remaining 10 (structural / system-mutation) stay parked above. *** 2026-07-21 Tue @ 08:00:00 -0500 Audit reconcile: the GRUB cmdline sed slice shipped f9da097 (2026-07-21) rewrote the boot-critical =GRUB_CMDLINE_LINUX_DEFAULT= sed into a guarded awk+mv merge with a backup and added characterization tests (tests/installer-steps/test_grub_cmdline.py). So the "GRUB sed clusters want characterization coverage before rewrite" caveat no longer covers the cmdline line — only the 4 cosmetic GRUB sed lines (timeout/default/terminal/gfxmode, which overwrite fixed values with nothing to preserve) remain in that slice. -** TODO [#D] Test-framework + prototype refactor cluster :refactor: +** TODO [#D] Test-framework + prototype refactor cluster :refactor:solo: Grading: no behavior change; parking lot. Refactors from the S5-S7 audit, distinct from the installer refactor rollup above. scripts/testing/run-test.sh + run-test-baremetal.sh duplicate the run/poll/report skeleton and have drifted (VM uses setsid + copy helpers, baremetal uses nohup + hand-rolled sshpass scp) — extract the shared core so baremetal inherits the sturdier paths; run-maint-nspawn.sh:66 + run-maint-scenarios.sh:78 duplicate the transport-independent _scenario_var/_validate_scenario/run_scenario (a sourced lib/maint-scenario.sh); run-test.sh:251,265 uses two different mechanisms (pgrep vs ps|grep) for the same liveness check; docs/prototypes/gen_tokens.py:78 repeats the section-iteration skeleton across four emitters; gallery-widget.el:95,136 hardcodes SVG arc/hub path strings that duplicate the cx/cy/radius geometry (dial desyncs silently on a constant change); gallery-widget.el:72,84 leans on the private svg--append. See findings doc (S5, S6, S7). ** TODO [#D] Net doctor vNext :feature:dotfiles:network: -- cgit v1.2.3