From e0f1d43c2cb61765356efdea958158bc2d70e6bf Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Tue, 23 Dec 2025 17:03:21 -0800 Subject: [PATCH] fix: write startup config keys to config.yaml instead of SQLite (GH#536) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bd config set no-db true` now correctly writes to config.yaml instead of SQLite. Previously, startup flags like no-db, no-daemon, no-auto-flush were stored in SQLite but read from config.yaml at startup - making the command appear to work while having no effect. This adds: - yaml_config.go: Defines yaml-only keys and provides SetYamlConfig/GetYamlConfig - Updated config set/get commands to route yaml-only keys appropriately - Comprehensive tests for yaml config handling Startup flags affected: no-db, no-daemon, no-auto-flush, no-auto-import, json, auto-start-daemon, flush-debounce, lock-timeout, git.*, sync.*, routing.* 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/config.go | 63 +++++-- internal/config/yaml_config.go | 245 ++++++++++++++++++++++++++++ internal/config/yaml_config_test.go | 206 +++++++++++++++++++++++ 3 files changed, 504 insertions(+), 10 deletions(-) create mode 100644 internal/config/yaml_config.go create mode 100644 internal/config/yaml_config_test.go diff --git a/cmd/bd/config.go b/cmd/bd/config.go index 7e104b11..a53b5f4c 100644 --- a/cmd/bd/config.go +++ b/cmd/bd/config.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/spf13/cobra" + "github.com/steveyegge/beads/internal/config" "github.com/steveyegge/beads/internal/syncbranch" ) @@ -49,17 +50,38 @@ var configSetCmd = &cobra.Command{ Short: "Set a configuration value", Args: cobra.ExactArgs(2), Run: func(_ *cobra.Command, args []string) { - // Config operations work in direct mode only + key := args[0] + value := args[1] + + // Check if this is a yaml-only key (startup settings like no-db, no-daemon, etc.) + // These must be written to config.yaml, not SQLite, because they're read + // before the database is opened. (GH#536) + if config.IsYamlOnlyKey(key) { + if err := config.SetYamlConfig(key, value); err != nil { + fmt.Fprintf(os.Stderr, "Error setting config: %v\n", err) + os.Exit(1) + } + + if jsonOutput { + outputJSON(map[string]interface{}{ + "key": key, + "value": value, + "location": "config.yaml", + }) + } else { + fmt.Printf("Set %s = %s (in config.yaml)\n", key, value) + } + return + } + + // Database-stored config requires direct mode if err := ensureDirectMode("config set requires direct database access"); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } - key := args[0] - value := args[1] - ctx := rootCtx - + // Special handling for sync.branch to apply validation if strings.TrimSpace(key) == syncbranch.ConfigKey { if err := syncbranch.Set(ctx, store, value); err != nil { @@ -89,25 +111,46 @@ var configGetCmd = &cobra.Command{ Short: "Get a configuration value", Args: cobra.ExactArgs(1), Run: func(cmd *cobra.Command, args []string) { - // Config operations work in direct mode only + key := args[0] + + // Check if this is a yaml-only key (startup settings) + // These are read from config.yaml via viper, not SQLite. (GH#536) + if config.IsYamlOnlyKey(key) { + value := config.GetYamlConfig(key) + + if jsonOutput { + outputJSON(map[string]interface{}{ + "key": key, + "value": value, + "location": "config.yaml", + }) + } else { + if value == "" { + fmt.Printf("%s (not set in config.yaml)\n", key) + } else { + fmt.Printf("%s\n", value) + } + } + return + } + + // Database-stored config requires direct mode if err := ensureDirectMode("config get requires direct database access"); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) os.Exit(1) } - key := args[0] - ctx := rootCtx var value string var err error - + // Special handling for sync.branch to support env var override if strings.TrimSpace(key) == syncbranch.ConfigKey { value, err = syncbranch.Get(ctx, store) } else { value, err = store.GetConfig(ctx, key) } - + if err != nil { fmt.Fprintf(os.Stderr, "Error getting config: %v\n", err) os.Exit(1) diff --git a/internal/config/yaml_config.go b/internal/config/yaml_config.go new file mode 100644 index 00000000..f0b8027a --- /dev/null +++ b/internal/config/yaml_config.go @@ -0,0 +1,245 @@ +package config + +import ( + "bufio" + "fmt" + "os" + "path/filepath" + "regexp" + "strings" +) + +// YamlOnlyKeys are configuration keys that must be stored in config.yaml +// rather than the SQLite database. These are "startup" settings that are +// read before the database is opened. +// +// This fixes GH#536: users were confused when `bd config set no-db true` +// appeared to succeed but had no effect (because no-db is read from yaml +// at startup, not from SQLite). +var YamlOnlyKeys = map[string]bool{ + // Bootstrap flags (affect how bd starts) + "no-db": true, + "no-daemon": true, + "no-auto-flush": true, + "no-auto-import": true, + "json": true, + "auto-start-daemon": true, + + // Database and identity + "db": true, + "actor": true, + "identity": true, + + // Timing settings + "flush-debounce": true, + "lock-timeout": true, + "remote-sync-interval": true, + + // Git settings + "git.author": true, + "git.no-gpg-sign": true, + "no-push": true, + + // Sync settings + "sync-branch": true, + "sync.branch": true, + "sync.require_confirmation_on_mass_delete": true, + + // Routing settings + "routing.mode": true, + "routing.default": true, + "routing.maintainer": true, + "routing.contributor": true, + + // Create command settings + "create.require-description": true, +} + +// IsYamlOnlyKey returns true if the given key should be stored in config.yaml +// rather than the SQLite database. +func IsYamlOnlyKey(key string) bool { + // Check exact match + if YamlOnlyKeys[key] { + return true + } + + // Check prefix matches for nested keys + prefixes := []string{"routing.", "sync.", "git.", "directory.", "repos.", "external_projects."} + for _, prefix := range prefixes { + if strings.HasPrefix(key, prefix) { + return true + } + } + + return false +} + +// SetYamlConfig sets a configuration value in the project's config.yaml file. +// It handles both adding new keys and updating existing (possibly commented) keys. +func SetYamlConfig(key, value string) error { + configPath, err := findProjectConfigYaml() + if err != nil { + return err + } + + // Read existing config + content, err := os.ReadFile(configPath) + if err != nil { + return fmt.Errorf("failed to read config.yaml: %w", err) + } + + // Update or add the key + newContent, err := updateYamlKey(string(content), key, value) + if err != nil { + return err + } + + // Write back + if err := os.WriteFile(configPath, []byte(newContent), 0644); err != nil { + return fmt.Errorf("failed to write config.yaml: %w", err) + } + + return nil +} + +// GetYamlConfig gets a configuration value from config.yaml. +// Returns empty string if key is not found or is commented out. +func GetYamlConfig(key string) string { + if v == nil { + return "" + } + return v.GetString(key) +} + +// findProjectConfigYaml finds the project's .beads/config.yaml file. +func findProjectConfigYaml() (string, error) { + cwd, err := os.Getwd() + if err != nil { + return "", fmt.Errorf("failed to get working directory: %w", err) + } + + // Walk up parent directories to find .beads/config.yaml + for dir := cwd; dir != filepath.Dir(dir); dir = filepath.Dir(dir) { + configPath := filepath.Join(dir, ".beads", "config.yaml") + if _, err := os.Stat(configPath); err == nil { + return configPath, nil + } + } + + return "", fmt.Errorf("no .beads/config.yaml found (run 'bd init' first)") +} + +// updateYamlKey updates a key in yaml content, handling commented-out keys. +// If the key exists (commented or not), it updates it in place. +// If the key doesn't exist, it appends it at the end. +func updateYamlKey(content, key, value string) (string, error) { + // Format the value appropriately + formattedValue := formatYamlValue(value) + newLine := fmt.Sprintf("%s: %s", key, formattedValue) + + // Build regex to match the key (commented or not) + // Matches: "key: value" or "# key: value" with optional leading whitespace + keyPattern := regexp.MustCompile(`^(\s*)(#\s*)?` + regexp.QuoteMeta(key) + `\s*:`) + + found := false + var result []string + + scanner := bufio.NewScanner(strings.NewReader(content)) + for scanner.Scan() { + line := scanner.Text() + if keyPattern.MatchString(line) { + // Found the key - replace with new value (uncommented) + // Preserve leading whitespace + matches := keyPattern.FindStringSubmatch(line) + indent := "" + if len(matches) > 1 { + indent = matches[1] + } + result = append(result, indent+newLine) + found = true + } else { + result = append(result, line) + } + } + + if !found { + // Key not found - append at end + // Add blank line before if content doesn't end with one + if len(result) > 0 && result[len(result)-1] != "" { + result = append(result, "") + } + result = append(result, newLine) + } + + return strings.Join(result, "\n"), nil +} + +// formatYamlValue formats a value appropriately for YAML. +func formatYamlValue(value string) string { + // Boolean values + lower := strings.ToLower(value) + if lower == "true" || lower == "false" { + return lower + } + + // Numeric values - return as-is + if isNumeric(value) { + return value + } + + // Duration values (like "30s", "5m") - return as-is + if isDuration(value) { + return value + } + + // String values that need quoting + if needsQuoting(value) { + return fmt.Sprintf("%q", value) + } + + return value +} + +func isNumeric(s string) bool { + if s == "" { + return false + } + for i, c := range s { + if c == '-' && i == 0 { + continue + } + if c == '.' { + continue + } + if c < '0' || c > '9' { + return false + } + } + return true +} + +func isDuration(s string) bool { + if len(s) < 2 { + return false + } + suffix := s[len(s)-1] + if suffix != 's' && suffix != 'm' && suffix != 'h' { + return false + } + return isNumeric(s[:len(s)-1]) +} + +func needsQuoting(s string) bool { + // Quote if contains special YAML characters + special := []string{":", "#", "[", "]", "{", "}", ",", "&", "*", "!", "|", ">", "'", "\"", "%", "@", "`"} + for _, c := range special { + if strings.Contains(s, c) { + return true + } + } + // Quote if starts/ends with whitespace + if strings.TrimSpace(s) != s { + return true + } + return false +} diff --git a/internal/config/yaml_config_test.go b/internal/config/yaml_config_test.go new file mode 100644 index 00000000..6fefe8f3 --- /dev/null +++ b/internal/config/yaml_config_test.go @@ -0,0 +1,206 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +func TestIsYamlOnlyKey(t *testing.T) { + tests := []struct { + key string + expected bool + }{ + // Exact matches + {"no-db", true}, + {"no-daemon", true}, + {"no-auto-flush", true}, + {"json", true}, + {"auto-start-daemon", true}, + {"flush-debounce", true}, + {"git.author", true}, + {"git.no-gpg-sign", true}, + + // Prefix matches + {"routing.mode", true}, + {"routing.custom-key", true}, + {"sync.branch", true}, + {"sync.require_confirmation_on_mass_delete", true}, + {"directory.labels", true}, + {"repos.primary", true}, + {"external_projects.beads", true}, + + // SQLite keys (should return false) + {"jira.url", false}, + {"jira.project", false}, + {"linear.api_key", false}, + {"github.org", false}, + {"custom.setting", false}, + {"status.custom", false}, + {"issue_prefix", false}, + } + + for _, tt := range tests { + t.Run(tt.key, func(t *testing.T) { + got := IsYamlOnlyKey(tt.key) + if got != tt.expected { + t.Errorf("IsYamlOnlyKey(%q) = %v, want %v", tt.key, got, tt.expected) + } + }) + } +} + +func TestUpdateYamlKey(t *testing.T) { + tests := []struct { + name string + content string + key string + value string + expected string + }{ + { + name: "update commented key", + content: "# no-db: false\nother: value", + key: "no-db", + value: "true", + expected: "no-db: true\nother: value", + }, + { + name: "update existing key", + content: "no-db: false\nother: value", + key: "no-db", + value: "true", + expected: "no-db: true\nother: value", + }, + { + name: "add new key", + content: "other: value", + key: "no-db", + value: "true", + expected: "other: value\n\nno-db: true", + }, + { + name: "preserve indentation", + content: " # no-db: false\nother: value", + key: "no-db", + value: "true", + expected: " no-db: true\nother: value", + }, + { + name: "handle string value", + content: "# actor: \"\"\nother: value", + key: "actor", + value: "steve", + expected: "actor: steve\nother: value", + }, + { + name: "handle duration value", + content: "# flush-debounce: \"5s\"", + key: "flush-debounce", + value: "30s", + expected: "flush-debounce: 30s", + }, + { + name: "quote special characters", + content: "other: value", + key: "actor", + value: "user: name", + expected: "other: value\n\nactor: \"user: name\"", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := updateYamlKey(tt.content, tt.key, tt.value) + if err != nil { + t.Fatalf("updateYamlKey() error = %v", err) + } + if got != tt.expected { + t.Errorf("updateYamlKey() =\n%q\nwant:\n%q", got, tt.expected) + } + }) + } +} + +func TestFormatYamlValue(t *testing.T) { + tests := []struct { + value string + expected string + }{ + {"true", "true"}, + {"false", "false"}, + {"TRUE", "true"}, + {"FALSE", "false"}, + {"123", "123"}, + {"3.14", "3.14"}, + {"30s", "30s"}, + {"5m", "5m"}, + {"simple", "simple"}, + {"has space", "has space"}, + {"has:colon", "\"has:colon\""}, + {"has#hash", "\"has#hash\""}, + {" leading", "\" leading\""}, + } + + for _, tt := range tests { + t.Run(tt.value, func(t *testing.T) { + got := formatYamlValue(tt.value) + if got != tt.expected { + t.Errorf("formatYamlValue(%q) = %q, want %q", tt.value, got, tt.expected) + } + }) + } +} + +func TestSetYamlConfig(t *testing.T) { + // Create a temp directory with .beads/config.yaml + tmpDir, err := os.MkdirTemp("", "beads-yaml-test-*") + if err != nil { + t.Fatalf("Failed to create temp dir: %v", err) + } + defer os.RemoveAll(tmpDir) + + beadsDir := filepath.Join(tmpDir, ".beads") + if err := os.MkdirAll(beadsDir, 0755); err != nil { + t.Fatalf("Failed to create .beads dir: %v", err) + } + + configPath := filepath.Join(beadsDir, "config.yaml") + initialConfig := `# Beads Config +# no-db: false +other-setting: value +` + if err := os.WriteFile(configPath, []byte(initialConfig), 0644); err != nil { + t.Fatalf("Failed to write config.yaml: %v", err) + } + + // Change to temp directory for the test + oldWd, _ := os.Getwd() + if err := os.Chdir(tmpDir); err != nil { + t.Fatalf("Failed to chdir: %v", err) + } + defer os.Chdir(oldWd) + + // Test SetYamlConfig + if err := SetYamlConfig("no-db", "true"); err != nil { + t.Fatalf("SetYamlConfig() error = %v", err) + } + + // Read back and verify + content, err := os.ReadFile(configPath) + if err != nil { + t.Fatalf("Failed to read config.yaml: %v", err) + } + + contentStr := string(content) + if !strings.Contains(contentStr, "no-db: true") { + t.Errorf("config.yaml should contain 'no-db: true', got:\n%s", contentStr) + } + if strings.Contains(contentStr, "# no-db") { + t.Errorf("config.yaml should not have commented no-db, got:\n%s", contentStr) + } + if !strings.Contains(contentStr, "other-setting: value") { + t.Errorf("config.yaml should preserve other settings, got:\n%s", contentStr) + } +}