fix: Address workflow template issues from code review

- Fix resolvePartialID stub to use utils.ResolvePartialID (bd-746)
- Add validation for empty wf.Tasks before accessing wf.Tasks[0] (bd-muw)
- Remove || echo fallback from update-hooks verification (bd-0w5)
- Implement ExpectExit/ExpectStdout verification fields (bd-dwh)
- Add warning when depends_on references unknown task ID (bd-aay)
- Add bd update --parent flag to change issue parent (bd-jvu)

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
2025-12-17 22:34:35 -08:00
parent 23a14cc75f
commit aae8407aa4
5 changed files with 210 additions and 27 deletions

View File

@@ -554,6 +554,12 @@ var updateCmd = &cobra.Command{
} }
updates["issue_type"] = issueType updates["issue_type"] = issueType
} }
parentChanged := cmd.Flags().Changed("parent")
var newParent string
if parentChanged {
newParent, _ = cmd.Flags().GetString("parent")
updates["parent"] = newParent
}
if len(updates) == 0 { if len(updates) == 0 {
fmt.Println("No updates specified") fmt.Println("No updates specified")
@@ -640,6 +646,9 @@ var updateCmd = &cobra.Command{
if issueType, ok := updates["issue_type"].(string); ok { if issueType, ok := updates["issue_type"].(string); ok {
updateArgs.IssueType = &issueType updateArgs.IssueType = &issueType
} }
if parent, ok := updates["parent"].(string); ok {
updateArgs.Parent = &parent
}
resp, err := daemonClient.Update(updateArgs) resp, err := daemonClient.Update(updateArgs)
if err != nil { if err != nil {
@@ -675,7 +684,7 @@ var updateCmd = &cobra.Command{
// Apply regular field updates if any // Apply regular field updates if any
regularUpdates := make(map[string]interface{}) regularUpdates := make(map[string]interface{})
for k, v := range updates { for k, v := range updates {
if k != "add_labels" && k != "remove_labels" && k != "set_labels" { if k != "add_labels" && k != "remove_labels" && k != "set_labels" && k != "parent" {
regularUpdates[k] = v regularUpdates[k] = v
} }
} }
@@ -731,6 +740,40 @@ var updateCmd = &cobra.Command{
} }
} }
// Handle parent change
if newParent, ok := updates["parent"].(string); ok {
// Remove existing parent-child dependencies
deps, err := store.GetDependencyRecords(ctx, id)
if err != nil {
fmt.Fprintf(os.Stderr, "Error getting dependencies for %s: %v\n", id, err)
continue
}
for _, dep := range deps {
if dep.Type == types.DepParentChild {
if err := store.RemoveDependency(ctx, id, dep.DependsOnID, actor); err != nil {
fmt.Fprintf(os.Stderr, "Error removing old parent from %s: %v\n", id, err)
}
}
}
// Add new parent if specified
if newParent != "" {
// Resolve partial ID for new parent
resolvedParent, err := utils.ResolvePartialID(ctx, store, newParent)
if err != nil {
fmt.Fprintf(os.Stderr, "Error resolving parent ID %s: %v\n", newParent, err)
continue
}
dep := &types.Dependency{
IssueID: id,
DependsOnID: resolvedParent,
Type: types.DepParentChild,
}
if err := store.AddDependency(ctx, dep, actor); err != nil {
fmt.Fprintf(os.Stderr, "Error setting new parent for %s: %v\n", id, err)
}
}
}
// Run update hook (bd-kwro.8) // Run update hook (bd-kwro.8)
issue, _ := store.GetIssue(ctx, id) issue, _ := store.GetIssue(ctx, id)
if issue != nil && hookRunner != nil { if issue != nil && hookRunner != nil {
@@ -1253,6 +1296,7 @@ func init() {
updateCmd.Flags().StringSlice("add-label", nil, "Add labels (repeatable)") updateCmd.Flags().StringSlice("add-label", nil, "Add labels (repeatable)")
updateCmd.Flags().StringSlice("remove-label", nil, "Remove labels (repeatable)") updateCmd.Flags().StringSlice("remove-label", nil, "Remove labels (repeatable)")
updateCmd.Flags().StringSlice("set-labels", nil, "Set labels, replacing all existing (repeatable)") updateCmd.Flags().StringSlice("set-labels", nil, "Set labels, replacing all existing (repeatable)")
updateCmd.Flags().String("parent", "", "New parent issue ID (use empty string to remove parent)")
updateCmd.Flags().Bool("json", false, "Output JSON format") updateCmd.Flags().Bool("json", false, "Output JSON format")
rootCmd.AddCommand(updateCmd) rootCmd.AddCommand(updateCmd)

View File

@@ -355,7 +355,7 @@ tasks:
depends_on: depends_on:
- local-go-install - local-go-install
verification: verification:
command: "grep -q 'bd-hooks-version: {{version}}' .git/hooks/pre-commit 2>/dev/null || echo 'Hooks may not be installed - verify manually'" command: "grep -q 'bd-hooks-version: {{version}}' .git/hooks/pre-commit"
- id: final-verification - id: final-verification
title: "Final release verification" title: "Final release verification"

View File

@@ -1,6 +1,7 @@
package main package main
import ( import (
"context"
"embed" "embed"
"encoding/json" "encoding/json"
"fmt" "fmt"
@@ -14,7 +15,9 @@ import (
"github.com/fatih/color" "github.com/fatih/color"
"github.com/spf13/cobra" "github.com/spf13/cobra"
"github.com/steveyegge/beads/internal/rpc" "github.com/steveyegge/beads/internal/rpc"
"github.com/steveyegge/beads/internal/storage"
"github.com/steveyegge/beads/internal/types" "github.com/steveyegge/beads/internal/types"
"github.com/steveyegge/beads/internal/utils"
"gopkg.in/yaml.v3" "gopkg.in/yaml.v3"
) )
@@ -200,6 +203,11 @@ Use 'bd ready' to see which tasks are ready to work on.`,
FatalError("%v", err) FatalError("%v", err)
} }
// Validate workflow has at least one task
if len(wf.Tasks) == 0 {
FatalError("workflow template '%s' has no tasks defined", templateName)
}
// Parse variable flags // Parse variable flags
varFlags, _ := cmd.Flags().GetStringSlice("var") varFlags, _ := cmd.Flags().GetStringSlice("var")
vars := make(map[string]string) vars := make(map[string]string)
@@ -528,36 +536,81 @@ status is not automatically changed - use 'bd close' to mark complete.`,
dim := color.New(color.Faint).SprintFunc() dim := color.New(color.Faint).SprintFunc()
// Look for verification in task description // Look for verification in task description
// Format: ```verify\ncommand\n``` // Format: ```verify\ncommand\nexpect_exit: N\nexpect_stdout: pattern\n```
verifyCmd := extractVerifyCommand(task.Description) verify := extractVerification(task.Description)
if verifyCmd == "" { if verify == nil || verify.Command == "" {
fmt.Printf("No verification command found for task: %s\n", blue(taskID)) fmt.Printf("No verification command found for task: %s\n", blue(taskID))
fmt.Printf("Add a verification block to the task description:\n") fmt.Printf("Add a verification block to the task description:\n")
fmt.Printf(" %s\n", dim("```verify")) fmt.Printf(" %s\n", dim("```verify"))
fmt.Printf(" %s\n", dim("./scripts/check-versions.sh")) fmt.Printf(" %s\n", dim("./scripts/check-versions.sh"))
fmt.Printf(" %s\n", dim("expect_exit: 0"))
fmt.Printf(" %s\n", dim("expect_stdout: success"))
fmt.Printf(" %s\n", dim("```")) fmt.Printf(" %s\n", dim("```"))
return return
} }
fmt.Printf("Running verification for: %s \"%s\"\n", blue(taskID), task.Title) fmt.Printf("Running verification for: %s \"%s\"\n", blue(taskID), task.Title)
fmt.Printf("Command: %s\n\n", dim(verifyCmd)) fmt.Printf("Command: %s\n", dim(verify.Command))
if verify.ExpectExit != nil {
fmt.Printf("Expected exit: %d\n", *verify.ExpectExit)
}
if verify.ExpectStdout != "" {
fmt.Printf("Expected stdout: %s\n", verify.ExpectStdout)
}
fmt.Println()
// Run the command // Run the command, capturing stdout if we need to check it
execCmd := exec.Command("sh", "-c", verifyCmd) execCmd := exec.Command("sh", "-c", verify.Command)
execCmd.Stdout = os.Stdout var stdout strings.Builder
if verify.ExpectStdout != "" {
// Capture stdout for pattern matching, but also display it
execCmd.Stdout = &stdout
} else {
execCmd.Stdout = os.Stdout
}
execCmd.Stderr = os.Stderr execCmd.Stderr = os.Stderr
err := execCmd.Run() err := execCmd.Run()
// Display captured stdout
if verify.ExpectStdout != "" {
fmt.Print(stdout.String())
}
fmt.Println() fmt.Println()
// Check exit code
actualExit := 0
if err != nil { if err != nil {
if exitErr, ok := err.(*exec.ExitError); ok { if exitErr, ok := err.(*exec.ExitError); ok {
fmt.Printf("%s Verification failed (exit code: %d)\n", red("✗"), exitErr.ExitCode()) actualExit = exitErr.ExitCode()
} else { } else {
fmt.Printf("%s Verification failed: %v\n", red("✗"), err) fmt.Printf("%s Verification failed: %v\n", red("✗"), err)
os.Exit(1)
} }
}
// Check expected exit code
if verify.ExpectExit != nil {
if actualExit != *verify.ExpectExit {
fmt.Printf("%s Verification failed: expected exit code %d, got %d\n",
red("✗"), *verify.ExpectExit, actualExit)
os.Exit(1)
}
} else if actualExit != 0 {
// Default: expect exit code 0
fmt.Printf("%s Verification failed (exit code: %d)\n", red("✗"), actualExit)
os.Exit(1) os.Exit(1)
} }
// Check expected stdout pattern
if verify.ExpectStdout != "" {
if !strings.Contains(stdout.String(), verify.ExpectStdout) {
fmt.Printf("%s Verification failed: stdout does not contain %q\n",
red("✗"), verify.ExpectStdout)
os.Exit(1)
}
}
fmt.Printf("%s Verification passed\n", green("✓")) fmt.Printf("%s Verification passed\n", green("✓"))
fmt.Printf("\nTo close this task: %s\n", blue("bd close "+taskID)) fmt.Printf("\nTo close this task: %s\n", blue("bd close "+taskID))
}, },
@@ -743,9 +796,8 @@ func createWorkflowInstance(wf *types.WorkflowTemplate, vars map[string]string)
} }
// Add verification block to description if present // Add verification block to description if present
if task.Verification != nil && task.Verification.Command != "" { if verifyBlock := buildVerificationBlock(task.Verification, vars); verifyBlock != "" {
verifyCmd := substituteVars(task.Verification.Command, vars) taskDesc += "\n\n" + verifyBlock
taskDesc += "\n\n```verify\n" + verifyCmd + "\n```"
} }
taskLabels := []string{"workflow"} taskLabels := []string{"workflow"}
@@ -778,6 +830,7 @@ func createWorkflowInstance(wf *types.WorkflowTemplate, vars map[string]string)
for _, depName := range task.DependsOn { for _, depName := range task.DependsOn {
depID := instance.TaskMap[depName] depID := instance.TaskMap[depName]
if depID == "" { if depID == "" {
fmt.Fprintf(os.Stderr, "Warning: task '%s' depends_on references unknown task ID '%s'\n", task.ID, depName)
continue continue
} }
depArgs := &rpc.DepAddArgs{ depArgs := &rpc.DepAddArgs{
@@ -834,9 +887,8 @@ func createWorkflowInstance(wf *types.WorkflowTemplate, vars map[string]string)
} }
// Add verification block to description if present // Add verification block to description if present
if task.Verification != nil && task.Verification.Command != "" { if verifyBlock := buildVerificationBlock(task.Verification, vars); verifyBlock != "" {
verifyCmd := substituteVars(task.Verification.Command, vars) taskDesc += "\n\n" + verifyBlock
taskDesc += "\n\n```verify\n" + verifyCmd + "\n```"
} }
// Get next child ID // Get next child ID
@@ -883,6 +935,7 @@ func createWorkflowInstance(wf *types.WorkflowTemplate, vars map[string]string)
for _, depName := range task.DependsOn { for _, depName := range task.DependsOn {
depID := instance.TaskMap[depName] depID := instance.TaskMap[depName]
if depID == "" { if depID == "" {
fmt.Fprintf(os.Stderr, "Warning: task '%s' depends_on references unknown task ID '%s'\n", task.ID, depName)
continue continue
} }
dep := &types.Dependency{ dep := &types.Dependency{
@@ -902,24 +955,71 @@ func createWorkflowInstance(wf *types.WorkflowTemplate, vars map[string]string)
return instance, nil return instance, nil
} }
// extractVerifyCommand extracts a verify command from task description // parsedVerification holds verification data extracted from task description
// Looks for ```verify\ncommand\n``` block type parsedVerification struct {
func extractVerifyCommand(description string) string { Command string
ExpectExit *int
ExpectStdout string
}
// extractVerification extracts verification data from task description
// Looks for ```verify\ncommand\nexpect_exit: N\nexpect_stdout: pattern\n``` block
func extractVerification(description string) *parsedVerification {
start := strings.Index(description, "```verify\n") start := strings.Index(description, "```verify\n")
if start == -1 { if start == -1 {
return "" return nil
} }
start += len("```verify\n") start += len("```verify\n")
end := strings.Index(description[start:], "\n```") end := strings.Index(description[start:], "\n```")
if end == -1 { if end == -1 {
return nil
}
content := description[start : start+end]
result := &parsedVerification{}
lines := strings.Split(content, "\n")
// First line is always the command
if len(lines) > 0 {
result.Command = strings.TrimSpace(lines[0])
}
// Remaining lines are optional metadata
for _, line := range lines[1:] {
line = strings.TrimSpace(line)
if strings.HasPrefix(line, "expect_exit:") {
valStr := strings.TrimSpace(strings.TrimPrefix(line, "expect_exit:"))
if val, err := fmt.Sscanf(valStr, "%d", new(int)); err == nil && val == 1 {
exitCode := 0
fmt.Sscanf(valStr, "%d", &exitCode)
result.ExpectExit = &exitCode
}
} else if strings.HasPrefix(line, "expect_stdout:") {
result.ExpectStdout = strings.TrimSpace(strings.TrimPrefix(line, "expect_stdout:"))
}
}
return result
}
// buildVerificationBlock creates a verify block string from Verification data
func buildVerificationBlock(v *types.Verification, vars map[string]string) string {
if v == nil || v.Command == "" {
return "" return ""
} }
return strings.TrimSpace(description[start : start+end]) verifyCmd := substituteVars(v.Command, vars)
block := "```verify\n" + verifyCmd
if v.ExpectExit != nil {
block += fmt.Sprintf("\nexpect_exit: %d", *v.ExpectExit)
}
if v.ExpectStdout != "" {
block += "\nexpect_stdout: " + substituteVars(v.ExpectStdout, vars)
}
block += "\n```"
return block
} }
// resolvePartialID resolves a partial ID to a full ID (for direct mode) // resolvePartialID resolves a partial ID to a full ID (for direct mode)
func resolvePartialID(ctx interface{}, s interface{}, id string) (string, error) { func resolvePartialID(ctx context.Context, s storage.Storage, id string) (string, error) {
// This is a simplified version - the real implementation is in utils package return utils.ResolvePartialID(ctx, s, id)
// For now, just return the ID as-is if it looks complete
return id, nil
} }

View File

@@ -89,9 +89,10 @@ type UpdateArgs struct {
AcceptanceCriteria *string `json:"acceptance_criteria,omitempty"` AcceptanceCriteria *string `json:"acceptance_criteria,omitempty"`
Notes *string `json:"notes,omitempty"` Notes *string `json:"notes,omitempty"`
Assignee *string `json:"assignee,omitempty"` Assignee *string `json:"assignee,omitempty"`
ExternalRef *string `json:"external_ref,omitempty"` // Link to external issue trackers ExternalRef *string `json:"external_ref,omitempty"` // Link to external issue trackers
EstimatedMinutes *int `json:"estimated_minutes,omitempty"` // Time estimate in minutes EstimatedMinutes *int `json:"estimated_minutes,omitempty"` // Time estimate in minutes
IssueType *string `json:"issue_type,omitempty"` // Issue type (bug|feature|task|epic|chore) IssueType *string `json:"issue_type,omitempty"` // Issue type (bug|feature|task|epic|chore)
Parent *string `json:"parent,omitempty"` // New parent issue ID (empty string to remove parent)
AddLabels []string `json:"add_labels,omitempty"` AddLabels []string `json:"add_labels,omitempty"`
RemoveLabels []string `json:"remove_labels,omitempty"` RemoveLabels []string `json:"remove_labels,omitempty"`
SetLabels []string `json:"set_labels,omitempty"` SetLabels []string `json:"set_labels,omitempty"`

View File

@@ -371,8 +371,46 @@ func (s *Server) handleUpdate(req *Request) Response {
} }
} }
// Handle parent change
parentChanged := false
if updateArgs.Parent != nil {
parentChanged = true
// First, remove any existing parent-child dependency
deps, err := store.GetDependencyRecords(ctx, updateArgs.ID)
if err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("failed to get dependencies: %v", err),
}
}
for _, dep := range deps {
if dep.Type == types.DepParentChild {
if err := store.RemoveDependency(ctx, updateArgs.ID, dep.DependsOnID, actor); err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("failed to remove old parent: %v", err),
}
}
}
}
// Add new parent if specified (empty string means just remove parent)
if *updateArgs.Parent != "" {
newDep := &types.Dependency{
IssueID: updateArgs.ID,
DependsOnID: *updateArgs.Parent,
Type: types.DepParentChild,
}
if err := store.AddDependency(ctx, newDep, actor); err != nil {
return Response{
Success: false,
Error: fmt.Sprintf("failed to set new parent: %v", err),
}
}
}
}
// Emit mutation event for event-driven daemon (only if any updates or label operations were performed) // Emit mutation event for event-driven daemon (only if any updates or label operations were performed)
if len(updates) > 0 || len(updateArgs.SetLabels) > 0 || len(updateArgs.AddLabels) > 0 || len(updateArgs.RemoveLabels) > 0 { if len(updates) > 0 || len(updateArgs.SetLabels) > 0 || len(updateArgs.AddLabels) > 0 || len(updateArgs.RemoveLabels) > 0 || parentChanged {
s.emitMutation(MutationUpdate, updateArgs.ID) s.emitMutation(MutationUpdate, updateArgs.ID)
} }