fix: make LoadConfig read-only to prevent panic on read-only filesystems

LoadConfig() tried to create directories and write opal.yml as a side
effect of loading config. On the server (where /etc/opal is in systemd
ReadOnlyPaths), this failed, returning nil. All internal GetConfig()
callers discarded the error, passing nil to BuildUrgencyCoefficients()
which panicked on nil dereference.

Redesign the config system with layered, read-only loading:
- Defaults (always present) → YAML file (if exists) → OPAL_ env vars
- LoadConfig never writes to the filesystem or returns nil
- File creation moved to explicit InitConfig() for CLI first-run/setup
- SaveConfig uses yaml.Marshal instead of manual field-by-field Viper
  calls, eliminating the three-place maintenance burden

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

opal-task/cmd/root.go

@@ -244,16 +244,20 @@ func initializeApp() {
 		os.Exit(1)
 	}
 
-	// Load config
+	// On first run, create the config file with defaults
+	if isFirstRun {
+		if err := engine.InitConfig(); err != nil {
+			fmt.Fprintf(os.Stderr, "Error creating config: %v\n", err)
+			os.Exit(1)
+		}
+		showFirstRunMessage()
+	}
+
+	// Load config (reads file if present, otherwise uses defaults)
 	if _, err := engine.LoadConfig(); err != nil {
 		fmt.Fprintf(os.Stderr, "Error loading config: %v\n", err)
 		os.Exit(1)
 	}
