refactor: simplify the system interface

.github/copilot-instructions.md

@@ -34,9 +34,9 @@ This file provides specific guidance for code review in the concierge repository
 - **Use the Worker interface**: All system commands must go through `system.Worker`
   - Direct use of `exec.Command()` is prohibited
   - Use `system.NewCommand()` to construct commands
-  - Prefer `system.RunMany()` for sequential independent operations
+  - Prefer `system.RunMany(w, ...)` helper for sequential independent operations
 
-- **Exclusive operations**: Use `RunExclusive()` for operations requiring locks
+- **Exclusive operations**: Use `system.RunExclusive(w, cmd)` for operations requiring locks
   - Package installations (snap, apt)
   - State-modifying operations that can conflict
 

AGENTS.md

@@ -73,7 +73,7 @@ Note: The binary must be run with `sudo` for most operations since it installs s
 
 - **`internal/system/`**: System abstraction layer
   - `interface.go`: Worker interface for system operations
-  - `runner.go`: Executes shell commands with retries and locking
+  - `runner.go`: Executes shell commands
   - `snap.go`: Snap-specific utilities
 
 ### Key Design Patterns
@@ -87,8 +87,8 @@ Note: The binary must be run with `sudo` for most operations since it installs s
 
 3. **Worker Interface**: All system operations go through the `system.Worker` interface, enabling:
    - Testability via mock implementations
-   - Consistent command execution with retries
-   - Safe file operations in user home directories
+   - Consistent command execution with package-level helpers (e.g. `system.RunExclusive(w, cmd)`, `system.RunWithRetries(w, cmd, d)`, `system.RunMany(w, cmds...)`)
+   - Safe file operations via helper functions (e.g. `system.WriteHomeDirFile`, `system.ReadHomeDirFile`)
 
 4. **Runtime Config Caching**: During `prepare`, the merged configuration (including all overrides) is saved to `~/.cache/concierge/concierge.yaml`. The `restore` command reads this file to undo exactly what was provisioned.
 

internal/concierge/manager.go

@@ -97,7 +97,7 @@ func (m *Manager) recordRuntimeConfig(status config.Status) error {
 	}
 
 	filepath := path.Join(".cache", "concierge", "concierge.yaml")
-	err = m.system.WriteHomeDirFile(filepath, configYaml)
+	err = system.WriteHomeDirFile(m.system, filepath, configYaml)
 	if err != nil {
 		return fmt.Errorf("failed to write runtime config file: %w", err)
 	}
