Add errors to Provider convert/fix and resolver

Propagate errors from provider commands and improve provider resolution flow.

- Extended loop.Provider ConvertCommand and FixJSONCommand to return an error and updated implementations (Claude, Codex) to follow the new signature. Codex now returns explicit errors when temp files cannot be created.
- Updated callers (internal/cmd/convert.go, runFixJSONWithProvider, loop tests, agent tests) to handle the new error return values.
- Changed Resolve to return (loop.Provider, error) and make unknown providers produce a clear error; added mustResolve helper in tests and a TestResolve_unknownProvider.
- Added resolveProvider helper in cmd/chief to load config and resolve the provider (exiting on error), and improved CLI flag parsing for --agent/--agent-path (support both --flag value and --flag=value). Minor test and docstring tweaks to reflect agent-agnostic language.

Author: Simon-BEE

cmd/chief/main.go

@@ -12,6 +12,7 @@ import (
 	"github.com/minicodemonkey/chief/internal/cmd"
 	"github.com/minicodemonkey/chief/internal/config"
 	"github.com/minicodemonkey/chief/internal/git"
+	"github.com/minicodemonkey/chief/internal/loop"
 	"github.com/minicodemonkey/chief/internal/prd"
 	"github.com/minicodemonkey/chief/internal/tui"
 )
