1 files changed, 79 insertions, 3 deletions

diff --git a/docs/design/2026-07-19-sentry-code-findings.org b/docs/design/2026-07-19-sentry-code-findings.org
index 3816ebc..add1829 100644
--- a/docs/design/2026-07-19-sentry-code-findings.org
+++ b/docs/design/2026-07-19-sentry-code-findings.org
@@ -26,9 +26,9 @@ Each fire takes the next uncovered slice. Mark done as fires complete.
 - [X] S2 — archsetup installer, lines ~1050-2100 (post-chroot system config: locale, mirrors, user, dotfiles/stow, AUR, services, snapshot tooling, WM packages). NB still NO partitioning/mkfs/mount in this range either — the disk path is in S3 or delegated to a separate tool.
 - [X] S3 — archsetup installer, lines ~2100-3124 (desktop/WM, gaming, boot_ux: mkinitcpio/GRUB, finalize). RESOLVED: archsetup does NO partitioning/mkfs/mount anywhere in the file — it is a post-install config layer; disk setup is upstream (archinstall/separate tool).
 - [X] S4 — scripts/*.sh production scripts (audit-packages, cmail-setup-finish, post-install, import-wireguard, normalize-notify-sounds, generate-palette, easyeffects, games, setup-chess)
-- [ ] S5 — scripts/testing/ framework + lib (run-test, vm-utils, validation, logging, network-diagnostics, testinfra, run-maint*, create-base-vm, cleanup, debug, setup-testing-env)
-- [ ] S6 — Python: gen_tokens + test suites (refactor lens; test-quality per testing.md)
-- [ ] S7 — elisp (gallery-widget.el)
+- [X] S5 — scripts/testing/ framework + lib (run-test, vm-utils, validation, logging, network-diagnostics, testinfra, run-maint*, create-base-vm, cleanup, debug, setup-testing-env)
+- [X] S6 — Python: gen_tokens + test suites (refactor lens; test-quality per testing.md)
+- [X] S7 — elisp (gallery-widget.el)
 
 * Findings
 
@@ -172,3 +172,79 @@ info/ok/skip/warn/err printf helpers re-defined in setup-chess.sh:61-65, cmail-s
 Per-file: audit-packages clean; cmail 1 (Major); post-install clean (deliberate best-effort); import-wireguard 1 (Minor, key handling itself correct); normalize-notify 1 (Minor); games clean; setup-chess 2 refactors; generate-palette clean; easyeffects clean.
 
 S4 verdict: 3 bugs (1 Major, 2 Minor), 2 refactors (2 Low). Plus a SECURITY item escalated to the approval queue (tracked WireGuard private keys).
+
+** S5 — scripts/testing/ framework + lib (speedrun audit, 2026-07-20 ~16:00)
+
+*** [BUG Major] scripts/testing/run-net-scenarios.sh:103 — diagnose-check failure is printed but never counted, run exits green
+The scenario_diagnose_expect branch prints fail "$name: diagnose did NOT name it" but does not exit 1 (unlike the break and assert branches). Because the whole scenario runs inside ( ... ) || fails=$((fails+1)), a subshell whose only failure is the diagnose check still exits 0, so fails is not incremented and the script prints "all scenarios passed" and exit 0. A regression where net doctor can no longer identify a fault it is supposed to name is reported as a visible FAIL line yet a green exit code, so CI/automated callers treat the run as passing. Fix: make the diagnose-else branch force a non-zero subshell exit (exit 1 like the other two checks). First-draft harness per its own header — reviewed, not yet relied on.
+
+*** [BUG Minor] scripts/testing/debug-vm.sh:49 — hardcoded base-disk name ignores FS_PROFILE
+BASE_DISK is hardcoded to the btrfs image archsetup-base.qcow2, while init_vm_paths (line 56) already computes the profile-correct DISK_PATH (archsetup-base-zfs.qcow2 for FS_PROFILE=zfs). FS_PROFILE=zfs ./debug-vm.sh boots the btrfs base or fatals "Base disk not found." Fix: use DISK_PATH from init_vm_paths. Debug-only tool, zfs path only.
+
+*** [BUG Minor] scripts/testing/lib/vm-utils.sh:284 — snapshot restore races the force-killed QEMU's qcow2 lock
+kill_qemu sends kill -9 and deletes the PID file without waiting for reaping, so vm_is_running reports false at once. In the force-kill fallback, restore_snapshot then runs qemu-img snapshot -a while the dying qemu may still hold the qcow2 lock; qemu-img refuses a locked image, the restore fails, and in the trap it is swallowed by || true, leaving the base image dirty with no signal. Only bites on ACPI-powerdown timeout. Fix: wait for the killed PID (or poll /proc/$pid) before returning.
+
+*** [BUG Minor] scripts/testing/lib/vm-utils.sh:69 — runtime paths not profile-suffixed, parallel btrfs+zfs runs collide
+init_vm_paths suffixes DISK_PATH and OVMF_VARS by FS_PROFILE but leaves PID_FILE, MONITOR_SOCK, SERIAL_LOG fixed. A concurrent btrfs and zfs run shares those three: the second run's stop_qemu/vm_is_running reads the first run's PID file and can shutdown/kill the other VM; both truncate the same serial log. The default SSH_PORT check partially masks it. Fix: suffix these three like DISK_PATH.
+
+*** [BUG Minor] scripts/testing/run-test.sh:287 — ARCHSETUP_EXIT_CODE never holds the installer's real exit status
+The variable is set from a completion-marker grep, not archsetup's real exit code — the installer is launched detached (setsid ... &) and runs with set -e off, so it can error mid-run, continue, reach the end, and still write the marker. The report prints "ArchSetup Exit Code: 0" and the final gate treats that as success. Testinfra is the real backstop, so a broken install is still caught, but the reported code is misleading. Same pattern run-test-baremetal.sh:234. Fix: rename (ARCHSETUP_COMPLETED) and/or capture the true exit status via a file.
+
+*** [REFACTOR Medium] scripts/testing/run-test.sh + run-test-baremetal.sh — large-scale duplication of the run/poll/report skeleton
+The two runners duplicate the git-bundle transfer, the 30s MAX_POLLS loop, the completion-marker exit derivation, the Testinfra invocation, and the near-identical report heredoc — and have drifted (VM: setsid launch + copy_to_vm/copy_from_vm helpers; baremetal: nohup + hand-rolled sshpass scp). Extract the shared poll-and-report core into a sourced helper so baremetal inherits the sturdier launch/copy paths.
+
+*** [REFACTOR Low] scripts/testing/run-maint-nspawn.sh:66 + run-maint-scenarios.sh:78 — duplicated scenario plumbing beyond the acknowledged helper trio
+_scenario_var, _validate_scenario, and the mexec/mmaint/mfix/massert_metric helpers are byte-for-byte duplicated. The transport-specific trio (ssh vs nspawn) is defensibly duplicated, but _scenario_var and _validate_scenario are transport-independent pure functions that could live in a sourced lib/maint-scenario.sh. run_scenario is also nearly identical.
+
+*** [REFACTOR Low] scripts/testing/run-test.sh:251,265 — two different mechanisms to detect the same process
+The launch check uses pgrep -f 'bash archsetup' (251) and the poll loop uses ps aux | grep '[b]ash archsetup' (265) for the identical liveness question. Use one mechanism to remove a maintenance trap.
+
+S5 verdict: 5 bugs (1 Major, 4 Minor), 3 refactors (1 Medium, 2 Low). No syntax errors; temp-file traps, snapshot-stopped guards, mfix PIPESTATUS, and the errexit-off window handled correctly.
+
+** S6 — Python: gen_tokens + test suites (speedrun audit, 2026-07-20 ~16:00)
+
+*** [TEST-QUALITY High] tests/installer-steps/test_pacman_hook_order.py:20 — ordering "invariant" is a tautology that never touches the file
+test_safety_hooks_precede_mkinitcpio_removal claims to guard that 05-zfs-snapshot and 10-hypr-live-update-guard hooks are created before 60-mkinitcpio-remove. But the two assertLess calls compare two string literals ("05..." < "60...") — a constant ASCII fact always true regardless of file content. The ordering is never verified; the test would still pass if the hooks appeared after the removal or the removal moved above them. Only the assertIn presence checks do real work. Fix: assert on positions — text.index("05-zfs-snapshot.hook") < text.index("60-mkinitcpio-remove.hook") (and the guard hook). Matters because a real reorder can remove the current initramfs without a rebuild (unbootable) and this test would ship it green.
+
+*** [BUG Minor] scripts/testing/tests/test_desktop.py:96 — shell glob passed to `test -S` breaks on zero or multiple compositor sockets
+test_hyprland_socket runs host.run("test -S /tmp/hypr/*/.socket.sock"). The remote shell expands the glob: zero matches leaves the literal pattern (rc 1), two+ instances pass multiple args (rc 2 "too many arguments"). Either way the assertion fails for a reason unrelated to socket existence. Masked today because the test gates on compositor_running, which the headless VM never satisfies (always skips). Fix: resolve one socket first (find /tmp/hypr -name .socket.sock -type s | head -1).
+
+*** [TEST-QUALITY Low] tests/gallery-tokens/test_gen_tokens.py:181 — degenerate-marker test asserts properties too weak to notice garbled output
+test_start_marker_on_final_line_without_newline feeds "keep\n<START> <END>" and asserts only startswith("keep\n"), END present, "X" present — all hold, but the actual output is malformed (duplicated start marker, content spliced mid-line). Passes on broken output. The input can't occur in real HTML (markers always on their own newline-terminated lines), so severity is low; tighten to the exact expected string or drop as impossible.
+
+*** [REFACTOR Low] docs/prototypes/gen_tokens.py:78 — four emitters repeat the section-iteration skeleton
+emit_web_css, emit_waybar_gtk, emit_elisp each walk COLOR_SECTIONS then glow (resolving through resolve_color), then font/timing for two of three. A generator yielding resolved (name, color_value) pairs with each emitter supplying its formatter would remove the triplicated traversal. Readability only — divergences are load-bearing (waybar's font/timing omission is deliberate); keep them visible.
+
+S6 verdict: 1 bug (Minor), 2 test-quality (1 High, 1 Low), 1 refactor (Low). gen_tokens.py has no correctness bugs; the sed-extract unit tiers import real function bodies, fake only at the system boundary, assert behavior not prose, carry N/B/E coverage, no shared state or hardcoded dates. Testinfra tiers sound.
+
+** S7 — elisp: gallery-widget.el + gallery-tokens.el (speedrun audit, 2026-07-20 ~16:00)
+
+*** [BUG Minor] docs/prototypes/gallery-widget.el:139 — readout renders raw value while the needle clamps
+The needle angle is clamped 0-100 in gallery-widget--needle-angle, but the readout at line 139 uses (round value) on the unclamped input. Value 150 draws the needle pinned at +60 degrees (x2=82.64) while the text reads "150%" — confirmed live. The two halves disagree at any out-of-range input. Fix: compute a single clamped value once and format both the angle and the readout from it.
+
+*** [BUG Minor] docs/prototypes/gallery-widget.el:69 — cl-loop used without (require 'cl-lib)
+gallery-widget--node calls cl-loop, but the file requires only svg and dom. Works today only via cl-loop's autoload cookie; bites on byte-compile in a context that macroexpands cl-loop without cl-lib, or if a future cl- macro isn't autoloaded (cold-load void-function). Fix: add (require 'cl-lib).
+
+*** [BUG Minor] docs/prototypes/gallery-widget.el:29 — file-name-directory on a possibly-nil path
+The token load computes its directory from (or load-file-name buffer-file-name); both are nil when the form is eval'd outside a load and outside a file buffer, so file-name-directory receives nil and errors. Doesn't bite in the normal load/require path or the test harness, but makes interactive re-eval fragile. Fix: fall back to default-directory when neither is set.
+
+*** [REFACTOR Medium] docs/prototypes/gallery-widget.el:95,136 — hardcoded SVG path strings duplicate the geometry the constants encode
+The arc path "M 1 48 A 47 47 0 0 1 95 48" and hub path "M 44 48 A 4 4 0 0 1 52 48 Z" bake in cx=48, cy=48, radius 47, hub radius 4 — the same values expressed numerically elsewhere and passed to --polar for ticks/needle. Change cx/cy/radii and ticks/needle move while arc/hub silently stay put (dial desyncs, no error). Derive the arc endpoints and hub path from cx/cy/radius via the existing --polar/--fmt machinery, or name the magic numbers as shared defconsts.
+
+*** [REFACTOR Low] docs/prototypes/gallery-widget.el:72,84 — reliance on the private svg--append internal
+gallery-widget--node (72) and the defs/filter insertion (84) call svg--append, a double-dash private of svg.el with no cross-version stability guarantee. Low severity (prototype pinned to a known Emacs); prefer public svg.el builders or isolate the private call behind the single --node helper.
+
+*** [TEST-QUALITY Medium] tests/gallery-widgets/test-gallery-widget.el:77 — obtuse tick count via split-string + cl-count-if :start 1
+The "exactly three ticks" assertion splits on class="tick" then cl-count-if with an always-true predicate and :start 1 to count length-minus-one — a coincidence of split semantics, not a match count. Brittle: an empty leading/trailing segment or a stray class="tick" substring elsewhere throws it off. Replace with a direct occurrence count.
+
+*** [TEST-QUALITY Medium] tests/gallery-widgets/test-gallery-widget.el:47 — clamp coverage tests the angle helper but never the rendered readout
+The boundary test asserts gallery-widget--needle-angle clamps -5 and 150, but nothing asserts the full gauge's readout at an out-of-range value — exactly why the readout/needle mismatch (S7 bug 1) ships green. Add a gauge-level boundary case checking both the needle endpoint and the readout string at an out-of-range input.
+
+*** [TEST-QUALITY Low] tests/gallery-widgets/test-gallery-widget.el:159 — output helper gallery-widget-write-svg is untested
+gallery-widget-svg-string is exercised indirectly, but gallery-widget-write-svg has no coverage. A Normal case (write to temp, assert file exists, head is "<svg", return value equals the path) closes the last uncovered public entry point.
+
+S7 verdict: 3 bugs (3 Minor), 2 refactors (1 Medium, 1 Low), 3 test-quality (2 Medium, 1 Low). No Critical/Major; SVG geometry, needle math, token lookup, error handling otherwise sound; all 10 existing tests pass.
+
+** Audit complete (S1-S7)
+
+Installer (S1-S3): 16 bugs, 10 refactors. Scripts (S4): 3 bugs, 2 refactors + WireGuard secrets (remediated). Test framework (S5): 5 bugs, 3 refactors. Python (S6): 1 bug, 2 test-quality, 1 refactor. Elisp (S7): 3 bugs, 2 refactors, 3 test-quality. All slices covered. Speedrun triaged S1-S4 bugs into tasks (chpasswd, df, run_step, sudoers, wireguard, idempotency, chmod/cp, normalize-notify shipped; GRUB-overwrite + others filed). S5-S7 triaged into the tasks below.