From cdbca65ed4ba57683d4bf1a1729bba8c313ce06d Mon Sep 17 00:00:00 2001 From: "Charles P. Cross" <8572939+cpdata@users.noreply.github.com> Date: Wed, 24 Dec 2025 15:35:54 -0500 Subject: [PATCH] feat(config): add override notification for config parameters (#731) feat(config): add override notification for config parameters Adds verbose logging when config values are overridden by flags/env vars. --- cmd/bd/main.go | 59 +++++++++++++ internal/config/config.go | 147 +++++++++++++++++++++++++++++++++ internal/config/config_test.go | 112 +++++++++++++++++++++++++ 3 files changed, 318 insertions(+) diff --git a/cmd/bd/main.go b/cmd/bd/main.go index ee2ba2af..12f27cf5 100644 --- a/cmd/bd/main.go +++ b/cmd/bd/main.go @@ -320,33 +320,92 @@ var rootCmd = &cobra.Command{ // Priority: flags > viper (config file + env vars) > defaults // Do this BEFORE early-return so init/version/help respect config + // Track flag overrides for notification (only in verbose mode) + flagOverrides := make(map[string]struct { + Value interface{} + WasSet bool + }) + // If flag wasn't explicitly set, use viper value if !cmd.Flags().Changed("json") { jsonOutput = config.GetBool("json") + } else { + flagOverrides["json"] = struct { + Value interface{} + WasSet bool + }{jsonOutput, true} } if !cmd.Flags().Changed("no-daemon") { noDaemon = config.GetBool("no-daemon") + } else { + flagOverrides["no-daemon"] = struct { + Value interface{} + WasSet bool + }{noDaemon, true} } if !cmd.Flags().Changed("no-auto-flush") { noAutoFlush = config.GetBool("no-auto-flush") + } else { + flagOverrides["no-auto-flush"] = struct { + Value interface{} + WasSet bool + }{noAutoFlush, true} } if !cmd.Flags().Changed("no-auto-import") { noAutoImport = config.GetBool("no-auto-import") + } else { + flagOverrides["no-auto-import"] = struct { + Value interface{} + WasSet bool + }{noAutoImport, true} } if !cmd.Flags().Changed("no-db") { noDb = config.GetBool("no-db") + } else { + flagOverrides["no-db"] = struct { + Value interface{} + WasSet bool + }{noDb, true} } if !cmd.Flags().Changed("readonly") { readonlyMode = config.GetBool("readonly") + } else { + flagOverrides["readonly"] = struct { + Value interface{} + WasSet bool + }{readonlyMode, true} } if !cmd.Flags().Changed("lock-timeout") { lockTimeout = config.GetDuration("lock-timeout") + } else { + flagOverrides["lock-timeout"] = struct { + Value interface{} + WasSet bool + }{lockTimeout, true} } if !cmd.Flags().Changed("db") && dbPath == "" { dbPath = config.GetString("db") + } else if cmd.Flags().Changed("db") { + flagOverrides["db"] = struct { + Value interface{} + WasSet bool + }{dbPath, true} } if !cmd.Flags().Changed("actor") && actor == "" { actor = config.GetString("actor") + } else if cmd.Flags().Changed("actor") { + flagOverrides["actor"] = struct { + Value interface{} + WasSet bool + }{actor, true} + } + + // Check for and log configuration overrides (only in verbose mode) + if verboseFlag { + overrides := config.CheckOverrides(flagOverrides) + for _, override := range overrides { + config.LogOverride(override) + } } // Protect forks from accidentally committing upstream issue database diff --git a/internal/config/config.go b/internal/config/config.go index 74484a29..67c51236 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -140,6 +140,153 @@ func Initialize() error { return nil } +// ConfigSource represents where a configuration value came from +type ConfigSource string + +const ( + SourceDefault ConfigSource = "default" + SourceConfigFile ConfigSource = "config_file" + SourceEnvVar ConfigSource = "env_var" + SourceFlag ConfigSource = "flag" +) + +// ConfigOverride represents a detected configuration override +type ConfigOverride struct { + Key string + EffectiveValue interface{} + OverriddenBy ConfigSource + OriginalSource ConfigSource + OriginalValue interface{} +} + +// GetValueSource returns the source of a configuration value. +// Priority (highest to lowest): env var > config file > default +// Note: Flag overrides are handled separately in main.go since viper doesn't know about cobra flags. +func GetValueSource(key string) ConfigSource { + if v == nil { + return SourceDefault + } + + // Check if value is set from environment variable + // Viper's IsSet returns true if the key is set from any source (env, config, or default) + // We need to check specifically for env var by looking at the env var directly + envKey := "BD_" + strings.ToUpper(strings.ReplaceAll(strings.ReplaceAll(key, "-", "_"), ".", "_")) + if os.Getenv(envKey) != "" { + return SourceEnvVar + } + + // Check BEADS_ prefixed env vars for legacy compatibility + beadsEnvKey := "BEADS_" + strings.ToUpper(strings.ReplaceAll(strings.ReplaceAll(key, "-", "_"), ".", "_")) + if os.Getenv(beadsEnvKey) != "" { + return SourceEnvVar + } + + // Check if value is set in config file (as opposed to being a default) + if v.InConfig(key) { + return SourceConfigFile + } + + return SourceDefault +} + +// CheckOverrides checks for configuration overrides and returns a list of detected overrides. +// This is useful for informing users when env vars or flags override config file values. +// flagOverrides is a map of key -> (flagValue, flagWasSet) for flags that were explicitly set. +func CheckOverrides(flagOverrides map[string]struct{ Value interface{}; WasSet bool }) []ConfigOverride { + var overrides []ConfigOverride + + for key, flagInfo := range flagOverrides { + if !flagInfo.WasSet { + continue + } + + source := GetValueSource(key) + if source == SourceConfigFile || source == SourceEnvVar { + // Flag is overriding a config file or env var value + var originalValue interface{} + switch v := flagInfo.Value.(type) { + case bool: + originalValue = GetBool(key) + case string: + originalValue = GetString(key) + case int: + originalValue = GetInt(key) + default: + originalValue = v + } + + overrides = append(overrides, ConfigOverride{ + Key: key, + EffectiveValue: flagInfo.Value, + OverriddenBy: SourceFlag, + OriginalSource: source, + OriginalValue: originalValue, + }) + } + } + + // Check for env var overriding config file + if v != nil { + for _, key := range v.AllKeys() { + envSource := GetValueSource(key) + if envSource == SourceEnvVar && v.InConfig(key) { + // Env var is overriding config file value + // Get the config file value by temporarily unsetting the env + envKey := "BD_" + strings.ToUpper(strings.ReplaceAll(strings.ReplaceAll(key, "-", "_"), ".", "_")) + envValue := os.Getenv(envKey) + if envValue == "" { + envKey = "BEADS_" + strings.ToUpper(strings.ReplaceAll(strings.ReplaceAll(key, "-", "_"), ".", "_")) + envValue = os.Getenv(envKey) + } + + // Skip if no env var actually set (shouldn't happen but be safe) + if envValue == "" { + continue + } + + overrides = append(overrides, ConfigOverride{ + Key: key, + EffectiveValue: v.Get(key), + OverriddenBy: SourceEnvVar, + OriginalSource: SourceConfigFile, + OriginalValue: nil, // We can't easily get the config file value separately + }) + } + } + } + + return overrides +} + +// LogOverride logs a message about a configuration override in verbose mode. +func LogOverride(override ConfigOverride) { + var sourceDesc string + switch override.OriginalSource { + case SourceConfigFile: + sourceDesc = "config file" + case SourceEnvVar: + sourceDesc = "environment variable" + case SourceDefault: + sourceDesc = "default" + default: + sourceDesc = string(override.OriginalSource) + } + + var overrideDesc string + switch override.OverriddenBy { + case SourceFlag: + overrideDesc = "command-line flag" + case SourceEnvVar: + overrideDesc = "environment variable" + default: + overrideDesc = string(override.OverriddenBy) + } + + // Always emit to stderr when verbose mode is enabled (caller guards on verbose) + fmt.Fprintf(os.Stderr, "Config: %s overridden by %s (was: %v from %s, now: %v)\n", + override.Key, overrideDesc, override.OriginalValue, sourceDesc, override.EffectiveValue) +} + // GetString retrieves a string configuration value func GetString(key string) string { if v == nil { diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b0d7ee48..c8a0bc68 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -637,3 +637,115 @@ func TestGetIdentityFromConfig(t *testing.T) { t.Errorf("GetIdentity(flag-override) = %q, want \"flag-override\"", got) } } + +func TestGetValueSource(t *testing.T) { + // Initialize config + if err := Initialize(); err != nil { + t.Fatalf("Initialize() returned error: %v", err) + } + + tests := []struct { + name string + key string + setup func() + cleanup func() + expected ConfigSource + }{ + { + name: "default value returns SourceDefault", + key: "json", + setup: func() {}, + cleanup: func() {}, + expected: SourceDefault, + }, + { + name: "env var returns SourceEnvVar", + key: "json", + setup: func() { + os.Setenv("BD_JSON", "true") + }, + cleanup: func() { + os.Unsetenv("BD_JSON") + }, + expected: SourceEnvVar, + }, + { + name: "BEADS_ prefixed env var returns SourceEnvVar", + key: "identity", + setup: func() { + os.Setenv("BEADS_IDENTITY", "test-identity") + }, + cleanup: func() { + os.Unsetenv("BEADS_IDENTITY") + }, + expected: SourceEnvVar, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Reinitialize to clear state + if err := Initialize(); err != nil { + t.Fatalf("Initialize() returned error: %v", err) + } + + tt.setup() + defer tt.cleanup() + + got := GetValueSource(tt.key) + if got != tt.expected { + t.Errorf("GetValueSource(%q) = %v, want %v", tt.key, got, tt.expected) + } + }) + } +} + +func TestCheckOverrides_FlagOverridesEnvVar(t *testing.T) { + // Initialize config + if err := Initialize(); err != nil { + t.Fatalf("Initialize() returned error: %v", err) + } + + // Set an env var + os.Setenv("BD_JSON", "true") + defer os.Unsetenv("BD_JSON") + + // Simulate flag override + flagOverrides := map[string]struct { + Value interface{} + WasSet bool + }{ + "json": {Value: false, WasSet: true}, + } + + overrides := CheckOverrides(flagOverrides) + + // Should detect that flag overrides env var + found := false + for _, o := range overrides { + if o.Key == "json" && o.OverriddenBy == SourceFlag { + found = true + break + } + } + + if !found { + t.Error("Expected to find flag override for 'json' key") + } +} + +func TestConfigSourceConstants(t *testing.T) { + // Verify source constants have expected string values + if SourceDefault != "default" { + t.Errorf("SourceDefault = %q, want \"default\"", SourceDefault) + } + if SourceConfigFile != "config_file" { + t.Errorf("SourceConfigFile = %q, want \"config_file\"", SourceConfigFile) + } + if SourceEnvVar != "env_var" { + t.Errorf("SourceEnvVar = %q, want \"env_var\"", SourceEnvVar) + } + if SourceFlag != "flag" { + t.Errorf("SourceFlag = %q, want \"flag\"", SourceFlag) + } +}