@@ -227,37 +228,41 @@ func parseTUIFlags() *TUIOptions {
 
 func runNew() {
 	opts := cmd.NewOptions{}
-	// Parse arguments: chief new [name] [context...]
-	if len(os.Args) > 2 {
-		opts.Name = os.Args[2]
-	}
-	if len(os.Args) > 3 {
-		opts.Context = strings.Join(os.Args[3:], " ")
-	}
-	// Resolve provider (support --agent/--agent-path after "new")
-	cwd, _ := os.Getwd()
-	cfg, _ := config.Load(cwd)
 	flagAgent, flagPath := "", ""
+	var positional []string
+
+	// Parse arguments: chief new [name] [context...] [--agent X] [--agent-path X]
 	for i := 2; i < len(os.Args); i++ {
-		switch os.Args[i] {
-		case "--agent":
+		arg := os.Args[i]
+		switch {
+		case arg == "--agent":
 			if i+1 < len(os.Args) {
 				i++
 				flagAgent = os.Args[i]
 			}
-		case "--agent-path":
+		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]
 			}
+		case strings.HasPrefix(arg, "--agent-path="):
+			flagPath = strings.TrimPrefix(arg, "--agent-path=")
+		case strings.HasPrefix(arg, "-"):
+			// skip unknown flags
+		default:
+			positional = append(positional, arg)
 		}
 	}
-	opts.Provider = agent.Resolve(flagAgent, flagPath, cfg)
-	if err := agent.CheckInstalled(opts.Provider); err != nil {
-		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
-		os.Exit(1)
+	if len(positional) > 0 {
+		opts.Name = positional[0]
+	}
+	if len(positional) > 1 {
+		opts.Context = strings.Join(positional[1:], " ")
 	}
 
+	opts.Provider = resolveProvider(flagAgent, flagPath)
 	if err := cmd.RunNew(opts); err != nil {
 		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		os.Exit(1)
@@ -267,38 +272,37 @@ func runNew() {
 func runEdit() {
 	opts := cmd.EditOptions{}
 	flagAgent, flagPath := "", ""
-	// Parse arguments: chief edit [name] [--merge] [--force] [--agent] [--agent-path]
+
+	// Parse arguments: chief edit [name] [--merge] [--force] [--agent X] [--agent-path X]
 	for i := 2; i < len(os.Args); i++ {
 		arg := os.Args[i]
-		switch arg {
-		case "--merge":
+		switch {
+		case arg == "--merge":
 			opts.Merge = true
-		case "--force":
+		case arg == "--force":
 			opts.Force = true
-		case "--agent":
+		case arg == "--agent":
 			if i+1 < len(os.Args) {
 				i++
 				flagAgent = os.Args[i]
 			}
-		case "--agent-path":
+		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]
 			}
+		case strings.HasPrefix(arg, "--agent-path="):
+			flagPath = strings.TrimPrefix(arg, "--agent-path=")
 		default:
 			if opts.Name == "" && !strings.HasPrefix(arg, "-") {
 				opts.Name = arg
 			}
 		}
 	}
-	cwd, _ := os.Getwd()
-	cfg, _ := config.Load(cwd)
-	opts.Provider = agent.Resolve(flagAgent, flagPath, cfg)
-	if err := agent.CheckInstalled(opts.Provider); err != nil {
-		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
-		os.Exit(1)
-	}
 
+	opts.Provider = resolveProvider(flagAgent, flagPath)
 	if err := cmd.RunEdit(opts); err != nil {
 		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		os.Exit(1)
@@ -337,15 +341,28 @@ func runList() {
 	}
 }
 
-func runTUIWithOptions(opts *TUIOptions) {
-	// Resolve agent provider early (used for conversion, app, new, edit)
-	cwd, _ := os.Getwd()
+// resolveProvider loads config and resolves the agent provider, exiting on error.
+func resolveProvider(flagAgent, flagPath string) loop.Provider {
+	cwd, err := os.Getwd()
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+		os.Exit(1)
+	}
 	cfg, _ := config.Load(cwd)
-	provider := agent.Resolve(opts.Agent, opts.AgentPath, cfg)
+	provider, err := agent.Resolve(flagAgent, flagPath, cfg)
+	if err != nil {
+		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
+		os.Exit(1)
+	}
 	if err := agent.CheckInstalled(provider); err != nil {
 		fmt.Fprintf(os.Stderr, "Error: %v\n", err)
 		os.Exit(1)
 	}
+	return provider
+}
+
+func runTUIWithOptions(opts *TUIOptions) {
+	provider := resolveProvider(opts.Agent, opts.AgentPath)
 
 	prdPath := opts.PRDPath
 

internal/agent/claude.go

@@ -48,17 +48,17 @@ func (p *ClaudeProvider) InteractiveCommand(workDir, prompt string) *exec.Cmd {
 }
 
 // ConvertCommand implements loop.Provider.
-func (p *ClaudeProvider) ConvertCommand(workDir, prompt string) (*exec.Cmd, loop.OutputMode, string) {
+func (p *ClaudeProvider) ConvertCommand(workDir, prompt string) (*exec.Cmd, loop.OutputMode, string, error) {
 	cmd := exec.Command(p.cliPath, "-p", "--tools", "")
 	cmd.Dir = workDir
 	cmd.Stdin = strings.NewReader(prompt)
-	return cmd, loop.OutputStdout, ""
+	return cmd, loop.OutputStdout, "", nil
 }
 
 // FixJSONCommand implements loop.Provider.
-func (p *ClaudeProvider) FixJSONCommand(prompt string) (*exec.Cmd, loop.OutputMode, string) {
+func (p *ClaudeProvider) FixJSONCommand(prompt string) (*exec.Cmd, loop.OutputMode, string, error) {
 	cmd := exec.Command(p.cliPath, "-p", prompt)
-	return cmd, loop.OutputStdout, ""
+	return cmd, loop.OutputStdout, "", nil
 }
 
 // ParseLine implements loop.Provider.

internal/agent/codex.go

@@ -2,6 +2,7 @@ package agent
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"os/exec"
 	"strings"
@@ -45,36 +46,30 @@ func (p *CodexProvider) InteractiveCommand(workDir, prompt string) *exec.Cmd {
 }
 
 // ConvertCommand implements loop.Provider.
-func (p *CodexProvider) ConvertCommand(workDir, prompt string) (*exec.Cmd, loop.OutputMode, string) {
+func (p *CodexProvider) ConvertCommand(workDir, prompt string) (*exec.Cmd, loop.OutputMode, string, error) {
 	f, err := os.CreateTemp("", "chief-codex-convert-*.txt")
 	if err != nil {
-		// Caller will fail when running cmd; return empty path
-		cmd := exec.Command(p.cliPath, "exec", "--sandbox", "read-only", "--output-last-message", "-o", "", "-")
-		cmd.Dir = workDir
-		cmd.Stdin = strings.NewReader(prompt)
-		return cmd, loop.OutputFromFile, ""
+		return nil, 0, "", fmt.Errorf("failed to create temp file for conversion output: %w", err)
 	}
 	outPath := f.Name()
 	f.Close()
 	cmd := exec.Command(p.cliPath, "exec", "--sandbox", "read-only", "--output-last-message", "-o", outPath, "-")
 	cmd.Dir = workDir
 	cmd.Stdin = strings.NewReader(prompt)
-	return cmd, loop.OutputFromFile, outPath
+	return cmd, loop.OutputFromFile, outPath, nil
 }
 
 // FixJSONCommand implements loop.Provider.
-func (p *CodexProvider) FixJSONCommand(prompt string) (*exec.Cmd, loop.OutputMode, string) {
+func (p *CodexProvider) FixJSONCommand(prompt string) (*exec.Cmd, loop.OutputMode, string, error) {
 	f, err := os.CreateTemp("", "chief-codex-fixjson-*.txt")
 	if err != nil {
-		cmd := exec.Command(p.cliPath, "exec", "--sandbox", "read-only", "--output-last-message", "-o", "", "-")
-		cmd.Stdin = strings.NewReader(prompt)
-		return cmd, loop.OutputFromFile, ""
+		return nil, 0, "", fmt.Errorf("failed to create temp file for fix output: %w", err)
 	}
 	outPath := f.Name()
 	f.Close()
 	cmd := exec.Command(p.cliPath, "exec", "--sandbox", "read-only", "--output-last-message", "-o", outPath, "-")
 	cmd.Stdin = strings.NewReader(prompt)
-	return cmd, loop.OutputFromFile, outPath
+	return cmd, loop.OutputFromFile, outPath, nil
 }
 
 // ParseLine implements loop.Provider.

internal/agent/codex_test.go

@@ -62,7 +62,10 @@ func TestCodexProvider_LoopCommand(t *testing.T) {
 
 func TestCodexProvider_ConvertCommand(t *testing.T) {
 	p := NewCodexProvider("codex")
-	cmd, mode, outPath := p.ConvertCommand("/prd/dir", "convert prompt")
+	cmd, mode, outPath, err := p.ConvertCommand("/prd/dir", "convert prompt")
+	if err != nil {
+		t.Fatalf("ConvertCommand unexpected error: %v", err)
+	}
 	if mode != loop.OutputFromFile {
 		t.Errorf("ConvertCommand mode = %v, want OutputFromFile", mode)
 	}
@@ -72,7 +75,6 @@ func TestCodexProvider_ConvertCommand(t *testing.T) {
 	if !strings.Contains(cmd.Path, "codex") {
 		t.Errorf("ConvertCommand Path = %q", cmd.Path)
 	}
-	// Should have -o outPath
 	foundO := false
 	for i, a := range cmd.Args {
 		if a == "-o" && i+1 < len(cmd.Args) && cmd.Args[i+1] == outPath {
@@ -90,14 +92,16 @@ func TestCodexProvider_ConvertCommand(t *testing.T) {
 
 func TestCodexProvider_FixJSONCommand(t *testing.T) {
 	p := NewCodexProvider("codex")
-	cmd, mode, outPath := p.FixJSONCommand("fix prompt")
+	cmd, mode, outPath, err := p.FixJSONCommand("fix prompt")
+	if err != nil {
+		t.Fatalf("FixJSONCommand unexpected error: %v", err)
+	}
 	if mode != loop.OutputFromFile {
 		t.Errorf("FixJSONCommand mode = %v, want OutputFromFile", mode)
 	}
 	if outPath == "" {
 		t.Error("FixJSONCommand outPath should be non-empty temp file")
 	}
-	// -o outPath present
 	foundO := false
 	for i, a := range cmd.Args {
 		if a == "-o" && i+1 < len(cmd.Args) && cmd.Args[i+1] == outPath {

internal/agent/resolve.go

@@ -12,7 +12,8 @@ import (
 
 // Resolve returns the agent Provider using priority: flagAgent > CHIEF_AGENT env > config > "claude".
 // flagPath overrides the CLI path when non-empty (flag > CHIEF_AGENT_PATH > config agent.cliPath).
-func Resolve(flagAgent, flagPath string, cfg *config.Config) loop.Provider {
+// Returns an error if the resolved provider name is not recognised.
+func Resolve(flagAgent, flagPath string, cfg *config.Config) (loop.Provider, error) {
 	providerName := "claude"
 	if flagAgent != "" {
 		providerName = strings.ToLower(strings.TrimSpace(flagAgent))
@@ -32,10 +33,12 @@ func Resolve(flagAgent, flagPath string, cfg *config.Config) loop.Provider {
 	}
 
 	switch providerName {
+	case "claude":
+		return NewClaudeProvider(cliPath), nil
 	case "codex":
-		return NewCodexProvider(cliPath)
+		return NewCodexProvider(cliPath), nil
 	default:
-		return NewClaudeProvider(cliPath)
+		return nil, fmt.Errorf("unknown agent provider %q: expected \"claude\" or \"codex\"", providerName)
 	}
 }
 

internal/agent/resolve_test.go

@@ -8,11 +8,21 @@ import (
 	"testing"
 
 	"github.com/minicodemonkey/chief/internal/config"
+	"github.com/minicodemonkey/chief/internal/loop"
 )
 
+func mustResolve(t *testing.T, flagAgent, flagPath string, cfg *config.Config) loop.Provider {
+	t.Helper()
+	p, err := Resolve(flagAgent, flagPath, cfg)
+	if err != nil {
+		t.Fatalf("Resolve(%q, %q, cfg) unexpected error: %v", flagAgent, flagPath, err)
+	}
+	return p
+}
+
 func TestResolve_priority(t *testing.T) {
 	// Default: no flag, no env, nil config -> Claude
-	got := Resolve("", "", nil)
+	got := mustResolve(t, "", "", nil)
 	if got.Name() != "Claude" {
 		t.Errorf("Resolve(_, _, nil) name = %q, want Claude", got.Name())
 	}
@@ -21,7 +31,7 @@ func TestResolve_priority(t *testing.T) {
 	}
 
 	// Flag overrides everything
-	got = Resolve("codex", "", nil)
+	got = mustResolve(t, "codex", "", nil)
 	if got.Name() != "Codex" {
 		t.Errorf("Resolve(codex, _, nil) name = %q, want Codex", got.Name())
 	}
@@ -30,7 +40,7 @@ func TestResolve_priority(t *testing.T) {
 	cfg := &config.Config{}
 	cfg.Agent.Provider = "codex"
 	cfg.Agent.CLIPath = "/usr/local/bin/codex"
-	got = Resolve("", "", cfg)
+	got = mustResolve(t, "", "", cfg)
 	if got.Name() != "Codex" {
 		t.Errorf("Resolve(_, _, config codex) name = %q, want Codex", got.Name())
 	}
@@ -39,12 +49,12 @@ func TestResolve_priority(t *testing.T) {
 	}
 
 	// Flag overrides config
-	got = Resolve("claude", "", cfg)
+	got = mustResolve(t, "claude", "", cfg)
 	if got.Name() != "Claude" {
 		t.Errorf("Resolve(claude, _, config codex) name = %q, want Claude", got.Name())
 	}
 	// flag path overrides config path
-	got = Resolve("codex", "/opt/codex", cfg)
+	got = mustResolve(t, "codex", "/opt/codex", cfg)
 	if got.CLIPath() != "/opt/codex" {
 		t.Errorf("Resolve(codex, /opt/codex, cfg) CLIPath = %q, want /opt/codex", got.CLIPath())
 	}
@@ -73,7 +83,7 @@ func TestResolve_env(t *testing.T) {
 
 	// Env provider when no flag
 	os.Setenv(keyAgent, "codex")
-	got := Resolve("", "", nil)
+	got := mustResolve(t, "", "", nil)
 	if got.Name() != "Codex" {
 		t.Errorf("with CHIEF_AGENT=codex, name = %q, want Codex", got.Name())
 	}
@@ -82,7 +92,7 @@ func TestResolve_env(t *testing.T) {
 	// Env path when no flag path
 	os.Setenv(keyAgent, "codex")
 	os.Setenv(keyPath, "/env/codex")
-	got = Resolve("", "", nil)
+	got = mustResolve(t, "", "", nil)
 	if got.CLIPath() != "/env/codex" {
 		t.Errorf("with CHIEF_AGENT_PATH, CLIPath = %q, want /env/codex", got.CLIPath())
 	}
@@ -91,13 +101,22 @@ func TestResolve_env(t *testing.T) {
 }
 
 func TestResolve_normalize(t *testing.T) {
-	// Case and spaces
-	got := Resolve("  CODEX  ", "", nil)
+	got := mustResolve(t, "  CODEX  ", "", nil)
 	if got.Name() != "Codex" {
 		t.Errorf("Resolve('  CODEX  ') name = %q, want Codex", got.Name())
 	}
 }
 
+func TestResolve_unknownProvider(t *testing.T) {
+	_, err := Resolve("typo", "", nil)
+	if err == nil {
+		t.Fatal("Resolve(typo) expected error, got nil")
+	}
+	if !strings.Contains(err.Error(), "typo") {
+		t.Errorf("error should mention the bad provider name: %v", err)
+	}
+}
+
 func TestCheckInstalled_notFound(t *testing.T) {
 	// Use a path that does not exist
 	p := NewCodexProvider("/nonexistent/codex-binary-that-does-not-exist")
@@ -141,7 +160,7 @@ agent:
 	if err != nil {
 		t.Fatal(err)
 	}
-	got := Resolve("", "", cfg)
+	got := mustResolve(t, "", "", cfg)
 	if got.Name() != "Codex" || got.CLIPath() != "/usr/local/bin/codex" {
 		t.Errorf("Resolve from config: name=%q path=%q", got.Name(), got.CLIPath())
 	}