refactor: reduce cyclomatic complexity in repair.go and config_values.go (#1214)

- repair.go: Extract validateRepairPaths(), findAllOrphans(), printOrphansText()
- config_values.go: Extract findConfigPath(), validateDurationConfig(), etc.
- Target: CC < 20 for each extracted function
This commit is contained in:
Bo
2026-01-21 22:50:03 -05:00
committed by GitHub
parent 9a9704b451
commit a0dac11e42
2 changed files with 220 additions and 226 deletions

View File

@@ -69,68 +69,126 @@ func CheckConfigValues(repoPath string) DoctorCheck {
}
}
// checkYAMLConfigValues validates values in config.yaml
func checkYAMLConfigValues(repoPath string) []string {
var issues []string
// Load config.yaml if it exists
v := viper.New()
v.SetConfigType("yaml")
// findConfigPath locates config.yaml in standard locations.
func findConfigPath(repoPath string) string {
configPath := filepath.Join(repoPath, ".beads", "config.yaml")
if _, err := os.Stat(configPath); os.IsNotExist(err) {
// No config.yaml, check user config dirs
configPath = ""
if configDir, err := os.UserConfigDir(); err == nil {
userConfigPath := filepath.Join(configDir, "bd", "config.yaml")
if _, err := os.Stat(userConfigPath); err == nil {
configPath = userConfigPath
if _, err := os.Stat(configPath); err == nil {
return configPath
}
if configDir, err := os.UserConfigDir(); err == nil {
userConfigPath := filepath.Join(configDir, "bd", "config.yaml")
if _, err := os.Stat(userConfigPath); err == nil {
return userConfigPath
}
}
if homeDir, err := os.UserHomeDir(); err == nil {
homeConfigPath := filepath.Join(homeDir, ".beads", "config.yaml")
if _, err := os.Stat(homeConfigPath); err == nil {
return homeConfigPath
}
}
return ""
}
// validateDurationConfig validates a duration config value.
func validateDurationConfig(v *viper.Viper, key string, minDuration time.Duration) []string {
if !v.IsSet(key) {
return nil
}
durStr := v.GetString(key)
if durStr == "" {
return nil
}
d, err := time.ParseDuration(durStr)
if err != nil {
return []string{fmt.Sprintf("%s: invalid duration %q (expected format like \"30s\", \"1m\", \"500ms\")", key, durStr)}
}
if minDuration > 0 && d > 0 && d < minDuration {
return []string{fmt.Sprintf("%s: %q is too low (minimum %s)", key, durStr, minDuration)}
}
return nil
}
// validateBooleanConfigs validates boolean config values.
func validateBooleanConfigs(v *viper.Viper, keys []string) []string {
var issues []string
for _, key := range keys {
if v.IsSet(key) {
strVal := v.GetString(key)
if strVal != "" && !isValidBoolString(strVal) {
issues = append(issues, fmt.Sprintf("%s: %q is not a valid boolean value (expected true/false, yes/no, 1/0, on/off)", key, strVal))
}
}
if configPath == "" {
if homeDir, err := os.UserHomeDir(); err == nil {
homeConfigPath := filepath.Join(homeDir, ".beads", "config.yaml")
if _, err := os.Stat(homeConfigPath); err == nil {
configPath = homeConfigPath
}
return issues
}
// validateRoutingPaths validates routing path config values.
func validateRoutingPaths(v *viper.Viper) []string {
var issues []string
for _, key := range []string{"routing.default", "routing.maintainer", "routing.contributor"} {
if v.IsSet(key) {
path := v.GetString(key)
if path != "" && path != "." {
expandedPath := expandPath(path)
if _, err := os.Stat(expandedPath); os.IsNotExist(err) {
issues = append(issues, fmt.Sprintf("%s: path %q does not exist", key, path))
}
}
}
}
return issues
}
// validateRepoPaths validates repos.primary and repos.additional paths.
func validateRepoPaths(v *viper.Viper) []string {
var issues []string
if v.IsSet("repos.primary") {
primary := v.GetString("repos.primary")
if primary != "" {
expandedPath := expandPath(primary)
if info, err := os.Stat(expandedPath); err == nil {
if !info.IsDir() {
issues = append(issues, fmt.Sprintf("repos.primary: %q is not a directory", primary))
}
} else if !os.IsNotExist(err) {
issues = append(issues, fmt.Sprintf("repos.primary: cannot access %q: %v", primary, err))
}
}
}
if v.IsSet("repos.additional") {
for _, path := range v.GetStringSlice("repos.additional") {
if path != "" {
expandedPath := expandPath(path)
if info, err := os.Stat(expandedPath); err == nil && !info.IsDir() {
issues = append(issues, fmt.Sprintf("repos.additional: %q is not a directory", path))
}
}
}
}
return issues
}
// checkYAMLConfigValues validates values in config.yaml
func checkYAMLConfigValues(repoPath string) []string {
var issues []string
configPath := findConfigPath(repoPath)
if configPath == "" {
// No config.yaml found anywhere
return issues
}
v := viper.New()
v.SetConfigType("yaml")
v.SetConfigFile(configPath)
if err := v.ReadInConfig(); err != nil {
issues = append(issues, fmt.Sprintf("config.yaml: failed to parse: %v", err))
return issues
}
// Validate flush-debounce (should be a valid duration)
if v.IsSet("flush-debounce") {
debounceStr := v.GetString("flush-debounce")
if debounceStr != "" {
_, err := time.ParseDuration(debounceStr)
if err != nil {
issues = append(issues, fmt.Sprintf("flush-debounce: invalid duration %q (expected format like \"30s\", \"1m\", \"500ms\")", debounceStr))
}
}
}
// Validate remote-sync-interval (should be a valid duration, min 5s)
if v.IsSet("remote-sync-interval") {
intervalStr := v.GetString("remote-sync-interval")
if intervalStr != "" {
d, err := time.ParseDuration(intervalStr)
if err != nil {
issues = append(issues, fmt.Sprintf("remote-sync-interval: invalid duration %q (expected format like \"30s\", \"1m\", \"5m\")", intervalStr))
} else if d > 0 && d < 5*time.Second {
issues = append(issues, fmt.Sprintf("remote-sync-interval: %q is too low (minimum 5s to prevent excessive load)", intervalStr))
}
}
}
// Validate duration configs
issues = append(issues, validateDurationConfig(v, "flush-debounce", 0)...)
issues = append(issues, validateDurationConfig(v, "remote-sync-interval", 5*time.Second)...)
// Validate issue-prefix (should be alphanumeric with dashes/underscores, reasonably short)
if v.IsSet("issue-prefix") {
@@ -168,23 +226,7 @@ func checkYAMLConfigValues(repoPath string) []string {
}
// Validate routing paths exist if set
for _, key := range []string{"routing.default", "routing.maintainer", "routing.contributor"} {
if v.IsSet(key) {
path := v.GetString(key)
if path != "" && path != "." {
// Expand ~ to home directory
if strings.HasPrefix(path, "~") {
if home, err := os.UserHomeDir(); err == nil {
path = filepath.Join(home, path[1:])
}
}
// Check if path exists (only warn, don't error - it might be created later)
if _, err := os.Stat(path); os.IsNotExist(err) {
issues = append(issues, fmt.Sprintf("%s: path %q does not exist", key, v.GetString(key)))
}
}
}
}
issues = append(issues, validateRoutingPaths(v)...)
// Validate actor (should be alphanumeric with common special chars if set)
if v.IsSet("actor") {
@@ -209,59 +251,12 @@ func checkYAMLConfigValues(repoPath string) []string {
}
}
// Validate boolean config values are actually booleans
for _, key := range []string{"json", "no-daemon", "no-auto-flush", "no-auto-import", "no-db", "auto-start-daemon"} {
if v.IsSet(key) {
// Try to get as string first to check if it's a valid boolean representation
strVal := v.GetString(key)
if strVal != "" {
// Valid boolean strings: true, false, 1, 0, yes, no, on, off (case insensitive)
if !isValidBoolString(strVal) {
issues = append(issues, fmt.Sprintf("%s: %q is not a valid boolean value (expected true/false, yes/no, 1/0, on/off)", key, strVal))
}
}
}
}
// Validate boolean config values
boolKeys := []string{"json", "no-daemon", "no-auto-flush", "no-auto-import", "no-db", "auto-start-daemon", "sync.require_confirmation_on_mass_delete"}
issues = append(issues, validateBooleanConfigs(v, boolKeys)...)
// Validate sync.require_confirmation_on_mass_delete (should be boolean)
if v.IsSet("sync.require_confirmation_on_mass_delete") {
strVal := v.GetString("sync.require_confirmation_on_mass_delete")
if strVal != "" && !isValidBoolString(strVal) {
issues = append(issues, fmt.Sprintf("sync.require_confirmation_on_mass_delete: %q is not a valid boolean value", strVal))
}
}
// Validate repos.primary (should be a directory path if set)
if v.IsSet("repos.primary") {
primary := v.GetString("repos.primary")
if primary != "" {
expandedPath := expandPath(primary)
if info, err := os.Stat(expandedPath); err == nil {
if !info.IsDir() {
issues = append(issues, fmt.Sprintf("repos.primary: %q is not a directory", primary))
}
} else if !os.IsNotExist(err) {
issues = append(issues, fmt.Sprintf("repos.primary: cannot access %q: %v", primary, err))
}
// Note: path not existing is OK - might be created later
}
}
// Validate repos.additional (should be directory paths if set)
if v.IsSet("repos.additional") {
additional := v.GetStringSlice("repos.additional")
for _, path := range additional {
if path != "" {
expandedPath := expandPath(path)
if info, err := os.Stat(expandedPath); err == nil {
if !info.IsDir() {
issues = append(issues, fmt.Sprintf("repos.additional: %q is not a directory", path))
}
}
// Note: path not existing is OK - might be created later
}
}
}
// Validate repos paths
issues = append(issues, validateRepoPaths(v)...)
return issues
}

View File

@@ -68,30 +68,106 @@ func outputJSONAndExit(result repairResult, exitCode int) {
os.Exit(exitCode)
}
func runRepair(cmd *cobra.Command, args []string) {
// Find .beads directory
// validateRepairPaths validates the beads directory and database exist.
func validateRepairPaths() (string, error) {
beadsDir := filepath.Join(repairPath, ".beads")
if _, err := os.Stat(beadsDir); os.IsNotExist(err) {
if repairJSON {
outputJSONAndExit(repairResult{
Status: "error",
Error: fmt.Sprintf(".beads directory not found at %s", beadsDir),
}, 1)
}
fmt.Fprintf(os.Stderr, "Error: .beads directory not found at %s\n", beadsDir)
os.Exit(1)
return "", fmt.Errorf(".beads directory not found at %s", beadsDir)
}
// Find database file
dbPath := filepath.Join(beadsDir, "beads.db")
if _, err := os.Stat(dbPath); os.IsNotExist(err) {
if repairJSON {
outputJSONAndExit(repairResult{
Status: "error",
Error: fmt.Sprintf("database not found at %s", dbPath),
}, 1)
return "", fmt.Errorf("database not found at %s", dbPath)
}
return dbPath, nil
}
// findAllOrphans finds all orphaned references in the database.
func findAllOrphans(db *sql.DB) (stats repairStats, orphans allOrphans, err error) {
orphans.depsIssueID, err = findOrphanedDepsIssueID(db)
if err != nil {
return stats, orphans, fmt.Errorf("checking orphaned deps (issue_id): %w", err)
}
stats.orphanedDepsIssueID = len(orphans.depsIssueID)
orphans.depsDependsOn, err = findOrphanedDepsDependsOn(db)
if err != nil {
return stats, orphans, fmt.Errorf("checking orphaned deps (depends_on_id): %w", err)
}
stats.orphanedDepsDependsOn = len(orphans.depsDependsOn)
orphans.labels, err = findOrphanedLabels(db)
if err != nil {
return stats, orphans, fmt.Errorf("checking orphaned labels: %w", err)
}
stats.orphanedLabels = len(orphans.labels)
orphans.comments, err = findOrphanedComments(db)
if err != nil {
return stats, orphans, fmt.Errorf("checking orphaned comments: %w", err)
}
stats.orphanedComments = len(orphans.comments)
orphans.events, err = findOrphanedEvents(db)
if err != nil {
return stats, orphans, fmt.Errorf("checking orphaned events: %w", err)
}
stats.orphanedEvents = len(orphans.events)
return stats, orphans, nil
}
// allOrphans holds all found orphaned references.
type allOrphans struct {
depsIssueID []orphanedDep
depsDependsOn []orphanedDep
labels []orphanedLabel
comments []orphanedComment
events []orphanedEvent
}
// printOrphansText prints orphan details in text mode.
func printOrphansText(stats repairStats, orphans allOrphans) {
fmt.Printf("Found %d orphaned reference(s):\n", stats.total())
if stats.orphanedDepsIssueID > 0 {
fmt.Printf(" • %d dependencies with missing issue_id\n", stats.orphanedDepsIssueID)
for _, dep := range orphans.depsIssueID {
fmt.Printf(" - %s → %s\n", dep.issueID, dep.dependsOnID)
}
fmt.Fprintf(os.Stderr, "Error: database not found at %s\n", dbPath)
}
if stats.orphanedDepsDependsOn > 0 {
fmt.Printf(" • %d dependencies with missing depends_on_id\n", stats.orphanedDepsDependsOn)
for _, dep := range orphans.depsDependsOn {
fmt.Printf(" - %s → %s\n", dep.issueID, dep.dependsOnID)
}
}
if stats.orphanedLabels > 0 {
fmt.Printf(" • %d labels with missing issue_id\n", stats.orphanedLabels)
for _, label := range orphans.labels {
fmt.Printf(" - %s: %s\n", label.issueID, label.label)
}
}
if stats.orphanedComments > 0 {
fmt.Printf(" • %d comments with missing issue_id\n", stats.orphanedComments)
for _, comment := range orphans.comments {
fmt.Printf(" - %s (by %s)\n", comment.issueID, comment.author)
}
}
if stats.orphanedEvents > 0 {
fmt.Printf(" • %d events with missing issue_id\n", stats.orphanedEvents)
for _, event := range orphans.events {
fmt.Printf(" - %s: %s\n", event.issueID, event.eventType)
}
}
fmt.Println()
}
func runRepair(cmd *cobra.Command, args []string) {
dbPath, err := validateRepairPaths()
if err != nil {
if repairJSON {
outputJSONAndExit(repairResult{Status: "error", Error: err.Error()}, 1)
}
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
@@ -103,72 +179,26 @@ func runRepair(cmd *cobra.Command, args []string) {
fmt.Println()
}
// Open database directly, bypassing beads storage layer
db, err := openRepairDB(dbPath)
if err != nil {
if repairJSON {
outputJSONAndExit(repairResult{
DatabasePath: dbPath,
Status: "error",
Error: fmt.Sprintf("opening database: %v", err),
}, 1)
outputJSONAndExit(repairResult{DatabasePath: dbPath, Status: "error", Error: fmt.Sprintf("opening database: %v", err)}, 1)
}
fmt.Fprintf(os.Stderr, "Error opening database: %v\n", err)
os.Exit(1)
}
defer db.Close()
// Collect repair statistics
stats := repairStats{}
// Helper for consistent error output
exitWithError := func(msg string, err error) {
// Find all orphaned references
stats, orphans, err := findAllOrphans(db)
if err != nil {
if repairJSON {
outputJSONAndExit(repairResult{
DatabasePath: dbPath,
Status: "error",
Error: fmt.Sprintf("%s: %v", msg, err),
}, 1)
outputJSONAndExit(repairResult{DatabasePath: dbPath, Status: "error", Error: err.Error()}, 1)
}
fmt.Fprintf(os.Stderr, "Error %s: %v\n", msg, err)
fmt.Fprintf(os.Stderr, "Error: %v\n", err)
os.Exit(1)
}
// 1. Find and clean orphaned dependencies (issue_id not in issues)
orphanedIssueID, err := findOrphanedDepsIssueID(db)
if err != nil {
exitWithError("checking orphaned deps (issue_id)", err)
}
stats.orphanedDepsIssueID = len(orphanedIssueID)
// 2. Find and clean orphaned dependencies (depends_on_id not in issues)
orphanedDependsOn, err := findOrphanedDepsDependsOn(db)
if err != nil {
exitWithError("checking orphaned deps (depends_on_id)", err)
}
stats.orphanedDepsDependsOn = len(orphanedDependsOn)
// 3. Find and clean orphaned labels
orphanedLabels, err := findOrphanedLabels(db)
if err != nil {
exitWithError("checking orphaned labels", err)
}
stats.orphanedLabels = len(orphanedLabels)
// 4. Find and clean orphaned comments
orphanedCommentsList, err := findOrphanedComments(db)
if err != nil {
exitWithError("checking orphaned comments", err)
}
stats.orphanedComments = len(orphanedCommentsList)
// 5. Find and clean orphaned events
orphanedEventsList, err := findOrphanedEvents(db)
if err != nil {
exitWithError("checking orphaned events", err)
}
stats.orphanedEvents = len(orphanedEventsList)
// Build JSON result structure (used for both JSON output and tracking)
jsonResult := repairResult{
DatabasePath: dbPath,
@@ -184,23 +214,23 @@ func runRepair(cmd *cobra.Command, args []string) {
}
// Build orphan details
for _, dep := range orphanedIssueID {
for _, dep := range orphans.depsIssueID {
jsonResult.OrphanDetails.DependenciesIssueID = append(jsonResult.OrphanDetails.DependenciesIssueID,
repairDepDetail{IssueID: dep.issueID, DependsOnID: dep.dependsOnID})
}
for _, dep := range orphanedDependsOn {
for _, dep := range orphans.depsDependsOn {
jsonResult.OrphanDetails.DependenciesDependsOn = append(jsonResult.OrphanDetails.DependenciesDependsOn,
repairDepDetail{IssueID: dep.issueID, DependsOnID: dep.dependsOnID})
}
for _, label := range orphanedLabels {
for _, label := range orphans.labels {
jsonResult.OrphanDetails.Labels = append(jsonResult.OrphanDetails.Labels,
repairLabelDetail{IssueID: label.issueID, Label: label.label})
}
for _, comment := range orphanedCommentsList {
for _, comment := range orphans.comments {
jsonResult.OrphanDetails.Comments = append(jsonResult.OrphanDetails.Comments,
repairCommentDetail{ID: comment.id, IssueID: comment.issueID, Author: comment.author})
}
for _, event := range orphanedEventsList {
for _, event := range orphans.events {
jsonResult.OrphanDetails.Events = append(jsonResult.OrphanDetails.Events,
repairEventDetail{ID: event.id, IssueID: event.issueID, EventType: event.eventType})
}
@@ -217,38 +247,7 @@ func runRepair(cmd *cobra.Command, args []string) {
// Print findings (text mode)
if !repairJSON {
fmt.Printf("Found %d orphaned reference(s):\n", stats.total())
if stats.orphanedDepsIssueID > 0 {
fmt.Printf(" • %d dependencies with missing issue_id\n", stats.orphanedDepsIssueID)
for _, dep := range orphanedIssueID {
fmt.Printf(" - %s → %s\n", dep.issueID, dep.dependsOnID)
}
}
if stats.orphanedDepsDependsOn > 0 {
fmt.Printf(" • %d dependencies with missing depends_on_id\n", stats.orphanedDepsDependsOn)
for _, dep := range orphanedDependsOn {
fmt.Printf(" - %s → %s\n", dep.issueID, dep.dependsOnID)
}
}
if stats.orphanedLabels > 0 {
fmt.Printf(" • %d labels with missing issue_id\n", stats.orphanedLabels)
for _, label := range orphanedLabels {
fmt.Printf(" - %s: %s\n", label.issueID, label.label)
}
}
if stats.orphanedComments > 0 {
fmt.Printf(" • %d comments with missing issue_id\n", stats.orphanedComments)
for _, comment := range orphanedCommentsList {
fmt.Printf(" - %s (by %s)\n", comment.issueID, comment.author)
}
}
if stats.orphanedEvents > 0 {
fmt.Printf(" • %d events with missing issue_id\n", stats.orphanedEvents)
for _, event := range orphanedEventsList {
fmt.Printf(" - %s: %s\n", event.issueID, event.eventType)
}
}
fmt.Println()
printOrphansText(stats, orphans)
}
// Handle dry-run
@@ -300,7 +299,7 @@ func runRepair(cmd *cobra.Command, args []string) {
var repairErr error
// Delete orphaned deps (issue_id) and mark affected issues dirty
if len(orphanedIssueID) > 0 && repairErr == nil {
if len(orphans.depsIssueID) > 0 && repairErr == nil {
// Note: orphanedIssueID contains deps where issue_id doesn't exist,
// so we can't mark them dirty (the issue is gone). But for depends_on orphans,
// the issue_id still exists and should be marked dirty.
@@ -317,9 +316,9 @@ func runRepair(cmd *cobra.Command, args []string) {
}
// Delete orphaned deps (depends_on_id) and mark parent issues dirty
if len(orphanedDependsOn) > 0 && repairErr == nil {
if len(orphans.depsDependsOn) > 0 && repairErr == nil {
// Mark parent issues as dirty for export
for _, dep := range orphanedDependsOn {
for _, dep := range orphans.depsDependsOn {
_, _ = tx.Exec("INSERT OR IGNORE INTO dirty_issues (issue_id) VALUES (?)", dep.issueID)
}
@@ -337,7 +336,7 @@ func runRepair(cmd *cobra.Command, args []string) {
}
// Delete orphaned labels
if len(orphanedLabels) > 0 && repairErr == nil {
if len(orphans.labels) > 0 && repairErr == nil {
// Labels reference non-existent issues, so no dirty marking needed
result, err := tx.Exec(`
DELETE FROM labels
@@ -352,7 +351,7 @@ func runRepair(cmd *cobra.Command, args []string) {
}
// Delete orphaned comments
if len(orphanedCommentsList) > 0 && repairErr == nil {
if len(orphans.comments) > 0 && repairErr == nil {
// Comments reference non-existent issues, so no dirty marking needed
result, err := tx.Exec(`
DELETE FROM comments
@@ -367,7 +366,7 @@ func runRepair(cmd *cobra.Command, args []string) {
}
// Delete orphaned events
if len(orphanedEventsList) > 0 && repairErr == nil {
if len(orphans.events) > 0 && repairErr == nil {
// Events reference non-existent issues, so no dirty marking needed
result, err := tx.Exec(`
DELETE FROM events