Refactor: extract duplicated validation and flag logic (bd-g5p7)

- Created internal/validation package for centralized validation logic
- Created cmd/bd/flags.go for shared flag registration
- Updated create and update commands to use shared logic
- Added support for 'P1' style priority to update command
- Added tests for validation logic

Amp-Thread-ID: https://ampcode.com/threads/T-c8d369a3-32f0-42a0-96d1-fd589e89bd6b
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
2025-11-20 19:11:27 -05:00
parent e9f5a0c35a
commit bbfedb060a
7 changed files with 102 additions and 59 deletions

View File

@@ -579,6 +579,17 @@ func TestCLI_PriorityFormats(t *testing.T) {
if issue["priority"].(float64) != 3 { if issue["priority"].(float64) != 3 {
t.Errorf("Expected priority 3, got: %v", issue["priority"]) t.Errorf("Expected priority 3, got: %v", issue["priority"])
} }
// Test update with P-format
id := issue["id"].(string)
runBDInProcess(t, tmpDir, "update", id, "-p", "P1")
out = runBDInProcess(t, tmpDir, "show", id, "--json")
var updated []map[string]interface{}
json.Unmarshal([]byte(out), &updated)
if updated[0]["priority"].(float64) != 1 {
t.Errorf("Expected priority 1 after update, got: %v", updated[0]["priority"])
}
} }
func TestCLI_Reopen(t *testing.T) { func TestCLI_Reopen(t *testing.T) {

View File

@@ -14,6 +14,7 @@ import (
"github.com/steveyegge/beads/internal/routing" "github.com/steveyegge/beads/internal/routing"
"github.com/steveyegge/beads/internal/rpc" "github.com/steveyegge/beads/internal/rpc"
"github.com/steveyegge/beads/internal/types" "github.com/steveyegge/beads/internal/types"
"github.com/steveyegge/beads/internal/validation"
) )
var createCmd = &cobra.Command{ var createCmd = &cobra.Command{
@@ -94,7 +95,7 @@ var createCmd = &cobra.Command{
// Parse priority (supports both "1" and "P1" formats) // Parse priority (supports both "1" and "P1" formats)
priorityStr, _ := cmd.Flags().GetString("priority") priorityStr, _ := cmd.Flags().GetString("priority")
priority := parsePriority(priorityStr) priority := validation.ParsePriority(priorityStr)
if priority == -1 { if priority == -1 {
fmt.Fprintf(os.Stderr, "Error: invalid priority %q (expected 0-4 or P0-P4)\n", priorityStr) fmt.Fprintf(os.Stderr, "Error: invalid priority %q (expected 0-4 or P0-P4)\n", priorityStr)
os.Exit(1) os.Exit(1)
@@ -394,18 +395,14 @@ func init() {
createCmd.Flags().StringP("file", "f", "", "Create multiple issues from markdown file") createCmd.Flags().StringP("file", "f", "", "Create multiple issues from markdown file")
createCmd.Flags().String("from-template", "", "Create issue from template (e.g., 'epic', 'bug', 'feature')") createCmd.Flags().String("from-template", "", "Create issue from template (e.g., 'epic', 'bug', 'feature')")
createCmd.Flags().String("title", "", "Issue title (alternative to positional argument)") createCmd.Flags().String("title", "", "Issue title (alternative to positional argument)")
createCmd.Flags().StringP("description", "d", "", "Issue description") registerPriorityFlag(createCmd, "2")
createCmd.Flags().String("design", "", "Design notes")
createCmd.Flags().String("acceptance", "", "Acceptance criteria")
createCmd.Flags().StringP("priority", "p", "2", "Priority (0-4 or P0-P4, 0=highest)")
createCmd.Flags().StringP("type", "t", "task", "Issue type (bug|feature|task|epic|chore)") createCmd.Flags().StringP("type", "t", "task", "Issue type (bug|feature|task|epic|chore)")
createCmd.Flags().StringP("assignee", "a", "", "Assignee") registerCommonIssueFlags(createCmd)
createCmd.Flags().StringSliceP("labels", "l", []string{}, "Labels (comma-separated)") createCmd.Flags().StringSliceP("labels", "l", []string{}, "Labels (comma-separated)")
createCmd.Flags().StringSlice("label", []string{}, "Alias for --labels") createCmd.Flags().StringSlice("label", []string{}, "Alias for --labels")
_ = createCmd.Flags().MarkHidden("label") _ = createCmd.Flags().MarkHidden("label")
createCmd.Flags().String("id", "", "Explicit issue ID (e.g., 'bd-42' for partitioning)") createCmd.Flags().String("id", "", "Explicit issue ID (e.g., 'bd-42' for partitioning)")
createCmd.Flags().String("parent", "", "Parent issue ID for hierarchical child (e.g., 'bd-a3f8e9')") createCmd.Flags().String("parent", "", "Parent issue ID for hierarchical child (e.g., 'bd-a3f8e9')")
createCmd.Flags().String("external-ref", "", "External reference (e.g., 'gh-9', 'jira-ABC')")
createCmd.Flags().StringSlice("deps", []string{}, "Dependencies in format 'type:id' or 'id' (e.g., 'discovered-from:bd-20,blocks:bd-15' or 'bd-20')") createCmd.Flags().StringSlice("deps", []string{}, "Dependencies in format 'type:id' or 'id' (e.g., 'discovered-from:bd-20,blocks:bd-15' or 'bd-20')")
createCmd.Flags().Bool("force", false, "Force creation even if prefix doesn't match database prefix") createCmd.Flags().Bool("force", false, "Force creation even if prefix doesn't match database prefix")
createCmd.Flags().String("repo", "", "Target repository for issue (overrides auto-routing)") createCmd.Flags().String("repo", "", "Target repository for issue (overrides auto-routing)")

17
cmd/bd/flags.go Normal file
View File

@@ -0,0 +1,17 @@
package main
import "github.com/spf13/cobra"
// registerCommonIssueFlags registers flags common to create and update commands.
func registerCommonIssueFlags(cmd *cobra.Command) {
cmd.Flags().StringP("assignee", "a", "", "Assignee")
cmd.Flags().StringP("description", "d", "", "Issue description")
cmd.Flags().String("design", "", "Design notes")
cmd.Flags().String("acceptance", "", "Acceptance criteria")
cmd.Flags().String("external-ref", "", "External reference (e.g., 'gh-9', 'jira-ABC')")
}
// registerPriorityFlag registers the priority flag with a specific default value.
func registerPriorityFlag(cmd *cobra.Command, defaultVal string) {
cmd.Flags().StringP("priority", "p", defaultVal, "Priority (0-4 or P0-P4, 0=highest)")
}

View File

@@ -14,6 +14,7 @@ import (
"github.com/fatih/color" "github.com/fatih/color"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"github.com/steveyegge/beads/internal/types" "github.com/steveyegge/beads/internal/types"
"github.com/steveyegge/beads/internal/validation"
) )
var ( var (
@@ -39,47 +40,7 @@ type IssueTemplate struct {
Dependencies []string Dependencies []string
} }
// parsePriority extracts and validates a priority value from content.
// Supports both numeric (0-4) and P-prefix format (P0-P4).
// Returns the parsed priority (0-4) or -1 if invalid.
func parsePriority(content string) int {
content = strings.TrimSpace(content)
// Handle "P1", "P0", etc. format
if strings.HasPrefix(strings.ToUpper(content), "P") {
content = content[1:] // Strip the "P" prefix
}
var p int
if _, err := fmt.Sscanf(content, "%d", &p); err == nil && p >= 0 && p <= 4 {
return p
}
return -1 // Invalid
}
// parseIssueType extracts and validates an issue type from content.
// Returns the validated type or empty string if invalid.
func parseIssueType(content, issueTitle string) types.IssueType {
issueType := types.IssueType(strings.TrimSpace(content))
// Validate issue type
validTypes := map[types.IssueType]bool{
types.TypeBug: true,
types.TypeFeature: true,
types.TypeTask: true,
types.TypeEpic: true,
types.TypeChore: true,
}
if !validTypes[issueType] {
// Warn but continue with default
fmt.Fprintf(os.Stderr, "Warning: invalid issue type '%s' in '%s', using default 'task'\n",
issueType, issueTitle)
return types.TypeTask
}
return issueType
}
// parseStringList extracts a list of strings from content, splitting by comma or whitespace. // parseStringList extracts a list of strings from content, splitting by comma or whitespace.
// This is a generic helper used by parseLabels and parseDependencies. // This is a generic helper used by parseLabels and parseDependencies.
@@ -116,11 +77,18 @@ func processIssueSection(issue *IssueTemplate, section, content string) {
switch strings.ToLower(section) { switch strings.ToLower(section) {
case "priority": case "priority":
if p := parsePriority(content); p != -1 { if p := validation.ParsePriority(content); p != -1 {
issue.Priority = p issue.Priority = p
} }
case "type": case "type":
issue.IssueType = parseIssueType(content, issue.Title) t, err := validation.ParseIssueType(content)
if err != nil {
fmt.Fprintf(os.Stderr, "Warning: invalid issue type '%s' in '%s', using default 'task'\n",
strings.TrimSpace(content), issue.Title)
issue.IssueType = types.TypeTask
} else {
issue.IssueType = t
}
case "description": case "description":
issue.Description = content issue.Description = content
case "design": case "design":

View File

@@ -13,6 +13,7 @@ import (
"github.com/steveyegge/beads/internal/rpc" "github.com/steveyegge/beads/internal/rpc"
"github.com/steveyegge/beads/internal/types" "github.com/steveyegge/beads/internal/types"
"github.com/steveyegge/beads/internal/utils" "github.com/steveyegge/beads/internal/utils"
"github.com/steveyegge/beads/internal/validation"
) )
var showCmd = &cobra.Command{ var showCmd = &cobra.Command{
@@ -343,7 +344,12 @@ var updateCmd = &cobra.Command{
updates["status"] = status updates["status"] = status
} }
if cmd.Flags().Changed("priority") { if cmd.Flags().Changed("priority") {
priority, _ := cmd.Flags().GetInt("priority") priorityStr, _ := cmd.Flags().GetString("priority")
priority := validation.ParsePriority(priorityStr)
if priority == -1 {
fmt.Fprintf(os.Stderr, "Error: invalid priority %q (expected 0-4 or P0-P4)\n", priorityStr)
os.Exit(1)
}
updates["priority"] = priority updates["priority"] = priority
} }
if cmd.Flags().Changed("title") { if cmd.Flags().Changed("title") {
@@ -802,16 +808,13 @@ func init() {
rootCmd.AddCommand(showCmd) rootCmd.AddCommand(showCmd)
updateCmd.Flags().StringP("status", "s", "", "New status") updateCmd.Flags().StringP("status", "s", "", "New status")
updateCmd.Flags().IntP("priority", "p", 0, "New priority") registerPriorityFlag(updateCmd, "")
updateCmd.Flags().String("title", "", "New title") updateCmd.Flags().String("title", "", "New title")
updateCmd.Flags().StringP("assignee", "a", "", "New assignee") registerCommonIssueFlags(updateCmd)
updateCmd.Flags().StringP("description", "d", "", "Issue description")
updateCmd.Flags().String("design", "", "Design notes")
updateCmd.Flags().String("notes", "", "Additional notes") updateCmd.Flags().String("notes", "", "Additional notes")
updateCmd.Flags().String("acceptance", "", "Acceptance criteria")
updateCmd.Flags().String("acceptance-criteria", "", "DEPRECATED: use --acceptance") updateCmd.Flags().String("acceptance-criteria", "", "DEPRECATED: use --acceptance")
_ = updateCmd.Flags().MarkHidden("acceptance-criteria") _ = updateCmd.Flags().MarkHidden("acceptance-criteria")
updateCmd.Flags().String("external-ref", "", "External reference (e.g., 'gh-9', 'jira-ABC')")
updateCmd.Flags().Bool("json", false, "Output JSON format") updateCmd.Flags().Bool("json", false, "Output JSON format")
rootCmd.AddCommand(updateCmd) rootCmd.AddCommand(updateCmd)

View File

@@ -0,0 +1,47 @@
package validation
import (
"fmt"
"strings"
"github.com/steveyegge/beads/internal/types"
)
// ParsePriority extracts and validates a priority value from content.
// Supports both numeric (0-4) and P-prefix format (P0-P4).
// Returns the parsed priority (0-4) or -1 if invalid.
func ParsePriority(content string) int {
content = strings.TrimSpace(content)
// Handle "P1", "P0", etc. format
if strings.HasPrefix(strings.ToUpper(content), "P") {
content = content[1:] // Strip the "P" prefix
}
var p int
if _, err := fmt.Sscanf(content, "%d", &p); err == nil && p >= 0 && p <= 4 {
return p
}
return -1 // Invalid
}
// ParseIssueType extracts and validates an issue type from content.
// Returns the validated type or error if invalid.
func ParseIssueType(content string) (types.IssueType, error) {
issueType := types.IssueType(strings.TrimSpace(content))
// Validate issue type
validTypes := map[types.IssueType]bool{
types.TypeBug: true,
types.TypeFeature: true,
types.TypeTask: true,
types.TypeEpic: true,
types.TypeChore: true,
}
if !validTypes[issueType] {
return types.TypeTask, fmt.Errorf("invalid issue type: %s", content)
}
return issueType, nil
}

View File

@@ -1,4 +1,4 @@
package main package validation
import ( import (
"testing" "testing"
@@ -43,9 +43,9 @@ func TestParsePriority(t *testing.T) {
for _, tt := range tests { for _, tt := range tests {
t.Run(tt.input, func(t *testing.T) { t.Run(tt.input, func(t *testing.T) {
got := parsePriority(tt.input) got := ParsePriority(tt.input)
if got != tt.expected { if got != tt.expected {
t.Errorf("parsePriority(%q) = %d, want %d", tt.input, got, tt.expected) t.Errorf("ParsePriority(%q) = %d, want %d", tt.input, got, tt.expected)
} }
}) })
} }