@@ -112,7 +112,7 @@ func (m *Manager) recordRuntimeConfig(status config.Status) error {
 func (m *Manager) loadRuntimeConfig() error {
 	recordPath := path.Join(".cache", "concierge", "concierge.yaml")
 
-	contents, err := m.system.ReadHomeDirFile(recordPath)
+	contents, err := system.ReadHomeDirFile(m.system, recordPath)
 	if err != nil {
 		return fmt.Errorf("failed to read file: %w", err)
 	}
@@ -139,7 +139,7 @@ func (m *Manager) loadRuntimeConfig() error {
 func (m *Manager) Status() (config.Status, error) {
 	recordPath := path.Join(".cache", "concierge", "concierge.yaml")
 
-	contents, err := m.system.ReadHomeDirFile(recordPath)
+	contents, err := system.ReadHomeDirFile(m.system, recordPath)
 	if err != nil {
 		return 0, fmt.Errorf("concierge has not prepared this machine and cannot report its status")
 	}

internal/juju/juju.go

@@ -157,7 +157,7 @@ func (j *JujuHandler) writeCredentials() error {
 		return fmt.Errorf("failed to marshal juju credentials to yaml: %w", err)
 	}
 
-	err = j.system.WriteHomeDirFile(path.Join(".local", "share", "juju", "credentials.yaml"), content)
+	err = system.WriteHomeDirFile(j.system, path.Join(".local", "share", "juju", "credentials.yaml"), content)
 	if err != nil {
 		return fmt.Errorf("failed to write credentials.yaml: %w", err)
 	}
@@ -238,7 +238,7 @@ func (j *JujuHandler) bootstrapProvider(provider providers.Provider) error {
 	user := j.system.User().Username
 
 	cmd := system.NewCommandAs(user, provider.GroupName(), "juju", bootstrapArgs)
-	_, err = j.system.RunWithRetries(cmd, (5 * time.Minute))
+	_, err = system.RunWithRetries(j.system, cmd, 5*time.Minute)
 	if err != nil {
 		return err
 	}

internal/juju/juju_test.go

@@ -176,7 +176,7 @@ func TestJujuHandlerWithCredentialedProvider(t *testing.T) {
 		t.Fatal(err.Error())
 	}
 
-	expectedFiles := map[string]string{".local/share/juju/credentials.yaml": string(expectedCredsFileContent)}
+	expectedFiles := map[string]string{path.Join(os.TempDir(), ".local", "share", "juju", "credentials.yaml"): string(expectedCredsFileContent)}
 
 	if !reflect.DeepEqual(expectedFiles, system.CreatedFiles) {
 		t.Fatalf("expected: %v, got: %v", expectedFiles, system.CreatedFiles)

internal/packages/deb_handler.go

@@ -62,7 +62,7 @@ func (h *DebHandler) Restore() error {
 
 	cmd := system.NewCommand("apt-get", []string{"autoremove", "-y"})
 
-	_, err := h.system.RunExclusive(cmd)
+	_, err := system.RunExclusive(h.system, cmd)
 	if err != nil {
 		return fmt.Errorf("failed to install apt package: %w", err)
 	}
@@ -74,7 +74,7 @@ func (h *DebHandler) Restore() error {
 func (h *DebHandler) installDeb(d *Deb) error {
 	cmd := system.NewCommand("apt-get", []string{"install", "-y", d.Name})
 
-	_, err := h.system.RunExclusive(cmd)
+	_, err := system.RunExclusive(h.system, cmd)
 	if err != nil {
 		return fmt.Errorf("failed to install apt package '%s': %w", d.Name, err)
 	}
@@ -87,7 +87,7 @@ func (h *DebHandler) installDeb(d *Deb) error {
 func (h *DebHandler) removeDeb(d *Deb) error {
 	cmd := system.NewCommand("apt-get", []string{"remove", "-y", d.Name})
 
-	_, err := h.system.RunExclusive(cmd)
+	_, err := system.RunExclusive(h.system, cmd)
 	if err != nil {
 		return fmt.Errorf("failed to remove apt package '%s': %w", d.Name, err)
 	}
@@ -100,7 +100,7 @@ func (h *DebHandler) removeDeb(d *Deb) error {
 func (h *DebHandler) updateAptCache() error {
 	cmd := system.NewCommand("apt-get", []string{"update"})
 
-	_, err := h.system.RunExclusive(cmd)
+	_, err := system.RunExclusive(h.system, cmd)
 	if err != nil {
 		return fmt.Errorf("failed to update apt package lists: %w", err)
 	}

internal/packages/snap_handler.go

@@ -79,7 +79,7 @@ func (h *SnapHandler) installSnap(s *system.Snap) error {
 	}
 
 	cmd := system.NewCommand("snap", args)
-	_, err = h.system.RunExclusive(cmd)
+	_, err = system.RunExclusive(h.system, cmd)
 	if err != nil {
 		return fmt.Errorf("command failed: %w", err)
 	}
@@ -99,7 +99,7 @@ func (h *SnapHandler) connectSnap(s *system.Snap) error {
 		args := append([]string{"connect"}, parts...)
 
 		cmd := system.NewCommand("snap", args)
-		_, err := h.system.RunExclusive(cmd)
+		_, err := system.RunExclusive(h.system, cmd)
 		if err != nil {
 			return fmt.Errorf("command failed: %w", err)
 		}
@@ -113,7 +113,7 @@ func (h *SnapHandler) removeSnap(s *system.Snap) error {
 	args := []string{"remove", s.Name, "--purge"}
 
 	cmd := system.NewCommand("snap", args)
-	_, err := h.system.RunExclusive(cmd)
+	_, err := system.RunExclusive(h.system, cmd)
 	if err != nil {
 		return fmt.Errorf("failed to remove snap '%s': %w", s.Name, err)
 	}

internal/providers/k8s.go

@@ -176,14 +176,14 @@ func (k *K8s) init() error {
 
 	if k.needsBootstrap() {
 		cmd := system.NewCommand("k8s", []string{"bootstrap"})
-		_, err := k.system.RunWithRetries(cmd, (5 * time.Minute))
+		_, err := system.RunWithRetries(k.system, cmd, 5*time.Minute)
 		if err != nil {
 			return err
 		}
 	}
 
 	cmd := system.NewCommand("k8s", []string{"status", "--wait-ready", "--timeout", "270s"})
-	_, err := k.system.RunWithRetries(cmd, (5 * time.Minute))
+	_, err := system.RunWithRetries(k.system, cmd, 5*time.Minute)
 
 	return err
 }
@@ -202,7 +202,7 @@ func (k *K8s) configureFeatures() error {
 		}
 
 		cmd := system.NewCommand("k8s", []string{"enable", featureName})
-		_, err := k.system.RunWithRetries(cmd, (5 * time.Minute))
+		_, err := system.RunWithRetries(k.system, cmd, 5*time.Minute)
 		if err != nil {
 			return fmt.Errorf("failed to enable K8s addon '%s': %w", featureName, err)
 		}
@@ -220,7 +220,7 @@ func (k *K8s) setupKubectl() error {
 		return fmt.Errorf("failed to fetch K8s configuration: %w", err)
 	}
 
-	return k.system.WriteHomeDirFile(path.Join(".kube", "config"), result)
+	return system.WriteHomeDirFile(k.system, path.Join(".kube", "config"), result)
 }
 
 func (k *K8s) needsBootstrap() bool {

internal/providers/k8s_test.go

@@ -94,7 +94,7 @@ func TestK8sPrepareCommands(t *testing.T) {
 	}
 
 	expectedFiles := map[string]string{
-		".kube/config": "",
+		path.Join(os.TempDir(), ".kube", "config"): "",
 	}
 
 	system := system.NewMockSystem()
@@ -137,7 +137,7 @@ func TestK8sPrepareCommandsAlreadyBootstrappedIptablesInstalled(t *testing.T) {
 	}
 
 	expectedFiles := map[string]string{
-		".kube/config": "",
+		path.Join(os.TempDir(), ".kube", "config"): "",
 	}
 
 	system := system.NewMockSystem()

internal/providers/lxd.go

@@ -122,7 +122,7 @@ func (l *LXD) install() error {
 	if restart {
 		args := []string{"start", l.Name()}
 		cmd := system.NewCommand("snap", args)
-		_, err = l.system.RunExclusive(cmd)
+		_, err = system.RunExclusive(l.system, cmd)
 		if err != nil {
 			return err
 		}
@@ -133,7 +133,7 @@ func (l *LXD) install() error {
 
 // init ensures that LXD is minimally configured, and ready.
 func (l *LXD) init() error {
-	return l.system.RunMany(
+	return system.RunMany(l.system,
 		system.NewCommand("lxd", []string{"waitready", "--timeout", "270"}),
 		system.NewCommand("lxd", []string{"init", "--minimal"}),
 		system.NewCommand("lxc", []string{"network", "set", "lxdbr0", "ipv6.address", "none"}),
@@ -144,7 +144,7 @@ func (l *LXD) init() error {
 func (l *LXD) enableNonRootUserControl() error {
 	username := l.system.User().Username
 
-	return l.system.RunMany(
+	return system.RunMany(l.system,
 		system.NewCommand("chmod", []string{"a+wr", "/var/snap/lxd/common/lxd/unix.socket"}),
 		system.NewCommand("usermod", []string{"-a", "-G", "lxd", username}),
 	)
@@ -154,7 +154,7 @@ func (l *LXD) enableNonRootUserControl() error {
 // This is to avoid a conflict with the default iptables rules that ship with
 // docker on Ubuntu.
 func (l *LXD) deconflictFirewall() error {
-	return l.system.RunMany(
+	return system.RunMany(l.system,
 		system.NewCommand("iptables", []string{"-F", "FORWARD"}),
 		system.NewCommand("iptables", []string{"-P", "FORWARD", "ACCEPT"}),
 	)
@@ -186,7 +186,7 @@ func (l *LXD) workaroundRefresh() (bool, error) {
 			"tracking", snapInfo.TrackingChannel, "target", l.Channel)
 		args := []string{"stop", l.Name()}
 		cmd := system.NewCommand("snap", args)
-		_, err = l.system.RunExclusive(cmd)
+		_, err = system.RunExclusive(l.system, cmd)
 		if err != nil {
 			return false, fmt.Errorf("command failed: %w", err)
 		}

internal/providers/microk8s.go

@@ -150,7 +150,7 @@ func (m *MicroK8s) install() error {
 // init ensures that MicroK8s is installed, minimally configured, and ready.
 func (m *MicroK8s) init() error {
 	cmd := system.NewCommand("microk8s", []string{"status", "--wait-ready", "--timeout", "270"})
-	_, err := m.system.RunWithRetries(cmd, (5 * time.Minute))
+	_, err := system.RunWithRetries(m.system, cmd, 5*time.Minute)
 
 	return err
 }
@@ -166,7 +166,7 @@ func (m *MicroK8s) enableAddons() error {
 		}
 
 		cmd := system.NewCommand("microk8s", []string{"enable", enableArg})
-		_, err := m.system.RunWithRetries(cmd, (5 * time.Minute))
+		_, err := system.RunWithRetries(m.system, cmd, 5*time.Minute)
 		if err != nil {
 			return fmt.Errorf("failed to enable MicroK8s addon '%s': %w", addon, err)
 		}
@@ -199,7 +199,7 @@ func (m *MicroK8s) setupKubectl() error {
 		return fmt.Errorf("failed to fetch MicroK8s configuration: %w", err)
 	}
 
-	return m.system.WriteHomeDirFile(path.Join(".kube", "config"), result)
+	return system.WriteHomeDirFile(m.system, path.Join(".kube", "config"), result)
 }
 
 // Try to compute the "correct" default channel. Concierge prefers that the 'strict'

internal/providers/microk8s_test.go

@@ -106,7 +106,7 @@ func TestMicroK8sPrepareCommands(t *testing.T) {
 	}
 
 	expectedFiles := map[string]string{
-		".kube/config": "",
+		path.Join(os.TempDir(), ".kube", "config"): "",
 	}
 
 	system := system.NewMockSystem()

internal/system/dryrun.go

@@ -7,8 +7,6 @@ import (
 	"os"
 	"os/exec"
 	"os/user"
-	"path"
-	"time"
 )
 
 // ErrNotInstalled is returned by DryRunWorker when a read-only command's
@@ -56,54 +54,12 @@ func (d *DryRunWorker) Run(c *Command) ([]byte, error) {
 	return []byte{}, nil
 }
 
-// RunMany prints each command that would be executed and returns success.
-// Read-only commands are delegated to the real system for accurate results.
-func (d *DryRunWorker) RunMany(commands ...*Command) error {
-	for _, c := range commands {
-		if c.ReadOnly {
-			_, err := d.runReadOnly(c)
-			if err != nil {
-				return err
-			}
-		} else {
-			fmt.Fprintln(d.out, c.CommandString())
-		}
-	}
+// WriteFile prints what file would be written and returns success.
+func (d *DryRunWorker) WriteFile(filePath string, contents []byte, perm os.FileMode) error {
+	fmt.Fprintln(d.out, "# Write file:", filePath)
 	return nil
 }
 
-// RunExclusive prints the command that would be executed and returns success.
-// Read-only commands are delegated to the real system for accurate results.
-func (d *DryRunWorker) RunExclusive(c *Command) ([]byte, error) {
-	if c.ReadOnly {
-		return d.runReadOnly(c)
-	}
-	fmt.Fprintln(d.out, c.CommandString())
-	return []byte{}, nil
-}
-
-// RunWithRetries prints the command that would be executed and returns success.
-// Read-only commands are delegated to the real system for accurate results.
-func (d *DryRunWorker) RunWithRetries(c *Command, maxDuration time.Duration) ([]byte, error) {
-	if c.ReadOnly {
-		return d.runReadOnly(c)
-	}
-	fmt.Fprintln(d.out, c.CommandString())
-	return []byte{}, nil
-}
-
-// WriteHomeDirFile prints what file would be written and returns success.
-func (d *DryRunWorker) WriteHomeDirFile(filepath string, contents []byte) error {
-	fullPath := path.Join(d.realSystem.User().HomeDir, filepath)
-	fmt.Fprintln(d.out, "# Write file:", fullPath)
-	return nil
-}
-
-// ReadHomeDirFile delegates to real system for accurate conditional logic.
-func (d *DryRunWorker) ReadHomeDirFile(filepath string) ([]byte, error) {
-	return d.realSystem.ReadHomeDirFile(filepath)
-}
-
 // ReadFile delegates to real system for accurate conditional logic.
 func (d *DryRunWorker) ReadFile(filePath string) ([]byte, error) {
 	return d.realSystem.ReadFile(filePath)