refactor: address code review notes (backward compat, Gemini tests, unknown agent test)

5 files changed, 172 insertions, 0 deletions

diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go
index 2f205f8..9ad0617 100644
--- a/internal/executor/executor_test.go
+++ b/internal/executor/executor_test.go
@@ -346,3 +346,29 @@ func TestPool_ConcurrentExecution(t *testing.T) {
 		t.Errorf("calls: want 3, got %d", runner.callCount())
 	}
 }
+
+func TestPool_UnsupportedAgent(t *testing.T) {
+	store := testStore(t)
+	runners := map[string]Runner{"claude": &mockRunner{}}
+	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError}))
+	pool := NewPool(2, runners, store, logger)
+
+	tk := makeTask("bad-agent")
+	tk.Agent.Type = "super-ai"
+	store.CreateTask(tk)
+
+	if err := pool.Submit(context.Background(), tk); err != nil {
+		t.Fatalf("submit: %v", err)
+	}
+
+	result := <-pool.Results()
+	if result.Err == nil {
+		t.Fatal("expected error for unsupported agent")
+	}
+	if !strings.Contains(result.Err.Error(), "unsupported agent type") {
+		t.Errorf("expected 'unsupported agent type' in error, got: %v", result.Err)
+	}
+	if result.Execution.Status != "FAILED" {
+		t.Errorf("status: want FAILED, got %q", result.Execution.Status)
+	}
+}
diff --git a/internal/executor/gemini_test.go b/internal/executor/gemini_test.go
new file mode 100644
index 0000000..c7acc3c
--- /dev/null
+++ b/internal/executor/gemini_test.go
@@ -0,0 +1,103 @@
+package executor
+
+import (
+	"context"
+	"io"
+	"log/slog"
+	"strings"
+	"testing"
+
+	"github.com/thepeterstone/claudomator/internal/storage"
+	"github.com/thepeterstone/claudomator/internal/task"
+)
+
+func TestGeminiRunner_BuildArgs_BasicTask(t *testing.T) {
+	r := &GeminiRunner{}
+	tk := &task.Task{
+		Agent: task.AgentConfig{
+			Type:         "gemini",
+			Instructions: "fix the bug",
+			Model:        "gemini-2.0-flash",
+			SkipPlanning: true,
+		},
+	}
+
+	args := r.buildArgs(tk, &storage.Execution{ID: "test-exec"}, "/tmp/q.json")
+
+	// Gemini CLI: instructions is the first positional arg
+	if len(args) < 1 || args[0] != "fix the bug" {
+		t.Errorf("expected instructions as first arg, got: %v", args)
+	}
+
+	argMap := make(map[string]bool)
+	for _, a := range args {
+		argMap[a] = true
+	}
+	for _, want := range []string{"--output-format", "stream-json", "--model", "gemini-2.0-flash"} {
+		if !argMap[want] {
+			t.Errorf("missing arg %q in %v", want, args)
+		}
+	}
+}
+
+func TestGeminiRunner_BuildArgs_PreamblePrepended(t *testing.T) {
+	r := &GeminiRunner{}
+	tk := &task.Task{
+		Agent: task.AgentConfig{
+			Type:         "gemini",
+			Instructions: "fix the bug",
+			SkipPlanning: false,
+		},
+	}
+
+	args := r.buildArgs(tk, &storage.Execution{ID: "test-exec"}, "/tmp/q.json")
+
+	if len(args) < 1 {
+		t.Fatalf("expected at least 1 arg, got: %v", args)
+	}
+	if !strings.HasPrefix(args[0], planningPreamble) {
+		t.Errorf("instructions should start with planning preamble")
+	}
+	if !strings.HasSuffix(args[0], "fix the bug") {
+		t.Errorf("instructions should end with original instructions")
+	}
+}
+
+func TestGeminiRunner_Run_InaccessibleWorkingDir_ReturnsError(t *testing.T) {
+	r := &GeminiRunner{
+		BinaryPath: "true", // would succeed if it ran
+		Logger:     slog.New(slog.NewTextHandler(io.Discard, nil)),
+		LogDir:     t.TempDir(),
+	}
+	tk := &task.Task{
+		Agent: task.AgentConfig{
+			Type:         "gemini",
+			WorkingDir:   "/nonexistent/path/does/not/exist",
+			SkipPlanning: true,
+		},
+	}
+	exec := &storage.Execution{ID: "test-exec"}
+
+	err := r.Run(context.Background(), tk, exec)
+
+	if err == nil {
+		t.Fatal("expected error for inaccessible working_dir, got nil")
+	}
+	if !strings.Contains(err.Error(), "working_dir") {
+		t.Errorf("expected 'working_dir' in error, got: %v", err)
+	}
+}
+
+func TestGeminiRunner_BinaryPath_Default(t *testing.T) {
+	r := &GeminiRunner{}
+	if r.binaryPath() != "gemini" {
+		t.Errorf("want 'gemini', got %q", r.binaryPath())
+	}
+}
+
+func TestGeminiRunner_BinaryPath_Custom(t *testing.T) {
+	r := &GeminiRunner{BinaryPath: "/usr/local/bin/gemini"}
+	if r.binaryPath() != "/usr/local/bin/gemini" {
+		t.Errorf("want custom path, got %q", r.binaryPath())
+	}
+}
diff --git a/internal/storage/db.go b/internal/storage/db.go
index 31927a0..c396bbe 100644
--- a/internal/storage/db.go
+++ b/internal/storage/db.go
@@ -515,6 +515,15 @@ func scanTask(row scanner) (*task.Task, error) {
 	if err := json.Unmarshal([]byte(configJSON), &t.Agent); err != nil {
 		return nil, fmt.Errorf("unmarshaling agent config: %w", err)
 	}
+	// Fallback for legacy 'claude' field
+	if t.Agent.Instructions == "" {
+		var legacy struct {
+			Claude task.AgentConfig `json:"claude"`
+		}
+		if err := json.Unmarshal([]byte(configJSON), &legacy); err == nil && legacy.Claude.Instructions != "" {
+			t.Agent = legacy.Claude
+		}
+	}
 	if err := json.Unmarshal([]byte(retryJSON), &t.Retry); err != nil {
 		return nil, fmt.Errorf("unmarshaling retry: %w", err)
 	}
diff --git a/internal/storage/db_test.go b/internal/storage/db_test.go
index 4c3f011..36f1644 100644
--- a/internal/storage/db_test.go
+++ b/internal/storage/db_test.go
@@ -548,3 +548,36 @@ func TestStorage_GetLatestExecution(t *testing.T) {
 		t.Errorf("want le-2, got %q", got.ID)
 	}
 }
