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 `Run(cmd, system.Exclusive())` for operations requiring locks
   - Package installations (snap, apt)
   - State-modifying operations that can conflict
 

AGENTS.md

@@ -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 options (e.g. `system.Exclusive()`, `system.WithRetries(d)`)
+   - 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

@@ -88,7 +88,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)
 	}
@@ -102,7 +102,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)
 	}
@@ -124,7 +124,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

@@ -156,7 +156,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)
 	}
@@ -237,7 +237,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 = j.system.Run(cmd, system.WithRetries(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 := h.system.Run(cmd, system.Exclusive())
 	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 := h.system.Run(cmd, system.Exclusive())
 	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 := h.system.Run(cmd, system.Exclusive())
 	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 := h.system.Run(cmd, system.Exclusive())
 	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 = h.system.Run(cmd, system.Exclusive())
 	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 := h.system.Run(cmd, system.Exclusive())
 		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 := h.system.Run(cmd, system.Exclusive())
 	if err != nil {
 		return fmt.Errorf("failed to remove snap '%s': %w", s.Name, err)
 	}

internal/providers/k8s.go

@@ -174,14 +174,14 @@ func (k *K8s) init() error {
 
 	if k.needsBootstrap() {
 		cmd := system.NewCommand("k8s", []string{"bootstrap"})
-		_, err := k.system.RunWithRetries(cmd, (5 * time.Minute))
+		_, err := k.system.Run(cmd, system.WithRetries(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 := k.system.Run(cmd, system.WithRetries(5*time.Minute))
 
 	return err
 }
@@ -200,7 +200,7 @@ func (k *K8s) configureFeatures() error {
 		}
 
 		cmd := system.NewCommand("k8s", []string{"enable", featureName})
-		_, err := k.system.RunWithRetries(cmd, (5 * time.Minute))
+		_, err := k.system.Run(cmd, system.WithRetries(5*time.Minute))
 		if err != nil {
 			return fmt.Errorf("failed to enable K8s addon '%s': %w", featureName, err)
 		}
@@ -218,7 +218,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 = l.system.Run(cmd, system.Exclusive())
 		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 = l.system.Run(cmd, system.Exclusive())
 		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 := m.system.Run(cmd, system.WithRetries(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 := m.system.Run(cmd, system.WithRetries(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/interface.go

@@ -9,29 +9,38 @@ import (
 	"time"
 )
 
+// RunOption is a functional option for configuring the behaviour of Run.
+type RunOption func(*runConfig)
+
+type runConfig struct {
+	exclusive        bool
+	maxRetryDuration time.Duration
+}
+
+// Exclusive returns a RunOption that causes Run to acquire a per-executable
+// mutex, ensuring only one instance of that executable runs at a time.
+func Exclusive() RunOption {
+	return func(c *runConfig) { c.exclusive = true }
+}
+
+// WithRetries returns a RunOption that causes Run to retry the command using
+// exponential backoff, up to the specified maximum duration.
+func WithRetries(maxDuration time.Duration) RunOption {
+	return func(c *runConfig) { c.maxRetryDuration = maxDuration }
+}
+
 // Worker is an interface for a struct that can run commands on the underlying system.
 type Worker interface {
 	// User returns the 'real user' the system executes command as. This may be different from
 	// the current user since the command is often executed with `sudo`.
 	User() *user.User
 	// Run takes a single command and runs it, returning the combined output and an error value.
-	Run(c *Command) ([]byte, error)
-	// RunMany takes multiple commands and runs them in sequence, returning an error on the
-	// first error encountered.
-	RunMany(commands ...*Command) error
-	// RunExclusive is a wrapper around Run that uses a mutex to ensure that only one of that
-	// particular command can be run at a time.
-	RunExclusive(c *Command) ([]byte, error)
-	// RunWithRetries executes the command, retrying utilising an exponential backoff pattern,
-	// which starts at 1 second. Retries will be attempted up to the specified maximum duration.
-	RunWithRetries(c *Command, maxDuration time.Duration) ([]byte, error)
-	// WriteHomeDirFile takes a path relative to the real user's home dir, and writes the contents
-	// specified to it.
-	WriteHomeDirFile(filepath string, contents []byte) error
-	// ReadHomeDirFile reads a file from the user's home directory.
-	ReadHomeDirFile(filepath string) ([]byte, error)
+	// RunOptions can be provided to alter the behaviour (e.g. Exclusive, WithRetries).
+	Run(c *Command, opts ...RunOption) ([]byte, error)
 	// ReadFile reads a file with an arbitrary path from the system.
 	ReadFile(filePath string) ([]byte, error)
+	// WriteFile writes the given contents to the specified file path with the given permissions.
+	WriteFile(filePath string, contents []byte, perm os.FileMode) error
 	// SnapInfo returns information about a given snap, looking up details in the snap
 	// store using the snapd client API where necessary.
 	SnapInfo(snap string, channel string) (*SnapInfo, error)
@@ -45,6 +54,48 @@ type Worker interface {
 	ChownAll(path string, user *user.User) error
 }
 
+// RunMany takes multiple commands and runs them in sequence via the Worker,
+// returning an error on the first error encountered.
+func RunMany(w Worker, commands ...*Command) error {
+	for _, cmd := range commands {
+		_, err := w.Run(cmd)
+		if err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+// ReadHomeDirFile reads a file at a path relative to the real user's home directory.
+func ReadHomeDirFile(w Worker, filePath string) ([]byte, error) {
+	homePath := path.Join(w.User().HomeDir, filePath)
+	return w.ReadFile(homePath)
+}
+
+// WriteHomeDirFile writes contents to a path relative to the real user's home directory,
+// creating parent directories and adjusting ownership as needed.
+func WriteHomeDirFile(w Worker, filePath string, contents []byte) error {
+	dir := path.Dir(filePath)
+
+	err := MkHomeSubdirectory(w, dir)
+	if err != nil {
+		return err
+	}
+
+	absPath := path.Join(w.User().HomeDir, filePath)
+
+	if err := w.WriteFile(absPath, contents, 0644); err != nil {
+		return fmt.Errorf("failed to write file '%s': %w", absPath, err)
+	}
+
+	err = w.ChownAll(absPath, w.User())
+	if err != nil {
+		return fmt.Errorf("failed to change ownership of file '%s': %w", absPath, err)
+	}
+
+	return nil
+}
+
 // MkHomeSubdirectory is a helper function that takes a relative folder path and creates it
 // recursively in the real user's home directory using the Worker interface.
 func MkHomeSubdirectory(w Worker, subdirectory string) error {