From ad9965ae218b38d0ea2146088800624ef85434f7 Mon Sep 17 00:00:00 2001 From: Adam Hall Date: Mon, 22 Jun 2026 21:12:03 +0930 Subject: [PATCH] chore: prune legacy paths and stale docs for release Remove deprecated config keys and legacy/historical framing ahead of tagging: - Drop the legacy top-level `chezmoi:` key and its dotfiles.repo fallback; `dotfiles:` (manager + repo) is now the only dotfiles config. - Drop the `pacstrap_extra` deprecation guard; `pacstrap` is the single declarative package list. - Delete docs/ that described completed work (declarative-installs, wave4-handoff, extensibility-review); the wave process rules already live in CLAUDE.md. - Add docs/vm-validation.md for the one genuinely outstanding item: validating the reverse-engineered archinstall 4.3 schema in a QEMU VM. - Scrub "historical"/"backward compatible"/"byte-identical to before" framing from comments so they describe only current behavior. Co-Authored-By: Claude Opus 4.8 --- .github/workflows/e2e.yml | 2 +- README.md | 12 +- config.example.yaml | 21 +- docs/declarative-installs.md | 228 ---- docs/extensibility-review.md | 1067 ----------------- docs/vm-validation.md | 41 + docs/wave4-handoff.md | 119 -- .../archinstall/bootloader_render_test.go | 2 +- internal/archinstall/volumes_test.go | 2 +- internal/config/config.go | 24 +- internal/config/config_test.go | 15 +- internal/config/lvm_volumes_test.go | 2 +- internal/stages/dotfiles.go | 17 +- internal/stages/dotfiles_test.go | 13 +- internal/stages/postinstall.go | 3 +- internal/stages/stages_test.go | 2 +- 16 files changed, 75 insertions(+), 1495 deletions(-) delete mode 100644 docs/declarative-installs.md delete mode 100644 docs/extensibility-review.md create mode 100644 docs/vm-validation.md delete mode 100644 docs/wave4-handoff.md diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index 17beb6b..6a737a3 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -1,4 +1,4 @@ -# End-to-end testing, tiered by cost/fidelity (see CLAUDE.md "Testing"). +# End-to-end testing, tiered by cost/fidelity. # # Tier 1 (this file) — render matrix: no VM, no privileges. For each config # fixture it builds the binary, validates the config, runs the real install CLI diff --git a/README.md b/README.md index 2b515ac..caa7e19 100644 --- a/README.md +++ b/README.md @@ -421,17 +421,13 @@ kde: ### Dotfiles -The dotfiles stage supports a selectable manager. When the `dotfiles:` block is omitted, the -manager defaults to `chezmoi` and the repo falls back to `chezmoi.repo` — so the block is only -needed to pick a different manager or repo. +The dotfiles stage applies your dotfiles via a selectable manager. The `manager` defaults to +`chezmoi`, so it's optional; `repo` is required for any manager other than `none`. ```yaml -chezmoi: +dotfiles: repo: https://github.com/AdamJHall/dotfiles - -# dotfiles: -# manager: chezmoi # chezmoi (default) | yadm | bare-git | none -# repo: https://github.com/AdamJHall/dotfiles # defaults to chezmoi.repo when unset + # manager: chezmoi # chezmoi (default) | yadm | bare-git | none ``` | `manager` | What it runs | diff --git a/config.example.yaml b/config.example.yaml index 65520c2..2b24457 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -237,13 +237,9 @@ kde: # wallpaper: /usr/share/wallpapers/Next/contents/images/1920x1080.png # --- dotfiles --------------------------------------------------------------- -chezmoi: - repo: https://github.com/AdamJHall/dotfiles - -# The dotfiles stage (80) supports a selectable manager. When `dotfiles:` is -# omitted entirely, the manager defaults to `chezmoi` and the repo falls back to -# `chezmoi.repo` above — so the block below is optional and only needed to pick a -# different manager or repo. +# The dotfiles stage (80) applies your dotfiles via a selectable manager. The +# manager defaults to `chezmoi`, so `manager:` is optional; `repo:` is required +# for any manager other than `none`. # # manager: # chezmoi (default) — chezmoi init --apply , or `chezmoi apply` when @@ -251,12 +247,9 @@ chezmoi: # yadm — yadm clone , or `yadm pull` when already cloned. # bare-git — classic bare repo at ~/.dotfiles with --work-tree=$HOME. # none — skip dotfiles entirely (clean no-op). -# -# repo: defaults to chezmoi.repo when unset (backward compatible). -# -# dotfiles: -# manager: chezmoi -# repo: https://github.com/AdamJHall/dotfiles +dotfiles: + repo: https://github.com/AdamJHall/dotfiles + # manager: chezmoi # --- post-dotfiles setup ---------------------------------------------------- # Runs after the dotfiles stage (stage 85). For things a dotfiles repo references but can't @@ -274,7 +267,7 @@ chezmoi: # We clone oh-my-zsh itself rather than running its installer: the installer # refuses to run when ~/.oh-my-zsh exists (not re-runnable) and also tries to # chsh/launch zsh — whereas the shell is set by `user.shell` and the .zshrc comes -# from chezmoi. +# from your dotfiles repo. setup: steps: # oh-my-zsh itself FIRST — the custom plugins below clone into its tree. diff --git a/docs/declarative-installs.md b/docs/declarative-installs.md deleted file mode 100644 index 7ef65ef..0000000 --- a/docs/declarative-installs.md +++ /dev/null @@ -1,228 +0,0 @@ -# Declarative install sources — config is the source of truth - -A change spec for making **the config file the complete, explicit record of everything -installed**, removing the hardcoded package/remote/kernel defaults baked into the binary. -Today the code silently prepends a base package set to pacstrap, always registers the -Flathub remote, and always pacstraps the stock `linux` kernel — so `config.yaml` is *not* -the full truth of what lands on disk. This spec closes that gap. - -## Principle (and its one deliberate exception) - -> Nothing is installed unless `config.yaml` explicitly says so. - -**Decision (confirmed):** this applies to the explicit **lists** — packages, flatpak -remotes, the baseline kernel. It does **not** apply to **feature-implied tooling**: when you -set `dotfiles.manager: chezmoi`, installing `chezmoi` is the *consequence of an explicit -choice*, so `ensureTool` stays; likewise `yay` may install `git`/`base-devel` to build the -AUR helper you asked for. The feature selection **is** the declaration. We keep those two -(`ensureTool`, the yay build deps) exactly as they are — see "Out of scope" below. - -## Inventory — every place code installs something the config doesn't list - -| # | Site | What it injects today | In scope | -|---|------|-----------------------|----------| -| D1 | `archinstall.go:233` `bootstrapPackages` + `cfg.PacstrapExtra` (`archinstall.go:285`) | `base-devel git zsh sudo networkmanager efibootmgr` always prepended to pacstrap | **yes — headline** | -| D2 | `flatpak.go:28` (hardcoded Flathub remote) + `:37` (`install … flathub …`) | the `flathub` remote, always added; every app installed *from* `flathub` | **yes — headline** | -| D3 | `archinstall.go:289` `Kernels: []string{"linux"}` | stock `linux` kernel always pacstrapped (then maybe removed by `kernel.replace_stock`) | **yes** | -| D4 | `yay.go:27` `pacman -S … git base-devel` | build deps for the AUR helper | no (feature-implied) | -| D5 | `helpers.go:20` `ensureTool` | a feature's tool (chezmoi/flatpak/snapper/plymouth…) when its binary is absent | no (feature-implied) | - ---- - -## D1 — explicit `pacstrap` list (replaces `pacstrap_extra` + `bootstrapPackages`) - -**Today:** `Build` does `pkgs := append(bootstrapPackages, cfg.PacstrapExtra...)` -(`archinstall.go:285`). The base six packages live in code; the user can only *add*. - -**Change:** a single required `pacstrap` list that is rendered verbatim. Nothing prepended. - -```yaml -# config.example.yaml -pacstrap: # the COMPLETE Phase-A pacstrap set (nothing added in code) - - base-devel # needed by Phase B to build the AUR helper - - git # same - - zsh # the user's login shell (system.shell) - - sudo # Phase B runs as the user via sudo - - networkmanager # network at first boot (see system.network) - - efibootmgr # UEFI boot entry management - - intel-ucode # CPU microcode (or amd-ucode) — folded into initramfs -``` - -```go -// config.go — replace PacstrapExtra -Pacstrap []string `yaml:"pacstrap" validate:"required,min=1,dive,required"` -// DELETE: PacstrapExtra []string `yaml:"pacstrap_extra"` -``` - -```go -// archinstall.go Build — replace the append; DELETE the bootstrapPackages var -Packages: append([]string(nil), cfg.Pacstrap...), -``` - -### Hazard + guardrail (advisory, never injection) -Omitting `base-devel`/`git` breaks the Phase-B yay build; omitting `networkmanager` (with -`network: nm`) means no first-boot network; omitting microcode/kernel/`efibootmgr` are -boot-quality footguns. We do **not** silently re-add anything. Instead, **preflight emits a -warning** listing recommended-but-absent packages, conditioned on what the rest of the -config implies: -- `base-devel`/`git` absent **and** `aur:`/`aur_helper` set → warn (yay won't build). -- `networkmanager` absent **and** `system.network` is `nm`/unset → warn. -- no microcode (`*-ucode`) present → info. -- no kernel package present in `pacstrap` **and** `kernel.base` empty (see D3) → warn - (system may not boot). - -This is a guardrail, not a default — it changes no installed bytes, only prints. Lives in -`preflight.go` (Phase A) so it's seen before the destructive step. - -### Migration -`pacstrap_extra` → `pacstrap`, and the user folds the old six base packages in explicitly. -A one-line note in the release/PR and the example config covers it. Optionally: keep -accepting `pacstrap_extra` for one release as a deprecated alias that errors with a clear -"rename to `pacstrap` and add the base set" message (recommended — `pacstrap_extra` with no -`pacstrap` is unambiguous to detect). - ---- - -## D2 — explicit `flatpak_remotes` + per-app remote (no built-in Flathub) - -**Today:** `flatpak.go` always runs `remote-add … flathub …` and then installs **every** app -from `flathub` (`:37`). `flatpak_remotes` only *adds* extras; Flathub is implicit. An app -from a non-Flathub remote is impossible to express. - -**Change:** -1. `flatpak_remotes` is the **complete** remote list — nothing added in code. If you install - from Flathub you list Flathub. -2. Each flatpak names the remote it comes from, so the install isn't pinned to `flathub`. - Use a `remote:appid` ref (matches `flatpak install `), keeping the list - flat and the migration mechanical. - -```yaml -flatpak_remotes: # COMPLETE list; flathub is no longer implicit - - { name: flathub, url: https://flathub.org/repo/flathub.flatpakrepo } - -flatpaks: # each app names its remote (remote must be declared above) - - flathub:com.spotify.Client - - flathub:org.mozilla.firefox - - flathub:com.stremio.Stremio -``` - -```go -// flatpak.go Run — no hardcoded remote-add; add exactly the declared remotes, -// then install each app from its named remote. -for _, rem := range ctx.Cfg.FlatpakRemotes { - if err := ctx.R.Cmd("flatpak", "remote-add", "--if-not-exists", rem.Name, rem.URL); err != nil { - return err - } -} -for _, app := range ctx.Cfg.Flatpaks { - remote, appid, ok := strings.Cut(app, ":") - if !ok { /* validation guarantees this; defensive */ } - if err := ctx.R.Cmd("flatpak", "install", "-y", "--noninteractive", remote, appid); err != nil { - return err - } -} -``` - -> Note: this installs apps one-per-command instead of one batched `flatpak install … `. -> That's the cost of per-app remotes; acceptable (flatpak installs are already slow and the -> per-app command keeps remote attribution unambiguous). If batching matters, group apps by -> remote and emit one `install` per remote — a pure optimization, same observable result. - -### Validation (`config.go` semanticErrors) -- every `flatpaks` entry must be `remote:appid` (contains exactly one `:`, both halves - non-empty); -- its `remote` must be a `name` in `flatpak_remotes` (catches typos / undeclared remotes at - `validate` time, before anything runs). - -`FlatpakRemote` struct is unchanged (still `{name, url}`); only the doc comment ("the -built-in flathub remote is always added") is corrected. - -### Migration -`com.spotify.Client` → `flathub:com.spotify.Client`, and add the Flathub remote line. The -validator's error message names the fix. - ---- - -## D3 — explicit baseline kernel (`Kernels` no longer hardcoded `linux`) - -**Today:** `Build` hardcodes `Kernels: []string{"linux"}` (`archinstall.go:289`). The only -control is `kernel.replace_stock`, which *removes* stock `linux` post-pacstrap after a custom -kernel is installed in the chroot — i.e. you pacstrap a kernel you didn't ask for, then rip -it out. Not declarative, and wasteful. - -**Change:** add `kernel.base` — the kernel(s) archinstall pacstraps. Render it into the -archinstall `Kernels` field. This makes the baseline explicit and removes the -pacstrap-then-remove dance for the common "custom kernel only" case. - -```yaml -kernel: - base: [linux] # what archinstall pacstraps for a bootable baseline - packages: [linux-cachyos, linux-cachyos-headers] # extra kernels added in the chroot - default: linux-cachyos # GRUB/loader default; must be in base ∪ packages - # replace_stock is now redundant for most cases: set base: [linux-cachyos-...] directly. -``` - -```go -// archinstall.go Build -Kernels: kernelBase(cfg), // cfg.Kernel.Base, validated non-empty -``` - -### Validation -- `kernel.base` required, `min=1` (a system with no kernel doesn't boot); -- `kernel.default`, when set, must be in `kernel.base ∪ kernel.packages` (extend the existing - `kernel.default ∈ packages` check at `config.go:427`). - -### Compatibility -`replace_stock` stays (still valid: pacstrap `linux`, install cachyos in chroot, remove -`linux`) but the example steers people to `base: []` instead. Keeping -`replace_stock` avoids forcing a flag-day for configs that rely on it. **Caveat:** a kernel -in `base` must be in the official repos (archinstall pacstraps from the live ISO's repos, -before the custom `repos:` are configured in the chroot) — so `linux-cachyos` belongs in -`kernel.packages` (chroot, post-repo-setup), not `base`. Document this; the validator can't -know repo membership. - ---- - -## Out of scope (kept by decision — feature-implied, not list-driven) - -- **D4 `yay` build deps** (`git base-devel`) and **D5 `ensureTool`** (chezmoi/flatpak/ - snapper/plymouth…): these install a tool **because the config explicitly enabled the - feature that needs it**. The feature selection is the declaration, so they stay as-is. No - change. (If the strict-everywhere stance is ever wanted, the move is: `ensureTool` → - `ensurePresent` that errors instead of installing, and require the tools in `pacstrap`/ - `packages`. Not doing that now.) -- **`packages` (Phase B)** already has no code-side additions — it's rendered verbatim by - `packages.go`. No change; it's the model the others move toward. - ---- - -## Summary - -| # | Change | Files | Value | Cost | -|---|--------|-------|-------|------| -| D1 | `pacstrap` replaces `pacstrap_extra` + drop `bootstrapPackages`; advisory preflight warnings | `config.go`, `archinstall.go`, `preflight.go`, example | **HIGH** | LOW-MED | -| D2 | `flatpak_remotes` complete (no built-in flathub) + per-app `remote:appid` + validation | `config.go`, `flatpak.go`, example | **HIGH** | LOW-MED | -| D3 | `kernel.base` replaces hardcoded `Kernels: [linux]`; validation + compat | `config.go`, `archinstall.go`, example | MED | LOW | -| D4/D5 | feature-implied installs (yay deps, ensureTool) | — | — | **keep, no change** | - -## Tests (TDD; mirror the existing harness) -- **D1:** golden render — `pacstrap: [a,b,c]` renders `Packages` == exactly `[a,b,c]` (no - prepend); `config_test.go` table case for `required,min=1`; `preflight` advisory tests - (warn when base-devel+aur set; warn when no kernel) asserting on a captured warning, not - `.Plan`. Update the existing golden fixtures (their `Packages` field changes) — do it as a - separate commit so the diff is purely the removed base set, per the Wave 2 golden discipline. -- **D2:** `flatpak.go` dry-run plan — declared remotes added, each app installed from its - named remote, **no** unconditional flathub `remote-add`; `config_test.go` cases for the - `remote:appid` shape and the undeclared-remote error. -- **D3:** golden render — `Kernels` reflects `kernel.base`; `config_test.go` for `base` - `min=1` and the extended `default ∈ base ∪ packages` rule. - -All via `go test ./...` in dry-run (no disk touches), `go vet ./...`, -`go build -o archwright .`, `gofmt -l .` clean. - -## Sequencing note -D1/D2/D3 are independent and touch the now-familiar shared hotspots (`config.go`, -`archinstall.go`, the example) in disjoint regions — they fit the parallel-worktree model -(one agent each, distinct anchors, new `*_test.go` files only). Could land as its own wave or -fold into Wave 4 alongside the snapper/`--from`-`--to` work; the TUI agent doesn't touch any -of these files, so there's no contention with it. diff --git a/docs/extensibility-review.md b/docs/extensibility-review.md deleted file mode 100644 index 2681f6a..0000000 --- a/docs/extensibility-review.md +++ /dev/null @@ -1,1067 +0,0 @@ -# Archwright — extensibility & TUI review - -A review of the whole codebase with concrete recommendations for making it more -**generic, extensible, and configurable**, plus a design for converting the output -from a streaming CLI into a scrollable **TUI** (charmbracelet viewport). - -The codebase is clean and well-tested, but hardcoded to one opinionated path: -UEFI + GRUB, LVM + XFS root, post-install swapfile, KDE Plasma, `yay`, `chezmoi`. -The rigidity is concentrated in a few load-bearing spots, and most of it loosens with -the same move: **a discriminator field + small strategy dispatch + degrade-to-no-op**, -rather than removing anything. - -The review was done by three focused passes: **(A)** disk/archinstall rendering, -**(B)** the config/stage/runner framework, **(C)** the Phase B customization stages. - ---- - -## The three cross-cutting wins (do these first) - -Each one unlocks or cheapens everything else. - -### 1. Shared stage helpers — HIGH value, LOW cost - -Three patterns repeat across the Phase B stages and are the root of most duplication: - -- **ensure-tool-then-use** (`LookPath` → `pacman -S` fallback): - `flatpak.go:26`, `plymouth.go:32`, `chezmoi.go:28`, `yay.go:20`. -- **idempotent grep-guarded `sed /etc/default/grub`**: - `plymouth.go:39-54`, `grubtheme.go:50-53`. -- **clone-to-tmp-build-cleanup** (`mktemp -d … git clone … rm -rf`): - `yay.go:28-31`, `grubtheme.go:31-34`. - -Collapse into a small `internal/stages/helpers.go`: - -```go -// ensureTool installs pkg via pacman if bin isn't on PATH. -func ensureTool(ctx *Context, bin, pkg string) error { - if _, err := exec.LookPath(bin); err == nil { - return nil - } - return ctx.R.Root("pacman", "-S", "--needed", "--noconfirm", pkg) -} - -// ensureKernelParam adds one token to GRUB_CMDLINE_LINUX_DEFAULT, idempotently. -func ensureKernelParam(ctx *Context, tok string) error { /* sed from plymouth.go:48-53 */ } - -// cloneBuild runs an in-checkout shell snippet against a fresh shallow clone. -func cloneBuild(ctx *Context, url, shellInCheckout string) error { /* yay.go:28-31 idiom */ } -``` - -Low-risk: the *emitted command strings* (what `stages_test.go` asserts on) stay identical. -New stages then become a few lines — the stated goal in `CLAUDE.md`. - -### 2. A first-class hooks mechanism — HIGH value, MEDIUM cost - -The new `setup` stage (`internal/stages/setup.go`, order 85) already provides `clones` -+ `commands`, but at **one fixed point** (Phase B only, after everything, user-only). -Generalize it to user-defined commands keyed to lifecycle points. - -```go -// config.go -type Config struct { - // ...existing... - Hooks []Hook `yaml:"hooks" validate:"dive"` -} - -// Hook is a user-defined command run at a named lifecycle point. -type Hook struct { - Name string `yaml:"name"` - At string `yaml:"at" validate:"required,hookpoint"` - Run string `yaml:"run" validate:"required_without=Script"` - Script string `yaml:"script" validate:"omitempty,file"` - Root bool `yaml:"root"` // Root vs Cmd - Env map[string]string `yaml:"env"` - Dir string `yaml:"dir"` -} -``` - -`At` covers two flavours of lifecycle point in one field: - -- **Global:** `pre-install`, `post-install`, `pre-bootstrap`, `post-bootstrap`. -- **Per-stage:** `before:` / `after:` (e.g. `after:packages`). - -Fire them **centrally** in `runPhase` (`main.go:84`), so no stage needs to know about -hooks, and route through the `Runner` so they are dry-run-recorded and testable for free: - -```go -// main.go runPhase -fireHooks(ctx, phasePre(p)) -for _, s := range selected { - fireHooks(ctx, "before:"+s.Name()) - ui.Header(s.Order(), s.Name()) - if err := s.Run(ctx); err != nil { return /* ... */ } - fireHooks(ctx, "after:"+s.Name()) -} -fireHooks(ctx, phasePost(p)) -``` - -Validating `before:`/`after:` stage names needs the registry; do it in `semanticErrors()` -by passing known names into `Validate(knownStages []string)` (keeps `config` -dependency-free, avoids an import cycle). This subsumes `setup.commands` and -`Repo.Setup`, and becomes the escape hatch that means you *don't* build bespoke Go -stages for snap/cargo/gsettings/stow — they're all one-liners through a hook. - -### 3. Validate the rendered archinstall config against real archinstall — HIGH value - -The project's defining risk (`CLAUDE.md`) is archinstall schema drift. - -**Update (decided):** the original plan — vendor `archinstall/schema.json` from the 4.3 -checkout and validate golden renders against it at `go test` time — is **not viable**. -Investigation found that the shipped `schema.json` is a *stale legacy schema*: it requires -`mirror-region`, its `bootloader` enum is `systemd-bootctl/grub-install/efistub` (not the -`"Grub"` string we emit), it models `harddrives`/`keyboard-language`/`sys-encoding`, and it -is malformed JSON (trailing comma; object-form `required`). It does not describe the 4.3 -dataclass JSON we render, so it cannot serve as an oracle. - -**Chosen approach (Option A):** make the oracle *real archinstall itself*, exercised in the -existing end-to-end VM/container run (see [[e2e-testing-plan]]) rather than a unit test — -feed the rendered config to archinstall and assert it is accepted. This is the truthful -check and aligns with `CLAUDE.md`'s "must be validated against a real archinstall run in a -QEMU VM" rule. (Optional cheap add-on, if fast `go test` feedback is wanted later: a -self-maintained "shape" test asserting the canonical 4.3 fields are present/correct, -updated on each `Version` bump.) - -The review found **two latent drifts present today** — both still work in 4.3 but are on -its *deprecated* compat path, and the e2e acceptance check should flag them when fixed in -the A4 work: - -- the bare `bootloader: "Grub"` string (`archinstall.go:306`) — 4.3 wants a - `bootloader_config` object (`args.py:184-190`); -- the top-level `disk_encryption` (`archinstall.go:160`) — canonical is nested in - `disk_config` only (`args.py:153-156`); you currently emit both. - ---- - -## Subsystem A — disk / archinstall rendering - -The core limitation is `Build()` (`archinstall.go:190-331`): a 140-line function that -*is* the layout, with disk1-special-casing at `:207-219` (PV split), `:238-253` -(assembly), `:286-297` (single root LV). - -### A1. Layout-strategy refactor — HIGH / MEDIUM - -Add a discriminator and dispatch to a small internal interface. The existing LVM logic -moves wholesale into `lvmBuilder`; this is a refactor, not a rewrite, and it subsumes -every item below. - -```go -// config.go -type Disks struct { - Layout string `yaml:"layout" validate:"required,oneof=lvm btrfs plain"` - ESP ESPConfig `yaml:"esp"` - Swap SwapConfig `yaml:"swap"` - LVM *LVMLayout `yaml:"lvm"` // required when layout==lvm - Btrfs *BtrfsLayout `yaml:"btrfs"` // required when layout==btrfs - Plain *PlainLayout `yaml:"plain"` // required when layout==plain - Encryption *Encryption `yaml:"encryption"` -} - -// archinstall.go -type layoutBuilder interface { - build(geom Geometry) ([]Device, *LvmConfiguration, error) -} -``` - -`Build` shrinks to: parse cross-cutting fields → select a builder from `Disks.Layout` -→ call `builder.build(geom)` → wire encryption obj_ids → assemble `Config`. Cross-field -rules ("`lvm` block required when `layout: lvm`") live in `semanticErrors()`. - -### A2. btrfs + subvolumes — HIGH / MEDIUM - -The most-wanted modern Arch desktop layout (snapshots via snapper/timeshift). Your -`Partition.Btrfs []any` field (`archinstall.go:96`) **already serializes subvolumes** — -it is just always empty today. - -```go -type BtrfsLayout struct { - Device string `yaml:"device" validate:"required,startswith=/dev/"` - Compress string `yaml:"compress"` // zstd → mount_options - Snapshots string `yaml:"snapshots" validate:"omitempty,oneof=snapper none"` - Subvolumes []Subvol `yaml:"subvolumes" validate:"dive"` -} -type Subvol struct { - Name string `yaml:"name" validate:"required"` // @, @home, @log - Mountpoint string `yaml:"mountpoint" validate:"required"` // /, /home -} -``` - -Emit one root partition with `fs_type: "btrfs"`, `mountpoint: "/"`, a populated `Btrfs` -list, and add a top-level `disk_config.btrfs_options` for snapper. **Hazard:** a naive -swapfile on btrfs corrupts — a btrfs layout needs a `nocow`/no-compress swap subvolume -(see A5). - -### A3. LUKS encryption — MED-HIGH / MEDIUM - -Composes with LVM as `lvm_on_luks` (`EncryptionType`, `device.py:1403`). The password is -a **top-level** `encryption_password` field, not in creds. - -```go -type Encryption struct { - Type string `yaml:"type" validate:"omitempty,oneof=luks lvm_on_luks luks_on_lvm"` -} -type DiskEncryption struct { - EncryptionType string `json:"encryption_type"` - Partitions []string `json:"partitions"` // PV partition obj_ids - LvmVolumes []string `json:"lvm_volumes"` -} -``` - -⚠️ archinstall **rejects LVM encryption with >2 partitions** (`device.py:1476`), so -`lvm_on_luks` across multiple whole-disk PVs may not validate. This is the textbook -"reverse-engineered schema, must be VM-validated" gotcha — test in a VM. - -### A4. systemd-boot + canonical bootloader shape — MEDIUM / LOW-MED - -`Bootloader` enum (`bootloader.py:10`): `Systemd-boot`, `Grub`, `Efistub`, `Limine`, -`Refind`, `No bootloader`. Emit the modern object form: - -```go -type BootloaderConfig struct { - Bootloader string `json:"bootloader"` // "Grub" | "Systemd-boot" - UKI bool `json:"uki"` - Removable bool `json:"removable"` -} -``` - -**Caveat:** `installKernels` (`postinstall.go:94-117`) is GRUB-specific — it edits -`/etc/default/grub` (`GRUB_TOP_LEVEL`) and runs `grub-mkconfig`. systemd-boot needs -`bootctl set-default` / loader entries and has no `grub.cfg`. Bootloader choice touches -two files, not one. - -### A5. Swap options — MEDIUM / LOW - -```go -type SwapConfig struct { - Type string `yaml:"type" validate:"omitempty,oneof=swapfile zram partition none"` - Size string `yaml:"size" validate:"omitempty,size"` -} -``` - -- `zram` → set `Config.Swap = true` (archinstall's `swap` *is* zram). Best desktop - default; sidesteps both the LVM-format limitation and the btrfs-swapfile hazard. -- `swapfile` → current behaviour (the only LVM-compatible option). -- `partition` → `linux-swap` partition; valid only for `plain`/`btrfs` layouts. -- `none` → skip. - -### A6. Other rendering generality — LOW-MED / LOW - -- **Multiple LVs / separate `/home`:** make `LVMLayout.Volumes` a list; the "rest of VG" - math (`archinstall.go:279-283`) generalizes (sum fixed sizes, give the one `rest` - volume the remainder minus headroom). -- **plain ext4/xfs (no LVM):** falls out of the strategy refactor for free. -- **NTP:** add `system.ntp bool` (hardcoded `true` at `archinstall.go:311`). -- **Network:** `system.network: nm|systemd-networkd` (hardcoded `nm` at `:316`); low - priority for a desktop tool. -- **Multiple users:** `Creds.Users` (`archinstall.go:173`) already supports a list; only - the renderer is single-user (`:327`). Low value for a personal box. - -### Skip - -- **ZFS** — archinstall's model has no ZFS (`FilesystemType`, `device.py:785`); supporting - it means hand-rolling zpool/bootloader, which the project deliberately avoids. -- **BIOS/MBR** — UEFI is hard-required (`preflight.go:25`); high effort for hardware you - probably don't have. Hold the line. - -| # | Change | Value | Cost | Status | -|---|--------|-------|------|--------| -| A3-test | Validate rendered config via real archinstall in the e2e run (vendored `schema.json` is stale — unusable) | **HIGH** | LOW-MED | deferred (e2e) | -| A1 | Layout-strategy interface; de-special-case `Build` | **HIGH** | MED | ✅ Wave 2 | -| A2 | btrfs + subvolumes | **HIGH** | MED | ✅ Wave 2 | -| A4b | Canonical `bootloader_config` + nested-only encryption | **HIGH** | LOW | ✅ Wave 2 | -| A3 | LUKS (`luks` / `lvm_on_luks`) | MED-HIGH | MED | ✅ Wave 3 | -| A5 | Swap options (zram/partition/none) | MED | LOW | ✅ Wave 2 | -| A4 | systemd-boot | MED | LOW-MED | ✅ Wave 3 | -| A6 | separate /home (LVM volumes) ✅ (Wave 3), NTP ✅ (Wave 3); plain layout ✅ (Wave 2) | MED | LOW-MED | ✅ Wave 3 | -| — | ZFS, BIOS/MBR, multi-user | LOW | skip/defer | — | - ---- - -## Subsystem B — config / stage / runner framework - -### B1. CLI selection: `list-stages`, `--skip`, `stages.disable` — HIGH / LOW - -Today the only selection is `--only` (single stage). Add: - -- `archwright list-stages` — print order/name/phase from the registry (~15 lines). -- `--skip ` (repeatable) — the inverse of `--only`; "everything except kde" is - more useful in practice. -- `stages.disable: [kde]` in config — skip a stage without emptying its config block. -- Optionally `--from`/`--to` for resuming a half-finished run. - -Implement them as one selection filter in `runPhase`, with `--only` winning over the rest. - -### B2. Runner gaps — MEDIUM / LOW - -`run.go` is clean but minimal. Gaps that hooks/stages (and the TUI below) need: - -| Gap | Where | Fix | -|---|---|---| -| no env vars | `Cmd` `run.go:31`, `Shell` `:63` | `cmd.Env = append(os.Environ(), ...)` | -| no working dir | same | `cmd.Dir` | -| no output capture | `Cmd` streams to `os.Stdout` `:39` | `Capture(name, args) (string, error)` | -| no output sink | hardcoded `os.Stdout`/`os.Stderr` `:39`,`:70` | inject an `io.Writer` (see TUI section) | - -The capture gap has teeth: `preflight.go:41` already drops to raw `exec.Command`, -bypassing dry-run and the recorded plan — exactly what `CLAUDE.md` warns against. The -**output-sink** change is also the hinge for the TUI conversion. - -### B3. Smaller items - -- Delete `itoa` (`stages.go:61`) — hand-rolled int→string for no reason; use - `strconv.Itoa`. Trivial. -- **Env-var substitution** in `Load()` (`config.go:163`): run raw bytes through - `os.Expand` (error on unset) — ~8 lines, enables secrets and variants out of the - gitignored config. -- **Keep** the integer `Order()` scheme and flat `semanticErrors()` — both scale fine; - don't build a dependency graph or rule registry. -- **Defer** profile/overlay-file merging until you actually run >1 machine (list-merge - semantics are the sharp edge). - ---- - -## Subsystem C — Phase B customization stages - -Principle: **gate every tool-specific block behind a selector that degrades to a clean -no-op.** The `grub.theme.source: none` early-return and the per-field empty-skip in -`kde.go` are already the right model — apply it uniformly. - -### C1. Desktop-environment selector — HIGH / MEDIUM - -KDE is hardcoded (`kde.go:27-31`, `plasma-apply-*`). Don't build cross-DE theming -abstractions (a Breeze color scheme and a Hyprland config share nothing) — gate the KDE -stage and route everything else through hooks + the dotfiles repo. - -```go -Desktop struct { - Environment string `yaml:"environment" validate:"omitempty,oneof=kde gnome hyprland sway none"` -} `yaml:"desktop"` -``` - -```go -// kde.go Run() -if de := ctx.Cfg.Desktop.Environment; de != "" && de != "kde" { - ui.Info("desktop.environment is %q — skipping KDE stage", de) - return nil -} -``` - -### C2. Package-manager genericity — MEDIUM / LOW - -- **`aur_helper: yay|paru`** — paru is argument-compatible; near drop-in. `yay.go` - parameterizes on the package name (`yay-bin` → `paru-bin`); `aur.go:23` on the binary. -- **Flatpak remotes beyond Flathub** — model remotes as a list (`flatpak.go:32-36` - hardcodes the Flathub URL); let each app name a remote, defaulting to `flathub`. -- **snap/cargo/npm/pipx** — *don't* add as stages; they're one-liners through hooks. - -### C3. Decouple kernel-cmdline from Plymouth — MEDIUM / LOW-MED - -The cmdline edit lives in `plymouth.go:47-54`, but that's GRUB's concern, not Plymouth's. -Move it to a shared, bootloader-aware helper (`ensureKernelParam`, see cross-cutting #1) — -worth doing even if you never adopt systemd-boot. When/if Phase A gains systemd-boot, the -helper branches (sed `/etc/default/grub` vs edit `/etc/kernel/cmdline`) and -`grub-mkconfig` becomes `regenerateBootConfig(ctx)`. `grubtheme.go` no-ops for -systemd-boot (no theming) via its existing `case "none"` pattern. - -### C4. Dotfiles manager selector — LOW-MED / LOW - -`dotfiles.manager: chezmoi|yadm|bare-git|none`, keeping chezmoi's init-vs-apply -idempotency (`chezmoi.go:35-49`). yadm is near-identical; bare-git is `clone --bare` + -checkout into `$HOME`; stow → punt to hooks. - -| # | Change | Value | Cost | Status | -|---|--------|-------|------|--------| -| Cross-1 | Shared helpers | HIGH | LOW | ✅ Wave 0 | -| C1 | `desktop.environment` selector | HIGH | MED | ✅ Wave 1 | -| C2a | `aur_helper: yay\|paru` | MED | LOW | ✅ Wave 1 | -| C2b | Flatpak remotes | MED | LOW | ✅ Wave 1 | -| C3 | Decouple cmdline edit from Plymouth | MED | LOW-MED | ✅ Wave 2 | -| C4 | `dotfiles.manager` selector | LOW-MED | LOW | ✅ Wave 3 | - ---- - -## Suggested sequencing - -1. **Foundations:** shared helpers ✅, `strconv.Itoa` fix ✅, Runner - `Env`/`Dir`/`Capture`/sink ✅, config env-var substitution ✅ (Wave 0, landed). Config - acceptance is checked by real archinstall in the e2e run, not a unit test (see §3). -2. **Headline extensibility:** hooks mechanism ✅ + `list-stages`/`--skip`/`stages.disable` ✅ - (Wave 1, landed). -3. **Disk genericity:** layout-strategy refactor ✅ + fix the two deprecated archinstall - shapes (A4b) ✅, then btrfs ✅ / swap ✅ as independent builders, each with a golden - fixture (Wave 2, landed). LUKS (A3) ✅ and systemd-boot (A4) ✅ + multiple LVM volumes / - NTP (A6) ✅ (Wave 3, landed). -4. **Phase B selectors:** `desktop.environment` ✅, `aur_helper` ✅, flatpak remotes ✅ - (Wave 1, landed); decouple cmdline from Plymouth (C3) ✅ (Wave 2, landed); - `dotfiles.manager` (C4) ✅ (Wave 3, landed). - -**Wave 2 landed** the three Wave 1 correctness follow-ups (value-only env-substitution, -hooks `~`-expansion + dropping the eager `script` `file` check, and C3) plus the disk -genericity headline (A1 layout-strategy refactor, A4b canonical `bootloader_config` + -nested-only encryption, A2 btrfs+subvolumes, A5 swap options). See the Wave 2 retrospective -below. - -**Wave 3 landed** the remaining headline disk/boot extensibility items: A3 LUKS -(`luks` / `lvm_on_luks`, nested under `disk_config` + top-level `encryption_password`), -A4 systemd-boot (config-selected `bootloader.kind`, branching the C3 bootloader-aware -helpers and `installKernels`), and A6 (multiple LVM volumes for a separate `/home`, plus a -`system.ntp` toggle), and C4 (`dotfiles.manager`: chezmoi/yadm/bare-git/none). See the Wave 3 -retrospective below. - -**Wave 4 landed** snapper provisioning (✅ `internal/stages/snapper.go`, Order 25, no-op -unless `btrfs` + `snapshots: snapper`), the `--from`/`--to` stage-resume window -(✅ `stages.Within` post-filter), and the declarative-install trio from -`docs/declarative-installs.md`: D1 explicit `pacstrap` (✅ replaces `pacstrap_extra` + the -in-code base set; advisory preflight warnings), D2 explicit `flatpak_remotes` + per-app -`remote:appid` (✅ no implicit Flathub), and D3 explicit `kernel.base` (✅ replaces hardcoded -`Kernels: [linux]`). - -The **TUI conversion was built and then reverted** (❌ not landed). A scrollable bubbletea -viewport works for streamed output, but the alt-screen owns stdin, so interactive subprocess -prompts can't be answered inside it — and archwright runs arbitrary, unpredictably-interactive -user shell blobs (`repos[].setup`, e.g. `cachyos-repo.sh`'s Y/N questions). Surfacing those -without dropping them out of the viewport needs a full PTY multiplexer (brittle for curses -apps/password prompts, heavy for a single-machine tool). We reverted to plain streaming — the -original deliberate design (the "no bubbletea spinner" gotcha) was right. See the Wave 4 -retrospective below. - -**Wave 5 landed** the **remote/layered-config** item (the last big one): a new -`internal/configsrc` package that resolves `--config` refs (local path / github shorthand / -raw URL), recursively resolves a top-level `imports:` key, env-expands per layer, and -deep-merges (base-first, importer/later-wins; string lists union+dedup, name-keyed structured -lists merge by `name`, `!replace` escape hatch) into one flattened config. `--config` is now -repeatable; a new `render` command writes the flattened merge; Phase A resolves once and -stages the flattened bytes (`Context.FlatConfig`) so Phase B never re-fetches. See the Wave 5 -retrospective below. - -**Next wave starts here:** the headline extensibility roadmap is fully landed, and **Wave 6 -landed the Wave 5 trust/pinning UX + `render` provenance polish** (see the Wave 6 retrospective -below). What remains is the **e2e/VM validation** work ([[e2e-testing-plan]]) — the last open -item. Everything new since Wave 1 carries **VM-validation caveats** — confirm the -reverse-engineered shapes against a real archinstall 4.3 run before trusting on hardware: -the Wave 2 `bootloader_config`/btrfs-subvolume JSON, and the Wave 3 `disk_encryption` -obj_id wiring + `encryption_password` casing, the `lvm_on_luks` >2-partition limit, the -systemd-boot loader-entry default + `bootctl update` cmdline-refresh paths, the -multi-volume LVM "rest of VG" sizing, and the Wave 4 snapper timer-unit / `set-config` -key names (`snapper-timeline.timer`, `snapper-cleanup.timer`, `TIMELINE_LIMIT_*`). - -Recurring theme: **prefer one good escape hatch (hooks + dotfiles repo) over many bespoke -Go stages.** - ---- - -## Wave 1 retrospective — notes for the next agent - -Wave 1 (hooks, stage selection, Phase B selectors) was built by **three parallel worktree -agents** merged into one branch. What worked, and what to fix next: - -### Process that worked (reuse it) -- **Partition by file ownership, not just by feature.** `config.go` is the shared hotspot - — every wave adds fields. Give each agent a *distinct anchor* (top-of-struct vs. a named - sub-block vs. end-of-struct); the additive struct edits then auto-merge. The only - hand-merges needed were `main.go`'s `runPhase` (two agents touched it) and one adjacent - type-declaration spot. -- **New test files, never shared fixtures.** Agents added `*_test.go` files with - self-contained config snippets instead of editing the shared `testYAML`/`testConfig` in - `stages_test.go`. Zero test-file conflicts — keep this rule. -- **Background agents need pre-authorised tools.** They can't answer permission prompts, so - `Edit`/`Write`/`Bash(go …)`/`Bash(git …)` must be on the allowlist or every write - auto-denies. They also tend to sweep untracked files in via `git add -A` — integrate with - explicit `git add ` and check the net diff before pushing. -- **gofmt + build/vet/test after every merge.** Merges can leave struct-tag alignment - unformatted (`gofmt -l` caught `config.go`). Worth a CI `gofmt -l` gate. - -### Concrete follow-ups found while dogfooding — **fix these in the next wave** -These are small, high-value correctness fixes that came out of Wave 1; do them **first** in -the next wave, before new feature work, and cover each with a test: -- **Hooks `script`/`dir` don't expand `~`, and `script` is validated with `file`** (the - path must exist at validate time). This is inconsistent with `setup`'s `expandHome` - (`setup.go`). Recommend: run `script`/`dir` through `expandHome`, and reconsider the eager - `file` existence check — a hook script may be produced by an *earlier* hook/stage in the - same run, so validate-time existence is the wrong check. Until fixed, `script` must be an - existing absolute path. -- **Env-substitution scans the whole file, comments included.** A literal `$` in any comment - or value errors unless doubled (`$$`). This is a sharp edge for users (a `$` in a config - comment fails the run) and it bites the remote/layered-config wave, where substitution - composes with merge. Consider expanding only string *values* by walking the parsed YAML - node tree, instead of `os.Expand` over raw bytes (`config.go:expandEnv`). -- **C3 (decouple the kernel cmdline from Plymouth) is still pending** — the one Phase B - selector item not done in Wave 1. Cheap; fold it into the systemd-boot work (A4), since - both want a bootloader-aware `regenerateBootConfig`/`ensureKernelParam` helper. - -### Repo hygiene (orthogonal) -`.claude/settings.local.json` is tracked on `main` — machine-local editor/agent settings -that should be gitignored. Wave 1 deliberately left it as-is; a separate cleanup PR should -`git rm --cached` it and add the `.gitignore` entry. - ---- - -## Wave 2 retrospective — notes for the next agent - -Wave 2 (the three Wave 1 correctness follow-ups + disk genericity) was again built by **three -parallel worktree agents** merged into one branch. What landed and what to watch: - -### What landed -- **Correctness follow-ups:** value-only env-substitution (walks the `yaml.Node` tree and - expands only scalar VALUE nodes, so a `$` in a comment or key no longer errors; `$$`→`$` - and unset-var-errors preserved); hooks now `~`-expand `script`/`dir` via the existing - `expandHome`, and the eager `script` `file` existence check was dropped (a hook script may - be produced by an earlier step in the same run). -- **Disk genericity:** A1 layout-strategy refactor (`Disks.Layout` discriminator → - `layoutBuilder` interface; `lvmBuilder` + new `plainBuilder`; empty layout still defaults - to `lvm` so existing goldens were byte-unchanged after the refactor), A4b (canonical - `bootloader_config` object + nested-only `disk_encryption`), A2 (btrfs + subvolumes via a - `btrfsBuilder`, populating the long-dormant `Partition.Btrfs` field), A5 (swap `type`: - swapfile/zram/partition/none, swapfile still the default). - -### Process that worked (reuse it) -- **The Wave 1 partition rules held.** Three agents, distinct file/region ownership, new - test files only — and `git merge` auto-resolved every overlap, including two agents both - editing `config.go` (disjoint regions: the `Disks` block vs. `expandEnv`/`Load`/`Hook`) - and both touching `config_test.go`. **Zero hand-merges this wave.** The "distinct anchor in - the shared hotspot" rule is the load-bearing one — keep it. -- **Behaviour-preserving refactors gated on the golden snapshot.** A1 was committed - separately and proven by running `TestRenderGolden` *without* `-update` (goldens unchanged); - A4b then regenerated them so its diff was purely the bootloader/encryption shape change. - Doing the refactor and the shape change as separate commits made the intended-vs-incidental - golden diff trivially reviewable. Keep this two-commit discipline for schema changes. - -### Harness gotcha (cost us a retry) -- **Pre-created external worktrees were unreachable.** Manually `git worktree add`-ing dirs - *outside* the repo root and pointing agents at them failed: the agent sandbox confines tool - calls to the session's working directory, so every Read/Write/Bash against the external - path was denied, and `EnterWorktree` refuses to switch from the repo root. The fix that - works is the Agent tool's built-in `isolation: "worktree"`, which roots each agent in its - own worktree under `.claude/worktrees/` (inside the sandbox). Don't hand-roll worktrees for - subagents — let the harness create them. - -### VM-validation caveats (do before trusting on hardware) -Per `CLAUDE.md`'s archinstall-drift rule, these Wave 2 shapes are reverse-engineered and -**must be validated against a real archinstall 4.3 run** (the e2e/A3-test item): -- `bootloader_config: {bootloader, uki, removable}` field names/casing; -- btrfs subvolume JSON (`{name, mountpoint}`) — archinstall may want extra keys (per-subvol - compression, `nodatacow`), and `disk_config.btrfs_options` was intentionally *not* emitted; -- the swap `partition` shape (`fs_type: linux-swap`, flag `swap`). -Snapper (`btrfs.snapshots: snapper`) is validated and carried in config but not yet wired to -an actual post-install snapper setup — that belongs in a hook/stage in a later wave. - ---- - -## Wave 3 retrospective — notes for the next agent - -Wave 3 (A3 LUKS, A4 systemd-boot, A6 multiple LVM volumes + NTP, C4 dotfiles.manager) was -again built by **four parallel worktree agents** merged into one branch. What landed and what -to watch: - -### What landed -- **A3 LUKS:** `disks.encryption.type` (`luks` / `lvm_on_luks` / `luks_on_lvm`) renders a - nested `disk_config.disk_encryption` (`encryption_type` + `partitions`, derived from the - already-built structures — PV obj_ids for `lvm_on_luks`, the `/`-partition obj_id for - `luks`) plus a top-level `encryption_password` (`omitempty`). Cross-field rules - (`encryptionErrors()`): type-vs-layout match, and the archinstall ≤2-PV limit for LVM - encryption. -- **A4 systemd-boot:** `bootloader.kind` (`grub` default | `systemd-boot`) drives the - archinstall `bootloader_config.bootloader` string AND branches the two C3 bootloader-aware - seams (`ensureKernelParam` edits `/etc/kernel/cmdline`; `regenerateBootConfig` runs - `bootctl update`) plus `installKernels` (systemd-boot writes the loader.conf `default` - instead of `GRUB_TOP_LEVEL`+`grub-mkconfig`). The grub path is byte-identical to before. -- **A6:** `LVMLayout.Volumes` (optional list; one size-less volume takes the VG remainder, - exactly one mounted at `/`) for a separate `/home`; `system.ntp` toggle (`*bool`, unset = - true). Single-LV mode is byte-unchanged. -- **C4:** the `chezmoi` stage became `dotfiles` (order 80 unchanged), selectable - `dotfiles.manager`: chezmoi/yadm/bare-git/none, with `chezmoi.repo` kept as a backward- - compatible repo fallback. - -### Process that worked (reuse it) -- **The partition rules held for a 4th wave.** Four agents, distinct config.go anchors - (`System` / `DisksConfig` / end-of-`Config` / after-`Chezmoi`) and distinct archinstall.go - regions (the `Build` literal's bootloader/ntp/disk_config lines are non-adjacent, so they - auto-merged). New test files only. -- **One genuine conflict, trivially resolved.** Two agents appended a helper *call* line after - `diskErrors()` in `semanticErrors()` — git flagged the adjacent insert; the resolution was - "keep both call lines + both new helper funcs." That's the expected orchestrator hand-merge - (same shape as Wave 1's `runPhase`). -- **Default-preserves-behaviour gating worked again.** Every feature degrades to today's - output when its config is unset, so all five existing golden snapshots passed *without* - `-update`. New behaviour is proven by inline-config field-assertion tests (the - `btrfs_test.go` pattern), never by touching `renderCases`/`testdata`. - -### Cross-file follow-up the orchestrator fixed at merge -- **`rootDevice()` (in stages, owned by no agent) broke in multi-volume mode.** It keyed off - `LVMLayout.LV`, which is empty when `Volumes` is set, so the post-install remount used an - invalid `/dev//` path. The A6 agent correctly flagged this rather than reaching outside - its ownership region; the orchestrator fixed it (derive the root LV from the `/`-mounted - volume) and added a regression test. Lesson: when an agent's feature has a consequence in an - *unowned* file, having it report the follow-up instead of silently editing kept the merge - clean — then close the loop centrally. - -### VM-validation caveats (do before trusting on hardware) -Per `CLAUDE.md`'s archinstall-drift rule, these Wave 3 shapes are reverse-engineered and -**must be validated against a real archinstall 4.3 run**: -- `disk_config.disk_encryption` field names/casing (`encryption_type`, `partitions`, - `lvm_volumes`) and that `partitions` carries PV obj_ids (lvm_on_luks) vs the root-partition - obj_id (luks); top-level `encryption_password` placement; and the ≤2-PV LVM-encryption limit; -- systemd-boot: the loader-entry filename the `installKernels` default-setter globs for - (`*linux-*.conf` vs a machine-id-prefixed name), and whether `bootctl update` actually - propagates a `/etc/kernel/cmdline` change (it may be a kernel-install-hook no-op); -- multi-volume LVM "rest of VG" sizing (fixed volumes summed + per-PV headroom subtracted). - ---- - -## Wave 4 retrospective — notes for the next agent - -Wave 4 was built by **six parallel worktree agents** (A TUI, B snapper, C `--from`/`--to`, -plus D1/D2/D3 from `docs/declarative-installs.md`) merged into one branch, each TDD with -the `golang-patterns`/`golang-testing` skills. Largest fan-out so far; what worked and what -to watch: - -- **File-ownership partitioning held** for the disjoint work (A's `internal/tui/*`, B's two - new files, C's `stages.Within`). main.go's A↔C contention **auto-merged** because A - deliberately left the `stages.Select(...)` line + empty-check intact and C inserted its - one filter line right after — the handoff's "keep the shared line stable" rule paid off. -- **The TUI was built, then reverted in VM testing — a design dead-end worth remembering.** - The viewport rendered streamed output fine, but two stdin problems surfaced live: (1) a - `strings.Builder` *value* field in the bubbletea model panicked ("illegal use of non-zero - Builder copied by value") because the Elm loop value-copies the model every Update — fixed - with a `*strings.Builder`; (2) more fundamentally, the alt-screen owns stdin, so interactive - subprocess prompts can't be answered inside it. We patched the *known* prompts (pre-cache - sudo, hoist the install ERASE/password ahead of the alt-screen), but `repos[].setup` runs - **arbitrary** user shell (e.g. `cachyos-repo.sh`'s Y/N) that prompts unpredictably. The only - real fix is a PTY multiplexer (brittle for curses/password prompts, heavy here), so we - reverted to plain streaming. **Lesson:** the original "no bubbletea spinner" gotcha encoded a - real constraint — a capture-everything TUI fights a tool whose job is running interactive - subprocesses. Don't re-attempt without a PTY plan and VM testing up front. -- **The real merge cost was the declarative trio sharing required-field fixtures.** D1 - (`pacstrap` now `required,min=1`) and D3 (`kernel.base` now `required,min=1`) each added a - line to the *same anchor* in ~15 inline-YAML fixtures, so every one conflicted. Resolution - was a mechanical union (keep both lines) **except** `archinstall_test.go`, where the - "theirs" side still carried the old `pacstrap_extra` key D1 had turned into a hard error — - a blind union would have kept both `pacstrap:` and the now-illegal `pacstrap_extra:`. - **Lesson:** when two agents both make a field `required`, expect a conflict in *every* - shared fixture, and check each "theirs" hunk for content the *other* agent deleted. -- **Cross-agent registry coupling:** B's new snapper stage (Order 25) landed *after* C and - the `TestRegistry`/`TestWithin` expectations were written against the registry without it, - so the orchestrator centrally inserted `snapper`/`25` into those expected slices (the - documented "unowned file affected by a feature — agent reports, orchestrator fixes" case). -- **Semantic follow-up the agents flagged and the orchestrator wired:** D1's preflight - "no kernel" advisory was conditioned only on `kernel.packages` (since `kernel.base` didn't - exist on its branch); after D3 landed, the orchestrator extended it to also consider - `cfg.Kernel.Base`. -- **Process nit:** a `git add -A` during conflict resolution staged the agents' - `.claude/worktrees/` checkouts as embedded repos — added a `.gitignore` entry; future - waves should gitignore the worktree dir up front. - -Deferred after Wave 4: **remote/layered configuration** — landed in Wave 5 (below). -All Wave 2–4 reverse-engineered shapes still need a real archinstall 4.3 QEMU run before -hardware — now including the snapper timer/`set-config` names. - ---- - -## Wave 5 retrospective — notes for the next agent - -Wave 5 (remote & layered configuration — the last big roadmap item) was built by **two -parallel worktree agents in Round 1** (the cohesive `internal/configsrc` package; the docs) -plus **one Round-2 agent** (CLI/flatten wiring) layered on top once the package API was real. -Each agent worked TDD with the `golang-patterns`/`golang-testing` skills. - -### What landed -- **`internal/configsrc`** (`merge.go` / `resolve.go` / `source.go` + tests): `Merge(base, - over map[string]any)` (maps recurse; string slices union+dedup; structured slices key-merge - by `name`, else replace; `!replace`-tagged nodes replace wholesale via a parse-time marker) - and `Load(refs []string, opts Options) (*config.Config, []byte, error)` — three ref forms - (local / `github.com/O/R/p.yaml[@ref]` → raw.githubusercontent / raw URL), depth-first - `imports:` recursion (stripped before decode; importer/later wins), relative imports - resolved against the importing file's location, per-layer `${VAR}` expansion, cycle - detection + depth cap (32), URL+ref cache under `$XDG_CACHE_HOME/archwright` with - `Offline`, `Strict` unpinned-ref rejection, and `Token` bearer auth. Returns the merged - `*config.Config` **and** the flattened YAML bytes. -- **CLI:** `--config` is now a repeatable `StringArrayVar` (defaulted via a testable - `configRefs` helper to dodge cobra's append-onto-default pitfall); new `--offline`/`--strict` - flags + `GITHUB_TOKEN` env; `validate`/`runPhase` both route through `configsrc.Load`; new - `render --config ... [-o out.yaml]` (resolve+merge+validate, write flattened, no stages). -- **Flatten-once:** `Context.FlatConfig []byte`; `stageBinary` writes the flattened bytes into - the target when present (falls back to `cp ConfigPath` when nil), so Phase B reads one - concrete file with no re-fetch. - -### Process notes (reuse / watch) -- **A genuine dependency chain doesn't parallelize like additive features do.** Unlike Waves - 1–4 (independent struct fields auto-merging), this feature is layered: merge ← resolve ← - CLI. Forcing all three into one parallel round would have meant stub-and-reconcile churn. - Splitting into **Round 1 (cohesive package + independent docs, parallel) → Round 2 (CLI - wiring off the integrated branch)** kept every agent compiling and testing against real code. - Lesson: partition *additive* work in parallel; **sequence layered work into rounds**, fanning - out only within a round. -- **One agreed API contract up front made Round 2 frictionless.** The orchestrator specified - the exact `Load`/`Merge`/`Options` signatures in both prompts; Round 2 coded against them - with zero interface drift. -- **Worktree base gotcha (again).** The harness branches each worktree agent off the main - repo's HEAD; even with the orchestrator checked out on the wave branch, the Round-2 agent's - worktree started at `main` (no `configsrc`) and the agent had to `git reset` onto - `feat/extensibility-wave5` itself. Next time, hand the agent the explicit base branch in the - prompt so it doesn't have to discover the miss. -- **One additive cross-package export, cleanly scoped.** configsrc needed byte-identical env - semantics, so Agent A added exactly one line to `config.go`: `ExpandEnvNode` wrapping the - unchanged `expandEnv`. Naming the single allowed outside-edit in the prompt kept ownership - clean. - -### Follow-ups for the next agent (small) -- ✅ **Trust UX wired in Wave 6.** `configsrc.Load` now returns a `[]Source` provenance list - (configsrc stays UI-free; `main` does the printing): `runPhase` `ui.Warn`s on every unpinned - github ref and prints the resolved remote-source list before `runStages` (hence before the - `archinstall` stage's `ConfirmErase` prompt). `--strict` still hard-errors as before. -- ✅ **Provenance comment wired in Wave 6.** `render` prepends a `# Flattened by archwright - from: …` header (via `configsrc.ProvenanceComment`) listing each resolved source + resolved - URL when ≥1 remote source is present; pure-local renders stay header-free (byte-identical). -- **VM/e2e still owes config-acceptance.** The flatten-once path and merge output should be - exercised in the e2e run ([[e2e-testing-plan]]) feeding a real archinstall. - ---- - -## Wave 6 retrospective — notes for the next agent - -Wave 6 (the Wave 5 trust/pinning UX + `render` provenance polish — the last non-e2e roadmap -items) was built by **three worktree agents in two rounds**, each TDD with the -`golang-patterns`/`golang-testing`/`tdd` skills. - -### What landed -- **configsrc provenance (Agent A):** `Load` now returns `(*config.Config, []byte, []Source, - error)`. `Source{Ref, Kind, URL, Pinned, Unpinned}` is collected for every resolved ref + - import (base-first, deduped by `canonical()`), and `ProvenanceComment([]Source) string` - renders a `# `-prefixed YAML header — returning `""` when no remote (github/url) source is - present, so pure-local output is untouched. -- **Trust UX (Agent B):** `runPhase` `ui.Warn`s on each unpinned github ref (recommends - `@`; notes it drives destructive disk ops + arbitrary hooks) and prints the - resolved remote-source list *before* `runStages`, i.e. before the `archinstall` stage's - `ConfirmErase`. Pure-string helpers `remoteSources`/`unpinnedSources` keep the decision - unit-tested apart from the printing. -- **Render provenance (Agent C):** `renderConfig` prepends `ProvenanceComment(srcs)` before the - flattened body when non-empty; all-local renders stay byte-identical (guarded by the - untouched `TestRenderConfig_FlattensMerge`). - -### Process that worked (reuse it) -- **This was layered, not additive — so it was sequenced into rounds (the Wave 5 lesson held).** - All three deliverables consume one new configsrc API, so Round 1 was Agent A *alone* - (foundation), then Round 2 fanned out B + C in parallel off the integrated branch. Forcing - all three parallel would have meant stub-and-reconcile churn. -- **The orchestrator did the mechanical signature migration centrally.** After merging A, the - orchestrator flipped all three `main.go` `Load` call sites to the 4-value form with `_` - placeholders and committed that as a separate `chore:` — so B and C each only had to flip - *their own* `_`→`srcs` in *their own* function (`runPhase` vs `renderConfig`). Different - functions in the shared `main.go` hotspot → **git auto-merged both with zero conflicts.** -- **New test files only, again.** B added `main_warn_test.go`, C added - `main_provenance_test.go`; neither touched the shared `main_test.go`. The "" rule for - all-local provenance meant C didn't need to edit the existing render golden test at all. -- **An agreed API contract up front made Round 2 frictionless** (same as Wave 5): both - prompts carried A's exact `Source`/`Load`/`ProvenanceComment` signatures verbatim, so Round 2 - coded against real, already-merged code with no interface drift. -- **Worktree base gotcha (still true).** Each Round-2 worktree started at `main` (no Wave 6 - work); the prompts told B/C to `git fetch && git reset --hard feat/extensibility-wave6` - first, which worked cleanly. Keep handing agents the explicit base branch. - -### Follow-ups (small, optional) -- The unpinned-ref warning + source-list print are wired in `runPhase` only; `validate`/`render` - don't warn (they're non-destructive). Add there too if desired. -- `ProvenanceComment` does not yet record a resolved commit **SHA** for github refs (it lists the - ref + resolved HEAD/tag URL). Resolving the actual SHA would need an extra API call; deferred. - ---- - -## TUI conversion — scrollable viewport - -Goal: make `archwright` feel like a TUI, with all output in a scrollable -`charmbracelet/bubbles` viewport rather than scrolling off the terminal. - -### The hard constraint (read this first) - -`CLAUDE.md` deliberately forbids a bubbletea spinner today, for a real reason: - -> *No bubbletea/bubbles spinner anywhere, deliberately — it would swallow streamed -> pacstrap/yay output. The Runner streams stdout/stderr straight through.* - -A viewport TUI **does not get to ignore this** — it has to *solve* it. bubbletea takes -over the terminal (alt-screen) and owns the render loop, so subprocess output can no -longer go straight to `os.Stdout`; it must be **captured and fed into the model** as -messages, then rendered into the viewport. The whole design below is about doing that -capture correctly so long installers (`pacstrap`, `yay` building, `archinstall --silent`) -remain fully visible and live. - -This also means the constraint in `CLAUDE.md` should be **rewritten, not deleted**: the -rule becomes "output must be piped into the viewport line-stream, never written directly -to `os.Stdout` while the TUI owns the screen." - -### Architecture - -``` - ┌──────────────────────────────────────────────┐ - goroutine │ runPhase(stages...) │ - (worker) │ stage.Run(ctx) → ctx.R.Cmd(...) │ - │ │ writes lines │ - │ ▼ │ - │ Runner.Out (io.Writer) ── program.Send ──►│ tea.Msg{outputLine} - └──────────────────────────────────────────────┘ - │ - main thread ┌──────────────────────────────────────────────▼─┐ - (tea loop) │ model.Update → append line, autoscroll │ - │ model.View → header + viewport + footer │ - └─────────────────────────────────────────────────┘ -``` - -1. **Inject an output sink into the `Runner`** (the B2 change). Replace the hardcoded - `os.Stdout`/`os.Stderr` in `Cmd`/`Shell` (`run.go:39`, `:70`) with an `io.Writer` - field: - - ```go - type Runner struct { - DryRun bool - Sudo bool - Out io.Writer // defaults to os.Stdout in plain mode; a TUI pump in TUI mode - Plan []string - } - ``` - - Plain/CLI mode keeps `Out = os.Stdout` and behaves exactly as today (no regression for - non-TTY, CI, `--dry-run | less`). TUI mode sets `Out` to a writer that forwards each - line to the bubbletea program. - -2. **The pump writer** turns bytes into messages: - - ```go - type teaWriter struct{ p *tea.Program } - func (w teaWriter) Write(b []byte) (int, error) { - w.p.Send(outputMsg(string(b))) // one msg per write; split on \n in Update - return len(b), nil - } - ``` - - Subprocess stdout+stderr both point at this writer, so interleaving matches the - terminal. (For clean line handling, wrap with a `bufio.Scanner` in a small goroutine, - or buffer partial lines in the model.) - -3. **Run the phase in a goroutine**; the tea program runs on the main thread: - - ```go - p := tea.NewProgram(newModel(), tea.WithAltScreen(), tea.WithMouseCellMotion()) - ctx.R.Out = teaWriter{p} - go func() { - err := runStages(ctx, selected, func(s Stage){ p.Send(stageMsg{s.Order(), s.Name()}) }) - p.Send(doneMsg{err}) - }() - _, err := p.Run() - ``` - -4. **The model** is a viewport plus a header/footer: - - ```go - type model struct { - vp viewport.Model - buf strings.Builder // full scrollback - stage string // current stage header - spin spinner.Model // now allowed: output no longer goes to os.Stdout - done bool - err error - follow bool // auto-scroll to bottom unless the user scrolled up - } - ``` - - - `Update`: on `outputMsg`, append to `buf`, `vp.SetContent(buf.String())`, and if - `follow` then `vp.GotoBottom()`. On `tea.KeyMsg` (`PgUp`/`k`/mouse wheel) set - `follow=false`; `End`/`G` re-enables follow. On `tea.WindowSizeMsg` resize the - viewport (leave rows for header/footer). On `stageMsg` update the header; on - `doneMsg` set `done`/`err` and stop the spinner. - - `View`: `header (stage + spinner) + vp.View() + footer (scroll %, keybinds)`. - Reuse the existing `lipgloss` styles from `internal/ui/ui.go` for the header banner. - -5. **Interactive prompts** are the real complication: huh runs its *own* bubbletea - program, and you can't run two at once inside the alt-screen. Two clean options: - - - **(Recommended) Collect all input before the TUI starts.** The destructive - `ConfirmErase` and the `Password` prompt (`ui.go:57`, `:85`) already run *before* - any long work in `archinstall.go:38-53`. Run them first, in normal terminal mode, - then enter the alt-screen TUI for execution. Simplest and robust. - - **(Later) Embed huh forms as states** in the model (huh integrates with bubbletea): - the model transitions `confirm → password → running`. More work; only needed if a - prompt must appear mid-run. - -### Mode selection & fallback - -- Auto-detect: use the TUI only when stdout is a TTY (`term.IsTerminal`) **and** not - `--dry-run` piped. Add an explicit `--plain` (and/or `--tui`) flag to override. -- Plain mode is the existing code path unchanged — keep it as the fallback for CI, - non-TTY, and `| less`. This means the `ui` package grows a "plain vs program" notion; - `Step`/`OK`/`Info`/`Warn` either print to `os.Stderr` (today) or `program.Send` a - styled line (TUI). - -### Scope / sequencing for the TUI - -1. Land the **Runner output-sink** refactor first (it's the B2 item; valuable on its own). -2. Route `ui.Step`/`OK`/`Info`/`Warn`/`Header` through the same sink abstraction so all - output has one path. -3. Add the `internal/tui` package (model/update/view) and wire `runPhase` to it behind a - TTY check + `--plain` flag. -4. Promote `bubbletea`/`bubbles` from indirect to direct deps in `go.mod` (already in the - module graph via `huh`). -5. Update `CLAUDE.md`: replace the "no bubbletea" gotcha with the new rule — output must - flow through the viewport sink, never directly to `os.Stdout` while the TUI owns the - screen. - -### Risks / watch-items - -- **Output volume:** `pacstrap` and `yay` builds emit a lot; keep the full scrollback in a - buffer but consider a cap (e.g. last N lines / bytes) if memory or render cost bites. -- **ANSI in child output:** pacman/yay emit colour and progress carriage-returns. The - viewport renders text; `\r`-based progress bars will look messy. Either strip `\r` - redraws or accept that progress lines append. Test with a real `pacstrap`. -- **Performance:** one `program.Send` per write can flood the loop. Batch with a scanner - and send whole lines, or coalesce on a short tick. -- **Resize correctness:** recompute viewport height on every `WindowSizeMsg`. -- **Don't regress plain mode:** the existing streaming behaviour must remain byte-for-byte - for non-TTY/CI — that path is what tests and `--dry-run` pipelines rely on. - ---- - -## Remote & layered configuration (future) - -Goal: point `archwright` at a config that lives in a git repo / at a URL, and let that -config pull in and **merge** other configs — so machine-specific config can be tiny and -sit on top of a shared base. - -```sh -archwright install --config github.com/AdamJHall/dotfiles/archwright.desktop.yaml -``` - -```yaml -# archwright.desktop.yaml (the entry point: desktop-specific) -imports: - - archwright.base.yaml # sibling file in the same repo - - github.com/AdamJHall/dotfiles/archwright.kde.yaml # another file / repo - - https://example.com/teams/shared.yaml # raw URL - -system: - hostname: desktop-box # overrides whatever base set -packages: - - steam # added on top of base's packages -``` - -This **subsumes and supersedes** the deferred local-overlay idea (B3): the same deep-merge -engine powers both repeated `--config a --config b` (local overlays) and the in-file -`imports:` key (remote/relative). It also composes with env-var substitution (B3): expand -each file's `${VAR}` *after* fetch, *before* merge. (Heads-up from Wave 1: substitution -today scans raw bytes including comments — see the Wave 1 retrospective; moving to -value-only expansion is cleaner and a prerequisite for sane merging here.) - -### Config-source resolution - -`--config` (and each `imports:` entry) accepts three reference forms, distinguished by -shape: - -| Form | Example | Resolves to | -|------|---------|-------------| -| local path | `config.yaml`, `./desktop.yaml` | filesystem read (today's behaviour) | -| github shorthand | `github.com/OWNER/REPO/path/to.yaml[@ref]` | `https://raw.githubusercontent.com/OWNER/REPO//path/to.yaml` | -| raw URL | `https://…/file.yaml` | HTTP GET | - -A bare relative path **inside an `imports:` list resolves against the importing file's -location**, not the CWD — so a sibling in the same repo is just `archwright.base.yaml`, and -a github-rooted entry point makes its relative imports github-rooted too (fetch the sibling -raw URL). This is the key ergonomic the request asks for. - -### Merge semantics - -Layering is **base-first, importer-wins**, applied recursively: - -1. An imported file is resolved and merged *before* the file that imports it. -2. Among multiple `imports:`, **later entries override earlier** ones. -3. The importing file's own top-level keys override everything it imported. -4. Imports are processed depth-first; an imported file may itself have `imports:`. - -So in the example, effective precedence (low → high) is: -`base.yaml` → `kde.yaml` → `shared.yaml` → `desktop.yaml`. - -**The sharp edge is list fields.** Deep-merging maps is unambiguous; lists are not, and the -right answer differs per field: - -- **Union/append** is what you want for additive string lists: `packages`, `flatpaks`, - `aur`, `system.locales`, `user.groups`, `hooks`, `repos`. (Dedup plain string lists.) -- **Replace** is what you want for identity/layout lists where appending is nonsense: - `disks.lvm.pvs`, `disks.btrfs.subvolumes`. - -Recommended pragmatic default for a personal tool: **maps merge recursively; string-slice -fields union+dedup; structured-slice fields replace** (or, better, key-merge by `name` -where the element has one — `repos`, `hooks`, flatpak remotes — so a later layer can -override a single named entry). Provide an explicit escape hatch for the rare override: - -```yaml -packages: !replace [vim, git] # ignore inherited packages, use exactly this -``` - -Document the per-field strategy in one table next to the merge code, and add golden tests -for each (the existing golden harness makes this cheap). Don't try to be clever beyond -this. - -### The Phase A → Phase B flattening rule (important) - -Phase A runs from the live ISO (network + `curl`/`git` available, since archinstall needs -them) and **stages the binary + config into the target for Phase B** (`archinstall.go:243`, -`stageBinary`). With remote/layered config, do the fetch+merge **once** in Phase A, then -stage the **flattened, resolved config** (a single concrete YAML with no `imports:`) into -the target. Phase B then reads a plain local file and needs no network, no re-fetch, and is -guaranteed to see byte-identical config to Phase A. Never re-resolve remotely in Phase B. - -(`archwright render --config -o config.flat.yaml` — a "resolve & merge, write the -result, change nothing" command — is the natural primitive here, and doubles as the -debugging tool for the merge engine.) - -### Trust, pinning, caching, auth - -Fetching config that drives **destructive disk operations and arbitrary hook commands** -from a URL is a real trust boundary — treat it like one: - -- **Pin a ref.** Encourage `@` on github shorthands; warn (or, with - `--strict`, refuse) on an unpinned `main`. Optionally record the resolved commit SHA into - the flattened config's provenance comment. -- **Show before you run.** Phase A is already gated by the `ConfirmErase` prompt; for a - remote config, print the resolved source list + merged result (or a diff) before that - prompt so the user sees exactly what they're about to execute. -- **Cache + `--offline`.** Cache fetched files (e.g. under `$XDG_CACHE_HOME/archwright/`) - keyed by URL+ref; `--offline` uses the cache only. Useful when re-running on a flaky live - ISO network. -- **Private repos.** Public raw URLs cover public dotfiles. For private, support a token via - env (`GITHUB_TOKEN`) in the `Authorization` header, or fall back to `git clone` over the - user's existing credentials. Keep tokens out of the config file (they belong in env, which - the substitution feature already reads). -- **Bound recursion.** Detect import cycles (track visited canonical URLs) and cap import - depth; fail with the cycle path, not a stack overflow. - -### Implementation sketch - -A small `internal/configsrc` (resolver) sitting in front of the existing `config.Load`: - -```go -// Resolve fetches ref (local | github | url), expands env, recursively resolves its -// imports, and deep-merges into one *config.Config. visited guards against cycles. -func Resolve(ref string, base *url.URL, visited map[string]bool) (*config.Config, error) - -// raw bytes → struct merge; layer order is base-first, importer-wins (see table). -func merge(dst, src map[string]any, strategy fieldStrategy) // generic map/list merge -``` - -Notes: - -- Merge at the **generic `map[string]any`** level (parse each layer with `yaml.Node` / - `map[string]any`, merge, then `yaml.Unmarshal` the result into `config.Config` and - `Validate()` once at the end). This keeps the merge engine independent of the config - schema, so new config fields need no merge-code changes — only the per-field list - strategy table does. -- `imports:` is a resolver-level key, consumed and stripped before the final - `config.Config` unmarshal (it isn't a `Config` field). -- `--config` becoming repeatable (`[]string`, last wins) is the same merge applied to - CLI-supplied layers — implement once, reuse. - -### Priority / cost - -**✅ Landed in Wave 5.** (Originally deferred.) It was MEDIUM value (great quality-of-life -for a multi-machine dotfiles-driven setup), MEDIUM-HIGH cost — the generic deep-merge with a -sane per-field list strategy is the bulk of the work, and the trust/pinning story must land -with it, not after. Sequence it **after** the framework foundations (env-var substitution -and the Runner/selection work), since it reuses the substitution hook and the `render` -command. diff --git a/docs/vm-validation.md b/docs/vm-validation.md new file mode 100644 index 0000000..f0a9574 --- /dev/null +++ b/docs/vm-validation.md @@ -0,0 +1,41 @@ +# VM validation of the rendered archinstall config — outstanding work + +This is the one open item before trusting archwright on real hardware. + +archinstall's config JSON is **not a stable API** — its schema changes between releases. We +render against the pinned `Version` in `internal/archinstall/archinstall.go` (currently +`4.3`), and the JSON shape (LVM, swap, encryption, bootloader, PV `obj_id` wiring, creds +keys) was **reverse-engineered from archinstall source**. The render tests +(`go test ./internal/archinstall/`, the golden snapshots, and the `e2e` / `e2e-disks` +workflows) prove our output is *stable and self-consistent*, and `e2e-disks` confirms a real +archinstall *parses* it against loopback devices — but none of that proves a real archinstall +*does the right thing with it end-to-end on a booted system*. + +That last mile needs a QEMU run that boots a real systemd live ISO, feeds it the rendered +config, and verifies the machine partitions, installs, and **boots to a desktop**. The +harness exists (`test/vm.sh`, `task vm` / `task vm-fresh` / `task vm-disk`); what remains is +to actually run each layout/feature through it and confirm the reverse-engineered shapes +below. + +> Use `-cpu host` for local VM runs — otherwise the CachyOS repo setup skips and +> `linux-cachyos` fails with "target not found". + +## Shapes to confirm against a real archinstall 4.3 run + +Each was reverse-engineered and is unproven on hardware. Validate, then delete its row here. + +| Area | Shape to confirm | +|------|------------------| +| Bootloader | `bootloader_config: {bootloader, uki, removable}` field names/casing | +| Btrfs | subvolume JSON `{name, mountpoint}` — whether archinstall wants extra keys (per-subvol compression, `nodatacow`); `disk_config.btrfs_options` is intentionally not emitted | +| Swap | `partition` shape (`fs_type: linux-swap`, flag `swap`); zram and swapfile paths | +| Encryption | nested `disk_config.disk_encryption` (`encryption_type` + `partitions`) obj_id wiring; `encryption_password` casing; the `lvm_on_luks` >2-partition limit | +| systemd-boot | loader-entry default + the `bootctl update` cmdline-refresh path | +| LVM | multi-volume "rest of VG" sizing (fixed root + remainder-taking `/home`) | +| Snapper | timer-unit + `set-config` key names (`snapper-timeline.timer`, `snapper-cleanup.timer`, `TIMELINE_LIMIT_*`) | + +## After an archinstall version bump + +Diff the upstream schema and update `internal/archinstall/` **and** the `Version` constant +together, then re-run the VM validation above. Preflight only *warns* on a version mismatch; +it does not block. diff --git a/docs/wave4-handoff.md b/docs/wave4-handoff.md deleted file mode 100644 index 169ba67..0000000 --- a/docs/wave4-handoff.md +++ /dev/null @@ -1,119 +0,0 @@ -# Wave 4 — handoff / resume notes - -Status: **planning complete, not yet started.** Paused at 91% usage limit before launching -agents. Resume by launching the three worktree agents below, merging into one branch, and -opening a PR. Everything below is the result of reviewing `docs/extensibility-review.md` and -the current `main` (Wave 3 landed, commit `7d97bdd`). - -## What "next wave" means (from the review doc) - -`extensibility-review.md` "Next wave starts here" names three candidates: -1. **TUI conversion** (scrollable viewport) — big, fully designed in the doc's TUI section. -2. **Remote & layered configuration** — big, but **explicitly deferred** ("Defer until you - actually run >1 machine"). **Excluded from Wave 4** — respect the deferral. -3. **Snapper wiring** for `btrfs.snapshots: snapper` — carried in config since Wave 2 but - not provisioned (archinstall.go:626 leaves it "to a post-install/hook step"). - -## Chosen Wave 4 scope (3 parallel worktree agents → 1 branch → PR) - -Partitioned by **file ownership** per the Wave 1–3 retrospective rule (distinct anchors in -shared hotspots; new test files only, never shared fixtures; explicit `git add `). - -### Agent A — TUI conversion (headline) -- **New** `internal/tui/` package: bubbletea model/update/view (viewport + header/footer), - `teaWriter` pump that forwards Runner output as `tea.Msg`. Design is fully spec'd in - `extensibility-review.md` → "TUI conversion" section (model fields, follow/autoscroll, - WindowSizeMsg resize, doneMsg/stageMsg). -- Route `internal/ui/ui.go` output through a sink so Step/OK/Info/Warn/Header can go to the - viewport in TUI mode and to os.Stderr in plain mode. -- Wire `runPhase` (main.go) to run the phase in a goroutine under `tea.Program`, behind a - TTY check + new `--plain`/`--tui` flag. **Collect prompts (ConfirmErase/Password) BEFORE - entering the alt-screen** (recommended option in the doc). -- Promote `bubbletea`/`bubbles` from indirect → direct in `go.mod` (already in module graph - via huh; Runner `Out io.Writer` sink already exists from Wave 0 — `internal/run/run.go:26`). -- Plain mode must stay **byte-for-byte unchanged** for non-TTY/CI/`--dry-run | less`. -- Update `CLAUDE.md`: replace the "no bubbletea spinner" gotcha with "output must flow - through the viewport sink, never directly to os.Stdout while the TUI owns the screen." -- **Owns:** `internal/tui/*` (new), `internal/ui/ui.go`, `main.go` (runPhase + `--plain` - flag), `go.mod`/`go.sum`, `CLAUDE.md` gotcha line. -- **Tests:** table-driven model Update tests (outputMsg appends + autoscroll; key toggles - follow; resize), plain-mode-unchanged assertion. New `internal/tui/*_test.go`. - -### Agent B — Snapper provisioning stage (closes Wave 2 gap) -- **New** `internal/stages/snapper.go` + `internal/stages/snapper_test.go`. Phase B stage. -- No-op unless `ctx.Cfg.Disks.EffectiveLayout() == "btrfs"` **and** - `ctx.Cfg.Disks.Btrfs.Snapshots == "snapper"` (degrade-to-clean-no-op, the project's - uniform pattern — mirror `dotfiles.go`'s selector style). -- When active: `ensureTool` snapper + install `snap-pac`; create root config - (`snapper -c root create-config /`), set sane retention, enable - `snapper-timeline.timer` + `snapper-cleanup.timer` (all via `ctx.R.Root`, idempotent / - grep-guarded where needed — reuse `internal/stages/helpers.go`). -- Pick an Order that fits Phase B (e.g. after packages, before dotfiles — check current - orders via `list-stages`; keep stable). -- **Owns:** only the two new files. **Zero** overlap with A or C. -- **Tests:** dry-run plan assertions (active → expected commands; snapshots:none and - non-btrfs layouts → empty/skip). New test file, self-contained inline config. - -### Agent C — `--from` / `--to` stage resume (B1 remaining ergonomic) -- Add `--from ` / `--to ` persistent flags (name-or-number, like `--only`). -- Implement as a **post-filter applied AFTER `stages.Select(...)`** in runPhase — do NOT - change the `Select(...)` signature/call line (that line is in runPhase which Agent A - rewrites; keeping it intact avoids a guaranteed conflict). e.g. add - `stages.Within(selected, from, to) []Stage` in `stages.go` and call it on the result. -- `--only` still wins over everything; `--from/--to` compose with `--skip`/`disable`. -- **Owns:** `internal/stages/stages.go` (new `Within` func), `main.go` (flag declarations - block + one added filter line in runPhase), new `internal/stages/fromto_test.go`. -- **Tests:** table-driven `Within` (from only, to only, both, inverted bounds, unknown - stage). New test file. - -### Known merge points (orchestrator hand-merges — expected, per retrospectives) -- `main.go`: A and C both add persistent flags (distinct lines) and both touch runPhase - (A rewrites execution; C adds one filter line). Hand-merge the flag block + runPhase. -- Everything else auto-merges (distinct files / distinct regions). - -## Execution plan (resume here) - -1. Launch agents A, B, C in parallel via the **Agent tool with `isolation: "worktree"`** - (NOT hand-rolled `git worktree` — Wave 2 retro: external worktrees are outside the - sandbox and get denied). Run foreground or background; each returns its final summary. -2. Each agent: **TDD red-green-refactor**, idiomatic Go (`golang-patterns`), table-driven - tests (`golang-testing`), `gofmt`/`go vet`/`go build`/`go test ./...` clean before - reporting done. Conventional-commit messages (subject ≤50 chars, body only when "why" - isn't obvious), each commit trailer: - `Co-Authored-By: Claude Opus 4.8 `. -3. Create `wave4/tui-snapper-fromto` off `main`. Merge each agent branch/worktree in. - Hand-merge `main.go`. Run `gofmt -l .` (must be empty), `go vet ./...`, - `go build -o archwright .`, `go test ./...` — **execute tests, confirm green**. -4. Fix any cross-file follow-ups centrally (Wave 3 pattern: e.g. an unowned file affected - by a feature — agents should *report* such, orchestrator fixes + adds a regression test). -5. Update `docs/extensibility-review.md`: add a **Wave 4 retrospective** and flip the TUI / - snapper items to ✅; note remaining deferred items (remote/layered config; VM-validation - caveats carried from Waves 2–3). -6. Open PR with `gh` into `main`. PR body ends with: - `🤖 Generated with [Claude Code](https://claude.com/claude-code)`. -7. **Loop until complete** (the user's instruction): if tests fail or review surfaces - issues, iterate before declaring done. - -## Grounding facts gathered (so resume needs no re-discovery) - -- Runner already has `Out io.Writer` sink + `Env`/`Dir`/`Capture` (Wave 0) — - `internal/run/run.go:19-101`. TUI just needs to set `Out` to the pump writer. -- `bubbletea` v1.3.6 + `bubbles` v0.21.x already in `go.mod` as indirect (via huh). -- `ui` output all goes to `os.Stderr` today (`internal/ui/ui.go`); `Header/Step/OK` use - lipgloss, `Info/Warn/Error` use charmbracelet/log, prompts use huh. -- Config paths: `ctx.Cfg.Disks DisksConfig` (`config.go:46`), `DisksConfig.EffectiveLayout()` - (`config.go:139`), `Disks.Btrfs *BtrfsLayout` with `Snapshots` field validated - `oneof=snapper none` (`config.go:205-208`). Example config documents - `snapshots: snapper # ... (snapper setup itself is left to a hook)` (config.example.yaml:88). -- Stage pattern reference: `internal/stages/dotfiles.go` (selector + degrade-to-no-op), - shared helpers in `internal/stages/helpers.go` (`ensureTool`, grep-guarded sed, etc.). -- Stage selection: `stages.Select(p, only, skip, disable)` (`stages.go:66`); `For`, - `All` also there. `list-stages` command already exists (main.go:85). -- Tests never touch disks: run a stage in dry-run, assert on `ctx.R.Plan`. `go test ./...`, - `go vet ./...`, `go build -o archwright .`. - -## Excluded from Wave 4 (deliberate) -- **Remote/layered config** — explicitly deferred in the doc; big merge-engine + trust - story; do after >1 machine need. -- **VM-validation caveats** (Waves 2–3 reverse-engineered archinstall shapes) — need a real - archinstall 4.3 run in QEMU; tracked in `docs/e2e-testing-plan.md`, not a code wave. diff --git a/internal/archinstall/bootloader_render_test.go b/internal/archinstall/bootloader_render_test.go index 9bfebc3..c51a422 100644 --- a/internal/archinstall/bootloader_render_test.go +++ b/internal/archinstall/bootloader_render_test.go @@ -7,7 +7,7 @@ import ( "gopkg.in/yaml.v3" ) -// bootloaderYAML is a minimal lvm config (the historical default layout) used to +// bootloaderYAML is a minimal lvm config (the default layout) used to // exercise the bootloader mapping in Build. The bootloader block is appended by // the test so both kinds reuse the same base. const bootloaderBaseYAML = ` diff --git a/internal/archinstall/volumes_test.go b/internal/archinstall/volumes_test.go index 719932f..4efabbb 100644 --- a/internal/archinstall/volumes_test.go +++ b/internal/archinstall/volumes_test.go @@ -146,7 +146,7 @@ disks: } // TestBuild_LVMSingleVolumeUnchanged proves an empty-Volumes config still emits -// exactly one root LV (the historical single-LV path). +// exactly one root LV (the single-LV path). func TestBuild_LVMSingleVolumeUnchanged(t *testing.T) { yamlSrc := ` system: diff --git a/internal/config/config.go b/internal/config/config.go index 1e4913b..23bb7ed 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -97,12 +97,8 @@ type Config struct { Environment string `yaml:"environment" validate:"omitempty,oneof=kde gnome hyprland sway none"` } `yaml:"desktop"` - Chezmoi struct { - Repo string `yaml:"repo" validate:"omitempty,url"` - } `yaml:"chezmoi"` - // Dotfiles selects how dotfiles are applied in Phase B. Manager defaults to - // "chezmoi"; Repo defaults to chezmoi.repo when unset (backward compatible). + // "chezmoi". Repo is required for any manager other than "none". Dotfiles struct { Manager string `yaml:"manager" validate:"omitempty,oneof=chezmoi yadm bare-git none"` Repo string `yaml:"repo" validate:"omitempty,url"` @@ -121,12 +117,6 @@ type Config struct { // Empty defaults to "grub" (today's behavior). systemd-boot is reverse-engineered // and VM-validation-pending (see CLAUDE.md archinstall-drift rule). Bootloader BootloaderConfig `yaml:"bootloader"` - - // PacstrapExtraDeprecated captures the removed `pacstrap_extra` key so - // semanticErrors can emit a clear rename-to-`pacstrap` migration error for one - // release, rather than silently ignoring a config that used the old key. It is - // not part of the schema and has no validate rules. - PacstrapExtraDeprecated []string `yaml:"pacstrap_extra"` } // DisksConfig describes the disk layout archinstall should create. Layout is a @@ -187,11 +177,10 @@ func (s SwapConfig) EffectiveType() string { return s.Type } -// LVMLayout is the classic ESP + LVM-on-partitions root (the historical default). +// LVMLayout is the ESP + LVM-on-partitions root layout (the default). // // Two mutually-exclusive shapes (enforced in lvmVolumeErrors): -// - single root LV: set LV + Filesystem, leave Volumes empty (the historical -// shape; behaviour and rendered output are byte-identical). +// - single root LV: set LV + Filesystem, leave Volumes empty. // - multiple volumes: set Volumes (e.g. a fixed root + a /home that takes the // remainder), leave LV/Filesystem empty. type LVMLayout struct { @@ -246,8 +235,8 @@ type Encryption struct { Type string `yaml:"type" validate:"required,oneof=luks lvm_on_luks"` } -// SetupConfig drives the Phase B 85-setup stage, which runs after chezmoi has -// applied the dotfiles. It covers the things a dotfiles repo references but can't +// SetupConfig drives the Phase B 85-setup stage, which runs after the dotfiles +// stage has applied the dotfiles. It covers the things a dotfiles repo references but can't // vendor itself — oh-my-zsh and its custom plugins, tmux's TPM, theme repos. // // Steps run strictly in the order written, so a clone that lands inside another @@ -472,9 +461,6 @@ func (c *Config) Validate() error { // semanticErrors covers cross-field rules that struct tags can't express. func (c *Config) semanticErrors() []error { var errs []error - if len(c.PacstrapExtraDeprecated) > 0 { - errs = append(errs, fmt.Errorf("pacstrap_extra has been removed: rename it to `pacstrap` and add the base set (base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode) — see config.example.yaml")) - } k := c.Kernel if k.ReplaceStock && len(k.Packages) == 0 { errs = append(errs, fmt.Errorf("kernel.replace_stock requires at least one kernel.packages entry (otherwise the system would have no kernel)")) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 64a2f7f..2dc58c9 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -59,7 +59,7 @@ grub: name: tela kde: look_and_feel: org.kde.breezedark.desktop -chezmoi: +dotfiles: repo: https://github.com/AdamJHall/dotfiles setup: steps: @@ -91,15 +91,15 @@ func TestLoad_EnvSubstitution(t *testing.T) { t.Run("substitutes ${VAR} and $VAR", func(t *testing.T) { t.Setenv("AW_USER", "adam") t.Setenv("AW_REPO", "https://example.com/dotfiles") - cfg, err := Load(write(t, "user:\n name: ${AW_USER}\nchezmoi:\n repo: $AW_REPO\n")) + cfg, err := Load(write(t, "user:\n name: ${AW_USER}\ndotfiles:\n repo: $AW_REPO\n")) if err != nil { t.Fatalf("Load: %v", err) } if cfg.User.Name != "adam" { t.Errorf("user.name = %q, want adam", cfg.User.Name) } - if cfg.Chezmoi.Repo != "https://example.com/dotfiles" { - t.Errorf("chezmoi.repo = %q", cfg.Chezmoi.Repo) + if cfg.Dotfiles.Repo != "https://example.com/dotfiles" { + t.Errorf("dotfiles.repo = %q", cfg.Dotfiles.Repo) } }) @@ -232,13 +232,6 @@ func TestValidate_Errors(t *testing.T) { "pacstrap: []", 1), want: []string{"pacstrap must have at least 1 item(s)"}, }, - { - name: "deprecated pacstrap_extra key errors", - yaml: strings.Replace(validYAML, - "pacstrap: [base-devel, git, zsh, sudo, networkmanager, efibootmgr, intel-ucode]", - "pacstrap: [base]\npacstrap_extra: [intel-ucode]", 1), - want: []string{"pacstrap_extra has been removed", "rename it to `pacstrap`"}, - }, { name: "bad reflector sort", yaml: strings.Replace(validYAML, "sort: rate", "sort: bogus", 1), diff --git a/internal/config/lvm_volumes_test.go b/internal/config/lvm_volumes_test.go index a038df5..df17492 100644 --- a/internal/config/lvm_volumes_test.go +++ b/internal/config/lvm_volumes_test.go @@ -15,7 +15,7 @@ func TestValidate_LVMVolumes(t *testing.T) { wantErr []string // substrings; empty means must be valid }{ { - name: "single root LV (historical) is valid", + name: "single root LV is valid", disks: ` disks: esp: {device: /dev/nvme0n1, size: 1GiB} diff --git a/internal/stages/dotfiles.go b/internal/stages/dotfiles.go index 177686b..41e677a 100644 --- a/internal/stages/dotfiles.go +++ b/internal/stages/dotfiles.go @@ -12,8 +12,7 @@ import ( // manager and apply the dotfiles. Final Phase B step before setup. // // The manager is selectable (chezmoi, yadm, bare-git, none); chezmoi is the -// default and preserves the historical behaviour. The repo defaults to -// chezmoi.repo when dotfiles.repo is unset, so pre-existing configs keep working. +// default. type dotfiles struct{} func init() { register(dotfiles{}) } @@ -30,14 +29,6 @@ func (dotfiles) effectiveManager(ctx *Context) string { return "chezmoi" } -// effectiveRepo returns dotfiles.repo, falling back to the legacy chezmoi.repo. -func (dotfiles) effectiveRepo(ctx *Context) string { - if r := ctx.Cfg.Dotfiles.Repo; r != "" { - return r - } - return ctx.Cfg.Chezmoi.Repo -} - func (d dotfiles) Run(ctx *Context) error { manager := d.effectiveManager(ctx) if manager == "none" { @@ -45,7 +36,7 @@ func (d dotfiles) Run(ctx *Context) error { return nil } - repo := d.effectiveRepo(ctx) + repo := ctx.Cfg.Dotfiles.Repo if repo == "" { ui.Warn("no dotfiles repo configured — skipping") return nil @@ -72,8 +63,8 @@ func (d dotfiles) Run(ctx *Context) error { return nil } -// runChezmoi preserves the historical behaviour exactly: install chezmoi, then -// `chezmoi apply` if already initialized, else `chezmoi init --apply `. +// runChezmoi installs chezmoi, then runs `chezmoi apply` if already initialized, +// else `chezmoi init --apply `. func (dotfiles) runChezmoi(ctx *Context, repo string) error { if err := ensureTool(ctx, "chezmoi", "chezmoi"); err != nil { return err diff --git a/internal/stages/dotfiles_test.go b/internal/stages/dotfiles_test.go index ce0ab78..3d8c3f3 100644 --- a/internal/stages/dotfiles_test.go +++ b/internal/stages/dotfiles_test.go @@ -16,9 +16,9 @@ import ( func chezmoiInitialized() bool { return homeHas(".local/share/chezmoi/.git") } func TestDotfiles_ChezmoiDefault(t *testing.T) { - // Unset manager defaults to chezmoi; unset dotfiles.repo falls back to chezmoi.repo. + // Unset manager defaults to chezmoi; the repo comes from dotfiles.repo. plan := planForCfg(t, Bootstrap, "dotfiles", ` -chezmoi: +dotfiles: repo: https://github.com/example/dotfiles `) if chezmoiInitialized() { @@ -29,22 +29,17 @@ chezmoi: } func TestDotfiles_ChezmoiExplicit(t *testing.T) { - // Explicit manager + dotfiles.repo (which takes precedence over chezmoi.repo). + // Explicit manager + repo. plan := planForCfg(t, Bootstrap, "dotfiles", ` dotfiles: manager: chezmoi repo: https://github.com/example/df-explicit -chezmoi: - repo: https://github.com/example/legacy `) if chezmoiInitialized() { mustContain(t, plan, "chezmoi apply") return } mustContain(t, plan, "chezmoi init --apply https://github.com/example/df-explicit") - if j := strings.Join(plan, "\n"); strings.Contains(j, "legacy") { - t.Errorf("dotfiles.repo should win over chezmoi.repo, got plan:\n%s", j) - } } func TestDotfiles_Yadm(t *testing.T) { @@ -88,7 +83,7 @@ dotfiles: } func TestDotfiles_UnsetRepoSkips(t *testing.T) { - // A non-none manager with no repo (and no chezmoi.repo fallback) is a clean skip. + // A non-none manager with no repo is a clean skip. plan := planForCfg(t, Bootstrap, "dotfiles", ` dotfiles: manager: chezmoi diff --git a/internal/stages/postinstall.go b/internal/stages/postinstall.go index ddfb64d..96d9bea 100644 --- a/internal/stages/postinstall.go +++ b/internal/stages/postinstall.go @@ -116,8 +116,7 @@ func installKernels(ctx *Context, k config.KernelConfig) error { } // installKernelsGrub sets the GRUB default kernel (GRUB_TOP_LEVEL) and regenerates -// grub.cfg so the new entries (and default) take effect on first boot. This is the -// historical behaviour and stays byte-identical to before. +// grub.cfg so the new entries (and default) take effect on first boot. func installKernelsGrub(ctx *Context, k config.KernelConfig) error { if k.Default != "" { // GRUB_TOP_LEVEL pins a specific kernel image as the default menu entry. diff --git a/internal/stages/stages_test.go b/internal/stages/stages_test.go index 50e0753..a1d16be 100644 --- a/internal/stages/stages_test.go +++ b/internal/stages/stages_test.go @@ -69,7 +69,7 @@ grub: name: tela kde: look_and_feel: org.kde.breezedark.desktop -chezmoi: +dotfiles: repo: https://github.com/AdamJHall/dotfiles setup: steps: