Implement persistence tests

2025-11-05 16:44:22 +00:00 · 2025-08-02 22:52:50 +02:00
parent bed172bf73
commit 3cbd23a6e2
8 changed files with 453 additions and 93 deletions
--- a/README.md
+++ b/README.md
@@ -138,7 +138,7 @@ server:
 ```yaml
 instances:
  port_range: [8000, 9000]           # Port range for instances
-  log_directory: "/tmp/llamactl"     # Directory for instance logs
+  logs_dir: "/tmp/llamactl"     # Directory for instance logs
  max_instances: -1                  # Maximum instances (-1 = unlimited)
  llama_executable: "llama-server"   # Path to llama-server executable
  default_auto_restart: true         # Default auto-restart setting
@@ -148,7 +148,7 @@ instances:

 **Environment Variables:**
 - `LLAMACTL_INSTANCE_PORT_RANGE` - Port range (format: "8000-9000" or "8000,9000")
- `LLAMACTL_LOG_DIR` - Log directory path
+- `LLAMACTL_LOGS_DIR` - Log directory path
 - `LLAMACTL_MAX_INSTANCES` - Maximum number of instances
 - `LLAMACTL_LLAMA_EXECUTABLE` - Path to llama-server executable
 - `LLAMACTL_DEFAULT_AUTO_RESTART` - Default auto-restart setting (true/false)
--- a/cmd/server/main.go
+++ b/cmd/server/main.go
@@ -17,7 +17,8 @@ import (
 // @basePath /api/v1
 func main() {

-	config, err := llamactl.LoadConfig("")
+	configPath := os.Getenv("LLAMACTL_CONFIG_PATH")
+	config, err := llamactl.LoadConfig(configPath)
 	if err != nil {
 		fmt.Printf("Error loading config: %v\n", err)
 		fmt.Println("Using default configuration.")
@@ -25,13 +26,13 @@ func main() {

 	// Create the data directory if it doesn't exist
 	if config.Instances.AutoCreateDirs {
-		if err := os.MkdirAll(config.Instances.ConfigDir, 0755); err != nil {
-			fmt.Printf("Error creating config directory %s: %v\n", config.Instances.ConfigDir, err)
+		if err := os.MkdirAll(config.Instances.InstancesDir, 0755); err != nil {
+			fmt.Printf("Error creating config directory %s: %v\n", config.Instances.InstancesDir, err)
 			fmt.Println("Persistence will not be available.")
 		}

-		if err := os.MkdirAll(config.Instances.LogDir, 0755); err != nil {
-			fmt.Printf("Error creating log directory %s: %v\n", config.Instances.LogDir, err)
+		if err := os.MkdirAll(config.Instances.LogsDir, 0755); err != nil {
+			fmt.Printf("Error creating log directory %s: %v\n", config.Instances.LogsDir, err)
 			fmt.Println("Instance logs will not be available.")
 		}
 	}
--- a/pkg/config.go
+++ b/pkg/config.go
@@ -41,10 +41,10 @@ type InstancesConfig struct {
 	DataDir string `yaml:"data_dir"`

 	// Instance config directory override
-	ConfigDir string `yaml:"config_dir"`
+	InstancesDir string `yaml:"configs_dir"`

 	// Logs directory override
-	LogDir string `yaml:"logs_dir"`
+	LogsDir string `yaml:"logs_dir"`

 	// Automatically create the data directory if it doesn't exist
 	AutoCreateDirs bool `yaml:"auto_create_dirs"`
@@ -97,8 +97,8 @@ func LoadConfig(configPath string) (Config, error) {
 		Instances: InstancesConfig{
 			PortRange:           [2]int{8000, 9000},
 			DataDir:             getDefaultDataDirectory(),
-			ConfigDir:           filepath.Join(getDefaultDataDirectory(), "instances"),
-			LogDir:              filepath.Join(getDefaultDataDirectory(), "logs"),
+			InstancesDir:        filepath.Join(getDefaultDataDirectory(), "instances"),
+			LogsDir:             filepath.Join(getDefaultDataDirectory(), "logs"),
 			AutoCreateDirs:      true,
 			MaxInstances:        -1, // -1 means unlimited
 			LlamaExecutable:     "llama-server",
@@ -173,11 +173,11 @@ func loadEnvVars(cfg *Config) {
 	if dataDir := os.Getenv("LLAMACTL_DATA_DIRECTORY"); dataDir != "" {
 		cfg.Instances.DataDir = dataDir
 	}
-	if instancesDir := os.Getenv("LLAMACTL_INSTANCES_DIRECTORY"); instancesDir != "" {
-		cfg.Instances.ConfigDir = instancesDir
+	if instancesDir := os.Getenv("LLAMACTL_INSTANCES_DIR"); instancesDir != "" {
+		cfg.Instances.InstancesDir = instancesDir
 	}
-	if logsDir := os.Getenv("LLAMACTL_LOGS_DIRECTORY"); logsDir != "" {
-		cfg.Instances.LogDir = logsDir
+	if logsDir := os.Getenv("LLAMACTL_LOGS_DIR"); logsDir != "" {
+		cfg.Instances.LogsDir = logsDir
 	}
 	if autoCreate := os.Getenv("LLAMACTL_AUTO_CREATE_DATA_DIR"); autoCreate != "" {
 		if b, err := strconv.ParseBool(autoCreate); err == nil {
@@ -271,23 +271,13 @@ func getDefaultDataDirectory() string {

 	case "darwin":
 		// For macOS, use user's Application Support directory
-		// System-wide would be /usr/local/var/llamactl but requires sudo
-		homeDir, _ := os.UserHomeDir()
-		if homeDir != "" {
+		if homeDir, _ := os.UserHomeDir(); homeDir != "" {
 			return filepath.Join(homeDir, "Library", "Application Support", "llamactl")
 		}
 		return "/usr/local/var/llamactl" // Fallback

 	default:
 		// Linux and other Unix-like systems
-		// Try system directory first, fallback to user directory
-		if os.Geteuid() == 0 { // Running as root
-			return "/var/lib/llamactl"
-		}
-		// For non-root users, use XDG data home
-		if xdgDataHome := os.Getenv("XDG_DATA_HOME"); xdgDataHome != "" {
-			return filepath.Join(xdgDataHome, "llamactl")
-		}
 		if homeDir, _ := os.UserHomeDir(); homeDir != "" {
 			return filepath.Join(homeDir, ".local", "share", "llamactl")
 		}
@@ -298,61 +288,31 @@ func getDefaultDataDirectory() string {
 // getDefaultConfigLocations returns platform-specific config file locations
 func getDefaultConfigLocations() []string {
 	var locations []string
-
-	// Current directory (cross-platform)
-	locations = append(locations,
-		"./llamactl.yaml",
-		"./config.yaml",
-	)
-
 	homeDir, _ := os.UserHomeDir()

 	switch runtime.GOOS {
 	case "windows":
-		// Windows: Use APPDATA and ProgramData
+		// Windows: Use APPDATA if available, else user home, fallback to ProgramData
 		if appData := os.Getenv("APPDATA"); appData != "" {
 			locations = append(locations, filepath.Join(appData, "llamactl", "config.yaml"))
-		}
-		if programData := os.Getenv("PROGRAMDATA"); programData != "" {
-			locations = append(locations, filepath.Join(programData, "llamactl", "config.yaml"))
-		}
-		// Fallback to user home
-		if homeDir != "" {
+		} else if homeDir != "" {
 			locations = append(locations, filepath.Join(homeDir, "llamactl", "config.yaml"))
 		}
+		locations = append(locations, filepath.Join(os.Getenv("PROGRAMDATA"), "llamactl", "config.yaml"))

 	case "darwin":
-		// macOS: Use proper Application Support directories
+		// macOS: Use Application Support in user home, fallback to /Library/Application Support
 		if homeDir != "" {
-			locations = append(locations,
-				filepath.Join(homeDir, "Library", "Application Support", "llamactl", "config.yaml"),
-				filepath.Join(homeDir, ".config", "llamactl", "config.yaml"), // XDG fallback
-			)
+			locations = append(locations, filepath.Join(homeDir, "Library", "Application Support", "llamactl", "config.yaml"))
 		}
 		locations = append(locations, "/Library/Application Support/llamactl/config.yaml")
-		locations = append(locations, "/etc/llamactl/config.yaml") // Unix fallback

 	default:
-		// User config: $XDG_CONFIG_HOME/llamactl/config.yaml or ~/.config/llamactl/config.yaml
-		configHome := os.Getenv("XDG_CONFIG_HOME")
-		if configHome == "" && homeDir != "" {
-			configHome = filepath.Join(homeDir, ".config")
+		// Linux/Unix: Use ~/.config/llamactl/config.yaml, fallback to /etc/llamactl/config.yaml
+		if homeDir != "" {
+			locations = append(locations, filepath.Join(homeDir, ".config", "llamactl", "config.yaml"))
 		}
-		if configHome != "" {
-			locations = append(locations, filepath.Join(configHome, "llamactl", "config.yaml"))
-		}
-
-		// System config: /etc/llamactl/config.yaml
 		locations = append(locations, "/etc/llamactl/config.yaml")
-
-		// Additional system locations
-		if xdgConfigDirs := os.Getenv("XDG_CONFIG_DIRS"); xdgConfigDirs != "" {
-			for dir := range strings.SplitSeq(xdgConfigDirs, ":") {
-				if dir != "" {
-					locations = append(locations, filepath.Join(dir, "llamactl", "config.yaml"))
-				}
-			}
-		}
 	}

 	return locations
--- a/pkg/config_test.go
+++ b/pkg/config_test.go
@@ -22,8 +22,17 @@ func TestLoadConfig_Defaults(t *testing.T) {
 	if cfg.Server.Port != 8080 {
 		t.Errorf("Expected default port to be 8080, got %d", cfg.Server.Port)
 	}
-	if cfg.Instances.ConfigDir != "/var/lib/llamactl/instances" {
-		t.Errorf("Expected default instances directory '/var/lib/llamactl/instances', got %q", cfg.Instances.ConfigDir)
+
+	homedir, err := os.UserHomeDir()
+	if err != nil {
+		t.Fatalf("Failed to get user home directory: %v", err)
+	}
+
+	if cfg.Instances.InstancesDir != filepath.Join(homedir, ".local", "share", "llamactl", "instances") {
+		t.Errorf("Expected default instances directory '%s', got %q", filepath.Join(homedir, ".local", "share", "llamactl", "instances"), cfg.Instances.InstancesDir)
+	}
+	if cfg.Instances.LogsDir != filepath.Join(homedir, ".local", "share", "llamactl", "logs") {
+		t.Errorf("Expected default logs directory '%s', got %q", filepath.Join(homedir, ".local", "share", "llamactl", "logs"), cfg.Instances.LogsDir)
 	}
 	if !cfg.Instances.AutoCreateDirs {
 		t.Error("Expected default instances auto-create to be true")
@@ -31,9 +40,6 @@ func TestLoadConfig_Defaults(t *testing.T) {
 	if cfg.Instances.PortRange != [2]int{8000, 9000} {
 		t.Errorf("Expected default port range [8000, 9000], got %v", cfg.Instances.PortRange)
 	}
-	if cfg.Instances.LogDir != "/tmp/llamactl/logs" {
-		t.Errorf("Expected default logs directory '/tmp/llamactl/logs', got %q", cfg.Instances.LogDir)
-	}
 	if cfg.Instances.MaxInstances != -1 {
 		t.Errorf("Expected default max instances -1, got %d", cfg.Instances.MaxInstances)
 	}
@@ -62,7 +68,7 @@ server:
  port: 9090
 instances:
  port_range: [7000, 8000]
-  log_directory: "/custom/logs"
+  logs_dir: "/custom/logs"
  max_instances: 5
  llama_executable: "/usr/bin/llama-server"
  default_auto_restart: false
@@ -90,8 +96,8 @@ instances:
 	if cfg.Instances.PortRange != [2]int{7000, 8000} {
 		t.Errorf("Expected port range [7000, 8000], got %v", cfg.Instances.PortRange)
 	}
-	if cfg.Instances.LogDir != "/custom/logs" {
-		t.Errorf("Expected logs directory '/custom/logs', got %q", cfg.Instances.LogDir)
+	if cfg.Instances.LogsDir != "/custom/logs" {
+		t.Errorf("Expected logs directory '/custom/logs', got %q", cfg.Instances.LogsDir)
 	}
 	if cfg.Instances.MaxInstances != 5 {
 		t.Errorf("Expected max instances 5, got %d", cfg.Instances.MaxInstances)
@@ -116,7 +122,7 @@ func TestLoadConfig_EnvironmentOverrides(t *testing.T) {
 		"LLAMACTL_HOST":                  "0.0.0.0",
 		"LLAMACTL_PORT":                  "3000",
 		"LLAMACTL_INSTANCE_PORT_RANGE":   "5000-6000",
-		"LLAMACTL_LOG_DIR":               "/env/logs",
+		"LLAMACTL_LOGS_DIR":              "/env/logs",
 		"LLAMACTL_MAX_INSTANCES":         "20",
 		"LLAMACTL_LLAMA_EXECUTABLE":      "/env/llama-server",
 		"LLAMACTL_DEFAULT_AUTO_RESTART":  "false",
@@ -145,8 +151,8 @@ func TestLoadConfig_EnvironmentOverrides(t *testing.T) {
 	if cfg.Instances.PortRange != [2]int{5000, 6000} {
 		t.Errorf("Expected port range [5000, 6000], got %v", cfg.Instances.PortRange)
 	}
-	if cfg.Instances.LogDir != "/env/logs" {
-		t.Errorf("Expected logs directory '/env/logs', got %q", cfg.Instances.LogDir)
+	if cfg.Instances.LogsDir != "/env/logs" {
+		t.Errorf("Expected logs directory '/env/logs', got %q", cfg.Instances.LogsDir)
 	}
 	if cfg.Instances.MaxInstances != 20 {
 		t.Errorf("Expected max instances 20, got %d", cfg.Instances.MaxInstances)
--- a/pkg/instance.go
+++ b/pkg/instance.go
@@ -149,7 +149,7 @@ func NewInstance(name string, globalSettings *InstancesConfig, options *CreateIn
 	// Apply defaults
 	applyDefaultOptions(optionsCopy, globalSettings)
 	// Create the instance logger
-	logger := NewInstanceLogger(name, globalSettings.LogDir)
+	logger := NewInstanceLogger(name, globalSettings.LogsDir)

 	return &Instance{
 		Name:           name,
@@ -235,10 +235,12 @@ func (i *Instance) MarshalJSON() ([]byte, error) {
 		Name    string                 `json:"name"`
 		Options *CreateInstanceOptions `json:"options,omitempty"`
 		Running bool                   `json:"running"`
+		Created int64                  `json:"created,omitempty"`
 	}{
 		Name:    i.Name,
 		Options: i.options,
 		Running: i.Running,
+		Created: i.Created,
 	}

 	return json.Marshal(temp)
@@ -251,6 +253,7 @@ func (i *Instance) UnmarshalJSON(data []byte) error {
 		Name    string                 `json:"name"`
 		Options *CreateInstanceOptions `json:"options,omitempty"`
 		Running bool                   `json:"running"`
+		Created int64                  `json:"created,omitempty"`
 	}{}

 	if err := json.Unmarshal(data, &temp); err != nil {
@@ -260,6 +263,7 @@ func (i *Instance) UnmarshalJSON(data []byte) error {
 	// Set the fields
 	i.Name = temp.Name
 	i.Running = temp.Running
+	i.Created = temp.Created

 	// Handle options with validation but no defaults
 	if temp.Options != nil {
--- a/pkg/instance_test.go
+++ b/pkg/instance_test.go
@@ -9,7 +9,7 @@ import (

 func TestNewInstance(t *testing.T) {
 	globalSettings := &llamactl.InstancesConfig{
-		LogDir:              "/tmp/test",
+		LogsDir:             "/tmp/test",
 		DefaultAutoRestart:  true,
 		DefaultMaxRestarts:  3,
 		DefaultRestartDelay: 5,
@@ -54,7 +54,7 @@ func TestNewInstance(t *testing.T) {

 func TestNewInstance_WithRestartOptions(t *testing.T) {
 	globalSettings := &llamactl.InstancesConfig{
-		LogDir:              "/tmp/test",
+		LogsDir:             "/tmp/test",
 		DefaultAutoRestart:  true,
 		DefaultMaxRestarts:  3,
 		DefaultRestartDelay: 5,
@@ -91,7 +91,7 @@ func TestNewInstance_WithRestartOptions(t *testing.T) {

 func TestNewInstance_ValidationAndDefaults(t *testing.T) {
 	globalSettings := &llamactl.InstancesConfig{
-		LogDir:              "/tmp/test",
+		LogsDir:             "/tmp/test",
 		DefaultAutoRestart:  true,
 		DefaultMaxRestarts:  3,
 		DefaultRestartDelay: 5,
@@ -123,7 +123,7 @@ func TestNewInstance_ValidationAndDefaults(t *testing.T) {

 func TestSetOptions(t *testing.T) {
 	globalSettings := &llamactl.InstancesConfig{
-		LogDir:              "/tmp/test",
+		LogsDir:             "/tmp/test",
 		DefaultAutoRestart:  true,
 		DefaultMaxRestarts:  3,
 		DefaultRestartDelay: 5,
@@ -164,7 +164,7 @@ func TestSetOptions(t *testing.T) {

 func TestSetOptions_NilOptions(t *testing.T) {
 	globalSettings := &llamactl.InstancesConfig{
-		LogDir:              "/tmp/test",
+		LogsDir:             "/tmp/test",
 		DefaultAutoRestart:  true,
 		DefaultMaxRestarts:  3,
 		DefaultRestartDelay: 5,
@@ -191,7 +191,7 @@ func TestSetOptions_NilOptions(t *testing.T) {

 func TestGetProxy(t *testing.T) {
 	globalSettings := &llamactl.InstancesConfig{
-		LogDir: "/tmp/test",
+		LogsDir: "/tmp/test",
 	}

 	options := &llamactl.CreateInstanceOptions{
@@ -224,7 +224,7 @@ func TestGetProxy(t *testing.T) {

 func TestMarshalJSON(t *testing.T) {
 	globalSettings := &llamactl.InstancesConfig{
-		LogDir:              "/tmp/test",
+		LogsDir:             "/tmp/test",
 		DefaultAutoRestart:  true,
 		DefaultMaxRestarts:  3,
 		DefaultRestartDelay: 5,
@@ -406,7 +406,7 @@ func TestCreateInstanceOptionsValidation(t *testing.T) {
 	}

 	globalSettings := &llamactl.InstancesConfig{
-		LogDir: "/tmp/test",
+		LogsDir: "/tmp/test",
 	}

 	for _, tt := range tests {
--- a/pkg/manager.go
+++ b/pkg/manager.go
@@ -192,7 +192,7 @@ func (im *instanceManager) DeleteInstance(name string) error {
 	delete(im.instances, name)

 	// Delete the instance's config file if persistence is enabled
-	instancePath := filepath.Join(im.instancesConfig.ConfigDir, name+".json")
+	instancePath := filepath.Join(im.instancesConfig.InstancesDir, name+".json")
 	if err := os.Remove(instancePath); err != nil && !os.IsNotExist(err) {
 		return fmt.Errorf("failed to delete config file for instance %s: %w", name, err)
 	}
@@ -293,11 +293,11 @@ func (im *instanceManager) getNextAvailablePort() (int, error) {

 // persistInstance saves an instance to its JSON file
 func (im *instanceManager) persistInstance(instance *Instance) error {
-	if im.instancesConfig.ConfigDir == "" {
+	if im.instancesConfig.InstancesDir == "" {
 		return nil // Persistence disabled
 	}

-	instancePath := filepath.Join(im.instancesConfig.ConfigDir, instance.Name+".json")
+	instancePath := filepath.Join(im.instancesConfig.InstancesDir, instance.Name+".json")
 	tempPath := instancePath + ".tmp"

 	// Serialize instance to JSON
@@ -349,17 +349,17 @@ func (im *instanceManager) Shutdown() {

 // loadInstances restores all instances from disk
 func (im *instanceManager) loadInstances() error {
-	if im.instancesConfig.ConfigDir == "" {
+	if im.instancesConfig.InstancesDir == "" {
 		return nil // Persistence disabled
 	}

 	// Check if instances directory exists
-	if _, err := os.Stat(im.instancesConfig.ConfigDir); os.IsNotExist(err) {
+	if _, err := os.Stat(im.instancesConfig.InstancesDir); os.IsNotExist(err) {
 		return nil // No instances directory, start fresh
 	}

 	// Read all JSON files from instances directory
-	files, err := os.ReadDir(im.instancesConfig.ConfigDir)
+	files, err := os.ReadDir(im.instancesConfig.InstancesDir)
 	if err != nil {
 		return fmt.Errorf("failed to read instances directory: %w", err)
 	}
@@ -371,7 +371,7 @@ func (im *instanceManager) loadInstances() error {
 		}

 		instanceName := strings.TrimSuffix(file.Name(), ".json")
-		instancePath := filepath.Join(im.instancesConfig.ConfigDir, file.Name())
+		instancePath := filepath.Join(im.instancesConfig.InstancesDir, file.Name())

 		if err := im.loadInstance(instanceName, instancePath); err != nil {
 			log.Printf("Failed to load instance %s: %v", instanceName, err)
--- a/pkg/manager_test.go
+++ b/pkg/manager_test.go
@@ -1,6 +1,10 @@
 package llamactl_test

 import (
+	"encoding/json"
+	"os"
+	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"

@@ -10,7 +14,7 @@ import (
 func TestNewInstanceManager(t *testing.T) {
 	config := llamactl.InstancesConfig{
 		PortRange:           [2]int{8000, 9000},
-		LogDir:              "/tmp/test",
+		LogsDir:             "/tmp/test",
 		MaxInstances:        5,
 		LlamaExecutable:     "llama-server",
 		DefaultAutoRestart:  true,
@@ -486,11 +490,396 @@ func TestUpdateInstance_NotFound(t *testing.T) {
 	}
 }

+func TestPersistence_InstancePersistedOnCreation(t *testing.T) {
+	// Create temporary directory for persistence
+	tempDir := t.TempDir()
+
+	config := llamactl.InstancesConfig{
+		PortRange:    [2]int{8000, 9000},
+		InstancesDir: tempDir,
+		MaxInstances: 10,
+	}
+	manager := llamactl.NewInstanceManager(config)
+
+	options := &llamactl.CreateInstanceOptions{
+		LlamaServerOptions: llamactl.LlamaServerOptions{
+			Model: "/path/to/model.gguf",
+			Port:  8080,
+		},
+	}
+
+	// Create instance
+	_, err := manager.CreateInstance("test-instance", options)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
+
+	// Check that JSON file was created
+	expectedPath := filepath.Join(tempDir, "test-instance.json")
+	if _, err := os.Stat(expectedPath); os.IsNotExist(err) {
+		t.Errorf("Expected persistence file %s to exist", expectedPath)
+	}
+
+	// Verify file contains correct data
+	data, err := os.ReadFile(expectedPath)
+	if err != nil {
+		t.Fatalf("Failed to read persistence file: %v", err)
+	}
+
+	var persistedInstance map[string]interface{}
+	if err := json.Unmarshal(data, &persistedInstance); err != nil {
+		t.Fatalf("Failed to unmarshal persisted data: %v", err)
+	}
+
+	if persistedInstance["name"] != "test-instance" {
+		t.Errorf("Expected name 'test-instance', got %v", persistedInstance["name"])
+	}
+}
+
+func TestPersistence_InstancePersistedOnUpdate(t *testing.T) {
+	tempDir := t.TempDir()
+
+	config := llamactl.InstancesConfig{
+		PortRange:    [2]int{8000, 9000},
+		InstancesDir: tempDir,
+		MaxInstances: 10,
+	}
+	manager := llamactl.NewInstanceManager(config)
+
+	// Create instance
+	options := &llamactl.CreateInstanceOptions{
+		LlamaServerOptions: llamactl.LlamaServerOptions{
+			Model: "/path/to/model.gguf",
+			Port:  8080,
+		},
+	}
+	_, err := manager.CreateInstance("test-instance", options)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
+
+	// Update instance
+	newOptions := &llamactl.CreateInstanceOptions{
+		LlamaServerOptions: llamactl.LlamaServerOptions{
+			Model: "/path/to/new-model.gguf",
+			Port:  8081,
+		},
+	}
+	_, err = manager.UpdateInstance("test-instance", newOptions)
+	if err != nil {
+		t.Fatalf("UpdateInstance failed: %v", err)
+	}
+
+	// Verify persistence file was updated
+	expectedPath := filepath.Join(tempDir, "test-instance.json")
+	data, err := os.ReadFile(expectedPath)
+	if err != nil {
+		t.Fatalf("Failed to read persistence file: %v", err)
+	}
+
+	var persistedInstance map[string]interface{}
+	if err := json.Unmarshal(data, &persistedInstance); err != nil {
+		t.Fatalf("Failed to unmarshal persisted data: %v", err)
+	}
+
+	// Check that the options were updated
+	options_data, ok := persistedInstance["options"].(map[string]interface{})
+	if !ok {
+		t.Fatal("Expected options to be present in persisted data")
+	}
+
+	if options_data["model"] != "/path/to/new-model.gguf" {
+		t.Errorf("Expected updated model '/path/to/new-model.gguf', got %v", options_data["model"])
+	}
+}
+
+func TestPersistence_InstanceFileDeletedOnDeletion(t *testing.T) {
+	tempDir := t.TempDir()
+
+	config := llamactl.InstancesConfig{
+		PortRange:    [2]int{8000, 9000},
+		InstancesDir: tempDir,
+		MaxInstances: 10,
+	}
+	manager := llamactl.NewInstanceManager(config)
+
+	// Create instance
+	options := &llamactl.CreateInstanceOptions{
+		LlamaServerOptions: llamactl.LlamaServerOptions{
+			Model: "/path/to/model.gguf",
+		},
+	}
+	_, err := manager.CreateInstance("test-instance", options)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
+
+	expectedPath := filepath.Join(tempDir, "test-instance.json")
+
+	// Verify file exists
+	if _, err := os.Stat(expectedPath); os.IsNotExist(err) {
+		t.Fatal("Expected persistence file to exist before deletion")
+	}
+
+	// Delete instance
+	err = manager.DeleteInstance("test-instance")
+	if err != nil {
+		t.Fatalf("DeleteInstance failed: %v", err)
+	}
+
+	// Verify file was deleted
+	if _, err := os.Stat(expectedPath); !os.IsNotExist(err) {
+		t.Error("Expected persistence file to be deleted")
+	}
+}
+
+func TestPersistence_InstancesLoadedFromDisk(t *testing.T) {
+	tempDir := t.TempDir()
+
+	// Create JSON files manually (simulating previous run)
+	instance1JSON := `{
+		"name": "instance1",
+		"running": false,
+		"options": {
+			"model": "/path/to/model1.gguf",
+			"port": 8080
+		}
+	}`
+
+	instance2JSON := `{
+		"name": "instance2", 
+		"running": false,
+		"options": {
+			"model": "/path/to/model2.gguf",
+			"port": 8081
+		}
+	}`
+
+	// Write JSON files
+	err := os.WriteFile(filepath.Join(tempDir, "instance1.json"), []byte(instance1JSON), 0644)
+	if err != nil {
+		t.Fatalf("Failed to write test JSON file: %v", err)
+	}
+
+	err = os.WriteFile(filepath.Join(tempDir, "instance2.json"), []byte(instance2JSON), 0644)
+	if err != nil {
+		t.Fatalf("Failed to write test JSON file: %v", err)
+	}
+
+	// Create manager - should load instances from disk
+	config := llamactl.InstancesConfig{
+		PortRange:    [2]int{8000, 9000},
+		InstancesDir: tempDir,
+		MaxInstances: 10,
+	}
+	manager := llamactl.NewInstanceManager(config)
+
+	// Verify instances were loaded
+	instances, err := manager.ListInstances()
+	if err != nil {
+		t.Fatalf("ListInstances failed: %v", err)
+	}
+
+	if len(instances) != 2 {
+		t.Fatalf("Expected 2 loaded instances, got %d", len(instances))
+	}
+
+	// Check instances by name
+	instancesByName := make(map[string]*llamactl.Instance)
+	for _, instance := range instances {
+		instancesByName[instance.Name] = instance
+	}
+
+	instance1, exists := instancesByName["instance1"]
+	if !exists {
+		t.Error("Expected instance1 to be loaded")
+	} else {
+		if instance1.GetOptions().Model != "/path/to/model1.gguf" {
+			t.Errorf("Expected model '/path/to/model1.gguf', got %q", instance1.GetOptions().Model)
+		}
+		if instance1.GetOptions().Port != 8080 {
+			t.Errorf("Expected port 8080, got %d", instance1.GetOptions().Port)
+		}
+	}
+
+	instance2, exists := instancesByName["instance2"]
+	if !exists {
+		t.Error("Expected instance2 to be loaded")
+	} else {
+		if instance2.GetOptions().Model != "/path/to/model2.gguf" {
+			t.Errorf("Expected model '/path/to/model2.gguf', got %q", instance2.GetOptions().Model)
+		}
+		if instance2.GetOptions().Port != 8081 {
+			t.Errorf("Expected port 8081, got %d", instance2.GetOptions().Port)
+		}
+	}
+}
+
+func TestPersistence_PortMapPopulatedFromLoadedInstances(t *testing.T) {
+	tempDir := t.TempDir()
+
+	// Create JSON file with specific port
+	instanceJSON := `{
+		"name": "test-instance",
+		"running": false,
+		"options": {
+			"model": "/path/to/model.gguf",
+			"port": 8080
+		}
+	}`
+
+	err := os.WriteFile(filepath.Join(tempDir, "test-instance.json"), []byte(instanceJSON), 0644)
+	if err != nil {
+		t.Fatalf("Failed to write test JSON file: %v", err)
+	}
+
+	// Create manager - should load instance and register port
+	config := llamactl.InstancesConfig{
+		PortRange:    [2]int{8000, 9000},
+		InstancesDir: tempDir,
+		MaxInstances: 10,
+	}
+	manager := llamactl.NewInstanceManager(config)
+
+	// Try to create new instance with same port - should fail due to conflict
+	options := &llamactl.CreateInstanceOptions{
+		LlamaServerOptions: llamactl.LlamaServerOptions{
+			Model: "/path/to/model2.gguf",
+			Port:  8080, // Same port as loaded instance
+		},
+	}
+
+	_, err = manager.CreateInstance("new-instance", options)
+	if err == nil {
+		t.Error("Expected error for port conflict with loaded instance")
+	}
+	if !strings.Contains(err.Error(), "port") || !strings.Contains(err.Error(), "in use") {
+		t.Errorf("Expected port conflict error, got: %v", err)
+	}
+}
+
+func TestPersistence_CompleteInstanceDataRoundTrip(t *testing.T) {
+	tempDir := t.TempDir()
+
+	config := llamactl.InstancesConfig{
+		PortRange:           [2]int{8000, 9000},
+		InstancesDir:        tempDir,
+		MaxInstances:        10,
+		DefaultAutoRestart:  true,
+		DefaultMaxRestarts:  3,
+		DefaultRestartDelay: 5,
+	}
+
+	// Create first manager and instance with comprehensive options
+	manager1 := llamactl.NewInstanceManager(config)
+
+	autoRestart := false
+	maxRestarts := 10
+	restartDelay := 30
+
+	originalOptions := &llamactl.CreateInstanceOptions{
+		AutoRestart:  &autoRestart,
+		MaxRestarts:  &maxRestarts,
+		RestartDelay: &restartDelay,
+		LlamaServerOptions: llamactl.LlamaServerOptions{
+			Model:       "/path/to/model.gguf",
+			Port:        8080,
+			Host:        "localhost",
+			CtxSize:     4096,
+			GPULayers:   32,
+			Temperature: 0.7,
+			TopK:        40,
+			TopP:        0.9,
+			Verbose:     true,
+			FlashAttn:   false,
+			Lora:        []string{"adapter1.bin", "adapter2.bin"},
+			HFRepo:      "microsoft/DialoGPT-medium",
+		},
+	}
+
+	originalInstance, err := manager1.CreateInstance("roundtrip-test", originalOptions)
+	if err != nil {
+		t.Fatalf("CreateInstance failed: %v", err)
+	}
+
+	// Create second manager (simulating restart) - should load the instance
+	manager2 := llamactl.NewInstanceManager(config)
+
+	loadedInstance, err := manager2.GetInstance("roundtrip-test")
+	if err != nil {
+		t.Fatalf("GetInstance failed after reload: %v", err)
+	}
+
+	// Compare all data
+	if loadedInstance.Name != originalInstance.Name {
+		t.Errorf("Name mismatch: original=%q, loaded=%q", originalInstance.Name, loadedInstance.Name)
+	}
+
+	originalOpts := originalInstance.GetOptions()
+	loadedOpts := loadedInstance.GetOptions()
+
+	// Compare restart options
+	if *loadedOpts.AutoRestart != *originalOpts.AutoRestart {
+		t.Errorf("AutoRestart mismatch: original=%v, loaded=%v", *originalOpts.AutoRestart, *loadedOpts.AutoRestart)
+	}
+	if *loadedOpts.MaxRestarts != *originalOpts.MaxRestarts {
+		t.Errorf("MaxRestarts mismatch: original=%v, loaded=%v", *originalOpts.MaxRestarts, *loadedOpts.MaxRestarts)
+	}
+	if *loadedOpts.RestartDelay != *originalOpts.RestartDelay {
+		t.Errorf("RestartDelay mismatch: original=%v, loaded=%v", *originalOpts.RestartDelay, *loadedOpts.RestartDelay)
+	}
+
+	// Compare llama server options
+	if loadedOpts.Model != originalOpts.Model {
+		t.Errorf("Model mismatch: original=%q, loaded=%q", originalOpts.Model, loadedOpts.Model)
+	}
+	if loadedOpts.Port != originalOpts.Port {
+		t.Errorf("Port mismatch: original=%d, loaded=%d", originalOpts.Port, loadedOpts.Port)
+	}
+	if loadedOpts.Host != originalOpts.Host {
+		t.Errorf("Host mismatch: original=%q, loaded=%q", originalOpts.Host, loadedOpts.Host)
+	}
+	if loadedOpts.CtxSize != originalOpts.CtxSize {
+		t.Errorf("CtxSize mismatch: original=%d, loaded=%d", originalOpts.CtxSize, loadedOpts.CtxSize)
+	}
+	if loadedOpts.GPULayers != originalOpts.GPULayers {
+		t.Errorf("GPULayers mismatch: original=%d, loaded=%d", originalOpts.GPULayers, loadedOpts.GPULayers)
+	}
+	if loadedOpts.Temperature != originalOpts.Temperature {
+		t.Errorf("Temperature mismatch: original=%f, loaded=%f", originalOpts.Temperature, loadedOpts.Temperature)
+	}
+	if loadedOpts.TopK != originalOpts.TopK {
+		t.Errorf("TopK mismatch: original=%d, loaded=%d", originalOpts.TopK, loadedOpts.TopK)
+	}
+	if loadedOpts.TopP != originalOpts.TopP {
+		t.Errorf("TopP mismatch: original=%f, loaded=%f", originalOpts.TopP, loadedOpts.TopP)
+	}
+	if loadedOpts.Verbose != originalOpts.Verbose {
+		t.Errorf("Verbose mismatch: original=%v, loaded=%v", originalOpts.Verbose, loadedOpts.Verbose)
+	}
+	if loadedOpts.FlashAttn != originalOpts.FlashAttn {
+		t.Errorf("FlashAttn mismatch: original=%v, loaded=%v", originalOpts.FlashAttn, loadedOpts.FlashAttn)
+	}
+	if loadedOpts.HFRepo != originalOpts.HFRepo {
+		t.Errorf("HFRepo mismatch: original=%q, loaded=%q", originalOpts.HFRepo, loadedOpts.HFRepo)
+	}
+
+	// Compare slice fields
+	if !reflect.DeepEqual(loadedOpts.Lora, originalOpts.Lora) {
+		t.Errorf("Lora mismatch: original=%v, loaded=%v", originalOpts.Lora, loadedOpts.Lora)
+	}
+
+	// Verify created timestamp is preserved
+	if loadedInstance.Created != originalInstance.Created {
+		t.Errorf("Created timestamp mismatch: original=%d, loaded=%d", originalInstance.Created, loadedInstance.Created)
+	}
+}
+
 // Helper function to create a test manager with standard config
 func createTestManager() llamactl.InstanceManager {
 	config := llamactl.InstancesConfig{
 		PortRange:           [2]int{8000, 9000},
-		LogDir:              "/tmp/test",
+		LogsDir:             "/tmp/test",
 		MaxInstances:        10,
 		LlamaExecutable:     "llama-server",
 		DefaultAutoRestart:  true,