fix: respect hierarchy.max-depth config setting (GH#995) (#997)
* fix: respect hierarchy.max-depth config setting (GH#995) The hierarchy.max-depth config setting was being ignored because storage implementations had the depth limit hardcoded to 3. This fix: - Registers hierarchy.max-depth default (3) in config initialization - Adds hierarchy.max-depth to yaml-only keys for config.yaml storage - Updates SQLite and Memory storage to read max depth from config - Adds validation to reject hierarchy.max-depth values < 1 - Adds tests for configurable hierarchy depth Users can now set deeper hierarchies: bd config set hierarchy.max-depth 10 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * refactor: extract shared CheckHierarchyDepth function (GH#995) - Extract duplicated depth-checking logic to types.CheckHierarchyDepth() - Update sqlite and memory storage backends to use shared function - Add t.Cleanup() for proper test isolation in sqlite test - Add equivalent test coverage for memory storage backend - Add comprehensive unit tests for CheckHierarchyDepth function Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -69,7 +69,7 @@ func Initialize() error {
|
||||
// Environment variables take precedence over config file
|
||||
// E.g., BD_JSON, BD_NO_DAEMON, BD_ACTOR, BD_DB
|
||||
v.SetEnvPrefix("BD")
|
||||
|
||||
|
||||
// Replace hyphens and dots with underscores for env var mapping
|
||||
// This allows BD_NO_DAEMON to map to "no-daemon" config key
|
||||
v.SetEnvKeyReplacer(strings.NewReplacer(".", "_", "-", "_"))
|
||||
@@ -85,20 +85,20 @@ func Initialize() error {
|
||||
v.SetDefault("actor", "")
|
||||
v.SetDefault("issue-prefix", "")
|
||||
v.SetDefault("lock-timeout", "30s")
|
||||
|
||||
|
||||
// Additional environment variables (not prefixed with BD_)
|
||||
// These are bound explicitly for backward compatibility
|
||||
_ = v.BindEnv("flush-debounce", "BEADS_FLUSH_DEBOUNCE")
|
||||
_ = v.BindEnv("auto-start-daemon", "BEADS_AUTO_START_DAEMON")
|
||||
_ = v.BindEnv("identity", "BEADS_IDENTITY")
|
||||
_ = v.BindEnv("remote-sync-interval", "BEADS_REMOTE_SYNC_INTERVAL")
|
||||
|
||||
|
||||
// Set defaults for additional settings
|
||||
v.SetDefault("flush-debounce", "30s")
|
||||
v.SetDefault("auto-start-daemon", true)
|
||||
v.SetDefault("identity", "")
|
||||
v.SetDefault("remote-sync-interval", "30s")
|
||||
|
||||
|
||||
// Routing configuration defaults
|
||||
v.SetDefault("routing.mode", "auto")
|
||||
v.SetDefault("routing.default", ".")
|
||||
@@ -122,8 +122,13 @@ func Initialize() error {
|
||||
v.SetDefault("validation.on-create", "none")
|
||||
v.SetDefault("validation.on-sync", "none")
|
||||
|
||||
// Hierarchy configuration defaults (GH#995)
|
||||
// Maximum nesting depth for hierarchical IDs (e.g., bd-abc.1.2.3)
|
||||
// Default matches types.MaxHierarchyDepth constant
|
||||
v.SetDefault("hierarchy.max-depth", 3)
|
||||
|
||||
// Git configuration defaults (GH#600)
|
||||
v.SetDefault("git.author", "") // Override commit author (e.g., "beads-bot <beads@example.com>")
|
||||
v.SetDefault("git.author", "") // Override commit author (e.g., "beads-bot <beads@example.com>")
|
||||
v.SetDefault("git.no-gpg-sign", false) // Disable GPG signing for beads commits
|
||||
|
||||
// Directory-aware label scoping (GH#541)
|
||||
@@ -200,7 +205,10 @@ func GetValueSource(key string) ConfigSource {
|
||||
// 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 {
|
||||
func CheckOverrides(flagOverrides map[string]struct {
|
||||
Value interface{}
|
||||
WasSet bool
|
||||
}) []ConfigOverride {
|
||||
var overrides []ConfigOverride
|
||||
|
||||
for key, flagInfo := range flagOverrides {
|
||||
|
||||
@@ -6,6 +6,7 @@ import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
@@ -18,16 +19,16 @@ import (
|
||||
// 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,
|
||||
"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,
|
||||
"db": true,
|
||||
"actor": true,
|
||||
"identity": true,
|
||||
|
||||
// Timing settings
|
||||
@@ -36,14 +37,14 @@ var YamlOnlyKeys = map[string]bool{
|
||||
"remote-sync-interval": true,
|
||||
|
||||
// Git settings
|
||||
"git.author": true,
|
||||
"git.no-gpg-sign": true,
|
||||
"no-push": true,
|
||||
"no-git-ops": true, // Disable git ops in bd prime session close protocol (GH#593)
|
||||
"git.author": true,
|
||||
"git.no-gpg-sign": true,
|
||||
"no-push": true,
|
||||
"no-git-ops": true, // Disable git ops in bd prime session close protocol (GH#593)
|
||||
|
||||
// Sync settings
|
||||
"sync-branch": true,
|
||||
"sync.branch": true,
|
||||
"sync-branch": true,
|
||||
"sync.branch": true,
|
||||
"sync.require_confirmation_on_mass_delete": true,
|
||||
|
||||
// Daemon settings (GH#871: team-wide auto-sync config)
|
||||
@@ -64,6 +65,9 @@ var YamlOnlyKeys = map[string]bool{
|
||||
// Values: "warn" | "error" | "none"
|
||||
"validation.on-create": true,
|
||||
"validation.on-sync": true,
|
||||
|
||||
// Hierarchy settings (GH#995)
|
||||
"hierarchy.max-depth": true,
|
||||
}
|
||||
|
||||
// IsYamlOnlyKey returns true if the given key should be stored in config.yaml
|
||||
@@ -75,7 +79,7 @@ func IsYamlOnlyKey(key string) bool {
|
||||
}
|
||||
|
||||
// Check prefix matches for nested keys
|
||||
prefixes := []string{"routing.", "sync.", "git.", "directory.", "repos.", "external_projects.", "validation.", "daemon."}
|
||||
prefixes := []string{"routing.", "sync.", "git.", "directory.", "repos.", "external_projects.", "validation.", "daemon.", "hierarchy."}
|
||||
for _, prefix := range prefixes {
|
||||
if strings.HasPrefix(key, prefix) {
|
||||
return true
|
||||
@@ -105,6 +109,11 @@ func normalizeYamlKey(key string) string {
|
||||
// It handles both adding new keys and updating existing (possibly commented) keys.
|
||||
// Keys are normalized to their canonical yaml format (e.g., sync.branch -> sync-branch).
|
||||
func SetYamlConfig(key, value string) error {
|
||||
// Validate specific keys (GH#995)
|
||||
if err := validateYamlConfigValue(key, value); err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
configPath, err := findProjectConfigYaml()
|
||||
if err != nil {
|
||||
return err
|
||||
@@ -274,3 +283,20 @@ func needsQuoting(s string) bool {
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// validateYamlConfigValue validates a configuration value before setting.
|
||||
// Returns an error if the value is invalid for the given key.
|
||||
func validateYamlConfigValue(key, value string) error {
|
||||
switch key {
|
||||
case "hierarchy.max-depth":
|
||||
// Must be a positive integer >= 1 (GH#995)
|
||||
depth, err := strconv.Atoi(value)
|
||||
if err != nil {
|
||||
return fmt.Errorf("hierarchy.max-depth must be a positive integer, got %q", value)
|
||||
}
|
||||
if depth < 1 {
|
||||
return fmt.Errorf("hierarchy.max-depth must be at least 1, got %d", depth)
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -37,6 +37,10 @@ func TestIsYamlOnlyKey(t *testing.T) {
|
||||
{"daemon.auto_pull", true},
|
||||
{"daemon.custom_setting", true}, // prefix match
|
||||
|
||||
// Hierarchy settings (GH#995)
|
||||
{"hierarchy.max-depth", true},
|
||||
{"hierarchy.custom_setting", true}, // prefix match
|
||||
|
||||
// SQLite keys (should return false)
|
||||
{"jira.url", false},
|
||||
{"jira.project", false},
|
||||
@@ -164,10 +168,10 @@ func TestNormalizeYamlKey(t *testing.T) {
|
||||
input string
|
||||
expected string
|
||||
}{
|
||||
{"sync.branch", "sync-branch"}, // alias should be normalized
|
||||
{"sync-branch", "sync-branch"}, // already canonical
|
||||
{"no-db", "no-db"}, // no alias, unchanged
|
||||
{"json", "json"}, // no alias, unchanged
|
||||
{"sync.branch", "sync-branch"}, // alias should be normalized
|
||||
{"sync-branch", "sync-branch"}, // already canonical
|
||||
{"no-db", "no-db"}, // no alias, unchanged
|
||||
{"json", "json"}, // no alias, unchanged
|
||||
{"routing.mode", "routing.mode"}, // no alias for this one
|
||||
}
|
||||
|
||||
@@ -328,3 +332,53 @@ other-setting: value
|
||||
t.Errorf("config.yaml should preserve other settings, got:\n%s", contentStr)
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateYamlConfigValue_HierarchyMaxDepth tests validation of hierarchy.max-depth (GH#995)
|
||||
func TestValidateYamlConfigValue_HierarchyMaxDepth(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
value string
|
||||
expectErr bool
|
||||
errMsg string
|
||||
}{
|
||||
{"valid positive integer", "5", false, ""},
|
||||
{"valid minimum value", "1", false, ""},
|
||||
{"valid large value", "100", false, ""},
|
||||
{"invalid zero", "0", true, "hierarchy.max-depth must be at least 1, got 0"},
|
||||
{"invalid negative", "-1", true, "hierarchy.max-depth must be at least 1, got -1"},
|
||||
{"invalid non-integer", "abc", true, "hierarchy.max-depth must be a positive integer, got \"abc\""},
|
||||
{"invalid float", "3.5", true, "hierarchy.max-depth must be a positive integer, got \"3.5\""},
|
||||
{"invalid empty", "", true, "hierarchy.max-depth must be a positive integer, got \"\""},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
err := validateYamlConfigValue("hierarchy.max-depth", tt.value)
|
||||
if tt.expectErr {
|
||||
if err == nil {
|
||||
t.Errorf("expected error for value %q, got nil", tt.value)
|
||||
} else if err.Error() != tt.errMsg {
|
||||
t.Errorf("expected error %q, got %q", tt.errMsg, err.Error())
|
||||
}
|
||||
} else {
|
||||
if err != nil {
|
||||
t.Errorf("unexpected error for value %q: %v", tt.value, err)
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestValidateYamlConfigValue_OtherKeys tests that other keys are not validated
|
||||
func TestValidateYamlConfigValue_OtherKeys(t *testing.T) {
|
||||
// Other keys should pass validation regardless of value
|
||||
err := validateYamlConfigValue("no-db", "invalid")
|
||||
if err != nil {
|
||||
t.Errorf("unexpected error for no-db: %v", err)
|
||||
}
|
||||
|
||||
err = validateYamlConfigValue("routing.mode", "anything")
|
||||
if err != nil {
|
||||
t.Errorf("unexpected error for routing.mode: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user