fix: resolve the four bugs found by the VM e2e harness

.claude/settings.local.json

@@ -31,7 +31,8 @@
       "Bash(bash -n test/vm.sh)",
       "Bash(git worktree *)",
       "Bash(git --no-pager diff --stat)",
-      "Bash(python3 -c \"import yaml,sys; yaml.safe_load\\(open\\('config.example.yaml'\\)\\); print\\('config.example.yaml: YAML OK'\\)\")"
+      "Bash(python3 -c \"import yaml,sys; yaml.safe_load\\(open\\('config.example.yaml'\\)\\); print\\('config.example.yaml: YAML OK'\\)\")",
+      "Bash(pkill -9 -f qemu-system-x86_64)"
     ]
   }
 }

README.md

@@ -661,34 +661,13 @@ disks.lvm.pvs must have at least 1 item(s)
 ## Testing in a VM
 
 **Recommended before real hardware.** Phase A repartitions disks, so smoke-test the whole flow
-in QEMU with three virtual disks. This is also where you **validate the generated archinstall
-JSON** against the archinstall version on the ISO.
+in QEMU before trusting it on hardware — this is also where you **validate the generated
+archinstall JSON** against the archinstall version on the ISO.
 
-```sh
-# Three disks: 100G (disk 1: ESP+swap+PV) + 2× 50G (whole-disk PVs)
-qemu-img create -f qcow2 disk1.qcow2 100G
-qemu-img create -f qcow2 disk2.qcow2 50G
-qemu-img create -f qcow2 disk3.qcow2 50G
-
-qemu-system-x86_64 \
-  -enable-kvm -m 8G -smp 4 \
-  -cpu host \                                     # required: CachyOS repo setup probes CPU features
-  -bios /usr/share/edk2/x64/OVMF.4m.fd \          # UEFI firmware (edk2-ovmf)
-  -drive file=disk1.qcow2,if=virtio \
-  -drive file=disk2.qcow2,if=virtio \
-  -drive file=disk3.qcow2,if=virtio \
-  -cdrom archlinux-x86_64.iso \
-  -boot d
-```
-
-Inside the VM the disks appear as `/dev/vda`, `/dev/vdb`, `/dev/vdc` — set `config.yaml`
-accordingly (`esp.device: /dev/vda`, PVs `/dev/vda3`, `/dev/vdb`, `/dev/vdc`). Use
-`./archwright install --yes` to skip the interactive prompts during automated runs.
-
-`install --dry-run` prints the rendered config without running anything; a real `install` writes
-`/tmp/archinstall-config.json` + `/tmp/archinstall-creds.json` and invokes `archinstall
---silent`. If archinstall rejects the config after a version bump, diff its schema and update
-`internal/archinstall` + the pinned `Version`.
+The harnesses and the full test pyramid (fast checks → loopback integration → interactive VM →
+automated VM e2e) live in [CONTRIBUTING.md](CONTRIBUTING.md#testing). In short, `task vm-fresh`
+boots the live ISO with clean disks and shares the repo over 9p, and `task vm-e2e` runs the
+headless install → bootstrap → validate flow unattended.
 
 ## For contributors
 

internal/archinstall/archinstall.go

@@ -358,7 +358,7 @@ func buildEncryption(encType string, devices []Device, lvm *LvmConfiguration) (*
 		var rootObjID string
 		for _, dev := range devices {
 			for _, p := range dev.Partitions {
-				if p.Mountpoint != nil && *p.Mountpoint == "/" {
+				if partitionIsRoot(p) {
 					rootObjID = p.ObjID
 				}
 			}
@@ -447,10 +447,15 @@ func (b *lvmBuilder) build(geom Geometry) ([]Device, *LvmConfiguration, error) {
 	}
 	pvOnDisk1 := roundDownMiB(disk1Total - used)
 
-	// A PV partition carries the LV filesystem as its fs_type purely so parted can
-	// create it (archinstall 4.x requires a non-null fs_type per partition); the
-	// filesystem is never written, the partition is pvcreated.
+	// A PV partition carries an LV filesystem as its fs_type purely so parted can
+	// create it (archinstall 4.x rejects an empty fs_type per partition); the
+	// filesystem is never written, the partition is pvcreated. In single-LV mode
+	// this is the root LV's filesystem; in multi-volume mode Filesystem is empty by
+	// schema, so fall back to the first volume's filesystem (any valid fs works).
 	pvFs := b.lvm.Filesystem
+	if pvFs == "" && len(b.lvm.Volumes) > 0 {
+		pvFs = b.lvm.Volumes[0].Filesystem
+	}
 	espPart := espPartition(espBytes)
 	disk1PVPart := Partition{
 		ObjID: newObjID(), Status: "create", Type: "primary",
@@ -643,16 +648,44 @@ func singleDiskRoot(esp config.ESPConfig, swap config.SwapConfig, espBytes uint6
 
 	root := "/"
 	rootFs := spec.fsType
+
+	// When the root partition carries btrfs subvolumes, the subvolume entries
+	// (e.g. {"name":"@","mountpoint":"/"}) provide the mountpoints, so the
+	// partition's own mountpoint must be null — otherwise archinstall mounts the
+	// bare partition (top-level subvol, subvolid 5) at "/" and the configured @
+	// subvolume is created but never used as root. With no subvolumes
+	// (plain/ext4 layouts) the partition itself is mounted at "/".
+	rootMount := &root
+	if len(spec.btrfs) > 0 {
+		rootMount = nil
+	}
 	parts = append(parts, Partition{
 		ObjID: newObjID(), Status: "create", Type: "primary",
 		Start: bytes(offset), Size: bytes(rootBytes),
-		FsType: &rootFs, Mountpoint: &root,
+		FsType: &rootFs, Mountpoint: rootMount,
 		MountOptions: spec.mountOptions, Flags: []string{}, Btrfs: spec.btrfs,
 	})
 
 	return []Device{{Device: disk1, Wipe: true, Partitions: parts}}, nil
 }
 
+// partitionIsRoot reports whether p provides the "/" mount, accounting for both
+// shapes: a plain/ext4/lvm partition mounted directly at "/" (Mountpoint == "/"),
+// and a btrfs partition whose own Mountpoint is null but which carries a
+// subvolume (e.g. "@") mapped to "/". The btrfs case matters because the root
+// partition's Mountpoint is deliberately null when subvolumes drive the mounts.
+func partitionIsRoot(p Partition) bool {
+	if p.Mountpoint != nil && *p.Mountpoint == "/" {
+		return true
+	}
+	for _, sv := range p.Btrfs {
+		if bs, ok := sv.(BtrfsSubvolume); ok && bs.Mountpoint != nil && *bs.Mountpoint == "/" {
+			return true
+		}
+	}
+	return false
+}
+
 // --- btrfs layout -----------------------------------------------------------
 
 // btrfsBuilder is a single btrfs root partition carrying subvolumes (the common

internal/archinstall/btrfs_root_test.go

@@ -0,0 +1,100 @@
+package archinstall
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/AdamJHall/archwright/internal/config"
+)
+
+// btrfsRootYAML is a self-contained btrfs layout: a single NVMe disk with an ESP
+// and a btrfs root carrying @ (/) and @home (/home) subvolumes. It deliberately
+// does not edit any shared fixture so this regression test stands alone.
+const btrfsRootYAML = `
+system:
+  hostname: arch-btrfs-root
+  timezone: Europe/London
+  locale: en_GB.UTF-8
+  keymap: uk
+user:
+  name: adam
+pacstrap: [base-devel, networkmanager]
+kernel:
+  base: [linux]
+disks:
+  layout: btrfs
+  esp:
+    device: /dev/nvme0n1
+    size: 1GiB
+  swap:
+    type: zram
+  btrfs:
+    device: /dev/nvme0n1
+    compress: zstd
+    subvolumes:
+      - {name: "@", mountpoint: /}
+      - {name: "@home", mountpoint: /home}
+`
+
+// TestBtrfsRootPartitionMountpointNull asserts the bug fix: for a btrfs layout
+// the root partition itself must have a nil (null) mountpoint, while the @
+// subvolume entry is what maps to "/". Otherwise archinstall installs into the
+// top-level subvolume (subvolid 5) and leaves @ empty.
+func TestBtrfsRootPartitionMountpointNull(t *testing.T) {
+	dir := t.TempDir()
+	cfgPath := filepath.Join(dir, "btrfs-root.yaml")
+	if err := os.WriteFile(cfgPath, []byte(btrfsRootYAML), 0o644); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+
+	cfg, err := config.Load(cfgPath)
+	if err != nil {
+		t.Fatalf("load config: %v", err)
+	}
+	if err := cfg.Validate(); err != nil {
+		t.Fatalf("config invalid: %v", err)
+	}
+
+	geom := Geometry{"/dev/nvme0n1": 256 << 30}
+	c, _, err := Build(cfg, geom, "TESTPASS")
+	if err != nil {
+		t.Fatalf("Build: %v", err)
+	}
+
+	// Find the btrfs root partition (the one carrying subvolumes).
+	var root *Partition
+	for i := range c.DiskConfig.DeviceModifications {
+		for j := range c.DiskConfig.DeviceModifications[i].Partitions {
+			p := &c.DiskConfig.DeviceModifications[i].Partitions[j]
+			if len(p.Btrfs) > 0 {
+				root = p
+			}
+		}
+	}
+	if root == nil {
+		t.Fatal("no btrfs root partition found in rendered config")
+	}
+
+	if root.Mountpoint != nil {
+		t.Errorf("btrfs root partition mountpoint = %q, want nil (null) so @ drives the root mount", *root.Mountpoint)
+	}
+
+	// The @ subvolume must be the one mounted at "/".
+	var atRoot bool
+	for _, sv := range root.Btrfs {
+		bs, ok := sv.(BtrfsSubvolume)
+		if !ok {
+			t.Fatalf("subvolume entry has unexpected type %T", sv)
+		}
+		if bs.Name == "@" {
+			if bs.Mountpoint == nil || *bs.Mountpoint != "/" {
+				t.Errorf("@ subvolume mountpoint = %v, want %q", bs.Mountpoint, "/")
+			}
+			atRoot = true
+		}
+	}
+	if !atRoot {
+		t.Error("no @ subvolume mapping to / found on the btrfs root partition")
+	}
+}

internal/archinstall/btrfs_test.go

@@ -67,7 +67,10 @@ func TestBuild_BtrfsLayout(t *testing.T) {
 		t.Fatalf("want ESP + btrfs root, got %d", len(parts))
 	}
 	root := parts[1]
-	if root.FsType == nil || *root.FsType != "btrfs" || root.Mountpoint == nil || *root.Mountpoint != "/" {
+	// The btrfs root partition's own mountpoint is null: the @ subvolume below
+	// (mountpoint "/") is what archinstall mounts at /, so the system lands
+	// inside @ rather than the top-level subvolume.
+	if root.FsType == nil || *root.FsType != "btrfs" || root.Mountpoint != nil {
 		t.Errorf("btrfs root wrong: %+v", root)
 	}
 	if len(root.MountOptions) != 1 || root.MountOptions[0] != "compress=zstd" {

internal/archinstall/encryption_test.go

@@ -121,10 +121,12 @@ func TestBuild_LuksOnBtrfsRoot(t *testing.T) {
 	if len(enc.Partitions) != 1 {
 		t.Fatalf("want exactly 1 encrypted partition, got %d", len(enc.Partitions))
 	}
-	// The encrypted partition must be the one mounted at "/".
+	// The encrypted partition must be the one providing "/". For btrfs that is
+	// the partition carrying the @ subvolume mapped to "/" (its own mountpoint
+	// is null), not a partition mounted directly at "/".
 	var rootObjID string
 	for _, p := range c.DiskConfig.DeviceModifications[0].Partitions {
-		if p.Mountpoint != nil && *p.Mountpoint == "/" {
+		if partitionIsRoot(p) {
 			rootObjID = p.ObjID
 		}
 	}

internal/archinstall/golden_test.go

@@ -47,6 +47,10 @@ var renderCases = []struct {
 		name: "lvm-zram",
 		geom: Geometry{"/dev/nvme0n1": 256 << 30}, // 256 GiB
 	},
+	{
+		name: "lvm-volumes",
+		geom: Geometry{"/dev/nvme0n1": 256 << 30}, // 256 GiB
+	},
 }
 
 // TestRenderGolden renders every fixture config against fixed geometry and

internal/archinstall/lvm_volumes_test.go

@@ -0,0 +1,59 @@
+package archinstall
+
+import (
+	"testing"
+
+	"github.com/AdamJHall/archwright/internal/config"
+)
+
+// TestMultiVolumeLVMPVFsTypeNonEmpty guards the multi-volume LVM PV-partition
+// regression: in multi-volume mode lvm.filesystem is empty by schema, yet every
+// PV partition must still carry a non-empty fs_type or archinstall 4.x aborts
+// with "File system type is not set". The PV partitions inherit the first
+// volume's filesystem.
+func TestMultiVolumeLVMPVFsTypeNonEmpty(t *testing.T) {
+	defer setDeterministicObjIDs()()
+
+	b := &lvmBuilder{
+		esp:      config.ESPConfig{Device: "/dev/nvme0n1", Size: "1GiB"},
+		swap:     config.SwapConfig{Type: "swapfile", Size: "8GiB"},
+		espBytes: 1 << 30,
+		lvm: config.LVMLayout{
+			VG:  "vg0",
+			PVs: []string{"/dev/nvme0n1p2", "/dev/sda"},
+			Volumes: []config.LVMVolume{
+				{Name: "root", Mountpoint: "/", Filesystem: "xfs", Size: "50GiB"},
+				{Name: "home", Mountpoint: "/home", Filesystem: "ext4"},
+			},
+		},
+	}
+	geom := Geometry{
+		"/dev/nvme0n1": 256 << 30,
+		"/dev/sda":     512 << 30,
+	}
+
+	devices, lvm, err := b.build(geom)
+	if err != nil {
+		t.Fatalf("build: %v", err)
+	}
+	if lvm == nil {
+		t.Fatal("expected an LvmConfiguration")
+	}
+
+	var pvParts int
+	for _, dev := range devices {
+		for _, p := range dev.Partitions {
+			// PV partitions are the non-ESP partitions (no mountpoint).
+			if p.Mountpoint != nil {
+				continue
+			}
+			pvParts++
+			if p.FsType == nil || *p.FsType == "" {
+				t.Errorf("PV partition on %s has empty fs_type; archinstall would abort", dev.Device)
+			}
+		}
+	}
+	if pvParts == 0 {
+		t.Fatal("expected at least one PV partition")
+	}
+}

internal/archinstall/testdata/configs/lvm-volumes.yaml

@@ -0,0 +1,29 @@
+# Single NVMe disk, multi-volume LVM: a fixed root LV (xfs) plus a /home LV
+# (ext4) taking the rest of the VG. Exercises the multi-volume PV fs_type path.
+system:
+  hostname: arch-volumes
+  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:
+    size: 8GiB
+  lvm:
+    vg: vg0
+    pvs: [/dev/nvme0n1p2]
+    volumes:
+      - name: root
+        mountpoint: /
+        filesystem: xfs
+        size: 50GiB
+      - name: home
+        mountpoint: /home
+        filesystem: ext4

internal/archinstall/testdata/golden/btrfs-subvols.config.json

@@ -87,7 +87,7 @@
               }
             },
             "fs_type": "btrfs",
-            "mountpoint": "/",
+            "mountpoint": null,
             "mount_options": [
               "compress=zstd"
             ],