Add provider validation and flag error handling

Add runtime validation and better error handling across CLI and loop components. Key changes:

- cmd/chief: validate presence of values for --agent and --agent-path flags (print error and exit) and handle config.Load errors with a clear message.
- internal/cmd: enforce non-nil Provider for RunNew, RunEdit and runInteractiveAgent (return explicit errors) and add corresponding tests that assert provider is required. Minor comment/spacing cleanups in option structs.
- internal/cmd/convert: ensure temporary output files are removed on failed command start to avoid leftover artifacts.
- internal/loop: return an error if Loop.Run is invoked without a configured provider; require provider for Manager.Start; add tests for both failure cases. Also minor struct field alignment and test provider method formatting.

These changes make failures explicit and fail fast with clearer messages, and ensure temporary files are cleaned up on early command failures.

Author: Simon-BEE

cmd/chief/main.go

@@ -155,13 +155,19 @@ func parseTUIFlags() *TUIOptions {
 			if i+1 < len(os.Args) {
 				i++
 				opts.Agent = os.Args[i]
+			} else {
+				fmt.Fprintf(os.Stderr, "Error: --agent requires a value (claude or codex)\n")
+				os.Exit(1)
 			}
 		case strings.HasPrefix(arg, "--agent="):
 			opts.Agent = strings.TrimPrefix(arg, "--agent=")
 		case arg == "--agent-path":
 			if i+1 < len(os.Args) {
 				i++
 				opts.AgentPath = os.Args[i]
+			} else {
+				fmt.Fprintf(os.Stderr, "Error: --agent-path requires a value\n")
+				os.Exit(1)
 			}
 		case strings.HasPrefix(arg, "--agent-path="):
 			opts.AgentPath = strings.TrimPrefix(arg, "--agent-path=")
@@ -239,13 +245,19 @@ func runNew() {
 			if i+1 < len(os.Args) {
 				i++
 				flagAgent = os.Args[i]
+			} else {
+				fmt.Fprintf(os.Stderr, "Error: --agent requires a value (claude or codex)\n")
+				os.Exit(1)
 			}
 		case strings.HasPrefix(arg, "--agent="):
 			flagAgent = strings.TrimPrefix(arg, "--agent=")
 		case arg == "--agent-path":
 			if i+1 < len(os.Args) {
 				i++
 				flagPath = os.Args[i]
+			} else {
+				fmt.Fprintf(os.Stderr, "Error: --agent-path requires a value\n")
+				os.Exit(1)
 			}
 		case strings.HasPrefix(arg, "--agent-path="):
 			flagPath = strings.TrimPrefix(arg, "--agent-path=")
@@ -285,13 +297,19 @@ func runEdit() {
 			if i+1 < len(os.Args) {
 				i++
 				flagAgent = os.Args[i]
+			} else {
+				fmt.Fprintf(os.Stderr, "Error: --agent requires a value (claude or codex)\n")
+				os.Exit(1)
 			}
 		case strings.HasPrefix(arg, "--agent="):
 			flagAgent = strings.TrimPrefix(arg, "--agent=")
 		case arg == "--agent-path":
 			if i+1 < len(os.Args) {
 				i++
 				flagPath = os.Args[i]
+			} else {
+				fmt.Fprintf(os.Stderr, "Error: --agent-path requires a value\n")
+				os.Exit(1)
 			}
 		case strings.HasPrefix(arg, "--agent-path="):
 			flagPath = strings.TrimPrefix(arg, "--agent-path=")
@@ -348,7 +366,11 @@ func resolveProvider(flagAgent, flagPath string) loop.Provider {
 		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		os.Exit(1)
 	}
-	cfg, _ := config.Load(cwd)
+	cfg, err := config.Load(cwd)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Error: failed to load .chief/config.yaml: %v\n", err)
+		os.Exit(1)
+	}
 	provider, err := agent.Resolve(flagAgent, flagPath, cfg)
 	if err != nil {
 		fmt.Fprintf(os.Stderr, "Error: %v\n", err)

internal/cmd/convert.go

@@ -32,6 +32,9 @@ func runConversionWithProvider(provider loop.Provider, absPRDDir string) (string
 	cmd.Stderr = &stderr
 
 	if err := cmd.Start(); err != nil {
+		if outPath != "" {
+			_ = os.Remove(outPath)
+		}
 		return "", fmt.Errorf("failed to start %s: %w", provider.Name(), err)
 	}
 	if err := prd.WaitWithPanel(cmd, "Converting PRD", "Analyzing PRD...", &stderr); err != nil {
@@ -67,6 +70,9 @@ func runFixJSONWithProvider(provider loop.Provider, prompt string) (string, erro
 	cmd.Stderr = &stderr
 
 	if err := cmd.Start(); err != nil {
+		if outPath != "" {
+			_ = os.Remove(outPath)
+		}
 		return "", fmt.Errorf("failed to start %s: %w", provider.Name(), err)
 	}
 	if err := prd.WaitWithSpinner(cmd, "Fixing JSON", "Fixing prd.json...", &stderr); err != nil {

internal/cmd/edit.go

@@ -11,10 +11,10 @@ import (
 
 // EditOptions contains configuration for the edit command.
 type EditOptions struct {
-	Name     string       // PRD name (default: "main")
-	BaseDir  string       // Base directory for .chief/prds/ (default: current directory)
-	Merge    bool         // Auto-merge without prompting on conversion conflicts
-	Force    bool         // Auto-overwrite without prompting on conversion conflicts
+	Name     string        // PRD name (default: "main")
+	BaseDir  string        // Base directory for .chief/prds/ (default: current directory)
+	Merge    bool          // Auto-merge without prompting on conversion conflicts
+	Force    bool          // Auto-overwrite without prompting on conversion conflicts
 	Provider loop.Provider // Agent CLI provider (Claude or Codex)
 }
 
@@ -48,6 +48,9 @@ func RunEdit(opts EditOptions) error {
 
 	// Get the edit prompt with the PRD directory path
 	prompt := embed.GetEditPrompt(prdDir)
+	if opts.Provider == nil {
+		return fmt.Errorf("edit command requires Provider to be set")
+	}
 
 	// Launch interactive agent session
 	fmt.Printf("Editing PRD at %s...\n", prdDir)

internal/cmd/edit_test.go

@@ -120,6 +120,31 @@ func TestEditOptionsDefaults(t *testing.T) {
 	}
 }
 
+func TestRunEditRequiresProvider(t *testing.T) {
+	tmpDir := t.TempDir()
+	prdDir := filepath.Join(tmpDir, ".chief", "prds", "main")
+	if err := os.MkdirAll(prdDir, 0755); err != nil {
+		t.Fatalf("Failed to create directory: %v", err)
+	}
+	prdMdPath := filepath.Join(prdDir, "prd.md")
+	if err := os.WriteFile(prdMdPath, []byte("# Main PRD"), 0644); err != nil {
+		t.Fatalf("Failed to create prd.md: %v", err)
+	}
+
+	opts := EditOptions{
+		Name:    "main",
+		BaseDir: tmpDir,
+	}
+
+	err := RunEdit(opts)
+	if err == nil {
+		t.Fatal("expected provider validation error")
+	}
+	if !contains(err.Error(), "Provider") {
+		t.Fatalf("expected error to mention Provider, got: %v", err)
+	}
+}
+
 // Helper function to check if a string contains a substring
 func contains(s, substr string) bool {
 	return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsHelper(s, substr))

internal/cmd/new.go

@@ -15,9 +15,9 @@ import (
 
 // NewOptions contains configuration for the new command.
 type NewOptions struct {
-	Name     string       // PRD name (default: "main")
-	Context  string       // Optional context to pass to the agent
-	BaseDir  string       // Base directory for .chief/prds/ (default: current directory)
+	Name     string        // PRD name (default: "main")
+	Context  string        // Optional context to pass to the agent
+	BaseDir  string        // Base directory for .chief/prds/ (default: current directory)
 	Provider loop.Provider // Agent CLI provider (Claude or Codex)
 }
 
@@ -54,6 +54,9 @@ func RunNew(opts NewOptions) error {
 
 	// Get the init prompt with the PRD directory path
 	prompt := embed.GetInitPrompt(prdDir, opts.Context)
+	if opts.Provider == nil {
+		return fmt.Errorf("new command requires Provider to be set")
+	}
 
 	// Launch interactive agent session
 	fmt.Printf("Creating PRD in %s...\n", prdDir)
@@ -83,6 +86,9 @@ func RunNew(opts NewOptions) error {
 
 // runInteractiveAgent launches an interactive agent session in the specified directory.
 func runInteractiveAgent(provider loop.Provider, workDir, prompt string) error {
+	if provider == nil {
+		return fmt.Errorf("interactive agent requires Provider to be set")
+	}
 	cmd := provider.InteractiveCommand(workDir, prompt)
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
@@ -92,9 +98,9 @@ func runInteractiveAgent(provider loop.Provider, workDir, prompt string) error {
 
 // ConvertOptions contains configuration for the conversion command.
 type ConvertOptions struct {
-	PRDDir   string       // PRD directory containing prd.md
-	Merge    bool         // Auto-merge without prompting on conversion conflicts
-	Force    bool         // Auto-overwrite without prompting on conversion conflicts
+	PRDDir   string        // PRD directory containing prd.md
+	Merge    bool          // Auto-merge without prompting on conversion conflicts
+	Force    bool          // Auto-overwrite without prompting on conversion conflicts
 	Provider loop.Provider // Agent CLI provider for conversion
 }
 

internal/cmd/new_test.go

@@ -3,6 +3,7 @@ package cmd
 import (
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -103,3 +104,18 @@ func TestRunNewRejectsExistingPRD(t *testing.T) {
 		t.Error("Expected error for existing PRD")
 	}
 }
+
+func TestRunNewRequiresProvider(t *testing.T) {
+	opts := NewOptions{
+		Name:    "main",
+		BaseDir: t.TempDir(),
+	}
+
+	err := RunNew(opts)
+	if err == nil {
+		t.Fatal("expected provider validation error")
+	}
+	if !strings.Contains(err.Error(), "Provider") {
+		t.Fatalf("expected error to mention Provider, got: %v", err)
+	}
+}

internal/loop/loop.go

@@ -21,9 +21,9 @@ import (
 
 // RetryConfig configures automatic retry behavior on Claude crashes.
 type RetryConfig struct {
-	MaxRetries  int           // Maximum number of retry attempts (default: 3)
+	MaxRetries  int             // Maximum number of retry attempts (default: 3)
 	RetryDelays []time.Duration // Delays between retries (default: 0s, 5s, 15s)
-	Enabled     bool          // Whether retry is enabled (default: true)
+	Enabled     bool            // Whether retry is enabled (default: true)
 }
 
 // DefaultRetryConfig returns the default retry configuration.
@@ -99,6 +99,10 @@ func (l *Loop) Iteration() int {
 
 // Run executes the agent loop until completion or max iterations.
 func (l *Loop) Run(ctx context.Context) error {
+	if l.provider == nil {
+		return fmt.Errorf("loop provider is not configured")
+	}
+
 	// Open log file in PRD directory
 	prdDir := filepath.Dir(l.prdPath)
 	logPath := filepath.Join(prdDir, l.provider.LogFileName())

internal/loop/loop_test.go

@@ -17,11 +17,11 @@ type mockProvider struct {
 	cliPath string // if set, used as CLI path; otherwise "claude"
 }
 
-func (m *mockProvider) Name() string                              { return "Test" }
-func (m *mockProvider) CLIPath() string                            { return m.path() }
-func (m *mockProvider) InteractiveCommand(_, _ string) *exec.Cmd  { return exec.Command("true") }
-func (m *mockProvider) ParseLine(line string) *Event               { return ParseLine(line) }
-func (m *mockProvider) LogFileName() string                        { return "claude.log" }
+func (m *mockProvider) Name() string                             { return "Test" }
+func (m *mockProvider) CLIPath() string                          { return m.path() }
+func (m *mockProvider) InteractiveCommand(_, _ string) *exec.Cmd { return exec.Command("true") }
+func (m *mockProvider) ParseLine(line string) *Event             { return ParseLine(line) }
+func (m *mockProvider) LogFileName() string                      { return "claude.log" }
 
 func (m *mockProvider) ConvertCommand(_, _ string) (*exec.Cmd, OutputMode, string, error) {
 	return exec.Command("true"), OutputStdout, "", nil
@@ -439,3 +439,14 @@ func TestLoop_SetRetryConfig(t *testing.T) {
 		t.Errorf("Expected MaxRetries 5, got %d", l.retryConfig.MaxRetries)
 	}
 }
+
+func TestLoop_RunRequiresProvider(t *testing.T) {
+	l := NewLoop("/test/prd.json", "test", 1, nil)
+	err := l.Run(context.Background())
+	if err == nil {
+		t.Fatal("expected provider validation error")
+	}
+	if err.Error() != "loop provider is not configured" {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}

internal/loop/manager.go

@@ -67,16 +67,16 @@ type ManagerEvent struct {
 
 // Manager manages multiple Loop instances for parallel PRD execution.
 type Manager struct {
-	instances     map[string]*LoopInstance
-	events        chan ManagerEvent
-	maxIter       int
-	retryConfig  RetryConfig
-	provider      Provider
-	baseDir       string                               // Project root directory (for CLAUDE.md etc.)
-	config        *config.Config                      // Project config for post-completion actions
-	mu            sync.RWMutex
-	wg            sync.WaitGroup
-	onComplete    func(prdName string)                 // Callback when a PRD completes
+	instances      map[string]*LoopInstance
+	events         chan ManagerEvent
+	maxIter        int
+	retryConfig    RetryConfig
+	provider       Provider
+	baseDir        string         // Project root directory (for CLAUDE.md etc.)
+	config         *config.Config // Project config for post-completion actions
+	mu             sync.RWMutex
+	wg             sync.WaitGroup
+	onComplete     func(prdName string)                  // Callback when a PRD completes
 	onPostComplete func(prdName, branch, workDir string) // Callback for post-completion actions (push, PR)
 }
 
@@ -210,6 +210,10 @@ func (m *Manager) Unregister(name string) error {
 
 // Start starts the loop for a specific PRD.
 func (m *Manager) Start(name string) error {
+	if m.provider == nil {
+		return fmt.Errorf("manager provider is not configured")
+	}
+
 	m.mu.Lock()
 	instance, exists := m.instances[name]
 	m.mu.Unlock()

internal/loop/manager_test.go

@@ -222,6 +222,24 @@ func TestManagerStartNonExistent(t *testing.T) {
 	}
 }
 
+func TestManagerStartRequiresProvider(t *testing.T) {
+	tmpDir := t.TempDir()
+	prdPath := createTestPRDWithName(t, tmpDir, "test-prd")
+
+	m := NewManager(10, nil)
+	if err := m.Register("test-prd", prdPath); err != nil {
+		t.Fatalf("register failed: %v", err)
+	}
+
+	err := m.Start("test-prd")
+	if err == nil {
+		t.Fatal("expected provider validation error")
+	}
+	if err.Error() != "manager provider is not configured" {
+		t.Fatalf("unexpected error: %v", err)
+	}
+}
+
 func TestManagerConcurrentAccess(t *testing.T) {
 	tmpDir := t.TempDir()
 	prdPath := createTestPRDWithName(t, tmpDir, "test-prd")