+
+func TestGetTask_BackwardCompatibility(t *testing.T) {
+	db := testDB(t)
+	now := time.Now().UTC().Truncate(time.Second)
+
+	// Legacy config JSON using "claude" field instead of "agent"
+	legacyConfig := `{"claude":{"model":"haiku","instructions":"legacy instructions","max_budget_usd":0.5}}`
+
+	_, err := db.db.Exec(`
+		INSERT INTO tasks (id, name, description, config_json, priority, timeout_ns, retry_json, tags_json, depends_on_json, state, created_at, updated_at)
+		VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)`,
+		"legacy-id", "Legacy Task", "A legacy test", legacyConfig, "normal",
+		0, "{}", "[]", "[]", "PENDING", now, now,
+	)
+	if err != nil {
+		t.Fatalf("inserting legacy task: %v", err)
+	}
+
+	got, err := db.GetTask("legacy-id")
+	if err != nil {
+		t.Fatalf("getting legacy task: %v", err)
+	}
+
+	if got.Agent.Instructions != "legacy instructions" {
+		t.Errorf("instructions: want 'legacy instructions', got %q", got.Agent.Instructions)
+	}
+	if got.Agent.Model != "haiku" {
+		t.Errorf("model: want 'haiku', got %q", got.Agent.Model)
+	}
+	if got.Agent.MaxBudgetUSD != 0.5 {
+		t.Errorf("budget: want 0.5, got %f", got.Agent.MaxBudgetUSD)
+	}
+}
diff --git a/internal/task/task.go b/internal/task/task.go
index b3e93d3..c6a321d 100644
--- a/internal/task/task.go
+++ b/internal/task/task.go
@@ -51,6 +51,7 @@ type Task struct {
 	Name         string       `yaml:"name"          json:"name"`
 	Description string       `yaml:"description" json:"description"`
 	Agent       AgentConfig `yaml:"agent"     json:"agent"`
+	Claude      AgentConfig `yaml:"claude"    json:"claude"` // alias for backward compatibility
 	Timeout     Duration     `yaml:"timeout"    json:"timeout"`
 	Retry       RetryConfig  `yaml:"retry"      json:"retry"`
 	Priority    Priority     `yaml:"priority"   json:"priority"`