-
-	// Show first-run message after config is created
-	if isFirstRun {
-		showFirstRunMessage()
-	}
 }
 
 func showFirstRunMessage() {

opal-task/cmd/setup.go

@@ -121,33 +121,6 @@ func runQuickSetup() {
 		os.Exit(1)
 	}
 
-	// Create config
-	cfg := engine.Config{
-		DefaultFilter:      "status:pending",
-		DefaultSort:        "due,priority",
-		DefaultReport:      "list",
-		ColorOutput:        true,
-		WeekStartDay:       "monday",
-		DefaultDueTime:     "",
-		NextLimit:          5,
-		SyncEnabled:        false,
-		SyncStrategy:       "last-write-wins",
-		SyncQueueOffline:   true,
-		UrgencyDue:         12.0,
-		UrgencyPriorityH:   6.0,
-		UrgencyPriorityM:   3.9,
-		UrgencyPriorityD:   1.8,
-		UrgencyPriorityL:   0.0,
-		UrgencyActive:      4.0,
-		UrgencyAge:         2.0,
-		UrgencyAgeMax:      365,
-		UrgencyTags:        1.0,
-		UrgencyProject:     1.0,
-		UrgencyWaiting:     -3.0,
-		UrgencyUrgentTag:   "next",
-		UrgencyUrgentCoeff: 15.0,
-	}
-
 	// Create directories
 	if err := os.MkdirAll(configDir, 0755); err != nil {
 		wizard.PrintError(fmt.Sprintf("Failed to create config directory: %v", err))
@@ -158,8 +131,8 @@ func runQuickSetup() {
 		os.Exit(1)
 	}
 
-	// Save config
+	// Save default config
-	if err := engine.SaveConfig(&cfg); err != nil {
+	if err := engine.SaveConfig(engine.DefaultConfig()); err != nil {
 		wizard.PrintError(fmt.Sprintf("Failed to save config: %v", err))
 		os.Exit(1)
 	}
@@ -304,34 +277,15 @@ func runInteractiveSetup() {
 		return
 	}
 
-	// Create configuration
+	// Create configuration from defaults, then apply user choices
-	cfg := &engine.Config{
+	cfg := engine.DefaultConfig()
-		DefaultFilter:      defaultFilter,
+	cfg.DefaultFilter = defaultFilter
-		DefaultSort:        "due,priority",
+	cfg.DefaultReport = reportNames[defaultReport]
-		DefaultReport:      reportNames[defaultReport],
+	cfg.ColorOutput = colorOutput
-		ColorOutput:        colorOutput,
+	cfg.WeekStartDay = weekStartDay
-		WeekStartDay:       weekStartDay,
+	cfg.SyncEnabled = syncEnabled
-		DefaultDueTime:     "",
+	cfg.SyncURL = syncURL
-		NextLimit:          5,
+	cfg.SyncAPIKey = syncAPIKey
-		SyncEnabled:        syncEnabled,
-		SyncURL:            syncURL,
-		SyncAPIKey:         syncAPIKey,
-		SyncStrategy:       "last-write-wins",
-		SyncQueueOffline:   true,
-		UrgencyDue:         12.0,
-		UrgencyPriorityH:   6.0,
-		UrgencyPriorityM:   3.9,
-		UrgencyPriorityD:   1.8,
-		UrgencyPriorityL:   0.0,
-		UrgencyActive:      4.0,
-		UrgencyAge:         2.0,
-		UrgencyAgeMax:      365,
-		UrgencyTags:        1.0,
-		UrgencyProject:     1.0,
-		UrgencyWaiting:     -3.0,
-		UrgencyUrgentTag:   "next",
-		UrgencyUrgentCoeff: 15.0,
-	}
 
 	// Set directory overrides
 	engine.SetConfigDirOverride(configDir)

opal-task/internal/engine/config.go

@@ -8,39 +8,40 @@ import (
 	"time"
 
 	"github.com/spf13/viper"
+	"go.yaml.in/yaml/v3"
 )
 
 type Config struct {
-	DefaultFilter  string `mapstructure:"default_filter"`
+	DefaultFilter  string `mapstructure:"default_filter" yaml:"default_filter"`
-	DefaultSort    string `mapstructure:"default_sort"`
+	DefaultSort    string `mapstructure:"default_sort" yaml:"default_sort"`
-	DefaultReport  string `mapstructure:"default_report"`
+	DefaultReport  string `mapstructure:"default_report" yaml:"default_report"`
-	ColorOutput    bool   `mapstructure:"color_output"`
+	ColorOutput    bool   `mapstructure:"color_output" yaml:"color_output"`
-	WeekStartDay   string `mapstructure:"week_start_day"`
+	WeekStartDay   string `mapstructure:"week_start_day" yaml:"week_start_day"`
-	DefaultDueTime string `mapstructure:"default_due_time"`
+	DefaultDueTime string `mapstructure:"default_due_time" yaml:"default_due_time"`
 
 	// Urgency coefficients
-	UrgencyDue         float64 `mapstructure:"urgency_due_coefficient"`
+	UrgencyDue         float64 `mapstructure:"urgency_due_coefficient" yaml:"urgency_due_coefficient"`
-	UrgencyPriorityH   float64 `mapstructure:"urgency_priority_h_coefficient"`
+	UrgencyPriorityH   float64 `mapstructure:"urgency_priority_h_coefficient" yaml:"urgency_priority_h_coefficient"`
-	UrgencyPriorityM   float64 `mapstructure:"urgency_priority_m_coefficient"`
+	UrgencyPriorityM   float64 `mapstructure:"urgency_priority_m_coefficient" yaml:"urgency_priority_m_coefficient"`
-	UrgencyPriorityD   float64 `mapstructure:"urgency_priority_d_coefficient"`
+	UrgencyPriorityD   float64 `mapstructure:"urgency_priority_d_coefficient" yaml:"urgency_priority_d_coefficient"`
-	UrgencyPriorityL   float64 `mapstructure:"urgency_priority_l_coefficient"`
+	UrgencyPriorityL   float64 `mapstructure:"urgency_priority_l_coefficient" yaml:"urgency_priority_l_coefficient"`
-	UrgencyActive      float64 `mapstructure:"urgency_active_coefficient"`
+	UrgencyActive      float64 `mapstructure:"urgency_active_coefficient" yaml:"urgency_active_coefficient"`
-	UrgencyAge         float64 `mapstructure:"urgency_age_coefficient"`
+	UrgencyAge         float64 `mapstructure:"urgency_age_coefficient" yaml:"urgency_age_coefficient"`
-	UrgencyAgeMax      int     `mapstructure:"urgency_age_max"`
+	UrgencyAgeMax      int     `mapstructure:"urgency_age_max" yaml:"urgency_age_max"`
-	UrgencyTags        float64 `mapstructure:"urgency_tags_coefficient"`
+	UrgencyTags        float64 `mapstructure:"urgency_tags_coefficient" yaml:"urgency_tags_coefficient"`
-	UrgencyProject     float64 `mapstructure:"urgency_project_coefficient"`
+	UrgencyProject     float64 `mapstructure:"urgency_project_coefficient" yaml:"urgency_project_coefficient"`
-	UrgencyWaiting     float64 `mapstructure:"urgency_waiting_coefficient"`
+	UrgencyWaiting     float64 `mapstructure:"urgency_waiting_coefficient" yaml:"urgency_waiting_coefficient"`
-	UrgencyUrgentTag   string  `mapstructure:"urgency_urgent_tag"`
+	UrgencyUrgentTag   string  `mapstructure:"urgency_urgent_tag" yaml:"urgency_urgent_tag"`
-	UrgencyUrgentCoeff float64 `mapstructure:"urgency_urgent_coefficient"`
+	UrgencyUrgentCoeff float64 `mapstructure:"urgency_urgent_coefficient" yaml:"urgency_urgent_coefficient"`
-	NextLimit          int     `mapstructure:"next_limit"`
+	NextLimit          int     `mapstructure:"next_limit" yaml:"next_limit"`
 
 	// Sync settings
-	SyncEnabled      bool   `mapstructure:"sync_enabled"`
+	SyncEnabled      bool   `mapstructure:"sync_enabled" yaml:"sync_enabled"`
-	SyncURL          string `mapstructure:"sync_url"`
+	SyncURL          string `mapstructure:"sync_url" yaml:"sync_url"`
-	SyncAPIKey       string `mapstructure:"sync_api_key"`
+	SyncAPIKey       string `mapstructure:"sync_api_key" yaml:"sync_api_key"`
-	SyncClientID     string `mapstructure:"sync_client_id"`
+	SyncClientID     string `mapstructure:"sync_client_id" yaml:"sync_client_id"`
-	SyncStrategy     string `mapstructure:"sync_strategy"`
+	SyncStrategy     string `mapstructure:"sync_strategy" yaml:"sync_strategy"`
-	SyncQueueOffline bool   `mapstructure:"sync_queue_offline"`
+	SyncQueueOffline bool   `mapstructure:"sync_queue_offline" yaml:"sync_queue_offline"`
 }
 
 var globalConfig *Config
@@ -234,77 +235,105 @@ func IsFirstRun() bool {
 	return !ConfigExists()
 }
 
-// LoadConfig loads the configuration from file or creates default
+// DefaultConfig returns a Config populated with all default values.
+// This is the single source of truth for defaults.
+func DefaultConfig() *Config {
+	return &Config{
+		DefaultFilter:      "status:pending",
+		DefaultSort:        "due,priority",
+		DefaultReport:      "list",
+		ColorOutput:        true,
+		WeekStartDay:       "monday",
+		DefaultDueTime:     "",
+		UrgencyDue:         12.0,
+		UrgencyPriorityH:   6.0,
+		UrgencyPriorityM:   3.9,
+		UrgencyPriorityD:   1.8,
+		UrgencyPriorityL:   0.0,
+		UrgencyActive:      4.0,
+		UrgencyAge:         2.0,
+		UrgencyAgeMax:      365,
+		UrgencyTags:        1.0,
+		UrgencyProject:     1.0,
+		UrgencyWaiting:     -3.0,
+		UrgencyUrgentTag:   "next",
+		UrgencyUrgentCoeff: 15.0,
+		NextLimit:          5,
+		SyncEnabled:        false,
+		SyncURL:            "",
+		SyncAPIKey:         "",
+		SyncClientID:       "",
+		SyncStrategy:       "last-write-wins",
+		SyncQueueOffline:   true,
+	}
+}
+
+// setViperDefaults registers all default values with a Viper instance.
+func setViperDefaults(v *viper.Viper) {
+	d := DefaultConfig()
+	v.SetDefault("default_filter", d.DefaultFilter)
+	v.SetDefault("default_sort", d.DefaultSort)
+	v.SetDefault("default_report", d.DefaultReport)
+	v.SetDefault("color_output", d.ColorOutput)
+	v.SetDefault("week_start_day", d.WeekStartDay)
+	v.SetDefault("default_due_time", d.DefaultDueTime)
+	v.SetDefault("urgency_due_coefficient", d.UrgencyDue)
+	v.SetDefault("urgency_priority_h_coefficient", d.UrgencyPriorityH)
+	v.SetDefault("urgency_priority_m_coefficient", d.UrgencyPriorityM)
+	v.SetDefault("urgency_priority_d_coefficient", d.UrgencyPriorityD)
+	v.SetDefault("urgency_priority_l_coefficient", d.UrgencyPriorityL)
+	v.SetDefault("urgency_active_coefficient", d.UrgencyActive)
+	v.SetDefault("urgency_age_coefficient", d.UrgencyAge)
+	v.SetDefault("urgency_age_max", d.UrgencyAgeMax)
+	v.SetDefault("urgency_tags_coefficient", d.UrgencyTags)
+	v.SetDefault("urgency_project_coefficient", d.UrgencyProject)
+	v.SetDefault("urgency_waiting_coefficient", d.UrgencyWaiting)
+	v.SetDefault("urgency_urgent_tag", d.UrgencyUrgentTag)
+	v.SetDefault("urgency_urgent_coefficient", d.UrgencyUrgentCoeff)
+	v.SetDefault("next_limit", d.NextLimit)
+	v.SetDefault("sync_enabled", d.SyncEnabled)
+	v.SetDefault("sync_url", d.SyncURL)
+	v.SetDefault("sync_api_key", d.SyncAPIKey)
+	v.SetDefault("sync_client_id", d.SyncClientID)
+	v.SetDefault("sync_strategy", d.SyncStrategy)
+	v.SetDefault("sync_queue_offline", d.SyncQueueOffline)
+}
+
+// LoadConfig loads configuration using layered sources:
+//  1. Hardcoded defaults (always present)
+//  2. YAML config file (optional, read-only — never created as a side effect)
+//  3. Environment variables with OPAL_ prefix (override everything)
+//
+// Returns a valid *Config even if no config file exists. Never returns nil.
+// Returns an error only if the config file exists but is malformed.
 func LoadConfig() (*Config, error) {
 	if globalConfig != nil {
 		return globalConfig, nil
 	}
 
-	configDir, err := GetConfigDir()
-	if err != nil {
-		return nil, err
-	}
-
-	// Ensure config directory exists
-	if err := os.MkdirAll(configDir, 0755); err != nil {
-		return nil, fmt.Errorf("failed to create config directory: %w", err)
-	}
-
-	configPath, err := GetConfigPath()
-	if err != nil {
-		return nil, err
-	}
-
 	v := viper.New()
-	v.SetConfigFile(configPath)
 	v.SetConfigType("yaml")
 
-	// Set defaults
+	// Layer 1: Hardcoded defaults
-	v.SetDefault("default_filter", "status:pending")
+	setViperDefaults(v)
-	v.SetDefault("default_sort", "due,priority")
-	v.SetDefault("default_report", "list")
-	v.SetDefault("color_output", true)
-	v.SetDefault("week_start_day", "monday")
-	v.SetDefault("default_due_time", "")
 
-	// Urgency defaults (adjusted for Opal's simpler model)
+	// Layer 2: YAML config file (optional, read-only)
-	v.SetDefault("urgency_due_coefficient", 12.0)
+	if configPath, err := GetConfigPath(); err == nil {
-	v.SetDefault("urgency_priority_h_coefficient", 6.0)
+		v.SetConfigFile(configPath)
-	v.SetDefault("urgency_priority_m_coefficient", 3.9)
-	v.SetDefault("urgency_priority_d_coefficient", 1.8)
-	v.SetDefault("urgency_priority_l_coefficient", 0.0)
-	v.SetDefault("urgency_active_coefficient", 4.0)
-	v.SetDefault("urgency_age_coefficient", 2.0)
-	v.SetDefault("urgency_age_max", 365)
-	v.SetDefault("urgency_tags_coefficient", 1.0)
-	v.SetDefault("urgency_project_coefficient", 1.0)
-	v.SetDefault("urgency_waiting_coefficient", -3.0)
-	v.SetDefault("urgency_urgent_tag", "next")
-	v.SetDefault("urgency_urgent_coefficient", 15.0)
-	v.SetDefault("next_limit", 5)
-
-	// Sync defaults
-	v.SetDefault("sync_enabled", false)
-	v.SetDefault("sync_url", "")
-	v.SetDefault("sync_api_key", "")
-	v.SetDefault("sync_client_id", "")
-	v.SetDefault("sync_strategy", "last-write-wins")
-	v.SetDefault("sync_queue_offline", true)
-
-	// Try to read existing config
-	err = v.ReadInConfig()
-	if err != nil {
-		// Config doesn't exist, create it with defaults
-		// Write config with defaults (ignoring the error type check for now)
-		if err := v.WriteConfigAs(configPath); err != nil {
-			return nil, fmt.Errorf("failed to create config file: %w", err)
-		}
-		// Try reading again
 		if err := v.ReadInConfig(); err != nil {
-			return nil, fmt.Errorf("failed to read newly created config: %w", err)
+			// Distinguish "file doesn't exist" from "file is malformed"
+			if _, statErr := os.Stat(configPath); statErr == nil {
+				// File exists but couldn't be parsed
+				return nil, fmt.Errorf("config file %s is invalid: %w", configPath, err)
+			}
+			// File doesn't exist — that's fine, defaults apply
 		}
 	}
 
+	// Layer 3: Environment variable overrides (OPAL_DEFAULT_FILTER, etc.)
+	v.SetEnvPrefix("OPAL")
+	v.AutomaticEnv()
+
 	cfg := &Config{}
 	if err := v.Unmarshal(cfg); err != nil {
 		return nil, fmt.Errorf("failed to unmarshal config: %w", err)
@@ -314,52 +343,42 @@ func LoadConfig() (*Config, error) {
 	return cfg, nil
 }
 
-// SaveConfig saves the configuration to file
+// InitConfig creates the config directory and writes a default opal.yml.
+// This should be called explicitly during CLI first-run or setup, never as a
+// side effect of loading config.
+func InitConfig() error {
+	configDir, err := GetConfigDir()
+	if err != nil {
+		return err
+	}
+	if err := os.MkdirAll(configDir, 0755); err != nil {
+		return fmt.Errorf("failed to create config directory: %w", err)
+	}
+	return SaveConfig(DefaultConfig())
+}
+
+// SaveConfig writes the configuration to the opal.yml file.
 func SaveConfig(cfg *Config) error {
 	configPath, err := GetConfigPath()
 	if err != nil {
 		return err
 	}
 
-	v := viper.New()
+	data, err := yaml.Marshal(cfg)
-	v.SetConfigFile(configPath)
+	if err != nil {
-	v.SetConfigType("yaml")
+		return fmt.Errorf("failed to marshal config: %w", err)
-
+	}
-	v.Set("default_filter", cfg.DefaultFilter)
+	if err := os.WriteFile(configPath, data, 0644); err != nil {
-	v.Set("default_sort", cfg.DefaultSort)
+		return fmt.Errorf("failed to write config file: %w", err)
-	v.Set("default_report", cfg.DefaultReport)
-	v.Set("color_output", cfg.ColorOutput)
-	v.Set("week_start_day", cfg.WeekStartDay)
-	v.Set("default_due_time", cfg.DefaultDueTime)
-
-	// Urgency settings
-	v.Set("urgency_due_coefficient", cfg.UrgencyDue)
-	v.Set("urgency_priority_h_coefficient", cfg.UrgencyPriorityH)
-	v.Set("urgency_priority_m_coefficient", cfg.UrgencyPriorityM)
-	v.Set("urgency_priority_d_coefficient", cfg.UrgencyPriorityD)
-	v.Set("urgency_priority_l_coefficient", cfg.UrgencyPriorityL)
-	v.Set("urgency_active_coefficient", cfg.UrgencyActive)
-	v.Set("urgency_age_coefficient", cfg.UrgencyAge)
-	v.Set("urgency_age_max", cfg.UrgencyAgeMax)
-	v.Set("urgency_tags_coefficient", cfg.UrgencyTags)
-	v.Set("urgency_project_coefficient", cfg.UrgencyProject)
-	v.Set("urgency_waiting_coefficient", cfg.UrgencyWaiting)
-	v.Set("urgency_urgent_tag", cfg.UrgencyUrgentTag)
-	v.Set("urgency_urgent_coefficient", cfg.UrgencyUrgentCoeff)
-	v.Set("next_limit", cfg.NextLimit)
-
-	// Sync settings
-	v.Set("sync_enabled", cfg.SyncEnabled)
-	v.Set("sync_url", cfg.SyncURL)
-	v.Set("sync_api_key", cfg.SyncAPIKey)
-	v.Set("sync_client_id", cfg.SyncClientID)
-	v.Set("sync_strategy", cfg.SyncStrategy)
-	v.Set("sync_queue_offline", cfg.SyncQueueOffline)
-
-	return v.WriteConfig()
 	}
 
-// GetConfig returns the loaded config or loads it if not already loaded
+	// Update the cached singleton
+	globalConfig = cfg
+	return nil
+}
+
+// GetConfig returns the loaded config or loads it if not already loaded.
+// Never returns nil — falls back to defaults if no config file exists.
 func GetConfig() (*Config, error) {
 	if globalConfig != nil {
 		return globalConfig, nil