From b6ddf6e1ffd7e0f0d766113f39660969495f6103 Mon Sep 17 00:00:00 2001
From: Adam Hall <adam.hall.1992@live.com>
Date: Mon, 22 Jun 2026 19:36:21 +0930
Subject: [PATCH 1/5] refactor(archinstall): extract singleDiskRoot shared by
 plain/btrfs builders

plainBuilder.build and btrfsBuilder.build were ~90% duplicated: probe
disk1 geometry, build the ESP partition, optionally append a linux-swap
partition, compute the remaining root bytes, and append the root
partition. Only the root partition's fs_type, mount options and btrfs
subvolume list differed.

Extract singleDiskRoot(esp, swap, espBytes, geom, rootSpec) which builds
the ESP + optional swap prefix and the root partition, taking the three
varying fields via a small rootSpec struct. The newObjID() call order
(ESP, swap, root) is preserved, so golden renders stay byte-identical
and the archinstall golden tests pass without -update.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 internal/archinstall/archinstall.go | 97 ++++++++++++++---------------
 1 file changed, 47 insertions(+), 50 deletions(-)

diff --git a/internal/archinstall/archinstall.go b/internal/archinstall/archinstall.go
index 0c21ece..c26c678 100644
--- a/internal/archinstall/archinstall.go
+++ b/internal/archinstall/archinstall.go
@@ -575,22 +575,51 @@ type plainBuilder struct {
 }
 
 func (b *plainBuilder) build(geom Geometry) ([]Device, *LvmConfiguration, error) {
-	disk1 := b.esp.Device
-	espBytes := b.espBytes
+	rootFs := b.plain.Filesystem
+	devices, err := singleDiskRoot(b.esp, b.swap, b.espBytes, geom, rootSpec{
+		fsType:       rootFs,
+		mountOptions: []string{},
+		btrfs:        []any{},
+	})
+	if err != nil {
+		return nil, nil, err
+	}
+	return devices, nil, nil
+}
+
+// rootSpec describes the per-layout root partition that follows the shared ESP +
+// optional swap prefix on disk 1. Only these three fields differ between the
+// plain and btrfs layouts; everything else (Status/Type/Start/Size/Mountpoint at
+// "/"/Flags) is identical and supplied by singleDiskRoot.
+type rootSpec struct {
+	fsType       string
+	mountOptions []string
+	btrfs        []any
+}
+
+// singleDiskRoot builds the disk-1 layout shared by the plain and btrfs builders:
+// the ESP partition, an optional linux-swap partition (when swap.type is
+// "partition", sized from swap.size), and a root partition consuming the rest of
+// disk 1. The root partition's fs_type, mount options and btrfs subvolume list
+// come from spec; everything else is fixed. The newObjID() call order is ESP,
+// swap (if present), then root — matching both original builders so golden renders
+// stay byte-identical.
+func singleDiskRoot(esp config.ESPConfig, swap config.SwapConfig, espBytes uint64, geom Geometry, spec rootSpec) ([]Device, error) {
+	disk1 := esp.Device
 
 	disk1Total, ok := geom[disk1]
 	if !ok || disk1Total == 0 {
-		return nil, nil, fmt.Errorf("no geometry for disk 1 (%s)", disk1)
+		return nil, fmt.Errorf("no geometry for disk 1 (%s)", disk1)
 	}
 
 	parts := []Partition{espPartition(espBytes)}
 	offset := startOffset + espBytes
 
 	// Optional swap partition (plain/btrfs only). Sized from swap.size.
-	if b.swap.EffectiveType() == "partition" {
-		swapBytes, err := parseSize(b.swap.Size)
+	if swap.EffectiveType() == "partition" {
+		swapBytes, err := parseSize(swap.Size)
 		if err != nil {
-			return nil, nil, fmt.Errorf("swap size: %w", err)
+			return nil, fmt.Errorf("swap size: %w", err)
 		}
 		swapFs := "linux-swap"
 		parts = append(parts, Partition{
@@ -603,20 +632,20 @@ func (b *plainBuilder) build(geom Geometry) ([]Device, *LvmConfiguration, error)
 
 	used := offset + endReserve
 	if disk1Total <= used {
-		return nil, nil, fmt.Errorf("disk 1 (%s, %d bytes) too small for ESP+swap (%d bytes)", disk1, disk1Total, used)
+		return nil, fmt.Errorf("disk 1 (%s, %d bytes) too small for ESP+swap (%d bytes)", disk1, disk1Total, used)
 	}
 	rootBytes := roundDownMiB(disk1Total - used)
 
 	root := "/"
-	rootFs := b.plain.Filesystem
+	rootFs := spec.fsType
 	parts = append(parts, Partition{
 		ObjID: newObjID(), Status: "create", Type: "primary",
 		Start: bytes(offset), Size: bytes(rootBytes),
 		FsType: &rootFs, Mountpoint: &root,
-		MountOptions: []string{}, Flags: []string{}, Btrfs: []any{},
+		MountOptions: spec.mountOptions, Flags: []string{}, Btrfs: spec.btrfs,
 	})
 
-	return []Device{{Device: disk1, Wipe: true, Partitions: parts}}, nil, nil
+	return []Device{{Device: disk1, Wipe: true, Partitions: parts}}, nil
 }
 
 // --- btrfs layout -----------------------------------------------------------
@@ -642,39 +671,6 @@ type btrfsBuilder struct {
 }
 
 func (b *btrfsBuilder) build(geom Geometry) ([]Device, *LvmConfiguration, error) {
-	disk1 := b.esp.Device
-	espBytes := b.espBytes
-
-	disk1Total, ok := geom[disk1]
-	if !ok || disk1Total == 0 {
-		return nil, nil, fmt.Errorf("no geometry for disk 1 (%s)", disk1)
-	}
-
-	parts := []Partition{espPartition(espBytes)}
-	offset := startOffset + espBytes
-
-	if b.swap.EffectiveType() == "partition" {
-		swapBytes, err := parseSize(b.swap.Size)
-		if err != nil {
-			return nil, nil, fmt.Errorf("swap size: %w", err)
-		}
-		swapFs := "linux-swap"
-		parts = append(parts, Partition{
-			ObjID: newObjID(), Status: "create", Type: "primary",
-			Start: bytes(offset), Size: bytes(swapBytes),
-			FsType: &swapFs, MountOptions: []string{}, Flags: []string{"swap"}, Btrfs: []any{},
-		})
-		offset += swapBytes
-	}
-
-	used := offset + endReserve
-	if disk1Total <= used {
-		return nil, nil, fmt.Errorf("disk 1 (%s, %d bytes) too small for ESP+swap (%d bytes)", disk1, disk1Total, used)
-	}
-	rootBytes := roundDownMiB(disk1Total - used)
-
-	root := "/"
-	btrfsFs := "btrfs"
 	mountOpts := []string{}
 	if b.btrfs.Compress != "" {
 		mountOpts = append(mountOpts, "compress="+b.btrfs.Compress)
@@ -687,14 +683,15 @@ func (b *btrfsBuilder) build(geom Geometry) ([]Device, *LvmConfiguration, error)
 		subvols = append(subvols, BtrfsSubvolume{Name: sv.Name, Mountpoint: &mp})
 	}
 
-	parts = append(parts, Partition{
-		ObjID: newObjID(), Status: "create", Type: "primary",
-		Start: bytes(offset), Size: bytes(rootBytes),
-		FsType: &btrfsFs, Mountpoint: &root,
-		MountOptions: mountOpts, Flags: []string{}, Btrfs: subvols,
+	devices, err := singleDiskRoot(b.esp, b.swap, b.espBytes, geom, rootSpec{
+		fsType:       "btrfs",
+		mountOptions: mountOpts,
+		btrfs:        subvols,
 	})
-
-	return []Device{{Device: disk1, Wipe: true, Partitions: parts}}, nil, nil
+	if err != nil {
+		return nil, nil, err
+	}
+	return devices, nil, nil
 }
 
 // --- helpers ----------------------------------------------------------------

From b11d4690d85f43ef8cd6d7d86cfb8129da8ca028 Mon Sep 17 00:00:00 2001
From: Adam Hall <adam.hall.1992@live.com>
Date: Mon, 22 Jun 2026 19:39:17 +0930
Subject: [PATCH 2/5] refactor(run): drop reverted-TUI dead code; unify root
 privilege via RootShell/TryRoot

Issue #8: delete Capture (zero callers), the Out field + outWriter helper
(fed only the reverted TUI viewport), wiring Cmd/Shell straight to
os.Stdout/os.Stderr. Drop now-unused bytes/io imports. Add run_test.go
covering the dry-run recording seam for Cmd/Shell/Root(Sudo on+off)/Try.

Issue #6: add Runner.RootShell (run a shell pipeline as root: sudo bash -c
in Phase B, bash -c in Phase A) and Runner.TryRoot (best-effort Root). Convert
the snapper, grub-theme, and kernel-cmdline shell snippets that hardcoded inner
sudo to RootShell with the inner sudo removed, and snapper set-config to TryRoot.
The vinceliuice install.sh keeps inner sudo (cloneBuild runs as the user).
Equivalent root behaviour in Phase B; updated the affected plan assertions.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 internal/run/run.go                  |  73 ++++++++----------
 internal/run/run_test.go             | 111 +++++++++++++++++++++++++++
 internal/stages/bootloader_test.go   |   4 +-
 internal/stages/grubtheme.go         |  15 ++--
 internal/stages/helpers.go           |  11 +--
 internal/stages/snapper.go           |   6 +-
 internal/stages/snapper_test.go      |  15 ++--
 internal/stages/systemd_boot_test.go |  15 ++--
 8 files changed, 181 insertions(+), 69 deletions(-)
 create mode 100644 internal/run/run_test.go

diff --git a/internal/run/run.go b/internal/run/run.go
index 2d62ce1..fb63d4e 100644
--- a/internal/run/run.go
+++ b/internal/run/run.go
@@ -6,9 +6,7 @@
 package run
 
 import (
-	"bytes"
 	"fmt"
-	"io"
 	"os"
 	"os/exec"
 	"strings"
@@ -20,10 +18,6 @@ type Runner struct {
 	DryRun bool // print/record commands instead of executing them
 	Sudo   bool // prefix privileged commands with sudo (Phase B as user); false in Phase A (already root)
 
-	// Out, when non-nil, receives BOTH stdout and stderr of every executed
-	// command. It is the hook for a TUI viewport that captures streamed output;
-	// when nil, output streams to os.Stdout/os.Stderr exactly as before.
-	Out io.Writer
 	// Env, when non-empty, is layered (key=value) on top of the inherited process
 	// environment for every executed command. Dir, when set, is the working dir.
 	Env map[string]string
@@ -36,15 +30,6 @@ type Runner struct {
 
 func (r *Runner) record(line string) { r.Plan = append(r.Plan, line) }
 
-// outWriter returns r.Out when set, else the given default stream, so a nil Out
-// preserves the original os.Stdout/os.Stderr wiring exactly.
-func (r *Runner) outWriter(def io.Writer) io.Writer {
-	if r.Out != nil {
-		return r.Out
-	}
-	return def
-}
-
 // prepare applies the Runner's Env and Dir (if any) to a command before it runs.
 func (r *Runner) prepare(cmd *exec.Cmd) {
 	if len(r.Env) > 0 {
@@ -70,7 +55,7 @@ func (r *Runner) Cmd(name string, args ...string) error {
 		return nil
 	}
 	cmd := exec.Command(name, args...)
-	cmd.Stdout, cmd.Stderr, cmd.Stdin = r.outWriter(os.Stdout), r.outWriter(os.Stderr), os.Stdin
+	cmd.Stdout, cmd.Stderr, cmd.Stdin = os.Stdout, os.Stderr, os.Stdin
 	r.prepare(cmd)
 	if err := cmd.Run(); err != nil {
 		return fmt.Errorf("%s: %w", name, err)
@@ -78,28 +63,6 @@ func (r *Runner) Cmd(name string, args ...string) error {
 	return nil
 }
 
-// Capture runs name+args and returns the command's stdout as a string, recording
-// it in .Plan exactly like Cmd. stderr still streams (to Out or os.Stderr). In
-// dry-run it records and returns "" with a nil error. Use this for the rare
-// state-querying command whose output a stage needs, rather than dropping to
-// os/exec directly (which would bypass dry-run and the recorded plan).
-func (r *Runner) Capture(name string, args ...string) (string, error) {
-	line := strings.TrimSpace(name + " " + strings.Join(args, " "))
-	r.record(line)
-	ui.Step("%s", line)
-	if r.DryRun {
-		return "", nil
-	}
-	cmd := exec.Command(name, args...)
-	var out bytes.Buffer
-	cmd.Stdout, cmd.Stderr, cmd.Stdin = &out, r.outWriter(os.Stderr), os.Stdin
-	r.prepare(cmd)
-	if err := cmd.Run(); err != nil {
-		return out.String(), fmt.Errorf("%s: %w", name, err)
-	}
-	return out.String(), nil
-}
-
 // Root runs a command with root privileges: directly when already root (Phase A,
 // live ISO) or via sudo otherwise (Phase B, as the user).
 func (r *Runner) Root(name string, args ...string) error {
@@ -109,12 +72,44 @@ func (r *Runner) Root(name string, args ...string) error {
 	return r.Cmd(name, args...)
 }
 
+// RootShell runs a shell script with root privileges through `bash -c`: via
+// `sudo bash -c <script>` when not already root (Phase B, as the user) or
+// directly when already root (Phase A, live ISO). This lets a shell pipeline
+// run wholly as root without inner per-command `sudo`. It records/prints the
+// same `sh: <script>` line as Shell so the recorded .Plan stays uniform.
+func (r *Runner) RootShell(script string) error {
+	r.record("sh: " + script)
+	ui.Step("sh: %s", script)
+	if r.DryRun {
+		return nil
+	}
+	var cmd *exec.Cmd
+	if r.Sudo {
+		cmd = exec.Command("sudo", "bash", "-c", script)
+	} else {
+		cmd = exec.Command("bash", "-c", script)
+	}
+	cmd.Stdout, cmd.Stderr, cmd.Stdin = os.Stdout, os.Stderr, os.Stdin
+	r.prepare(cmd)
+	if err := cmd.Run(); err != nil {
+		return fmt.Errorf("shell: %w", err)
+	}
+	return nil
+}
+
 // Try runs a command but never fails the stage (the analogue of bash `|| true`),
 // for best-effort steps like partprobe/udevadm.
 func (r *Runner) Try(name string, args ...string) {
 	_ = r.Cmd(name, args...)
 }
 
+// TryRoot is the Root analogue of Try: a best-effort privileged command that
+// never fails the stage. It adds sudo in Phase B and runs direct in Phase A,
+// keyed off Sudo, exactly like Root.
+func (r *Runner) TryRoot(name string, args ...string) {
+	_ = r.Root(name, args...)
+}
+
 // Shell runs a string through `bash -c`, for pipes/redirects/conditionals
 // (genfstab, idempotent sed edits, gpg key import). Prefer Cmd where possible.
 func (r *Runner) Shell(script string) error {
@@ -124,7 +119,7 @@ func (r *Runner) Shell(script string) error {
 		return nil
 	}
 	cmd := exec.Command("bash", "-c", script)
-	cmd.Stdout, cmd.Stderr, cmd.Stdin = r.outWriter(os.Stdout), r.outWriter(os.Stderr), os.Stdin
+	cmd.Stdout, cmd.Stderr, cmd.Stdin = os.Stdout, os.Stderr, os.Stdin
 	r.prepare(cmd)
 	if err := cmd.Run(); err != nil {
 		return fmt.Errorf("shell: %w", err)
diff --git a/internal/run/run_test.go b/internal/run/run_test.go
new file mode 100644
index 0000000..4934c6f
--- /dev/null
+++ b/internal/run/run_test.go
@@ -0,0 +1,111 @@
+package run
+
+import (
+	"reflect"
+	"testing"
+)
+
+// These tests pin the dry-run recording seam: in dry-run every state-changing
+// helper must populate .Plan with the exact line it would have executed, and must
+// NOT actually run anything. (If they executed, these commands — sudo, bash -c
+// "false" — would fail or mutate the host; their passing proves dry-run is honoured.)
+
+func TestCmd_RecordsInDryRun(t *testing.T) {
+	r := &Runner{DryRun: true}
+	if err := r.Cmd("pacman", "-S", "--noconfirm", "git"); err != nil {
+		t.Fatalf("dry-run Cmd should not error: %v", err)
+	}
+	want := []string{"pacman -S --noconfirm git"}
+	if !reflect.DeepEqual(r.Plan, want) {
+		t.Errorf("Plan = %v, want %v", r.Plan, want)
+	}
+}
+
+func TestShell_RecordsInDryRun(t *testing.T) {
+	r := &Runner{DryRun: true}
+	// A script that would fail if executed; dry-run must not run it.
+	if err := r.Shell("false && echo nope"); err != nil {
+		t.Fatalf("dry-run Shell should not error: %v", err)
+	}
+	want := []string{"sh: false && echo nope"}
+	if !reflect.DeepEqual(r.Plan, want) {
+		t.Errorf("Plan = %v, want %v", r.Plan, want)
+	}
+}
+
+func TestRoot_PrefixesSudoWhenSudoTrue(t *testing.T) {
+	r := &Runner{DryRun: true, Sudo: true}
+	if err := r.Root("systemctl", "enable", "foo.timer"); err != nil {
+		t.Fatalf("dry-run Root should not error: %v", err)
+	}
+	want := []string{"sudo systemctl enable foo.timer"}
+	if !reflect.DeepEqual(r.Plan, want) {
+		t.Errorf("Plan = %v, want %v", r.Plan, want)
+	}
+}
+
+func TestRoot_NoSudoWhenSudoFalse(t *testing.T) {
+	r := &Runner{DryRun: true, Sudo: false}
+	if err := r.Root("systemctl", "enable", "foo.timer"); err != nil {
+		t.Fatalf("dry-run Root should not error: %v", err)
+	}
+	// Phase A is already root: the recorded line has no sudo prefix.
+	want := []string{"systemctl enable foo.timer"}
+	if !reflect.DeepEqual(r.Plan, want) {
+		t.Errorf("Plan = %v, want %v", r.Plan, want)
+	}
+}
+
+func TestRootShell_RecordsSameLineRegardlessOfSudo(t *testing.T) {
+	// RootShell records the plain `sh: <script>` line in both privilege modes; the
+	// sudo/no-sudo distinction is an execution detail, not a recorded one.
+	for _, sudo := range []bool{true, false} {
+		r := &Runner{DryRun: true, Sudo: sudo}
+		if err := r.RootShell("snapper -c root create-config /"); err != nil {
+			t.Fatalf("dry-run RootShell (sudo=%v) should not error: %v", sudo, err)
+		}
+		want := []string{"sh: snapper -c root create-config /"}
+		if !reflect.DeepEqual(r.Plan, want) {
+			t.Errorf("sudo=%v: Plan = %v, want %v", sudo, r.Plan, want)
+		}
+	}
+}
+
+func TestTry_RecordsAndIgnoresError(t *testing.T) {
+	r := &Runner{DryRun: true}
+	// Try has no return value; it must still record the command in dry-run.
+	r.Try("udevadm", "settle")
+	want := []string{"udevadm settle"}
+	if !reflect.DeepEqual(r.Plan, want) {
+		t.Errorf("Plan = %v, want %v", r.Plan, want)
+	}
+}
+
+func TestTryRoot_RecordsWithSudoPrefix(t *testing.T) {
+	r := &Runner{DryRun: true, Sudo: true}
+	r.TryRoot("snapper", "-c", "root", "set-config", "NUMBER_LIMIT=50")
+	want := []string{"sudo snapper -c root set-config NUMBER_LIMIT=50"}
+	if !reflect.DeepEqual(r.Plan, want) {
+		t.Errorf("Plan = %v, want %v", r.Plan, want)
+	}
+}
+
+func TestTryRoot_NoSudoWhenSudoFalse(t *testing.T) {
+	r := &Runner{DryRun: true, Sudo: false}
+	r.TryRoot("snapper", "-c", "root", "set-config", "NUMBER_LIMIT=50")
+	want := []string{"snapper -c root set-config NUMBER_LIMIT=50"}
+	if !reflect.DeepEqual(r.Plan, want) {
+		t.Errorf("Plan = %v, want %v", r.Plan, want)
+	}
+}
+
+func TestChroot_RecordsArchChrootPrefix(t *testing.T) {
+	r := &Runner{DryRun: true}
+	if err := r.Chroot("/mnt", "pacman", "-Syu"); err != nil {
+		t.Fatalf("dry-run Chroot should not error: %v", err)
+	}
+	want := []string{"arch-chroot /mnt pacman -Syu"}
+	if !reflect.DeepEqual(r.Plan, want) {
+		t.Errorf("Plan = %v, want %v", r.Plan, want)
+	}
+}
diff --git a/internal/stages/bootloader_test.go b/internal/stages/bootloader_test.go
index 2138c85..d29545a 100644
--- a/internal/stages/bootloader_test.go
+++ b/internal/stages/bootloader_test.go
@@ -17,9 +17,9 @@ grub:
 	mustContain(t, plan,
 		// each cmdline token appended to GRUB_CMDLINE_LINUX_DEFAULT, idempotently
 		`grep -qE 'GRUB_CMDLINE_LINUX_DEFAULT="[^"]*\bquiet\b' /etc/default/grub || `+
-			`sudo sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1quiet /' /etc/default/grub`,
+			`sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1quiet /' /etc/default/grub`,
 		`grep -qE 'GRUB_CMDLINE_LINUX_DEFAULT="[^"]*\bsplash\b' /etc/default/grub || `+
-			`sudo sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1splash /' /etc/default/grub`,
+			`sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1splash /' /etc/default/grub`,
 		// bootloader config regenerated after the cmdline/theme change
 		"grub-mkconfig -o /boot/grub/grub.cfg",
 	)
diff --git a/internal/stages/grubtheme.go b/internal/stages/grubtheme.go
index 8300010..d577a3d 100644
--- a/internal/stages/grubtheme.go
+++ b/internal/stages/grubtheme.go
@@ -28,6 +28,8 @@ func (grubtheme) Run(ctx *Context) error {
 			return fmt.Errorf("grub.theme.name required for source 'vinceliuice'")
 		}
 		// Clone + run the upstream installer (it sets GRUB_THEME and regenerates).
+		// cloneBuild runs as the user (it mktemp/git-clones into the user's tree),
+		// so install.sh keeps its inner `sudo` to gain root for the system writes.
 		if err := cloneBuild(ctx,
 			"--depth 1 https://github.com/vinceliuice/grub2-themes",
 			fmt.Sprintf("sudo ./install.sh -t %s", t.Name)); err != nil {
@@ -41,16 +43,17 @@ func (grubtheme) Run(ctx *Context) error {
 			return fmt.Errorf("grub.theme.name and grub.theme.url required for source 'url'")
 		}
 		dest := "/boot/grub/themes/" + t.Name
-		if err := ctx.R.Shell(fmt.Sprintf(
+		// Whole pipeline runs as root via RootShell, so no inner per-command sudo.
+		if err := ctx.R.RootShell(fmt.Sprintf(
 			`tmp="$(mktemp -d)" && curl -fsSL %[1]q -o "$tmp/theme.tar.gz" && `+
-				`sudo mkdir -p %[2]q && sudo tar -xf "$tmp/theme.tar.gz" -C %[2]q --strip-components=1 && rm -rf "$tmp"`,
+				`mkdir -p %[2]q && tar -xf "$tmp/theme.tar.gz" -C %[2]q --strip-components=1 && rm -rf "$tmp"`,
 			t.URL, dest)); err != nil {
 			return err
 		}
-		if err := ctx.R.Shell(fmt.Sprintf(
-			`sudo sed -i '/^#\?GRUB_THEME=/d' /etc/default/grub && `+
-				`echo 'GRUB_THEME="%s/theme.txt"' | sudo tee -a /etc/default/grub >/dev/null && `+
-				`sudo grub-mkconfig -o /boot/grub/grub.cfg`, dest)); err != nil {
+		if err := ctx.R.RootShell(fmt.Sprintf(
+			`sed -i '/^#\?GRUB_THEME=/d' /etc/default/grub && `+
+				`echo 'GRUB_THEME="%s/theme.txt"' >>/etc/default/grub && `+
+				`grub-mkconfig -o /boot/grub/grub.cfg`, dest)); err != nil {
 			return err
 		}
 		ui.OK("GRUB theme %q installed from url", t.Name)
diff --git a/internal/stages/helpers.go b/internal/stages/helpers.go
index a48317b..e06c001 100644
--- a/internal/stages/helpers.go
+++ b/internal/stages/helpers.go
@@ -32,15 +32,16 @@ func ensureKernelParam(ctx *Context, tok string) error {
 	if ctx.Cfg.Bootloader.EffectiveKind() == "systemd-boot" {
 		// /etc/kernel/cmdline is a single space-separated line. Add tok only if its
 		// word boundary isn't already present; create the file if it doesn't exist.
-		return ctx.R.Shell(fmt.Sprintf(
+		// The whole pipeline runs as root via RootShell, so no inner per-command sudo.
+		return ctx.R.RootShell(fmt.Sprintf(
 			`grep -qE '\b%[1]s\b' /etc/kernel/cmdline 2>/dev/null || `+
-				`{ sudo touch /etc/kernel/cmdline && `+
-				`sudo sed -i -E '$ s/$/ %[1]s/' /etc/kernel/cmdline; }`,
+				`{ touch /etc/kernel/cmdline && `+
+				`sed -i -E '$ s/$/ %[1]s/' /etc/kernel/cmdline; }`,
 			tok))
 	}
-	return ctx.R.Shell(fmt.Sprintf(
+	return ctx.R.RootShell(fmt.Sprintf(
 		`grep -qE 'GRUB_CMDLINE_LINUX_DEFAULT="[^"]*\b%[1]s\b' /etc/default/grub || `+
-			`sudo sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1%[1]s /' /etc/default/grub`,
+			`sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1%[1]s /' /etc/default/grub`,
 		tok))
 }
 
diff --git a/internal/stages/snapper.go b/internal/stages/snapper.go
index 7531b9e..040fde5 100644
--- a/internal/stages/snapper.go
+++ b/internal/stages/snapper.go
@@ -49,9 +49,9 @@ func (s snapper) Run(ctx *Context) error {
 
 	// Create the root config only when it does not already exist — create-config
 	// errors out on an existing config, so grep-guard it for idempotent re-runs.
-	if err := ctx.R.Shell(
+	if err := ctx.R.RootShell(
 		`snapper list-configs 2>/dev/null | grep -qw root || ` +
-			`sudo snapper -c root create-config /`,
+			`snapper -c root create-config /`,
 	); err != nil {
 		return err
 	}
@@ -59,7 +59,7 @@ func (s snapper) Run(ctx *Context) error {
 	// Sane retention: keep the timeline tidy and cap the number of snapshots.
 	// Best-effort (Try) — a freshly-created config has these set, and a re-run
 	// must not fail the stage if set-config is fussy about an already-present key.
-	ctx.R.Try("sudo", "snapper", "-c", "root", "set-config",
+	ctx.R.TryRoot("snapper", "-c", "root", "set-config",
 		"TIMELINE_LIMIT_HOURLY=5",
 		"TIMELINE_LIMIT_DAILY=7",
 		"TIMELINE_LIMIT_WEEKLY=4",
diff --git a/internal/stages/snapper_test.go b/internal/stages/snapper_test.go
index 7ab62ec..54274bc 100644
--- a/internal/stages/snapper_test.go
+++ b/internal/stages/snapper_test.go
@@ -22,14 +22,15 @@ disks:
 
 	// snap-pac is always installed (it has no binary, so it goes through Root).
 	mustContain(t, plan, "sudo pacman -S --needed --noconfirm snap-pac")
-	// Root config creation is grep-guarded so a re-run with an existing config
-	// does not fail.
-	if !strings.Contains(j, "snapper -c root create-config /") {
-		t.Errorf("plan should create the root snapper config, got:\n%s", j)
-	}
-	if !strings.Contains(j, "snapper list-configs") {
-		t.Errorf("create-config should be grep-guarded on existing configs, got:\n%s", j)
+	// Root config creation runs as root via RootShell (recorded as a plain
+	// `sh: ...` line, no inner sudo) and is grep-guarded so a re-run with an
+	// existing config does not fail.
+	if !strings.Contains(j, "sh: snapper list-configs 2>/dev/null | grep -qw root || snapper -c root create-config /") {
+		t.Errorf("plan should create the root snapper config via RootShell, got:\n%s", j)
 	}
+	// set-config runs best-effort as root (TryRoot) — recorded with the sudo
+	// prefix in Phase B.
+	mustContain(t, plan, "sudo snapper -c root set-config")
 	// Timers are enabled.
 	mustContain(t,
 		plan,
diff --git a/internal/stages/systemd_boot_test.go b/internal/stages/systemd_boot_test.go
index 991fe35..62a98af 100644
--- a/internal/stages/systemd_boot_test.go
+++ b/internal/stages/systemd_boot_test.go
@@ -16,13 +16,14 @@ grub:
   cmdline_extra: "quiet splash"
 `)
 	mustContain(t, plan,
-		// each cmdline token appended to /etc/kernel/cmdline, idempotently
+		// each cmdline token appended to /etc/kernel/cmdline, idempotently. The
+		// whole pipeline runs as root via RootShell, so no inner per-command sudo.
 		`grep -qE '\bquiet\b' /etc/kernel/cmdline 2>/dev/null || `+
-			`{ sudo touch /etc/kernel/cmdline && `+
-			`sudo sed -i -E '$ s/$/ quiet/' /etc/kernel/cmdline; }`,
+			`{ touch /etc/kernel/cmdline && `+
+			`sed -i -E '$ s/$/ quiet/' /etc/kernel/cmdline; }`,
 		`grep -qE '\bsplash\b' /etc/kernel/cmdline 2>/dev/null || `+
-			`{ sudo touch /etc/kernel/cmdline && `+
-			`sudo sed -i -E '$ s/$/ splash/' /etc/kernel/cmdline; }`,
+			`{ touch /etc/kernel/cmdline && `+
+			`sed -i -E '$ s/$/ splash/' /etc/kernel/cmdline; }`,
 		// systemd-boot refresh, no grub.cfg regeneration
 		"bootctl update",
 	)
@@ -43,9 +44,9 @@ grub:
 `)
 	mustContain(t, plan,
 		`grep -qE 'GRUB_CMDLINE_LINUX_DEFAULT="[^"]*\bquiet\b' /etc/default/grub || `+
-			`sudo sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1quiet /' /etc/default/grub`,
+			`sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1quiet /' /etc/default/grub`,
 		`grep -qE 'GRUB_CMDLINE_LINUX_DEFAULT="[^"]*\bsplash\b' /etc/default/grub || `+
-			`sudo sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1splash /' /etc/default/grub`,
+			`sed -i -E 's/(GRUB_CMDLINE_LINUX_DEFAULT=")/\1splash /' /etc/default/grub`,
 		"grub-mkconfig -o /boot/grub/grub.cfg",
 	)
 	mustNotContain(t, plan,

From 8c6060cf7f3fdac4d6c75fd8f63ffb41a3f75ec6 Mon Sep 17 00:00:00 2001
From: Adam Hall <adam.hall.1992@live.com>
Date: Mon, 22 Jun 2026 19:39:31 +0930
Subject: [PATCH 3/5] fix(config): reject luks_on_lvm; require hook names for
 safe merge
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Issue #1: luks_on_lvm was silently rendered as lvm_on_luks (encrypting the
PVs, the wrong topology) because buildEncryption collapsed both into one
branch. The luks-on-lvm topology is unimplemented, so remove it from the
encryption oneof set and its renderer branch — Validate() now surfaces a
clear "must be one of" error instead of doing the wrong thing silently.

Issue #4: configsrc merge-by-name downgrades to wholesale-replace when any
slice element lacks a name, so a single nameless hook in an overriding layer
silently dropped the base layer's hooks. Make Hook.Name required so a
nameless hook fails validation, and document the sharp edge in merge.go.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 internal/archinstall/archinstall.go          | 23 +++---
 internal/config/config.go                    | 20 +++--
 internal/config/encryption_luksonlvm_test.go | 48 ++++++++++++
 internal/config/hook_name_required_test.go   | 82 ++++++++++++++++++++
 internal/config/hooks_config_test.go         | 10 +--
 internal/configsrc/merge.go                  | 13 +++-
 6 files changed, 176 insertions(+), 20 deletions(-)
 create mode 100644 internal/config/encryption_luksonlvm_test.go
 create mode 100644 internal/config/hook_name_required_test.go

diff --git a/internal/archinstall/archinstall.go b/internal/archinstall/archinstall.go
index 0c21ece..0f12b28 100644
--- a/internal/archinstall/archinstall.go
+++ b/internal/archinstall/archinstall.go
@@ -141,13 +141,16 @@ type LvmConfiguration struct {
 // DiskEncryption mirrors archinstall's DiskEncryption.json(). encryption_type is
 // "luks" (encrypt the single root partition) or "lvm_on_luks" (encrypt the PV
 // partitions under LVM). partitions holds the encrypted partition obj_ids (the
-// root partition for luks, the PV partitions for lvm_on_luks); lvm_volumes holds
-// LV obj_ids for the luks_on_lvm variant. The password is supplied separately as
-// the top-level encryption_password field.
+// root partition for luks, the PV partitions for lvm_on_luks). The password is
+// supplied separately as the top-level encryption_password field.
 type DiskEncryption struct {
 	EncryptionType string   `json:"encryption_type"` // "luks"/"lvm_on_luks"
 	Partitions     []string `json:"partitions"`
-	LvmVolumes     []string `json:"lvm_volumes"`
+	// LvmVolumes carries LV obj_ids for a per-LV (luks-on-lvm) encryption
+	// topology. That topology is reserved/unused for now — no encryption_type
+	// currently populates it — so it always renders as an empty list. Kept on
+	// the struct so the JSON shape matches archinstall's DiskEncryption.json().
+	LvmVolumes []string `json:"lvm_volumes"`
 }
 
 // DiskConfig mirrors DiskLayoutConfiguration.json(). disk_encryption is nested
@@ -333,14 +336,16 @@ func Build(cfg *config.Config, geom Geometry, password string) (*Config, *Creds,
 // and lvm config. For lvm_on_luks the encrypted partitions are exactly the VG's
 // LvmPvs; for luks it is the single partition mounted at "/". archinstall rejects
 // LVM encryption with more than two PV partitions (device_handler.py) — this is
-// guarded here as well as in config validation.
+// guarded here as well as in config validation. Only "luks" and "lvm_on_luks"
+// are accepted (config validation restricts the set); a per-LV luks-on-lvm
+// topology is not implemented.
 //
-// VM-validation-pending: the encryption_type values, the partitions/lvm_volumes
-// obj_id wiring, and the 2-PV limit are reverse-engineered from archinstall source
-// and must be confirmed against a real archinstall run in a VM.
+// VM-validation-pending: the encryption_type values, the partitions obj_id
+// wiring, and the 2-PV limit are reverse-engineered from archinstall source and
+// must be confirmed against a real archinstall run in a VM.
 func buildEncryption(encType string, devices []Device, lvm *LvmConfiguration) (*DiskEncryption, error) {
 	switch encType {
-	case "lvm_on_luks", "luks_on_lvm":
+	case "lvm_on_luks":
 		if lvm == nil || len(lvm.VolGroups) == 0 {
 			return nil, fmt.Errorf("%s encryption requires an lvm layout", encType)
 		}
diff --git a/internal/config/config.go b/internal/config/config.go
index e43ddb2..20cccb1 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -229,10 +229,16 @@ type Subvol struct {
 	Mountpoint string `yaml:"mountpoint" validate:"required"`
 }
 
-// Encryption enables LUKS. Type: "luks" (encrypt the single root partition,
-// for plain/btrfs) or "lvm_on_luks" (encrypt the PV partitions under LVM).
+// Encryption enables LUKS. Type is one of:
+//   - "luks": encrypt the single root partition (for the plain/btrfs layouts).
+//   - "lvm_on_luks": encrypt the PV partitions under LVM (for the lvm layout).
+//
+// The "luks_on_lvm" topology (encrypt individual LVs on top of an unencrypted
+// VG) is intentionally NOT accepted: it is unimplemented in the archinstall
+// renderer, so allowing it would silently produce the wrong (lvm_on_luks)
+// layout. Rejecting it here surfaces a clear validation error instead.
 type Encryption struct {
-	Type string `yaml:"type" validate:"required,oneof=luks lvm_on_luks luks_on_lvm"`
+	Type string `yaml:"type" validate:"required,oneof=luks lvm_on_luks"`
 }
 
 // SetupConfig drives the Phase B 85-setup stage, which runs after chezmoi has
@@ -269,8 +275,12 @@ type Clone struct {
 // Script and Dir have a leading `~` expanded to the user's home at run time.
 // Script existence is NOT checked at validate time: a hook script may be
 // produced by an earlier hook or stage in the same run.
+//
+// Name is required: it identifies the hook for layered-config merge-by-name (a
+// nameless element would silently flip the whole hooks slice from merge to
+// wholesale replace — see internal/configsrc/merge.go) and for diagnostics.
 type Hook struct {
-	Name   string            `yaml:"name"`
+	Name   string            `yaml:"name"   validate:"required"`
 	At     string            `yaml:"at"     validate:"required,hookpoint"`
 	Run    string            `yaml:"run"    validate:"required_without=Script"`
 	Script string            `yaml:"script" validate:"omitempty"`
@@ -509,7 +519,7 @@ func (c *Config) encryptionErrors() []error {
 	}
 	layout := d.EffectiveLayout()
 	switch d.Encryption.Type {
-	case "lvm_on_luks", "luks_on_lvm":
+	case "lvm_on_luks":
 		if layout != "lvm" {
 			errs = append(errs, fmt.Errorf("disks.encryption.type %s requires the lvm layout", d.Encryption.Type))
 		}
diff --git a/internal/config/encryption_luksonlvm_test.go b/internal/config/encryption_luksonlvm_test.go
new file mode 100644
index 0000000..13ed6a2
--- /dev/null
+++ b/internal/config/encryption_luksonlvm_test.go
@@ -0,0 +1,48 @@
+package config
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestEncryptionLuksOnLvmRejected pins that disks.encryption.type: luks_on_lvm is
+// no longer an accepted value. The archinstall renderer never implemented the
+// per-LV luks-on-lvm topology — it collapsed it into lvm_on_luks, encrypting the
+// PVs (the wrong layout) silently. Removing it from the oneof set makes Validate()
+// surface a clear "must be one of" error instead.
+func TestEncryptionLuksOnLvmRejected(t *testing.T) {
+	const y = `
+system:
+  hostname: arch
+  timezone: Europe/London
+  locale: en_GB.UTF-8
+  keymap: uk
+user:
+  name: adam
+pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode]
+kernel:
+  base: [linux]
+disks:
+  esp:
+    device: /dev/nvme0n1
+    size: 1GiB
+  swap:
+    type: swapfile
+    size: 4GiB
+  layout: lvm
+  lvm: {vg: vg0, lv: root, filesystem: ext4, pvs: [/dev/nvme0n1p2]}
+  encryption: {type: luks_on_lvm}
+`
+	err := validateYAML(t, y)
+	if err == nil {
+		t.Fatalf("want error for luks_on_lvm, got nil")
+	}
+	msg := err.Error()
+	if !strings.Contains(msg, "must be one of") {
+		t.Fatalf("error %q does not contain %q", msg, "must be one of")
+	}
+	// The accepted set must be named so the user knows the valid choices.
+	if !strings.Contains(msg, "luks") || !strings.Contains(msg, "lvm_on_luks") {
+		t.Fatalf("error %q should list the accepted types (luks, lvm_on_luks)", msg)
+	}
+}
diff --git a/internal/config/hook_name_required_test.go b/internal/config/hook_name_required_test.go
new file mode 100644
index 0000000..46fe1fe
--- /dev/null
+++ b/internal/config/hook_name_required_test.go
@@ -0,0 +1,82 @@
+package config
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestHookNameRequired pins that every hook must carry a name. A nameless hook
+// would silently flip the layered-config merge of the hooks list from
+// merge-by-name to wholesale replace (see internal/configsrc/merge.go), dropping
+// the base layer's hooks. Requiring a name fails validation instead, and the
+// failure is reported against the indexed YAML path hooks[N].name.
+func TestHookNameRequired(t *testing.T) {
+	const y = `
+system:
+  hostname: arch
+  timezone: Europe/London
+  locale: en_GB.UTF-8
+  keymap: uk
+user:
+  name: adam
+pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode]
+kernel:
+  base: [linux]
+disks:
+  esp:
+    device: /dev/nvme0n1
+    size: 1GiB
+  swap:
+    type: swapfile
+    size: 4GiB
+  layout: lvm
+  lvm: {vg: vg0, lv: root, filesystem: ext4, pvs: [/dev/nvme0n1p2]}
+hooks:
+  - at: post-install
+    run: echo hi
+`
+	err := validateYAML(t, y)
+	if err == nil {
+		t.Fatalf("want error for nameless hook, got nil")
+	}
+	msg := err.Error()
+	if !strings.Contains(msg, "hooks[0].name") {
+		t.Fatalf("error %q should reference hooks[0].name", msg)
+	}
+	if !strings.Contains(msg, "is required") {
+		t.Fatalf("error %q should say the field is required", msg)
+	}
+}
+
+// TestHookWithNameValid confirms a named hook still validates cleanly, so the new
+// requirement doesn't break correctly-written hooks.
+func TestHookWithNameValid(t *testing.T) {
+	const y = `
+system:
+  hostname: arch
+  timezone: Europe/London
+  locale: en_GB.UTF-8
+  keymap: uk
+user:
+  name: adam
+pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode]
+kernel:
+  base: [linux]
+disks:
+  esp:
+    device: /dev/nvme0n1
+    size: 1GiB
+  swap:
+    type: swapfile
+    size: 4GiB
+  layout: lvm
+  lvm: {vg: vg0, lv: root, filesystem: ext4, pvs: [/dev/nvme0n1p2]}
+hooks:
+  - name: greet
+    at: post-install
+    run: echo hi
+`
+	if err := validateYAML(t, y); err != nil {
+		t.Fatalf("want valid, got error: %v", err)
+	}
+}
diff --git a/internal/config/hooks_config_test.go b/internal/config/hooks_config_test.go
index b305642..233f0bd 100644
--- a/internal/config/hooks_config_test.go
+++ b/internal/config/hooks_config_test.go
@@ -12,19 +12,19 @@ func validateHook(h Hook) error {
 }
 
 func TestHook_ValidGlobalPoint(t *testing.T) {
-	if err := validateHook(Hook{At: "post-install", Run: "echo hi"}); err != nil {
+	if err := validateHook(Hook{Name: "h", At: "post-install", Run: "echo hi"}); err != nil {
 		t.Errorf("valid hook should pass: %v", err)
 	}
 }
 
 func TestHook_ValidPerStagePoint(t *testing.T) {
-	if err := validateHook(Hook{At: "before:packages", Run: "echo hi"}); err != nil {
+	if err := validateHook(Hook{Name: "h", At: "before:packages", Run: "echo hi"}); err != nil {
 		t.Errorf("before:packages is well-formed and should pass hookpoint: %v", err)
 	}
 }
 
 func TestHook_BadAtFailsHookpoint(t *testing.T) {
-	err := validateHook(Hook{At: "midway", Run: "echo hi"})
+	err := validateHook(Hook{Name: "h", At: "midway", Run: "echo hi"})
 	if err == nil {
 		t.Fatal("bad at should fail hookpoint")
 	}
@@ -34,7 +34,7 @@ func TestHook_BadAtFailsHookpoint(t *testing.T) {
 }
 
 func TestHook_EmptyStageTokenFailsHookpoint(t *testing.T) {
-	err := validateHook(Hook{At: "before:", Run: "echo hi"})
+	err := validateHook(Hook{Name: "h", At: "before:", Run: "echo hi"})
 	if err == nil {
 		t.Fatal("before: with empty stage should fail hookpoint")
 	}
@@ -44,7 +44,7 @@ func TestHook_EmptyStageTokenFailsHookpoint(t *testing.T) {
 }
 
 func TestHook_NeitherRunNorScriptFails(t *testing.T) {
-	err := validateHook(Hook{At: "post-install"})
+	err := validateHook(Hook{Name: "h", At: "post-install"})
 	if err == nil {
 		t.Fatal("hook with neither run nor script should fail required_without")
 	}
diff --git a/internal/configsrc/merge.go b/internal/configsrc/merge.go
index ae8bc4b..0360062 100644
--- a/internal/configsrc/merge.go
+++ b/internal/configsrc/merge.go
@@ -108,6 +108,14 @@ func mergeMaps(base, over map[string]any) (map[string]any, error) {
 
 // mergeSlices applies the slice strategy: union+dedup for string slices,
 // key-merge-by-name for slices of maps that carry a "name", replace otherwise.
+//
+// Sharp edge: the merge-by-name path applies ONLY when every element of BOTH
+// slices carries a non-empty "name" (see structuredByName). If even one element
+// in either layer lacks a name, the whole slice is replaced wholesale instead of
+// merged — so a single nameless element in an overriding layer silently drops the
+// base's elements. This is why structured-list schema fields whose lists are
+// meant to merge across layers (e.g. config Hooks) must make "name" required, so
+// a nameless element fails validation rather than degrading the merge.
 func mergeSlices(base, over []any) (any, error) {
 	if allStrings(base) && allStrings(over) {
 		return unionStrings(base, over), nil
@@ -146,7 +154,10 @@ func unionStrings(base, over []any) []any {
 }
 
 // structuredByName reports whether s is a non-empty slice whose every element is
-// a map carrying a non-empty string "name" key.
+// a map carrying a non-empty string "name" key. It is the gate for merge-by-name:
+// merge-by-name applies only when this holds for BOTH the base and the over
+// slice. A single nameless element makes this return false, which (in mergeSlices)
+// downgrades the merge to a wholesale replace — see the mergeSlices doc comment.
 func structuredByName(s []any) bool {
 	if len(s) == 0 {
 		return false

From 236dba87048d64c67b5470fc7ab5e2500064b8b3 Mon Sep 17 00:00:00 2001
From: Adam Hall <adam.hall.1992@live.com>
Date: Mon, 22 Jun 2026 19:39:45 +0930
Subject: [PATCH 4/5] fix(install): correct encrypted root handling,
 kernel-default gating, dry-run geometry warning

Phase-A postInstall fixes:

- Issue #2: encrypted layouts no longer remount the still-encrypted
  plaintext device (rootDevice returns the pre-LUKS path). Until the LUKS
  remount is implemented + VM-validated, encrypted installs skip the
  remount, all chroot post-install steps, and Phase B staging, emitting
  loud warnings (incl. that staging must be done manually). Unencrypted
  post-install remounts are now CHECKED (Root, aborts on failure) instead
  of best-effort Try; umounts stay best-effort.

- Issue #3: installKernels now runs when kernel.default is set even with
  no extra packages, so a default pointing at a base kernel is actually
  pinned in the bootloader. installKernels skips the empty pacman -S.

- Issue #11: probeGeometry warns when it falls back to the 512 GiB
  placeholder for an absent device under dry-run, instead of silently
  masking a typo'd device path.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 internal/stages/archinstall.go             |  39 ++++++-
 internal/stages/postinstall.go             |  11 +-
 internal/stages/postinstall_gating_test.go | 120 +++++++++++++++++++++
 3 files changed, 164 insertions(+), 6 deletions(-)
 create mode 100644 internal/stages/postinstall_gating_test.go

diff --git a/internal/stages/archinstall.go b/internal/stages/archinstall.go
index baadba3..61ad630 100644
--- a/internal/stages/archinstall.go
+++ b/internal/stages/archinstall.go
@@ -103,14 +103,39 @@ func (archinstallStage) Run(ctx *Context) error {
 // remount the root LV and the ESP (kernels/GRUB live on /boot) for the chroot
 // work, then unmount.
 func postInstall(ctx *Context) error {
+	// Encrypted layouts (Issue #2): rootDevice() returns the *plaintext* device
+	// (the bare partition or /dev/VG/LV), but after archinstall provisions LUKS the
+	// real root filesystem lives behind a /dev/mapper/* device that requires a LUKS
+	// remount (cryptsetup open with the passphrase) we have NOT implemented yet —
+	// encryption is VM-validation-pending. Mounting the still-encrypted device and
+	// running chroot work against it would either fail or, worse, silently no-op.
+	//
+	// Conservative behaviour until the LUKS remount is implemented and VM-validated:
+	// skip the remount and every chroot-dependent post-install step (swapfile,
+	// locales, repos, kernels). We also cannot stage the Phase B binary/config,
+	// because staging copies into /mnt/home/<user> which requires the (correctly
+	// mapped+mounted) root — so we warn that Phase B staging must be done manually.
+	// This keeps the encrypted path coherent and loud rather than silently wrong.
+	if ctx.Cfg.Disks.Encryption != nil {
+		ui.Warn("encrypted install: custom repos, kernels, locale extras, and swapfile are NOT applied yet (LUKS remount not implemented)")
+		ui.Warn("encrypted install: Phase B binary/config staging is SKIPPED — after first boot, copy the archwright binary + config.yaml into your home and run bootstrap manually")
+		return nil
+	}
+
 	rootDev, err := rootDevice(ctx.Cfg)
 	if err != nil {
 		return err
 	}
 	esp := partDev(ctx.Cfg.Disks.ESP.Device, 1)
 
-	ctx.R.Try("mount", rootDev, "/mnt")
-	ctx.R.Try("mount", esp, "/mnt/boot")
+	// Checked mounts (Issue #2): a failed remount must abort, not silently run the
+	// chroot steps against an empty /mnt. (The umounts below stay best-effort.)
+	if err := ctx.R.Root("mount", rootDev, "/mnt"); err != nil {
+		return err
+	}
+	if err := ctx.R.Root("mount", esp, "/mnt/boot"); err != nil {
+		return err
+	}
 
 	if err := setupSwapfile(ctx); err != nil {
 		return err
@@ -127,7 +152,10 @@ func postInstall(ctx *Context) error {
 			return err
 		}
 	}
-	if len(ctx.Cfg.Kernel.Packages) > 0 {
+	// Run installKernels when there are extra kernel packages to install OR a
+	// default kernel to pin (Issue #3): kernel.default may name a base kernel with
+	// no extra packages, and that default must still be written to the bootloader.
+	if len(ctx.Cfg.Kernel.Packages) > 0 || ctx.Cfg.Kernel.Default != "" {
 		if err := installKernels(ctx, ctx.Cfg.Kernel); err != nil {
 			return err
 		}
@@ -273,6 +301,11 @@ func probeGeometry(devs []string, dryRun bool) (archinstall.Geometry, error) {
 		out, err := exec.Command("blockdev", "--getsize64", dev).Output()
 		if err != nil {
 			if dryRun {
+				// Absent device under dry-run: fall back to a representative size so
+				// the rendered plan is still meaningful, but warn loudly (Issue #11)
+				// — silence here masks a typo'd device path, making the placeholder
+				// look like a successful probe.
+				ui.Warn("device not present; using 512 GiB placeholder size (dry-run)", "device", dev)
 				geom[dev] = 512 * (1 << 30) // 512 GiB placeholder
 				continue
 			}
diff --git a/internal/stages/postinstall.go b/internal/stages/postinstall.go
index e33031a..6c737f9 100644
--- a/internal/stages/postinstall.go
+++ b/internal/stages/postinstall.go
@@ -94,9 +94,14 @@ func configureLocales(ctx *Context, locales []string) error {
 // "make this kernel the default + regenerate boot config" tail differs per
 // bootloader, branched off the configured bootloader (defaulting to grub).
 func installKernels(ctx *Context, k config.KernelConfig) error {
-	args := append([]string{"-S", "--needed", "--noconfirm"}, k.Packages...)
-	if err := chrootCmd(ctx, "pacman", args...); err != nil {
-		return err
+	// Skip the pacman install when there are no extra kernel packages (Issue #3):
+	// postInstall now also calls this purely to pin a base kernel as the default,
+	// and `pacman -S` with zero package arguments would error.
+	if len(k.Packages) > 0 {
+		args := append([]string{"-S", "--needed", "--noconfirm"}, k.Packages...)
+		if err := chrootCmd(ctx, "pacman", args...); err != nil {
+			return err
+		}
 	}
 	if k.ReplaceStock {
 		// Validated to be safe: at least one replacement kernel is installed above.
diff --git a/internal/stages/postinstall_gating_test.go b/internal/stages/postinstall_gating_test.go
new file mode 100644
index 0000000..7a76e88
--- /dev/null
+++ b/internal/stages/postinstall_gating_test.go
@@ -0,0 +1,120 @@
+package stages
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/AdamJHall/archwright/internal/config"
+	"github.com/AdamJHall/archwright/internal/run"
+	"gopkg.in/yaml.v3"
+)
+
+// These tests cover the Phase-A postInstall fixes:
+//   - encrypted layouts skip the chroot/remount/staging steps (Issue #2),
+//   - the post-install remounts are checked, not best-effort (Issue #2),
+//   - installKernels runs when only kernel.default is set (Issue #3).
+// They use self-contained YAML so they don't touch the shared fixtures.
+
+// runArchinstall runs the archinstall stage in dry-run against a caller-supplied
+// config and returns the recorded plan. Unlike planForCfg it asserts the stage
+// succeeds and is reused across the cases below.
+func runArchinstall(t *testing.T, yamlBody string) []string {
+	t.Helper()
+	var c config.Config
+	if err := yaml.Unmarshal([]byte(yamlBody), &c); err != nil {
+		t.Fatalf("unmarshal config: %v", err)
+	}
+	ss := For(Install, "archinstall")
+	if len(ss) != 1 {
+		t.Fatalf("expected exactly one archinstall stage, got %d", len(ss))
+	}
+	r := &run.Runner{DryRun: true}
+	ctx := &Context{Cfg: &c, R: r, AssumeYes: true, ConfigPath: "/tmp/config.yaml"}
+	if err := ss[0].Run(ctx); err != nil {
+		t.Fatalf("archinstall stage errored in dry-run: %v", err)
+	}
+	return r.Plan
+}
+
+const encLVMYAML = `
+system: {hostname: arch-luks, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk}
+user: {name: adam}
+pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode]
+kernel: {base: [linux], packages: [linux-cachyos], default: linux-cachyos}
+repos:
+  - name: cachyos
+    server: https://example.invalid/$repo
+disks:
+  layout: lvm
+  esp: {device: /dev/nvme0n1, size: 1GiB}
+  swap: {type: swapfile, size: 4GiB}
+  lvm: {vg: vg0, lv: root, filesystem: ext4, pvs: [/dev/nvme0n1p2]}
+  encryption: {type: lvm_on_luks}
+`
+
+// Encrypted installs must NOT mount the (still-encrypted) device or run any
+// chroot/remount/staging steps: the plan ends at archinstall, and none of the
+// post-install side effects appear (Issue #2). archinstall itself still runs.
+func TestPostInstall_EncryptedSkipsChrootAndStaging(t *testing.T) {
+	plan := runArchinstall(t, encLVMYAML)
+	mustContain(t, plan, "archinstall --config")
+	// No remount of the plaintext device, no swapfile, no repo setup, no kernel
+	// install, no Phase B staging — all skipped for the encrypted layout.
+	mustNotContain(t, plan,
+		"mount /dev/vg0/root /mnt",
+		"mount /dev/nvme0n1p1 /mnt/boot",
+		"of=/mnt/swapfile",
+		"arch-chroot /mnt pacman -Sy",
+		"linux-cachyos",
+		"/mnt/home/adam",
+	)
+}
+
+const kernelDefaultOnlyYAML = `
+system: {hostname: arch-kd, timezone: Europe/London, locale: en_GB.UTF-8, keymap: uk}
+user: {name: adam}
+pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode]
+kernel: {base: [linux, linux-lts], default: linux-lts}
+disks:
+  layout: plain
+  esp: {device: /dev/nvme0n1, size: 1GiB}
+  swap: {type: none}
+  plain: {device: /dev/nvme0n1, filesystem: ext4}
+`
+
+// kernel.default pointing at a base kernel with no extra packages must still pin
+// the default in the bootloader (Issue #3): installKernels runs, the pacman -S
+// install is skipped (no packages), and GRUB's default is written + regenerated.
+func TestPostInstall_KernelDefaultOnlyPinsBootloader(t *testing.T) {
+	plan := runArchinstall(t, kernelDefaultOnlyYAML)
+	mustContain(t, plan,
+		`GRUB_TOP_LEVEL="/boot/vmlinuz-linux-lts"`,
+		"arch-chroot /mnt grub-mkconfig -o /boot/grub/grub.cfg",
+	)
+	// No packages to install -> no `pacman -S` kernel install.
+	joined := strings.Join(plan, "\n")
+	if strings.Contains(joined, "pacman -S --needed --noconfirm linux-lts") {
+		t.Errorf("did not expect a kernel package install for a base-only default.\nplan:\n%s", joined)
+	}
+}
+
+// installKernels with no packages but a default set still pins the bootloader and
+// skips the empty `pacman -S` (Issue #3 guard), tested at the function level so it
+// is independent of the full archinstall stage plan above.
+func TestInstallKernels_NoPackagesSkipsPacman(t *testing.T) {
+	r := &run.Runner{DryRun: true}
+	ctx := &Context{
+		Cfg: &config.Config{Kernel: config.KernelConfig{Base: []string{"linux"}, Default: "linux"}},
+		R:   r,
+	}
+	if err := installKernels(ctx, ctx.Cfg.Kernel); err != nil {
+		t.Fatalf("installKernels: %v", err)
+	}
+	joined := strings.Join(r.Plan, "\n")
+	if strings.Contains(joined, "pacman -S") {
+		t.Errorf("expected no pacman -S with empty packages.\nplan:\n%s", joined)
+	}
+	if !strings.Contains(joined, `GRUB_TOP_LEVEL="/boot/vmlinuz-linux"`) {
+		t.Errorf("expected GRUB default to be pinned.\nplan:\n%s", joined)
+	}
+}

From f4c8c62d26b99f92161ce067c01b8e3a116cd95d Mon Sep 17 00:00:00 2001
From: Adam Hall <adam.hall.1992@live.com>
Date: Mon, 22 Jun 2026 19:42:05 +0930
Subject: [PATCH 5/5] refactor: dedup partDev into archinstall.PartDev (#9)

The identical partDev was defined in both internal/archinstall and
internal/stages. Export it once from archinstall (stages already imports
that package) and drop the stages copy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 internal/archinstall/archinstall.go |  8 +++++---
 internal/stages/archinstall.go      |  4 ++--
 internal/stages/postinstall.go      | 11 -----------
 3 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/internal/archinstall/archinstall.go b/internal/archinstall/archinstall.go
index 810f8dc..d74e0fa 100644
--- a/internal/archinstall/archinstall.go
+++ b/internal/archinstall/archinstall.go
@@ -433,7 +433,7 @@ func (b *lvmBuilder) build(geom Geometry) ([]Device, *LvmConfiguration, error) {
 		}
 	}
 	if disk1PV == "" {
-		return nil, nil, fmt.Errorf("no LVM PV found on disk 1 (%s); expected a partition like %s", disk1, partDev(disk1, 2))
+		return nil, nil, fmt.Errorf("no LVM PV found on disk 1 (%s); expected a partition like %s", disk1, PartDev(disk1, 2))
 	}
 
 	// Disk 1: ESP + PV partition (no swap partition). Sizes computed from geometry.
@@ -730,8 +730,10 @@ func splitLocale(locale string) (lang, enc string) {
 	return locale, "UTF-8"
 }
 
-// partDev mirrors stages.partDev for the disk-1 PV error hint.
-func partDev(dev string, n int) string {
+// PartDev returns the kernel partition device for a base device and number:
+// /dev/sda -> /dev/sda1, but /dev/nvme0n1 -> /dev/nvme0n1p1. It is the single
+// shared definition; the stages package calls it rather than duplicating it.
+func PartDev(dev string, n int) string {
 	if len(dev) > 0 {
 		if last := dev[len(dev)-1]; last >= '0' && last <= '9' {
 			return fmt.Sprintf("%sp%d", dev, n)
diff --git a/internal/stages/archinstall.go b/internal/stages/archinstall.go
index 61ad630..7a4759b 100644
--- a/internal/stages/archinstall.go
+++ b/internal/stages/archinstall.go
@@ -126,7 +126,7 @@ func postInstall(ctx *Context) error {
 	if err != nil {
 		return err
 	}
-	esp := partDev(ctx.Cfg.Disks.ESP.Device, 1)
+	esp := archinstall.PartDev(ctx.Cfg.Disks.ESP.Device, 1)
 
 	// Checked mounts (Issue #2): a failed remount must abort, not silently run the
 	// chroot steps against an empty /mnt. (The umounts below stay best-effort.)
@@ -269,7 +269,7 @@ func rootDevice(cfg *config.Config) (string, error) {
 		if cfg.Disks.Swap.EffectiveType() == "partition" {
 			rootIdx = 3
 		}
-		return partDev(cfg.Disks.ESP.Device, rootIdx), nil
+		return archinstall.PartDev(cfg.Disks.ESP.Device, rootIdx), nil
 	default:
 		return "", fmt.Errorf("unknown disks.layout %q", cfg.Disks.Layout)
 	}
diff --git a/internal/stages/postinstall.go b/internal/stages/postinstall.go
index 6c737f9..ddfb64d 100644
--- a/internal/stages/postinstall.go
+++ b/internal/stages/postinstall.go
@@ -162,14 +162,3 @@ func installKernelsSystemdBoot(ctx *Context, k config.KernelConfig) error {
 	)
 	return chrootShell(ctx, set)
 }
-
-// partDev returns the kernel partition device for a base device and number:
-// /dev/sda -> /dev/sda1, but /dev/nvme0n1 -> /dev/nvme0n1p1.
-func partDev(dev string, n int) string {
-	if len(dev) > 0 {
-		if last := dev[len(dev)-1]; last >= '0' && last <= '9' {
-			return fmt.Sprintf("%sp%d", dev, n)
-		}
-	}
-	return fmt.Sprintf("%s%d", dev, n)
-}