refactor(agent): deduplicate flag parsing and convert helpers, improve test coverage

- Extract parseAgentFlags() in main.go to eliminate triple-duplicated
  --agent/--agent-path parsing across parseTUIFlags, runNew, and runEdit
- Extract runAgentCommand() in convert.go to share stdout/file output
  handling between runConversionWithProvider and runFixJSONWithProvider
- Improve OpenCode CleanOutput to use JSON parsing instead of fragile
  string matching heuristics
- Add Claude provider tests (claude_test.go) covering all Provider methods
- Add OpenCode CleanOutput tests for NDJSON extraction and edge cases
- Add OpenCode parser tests for tool_use start/no-state/completed events
- Update troubleshooting docs to clarify provider-specific log file names

Author: MiniCodeMonkey

cmd/chief/main.go

@@ -122,6 +122,40 @@ func listAvailablePRDs() []string {
 	return names
 }
 
+// parseAgentFlags extracts --agent and --agent-path from args[startIdx:],
+// returning the agent name, agent path, remaining args (with agent flags removed),
+// and the updated index offsets. It exits on missing values.
+func parseAgentFlags(args []string, startIdx int) (agentName, agentPath string, remaining []string) {
+	for i := startIdx; i < len(args); i++ {
+		arg := args[i]
+		switch {
+		case arg == "--agent":
+			if i+1 < len(args) {
+				i++
+				agentName = args[i]
+			} else {
+				fmt.Fprintf(os.Stderr, "Error: --agent requires a value (claude, codex, or opencode)\n")
+				os.Exit(1)
+			}
+		case strings.HasPrefix(arg, "--agent="):
+			agentName = strings.TrimPrefix(arg, "--agent=")
+		case arg == "--agent-path":
+			if i+1 < len(args) {
+				i++
+				agentPath = args[i]
+			} else {
+				fmt.Fprintf(os.Stderr, "Error: --agent-path requires a value\n")
+				os.Exit(1)
+			}
+		case strings.HasPrefix(arg, "--agent-path="):
+			agentPath = strings.TrimPrefix(arg, "--agent-path=")
+		default:
+			remaining = append(remaining, arg)
+		}
+	}
+	return
+}
+
 // parseTUIFlags parses command-line flags for TUI mode
 func parseTUIFlags() *TUIOptions {
 	opts := &TUIOptions{
@@ -133,6 +167,9 @@ func parseTUIFlags() *TUIOptions {
 		NoRetry:       false,
 	}
 
+	// Pre-extract agent flags so they don't interfere with positional arg parsing
+	opts.Agent, opts.AgentPath, _ = parseAgentFlags(os.Args, 1)
+
 	for i := 1; i < len(os.Args); i++ {
 		arg := os.Args[i]
 
@@ -151,26 +188,10 @@ func parseTUIFlags() *TUIOptions {
 			opts.Force = true
 		case arg == "--no-retry":
 			opts.NoRetry = true
-		case arg == "--agent":
-			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=")
+		case arg == "--agent" || arg == "--agent-path":
+			i++ // skip value (already parsed by parseAgentFlags)
+		case strings.HasPrefix(arg, "--agent=") || strings.HasPrefix(arg, "--agent-path="):
+			// already parsed by parseAgentFlags
 		case arg == "--max-iterations" || arg == "-n":
 			// Next argument should be the number
 			if i+1 < len(os.Args) {
@@ -234,44 +255,21 @@ func parseTUIFlags() *TUIOptions {
 
 func runNew() {
 	opts := cmd.NewOptions{}
-	flagAgent, flagPath := "", ""
-	var positional []string
 
 	// Parse arguments: chief new [name] [context...] [--agent X] [--agent-path X]
-	for i := 2; i < len(os.Args); i++ {
-		arg := os.Args[i]
-		switch {
-		case arg == "--agent":
-			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=")
-		case strings.HasPrefix(arg, "-"):
-			// skip unknown flags
-		default:
-			positional = append(positional, arg)
+	flagAgent, flagPath, positional := parseAgentFlags(os.Args, 2)
+	// Filter out remaining flags, keep only positional args
+	var args []string
+	for _, a := range positional {
+		if !strings.HasPrefix(a, "-") {
+			args = append(args, a)
 		}
 	}
-	if len(positional) > 0 {
-		opts.Name = positional[0]
+	if len(args) > 0 {
+		opts.Name = args[0]
 	}
-	if len(positional) > 1 {
-		opts.Context = strings.Join(positional[1:], " ")
+	if len(args) > 1 {
+		opts.Context = strings.Join(args[1:], " ")
 	}
 
 	opts.Provider = resolveProvider(flagAgent, flagPath)
@@ -283,36 +281,15 @@ func runNew() {
 
 func runEdit() {
 	opts := cmd.EditOptions{}
-	flagAgent, flagPath := "", ""
 
 	// Parse arguments: chief edit [name] [--merge] [--force] [--agent X] [--agent-path X]
-	for i := 2; i < len(os.Args); i++ {
-		arg := os.Args[i]
+	flagAgent, flagPath, remaining := parseAgentFlags(os.Args, 2)
+	for _, arg := range remaining {
 		switch {
 		case arg == "--merge":
 			opts.Merge = true
 		case arg == "--force":
 			opts.Force = true
-		case arg == "--agent":
-			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=")
 		default:
 			if opts.Name == "" && !strings.HasPrefix(arg, "-") {
 				opts.Name = arg

docs/troubleshooting/common-issues.md

@@ -57,15 +57,15 @@ Chief automatically runs Claude with permission prompts disabled for autonomous
 
 ## PRD Not Updating
 
-**Symptom:** Stories stay incomplete even though Claude seems to finish.
+**Symptom:** Stories stay incomplete even though the agent seems to finish.
 
-**Cause:** Claude didn't output the completion signal, or file watching failed.
+**Cause:** The agent didn't output the completion signal, or file watching failed.
 
 **Solution:**
 
-1. Check the agent log for errors (e.g. `claude.log`, `codex.log`, or `opencode.log` in the PRD directory):
+1. Check the agent log for errors (the log file matches your agent: `claude.log`, `codex.log`, or `opencode.log`):
    ```bash
-   tail -100 .chief/prds/your-prd/claude.log
+   tail -100 .chief/prds/your-prd/claude.log  # or codex.log / opencode.log
    ```
 
 2. Manually mark story complete if appropriate:
@@ -83,13 +83,13 @@ Chief automatically runs Claude with permission prompts disabled for autonomous
 
 **Symptom:** Chief runs but doesn't make progress on stories.
 
-**Cause:** Various—Claude may be stuck, context too large, or PRD unclear.
+**Cause:** Various—the agent may be stuck, context too large, or PRD unclear.
 
 **Solution:**
 
-1. Check the agent log (e.g. `claude.log`, `codex.log`, or `opencode.log`) for what the agent is doing:
+1. Check the agent log for what the agent is doing:
    ```bash
-   tail -f .chief/prds/your-prd/claude.log
+   tail -f .chief/prds/your-prd/claude.log  # or codex.log / opencode.log
    ```
 
 2. Simplify the current story's acceptance criteria

internal/agent/claude_test.go

@@ -0,0 +1,136 @@
+package agent
+
+import (
+	"context"
+	"testing"
+
+	"github.com/minicodemonkey/chief/internal/loop"
+)
+
+func TestClaudeProvider_Name(t *testing.T) {
+	p := NewClaudeProvider("")
+	if p.Name() != "Claude" {
+		t.Errorf("Name() = %q, want Claude", p.Name())
+	}
+}
+
+func TestClaudeProvider_CLIPath(t *testing.T) {
+	p := NewClaudeProvider("")
+	if p.CLIPath() != "claude" {
+		t.Errorf("CLIPath() empty arg = %q, want claude", p.CLIPath())
+	}
+	p2 := NewClaudeProvider("/usr/local/bin/claude")
+	if p2.CLIPath() != "/usr/local/bin/claude" {
+		t.Errorf("CLIPath() custom = %q, want /usr/local/bin/claude", p2.CLIPath())
+	}
+}
+
+func TestClaudeProvider_LogFileName(t *testing.T) {
+	p := NewClaudeProvider("")
+	if p.LogFileName() != "claude.log" {
+		t.Errorf("LogFileName() = %q, want claude.log", p.LogFileName())
+	}
+}
+
+func TestClaudeProvider_LoopCommand(t *testing.T) {
+	ctx := context.Background()
+	p := NewClaudeProvider("/bin/claude")
+	cmd := p.LoopCommand(ctx, "hello world", "/work/dir")
+
+	if cmd.Path != "/bin/claude" {
+		t.Errorf("LoopCommand Path = %q, want /bin/claude", cmd.Path)
+	}
+	wantArgs := []string{"/bin/claude", "--dangerously-skip-permissions", "-p", "hello world", "--output-format", "stream-json", "--verbose"}
+	if len(cmd.Args) != len(wantArgs) {
+		t.Fatalf("LoopCommand Args len = %d, want %d: %v", len(cmd.Args), len(wantArgs), cmd.Args)
+	}
+	for i, w := range wantArgs {
+		if cmd.Args[i] != w {
+			t.Errorf("LoopCommand Args[%d] = %q, want %q", i, cmd.Args[i], w)
+		}
+	}
+	if cmd.Dir != "/work/dir" {
+		t.Errorf("LoopCommand Dir = %q, want /work/dir", cmd.Dir)
+	}
+}
+
+func TestClaudeProvider_ConvertCommand(t *testing.T) {
+	p := NewClaudeProvider("/bin/claude")
+	cmd, mode, outPath, err := p.ConvertCommand("/prd/dir", "convert prompt")
+	if err != nil {
+		t.Fatalf("ConvertCommand unexpected error: %v", err)
+	}
+	if mode != loop.OutputStdout {
+		t.Errorf("ConvertCommand mode = %v, want OutputStdout", mode)
+	}
+	if outPath != "" {
+		t.Errorf("ConvertCommand outPath = %q, want empty string", outPath)
+	}
+	if cmd.Dir != "/prd/dir" {
+		t.Errorf("ConvertCommand Dir = %q, want /prd/dir", cmd.Dir)
+	}
+	// Should use -p flag with stdin
+	wantArgs := []string{"/bin/claude", "-p"}
+	if len(cmd.Args) != len(wantArgs) {
+		t.Fatalf("ConvertCommand Args = %v, want %v", cmd.Args, wantArgs)
+	}
+	for i, w := range wantArgs {
+		if cmd.Args[i] != w {
+			t.Errorf("ConvertCommand Args[%d] = %q, want %q", i, cmd.Args[i], w)
+		}
+	}
+	if cmd.Stdin == nil {
+		t.Error("ConvertCommand Stdin must be set (prompt via stdin)")
+	}
+}
+
+func TestClaudeProvider_FixJSONCommand(t *testing.T) {
+	p := NewClaudeProvider("/bin/claude")
+	cmd, mode, outPath, err := p.FixJSONCommand("fix prompt")
+	if err != nil {
+		t.Fatalf("FixJSONCommand unexpected error: %v", err)
+	}
+	if mode != loop.OutputStdout {
+		t.Errorf("FixJSONCommand mode = %v, want OutputStdout", mode)
+	}
+	if outPath != "" {
+		t.Errorf("FixJSONCommand outPath = %q, want empty string", outPath)
+	}
+	// Should pass prompt as arg to -p
+	wantArgs := []string{"/bin/claude", "-p", "fix prompt"}
+	if len(cmd.Args) != len(wantArgs) {
+		t.Fatalf("FixJSONCommand Args = %v, want %v", cmd.Args, wantArgs)
+	}
+}
+
+func TestClaudeProvider_InteractiveCommand(t *testing.T) {
+	p := NewClaudeProvider("/bin/claude")
+	cmd := p.InteractiveCommand("/work", "my prompt")
+	if cmd.Dir != "/work" {
+		t.Errorf("InteractiveCommand Dir = %q, want /work", cmd.Dir)
+	}
+	if len(cmd.Args) != 2 || cmd.Args[0] != "/bin/claude" || cmd.Args[1] != "my prompt" {
+		t.Errorf("InteractiveCommand Args = %v, want [/bin/claude my prompt]", cmd.Args)
+	}
+}
+
+func TestClaudeProvider_ParseLine(t *testing.T) {
+	p := NewClaudeProvider("")
+	// Valid assistant text event
+	line := `{"type":"assistant","message":{"type":"assistant","content":[{"type":"text","text":"hello"}]}}`
+	e := p.ParseLine(line)
+	if e == nil {
+		t.Fatal("ParseLine(assistant text) returned nil")
+	}
+	if e.Type != loop.EventAssistantText {
+		t.Errorf("ParseLine(assistant text) Type = %v, want EventAssistantText", e.Type)
+	}
+}
+
+func TestClaudeProvider_CleanOutput(t *testing.T) {
+	p := NewClaudeProvider("")
+	input := "some output"
+	if p.CleanOutput(input) != input {
+		t.Errorf("CleanOutput should return input unchanged")
+	}
+}