Revert "fix: Address workflow template issues from code review"

This reverts commit aae8407aa4.
This commit is contained in:
Steve Yegge
2025-12-17 22:44:23 -08:00
parent aae8407aa4
commit 6716395853
5 changed files with 27 additions and 210 deletions

View File

@@ -554,12 +554,6 @@ 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")
@@ -646,9 +640,6 @@ 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 {
@@ -684,7 +675,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" && k != "parent" { if k != "add_labels" && k != "remove_labels" && k != "set_labels" {
regularUpdates[k] = v regularUpdates[k] = v
} }
} }
@@ -740,40 +731,6 @@ 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 {
@@ -1296,7 +1253,6 @@ 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" command: "grep -q 'bd-hooks-version: {{version}}' .git/hooks/pre-commit 2>/dev/null || echo 'Hooks may not be installed - verify manually'"
- id: final-verification - id: final-verification
title: "Final release verification" title: "Final release verification"

View File

@@ -1,7 +1,6 @@
package main package main
import ( import (
"context"
"embed" "embed"
"encoding/json" "encoding/json"
"fmt" "fmt"
@@ -15,9 +14,7 @@ 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"
) )
@@ -203,11 +200,6 @@ 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)
@@ -536,81 +528,36 @@ 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\nexpect_exit: N\nexpect_stdout: pattern\n``` // Format: ```verify\ncommand\n```
verify := extractVerification(task.Description) verifyCmd := extractVerifyCommand(task.Description)
if verify == nil || verify.Command == "" { if verifyCmd == "" {
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", dim(verify.Command)) fmt.Printf("Command: %s\n\n", dim(verifyCmd))
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, capturing stdout if we need to check it // Run the command
execCmd := exec.Command("sh", "-c", verify.Command) execCmd := exec.Command("sh", "-c", verifyCmd)
var stdout strings.Builder execCmd.Stdout = os.Stdout
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 {
actualExit = exitErr.ExitCode() fmt.Printf("%s Verification failed (exit code: %d)\n", red("✗"), 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))
}, },
@@ -796,8 +743,9 @@ func createWorkflowInstance(wf *types.WorkflowTemplate, vars map[string]string)
} }
// Add verification block to description if present // Add verification block to description if present
if verifyBlock := buildVerificationBlock(task.Verification, vars); verifyBlock != "" { if task.Verification != nil && task.Verification.Command != "" {
taskDesc += "\n\n" + verifyBlock verifyCmd := substituteVars(task.Verification.Command, vars)
taskDesc += "\n\n```verify\n" + verifyCmd + "\n```"
} }
taskLabels := []string{"workflow"} taskLabels := []string{"workflow"}
@@ -830,7 +778,6 @@ 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{
@@ -887,8 +834,9 @@ func createWorkflowInstance(wf *types.WorkflowTemplate, vars map[string]string)
} }
// Add verification block to description if present // Add verification block to description if present
if verifyBlock := buildVerificationBlock(task.Verification, vars); verifyBlock != "" { if task.Verification != nil && task.Verification.Command != "" {
taskDesc += "\n\n" + verifyBlock verifyCmd := substituteVars(task.Verification.Command, vars)
taskDesc += "\n\n```verify\n" + verifyCmd + "\n```"
} }
// Get next child ID // Get next child ID
@@ -935,7 +883,6 @@ 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{
@@ -955,71 +902,24 @@ func createWorkflowInstance(wf *types.WorkflowTemplate, vars map[string]string)
return instance, nil return instance, nil
} }
// parsedVerification holds verification data extracted from task description // extractVerifyCommand extracts a verify command from task description
type parsedVerification struct { // Looks for ```verify\ncommand\n``` block
Command string func extractVerifyCommand(description string) 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 nil return ""
} }
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 ""
} }
verifyCmd := substituteVars(v.Command, vars) return strings.TrimSpace(description[start : start+end])
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 context.Context, s storage.Storage, id string) (string, error) { func resolvePartialID(ctx interface{}, s interface{}, id string) (string, error) {
return utils.ResolvePartialID(ctx, s, id) // This is a simplified version - the real implementation is in utils package
// For now, just return the ID as-is if it looks complete
return id, nil
} }

View File

@@ -89,10 +89,9 @@ 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,46 +371,8 @@ 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 || parentChanged { if len(updates) > 0 || len(updateArgs.SetLabels) > 0 || len(updateArgs.AddLabels) > 0 || len(updateArgs.RemoveLabels) > 0 {
s.emitMutation(MutationUpdate, updateArgs.ID) s.emitMutation(MutationUpdate, updateArgs.ID)
} }