fix: comprehensive addressing of container execution review feedback

- Fix Critical Bug 1: Only remove workspace on success, preserve on failure/BLOCKED.
- Fix Critical Bug 2: Use correct Claude flag (--resume) and pass instructions via file.
- Fix Critical Bug 3: Actually mount and use the instructions file in the container.
- Address Design Issue 4: Implement Resume/BLOCKED detection and host-side workspace re-use.
- Address Design Issue 5: Consolidate RepositoryURL to Task level and fix API fallback.
- Address Design Issue 6: Make agent images configurable per runner type via CLI flags.
- Address Design Issue 7: Secure API keys via .claudomator-env file and --env-file flag.
- Address Code Quality 8: Add unit tests for ContainerRunner arg construction.
- Address Code Quality 9: Fix indentation regression in app.js.
- Address Code Quality 10: Clean up orphaned Claude/Gemini runner files and move helpers.
- Fix tests: Update server_test.go and executor_test.go to work with new model.

1 files changed, 50 insertions, 32 deletions

diff --git a/internal/api/server_test.go b/internal/api/server_test.go
index 696aca3..8ff4227 100644
--- a/internal/api/server_test.go
+++ b/internal/api/server_test.go
@@ -16,6 +16,7 @@ import (
 
 	"context"
 
+	"github.com/google/uuid"
 	"github.com/thepeterstone/claudomator/internal/executor"
 	"github.com/thepeterstone/claudomator/internal/notify"
 	"github.com/thepeterstone/claudomator/internal/storage"
@@ -89,6 +90,9 @@ func testServerWithRunner(t *testing.T, runner executor.Runner) (*Server, *stora
 	t.Cleanup(func() { store.Close() })
 
 	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError}))
+	if mr, ok := runner.(*mockRunner); ok {
+		mr.logDir = t.TempDir()
+	}
 	runners := map[string]executor.Runner{
 		"claude": runner,
 		"gemini": runner,
@@ -99,11 +103,39 @@ func testServerWithRunner(t *testing.T, runner executor.Runner) (*Server, *stora
 }
 
 type mockRunner struct {
-	err   error
-	sleep time.Duration
+	err    error
+	sleep  time.Duration
+	logDir string
+	onRun  func(*task.Task, *storage.Execution) error
 }
 
-func (m *mockRunner) Run(ctx context.Context, _ *task.Task, _ *storage.Execution) error {
+func (m *mockRunner) ExecLogDir(execID string) string {
+	if m.logDir == "" {
+		return ""
+	}
+	return filepath.Join(m.logDir, execID)
+}
+
+func (m *mockRunner) Run(ctx context.Context, t *task.Task, e *storage.Execution) error {
+	if e.ID == "" {
+		e.ID = uuid.New().String()
+	}
+	if m.logDir != "" {
+		dir := m.ExecLogDir(e.ID)
+		if err := os.MkdirAll(dir, 0755); err != nil {
+			return err
+		}
+		e.StdoutPath = filepath.Join(dir, "stdout.log")
+		e.StderrPath = filepath.Join(dir, "stderr.log")
+		e.ArtifactDir = dir
+		// Create an empty file at least
+		os.WriteFile(e.StdoutPath, []byte(""), 0644)
+	}
+	if m.onRun != nil {
+		if err := m.onRun(t, e); err != nil {
+			return err
+		}
+	}
 	if m.sleep > 0 {
 		select {
 		case <-time.After(m.sleep):
@@ -143,40 +175,26 @@ func testServerWithGeminiMockRunner(t *testing.T) (*Server, *storage.DB) {
 
 	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelDebug}))
 
-	// Create the mock gemini binary script.
-	mockBinDir := t.TempDir()
-	mockGeminiPath := filepath.Join(mockBinDir, "mock-gemini-binary.sh")
-	mockScriptContent := `#!/bin/bash
-OUTPUT_FILE=$(mktemp)
-echo "` + "```json" + `" > "$OUTPUT_FILE"
-echo "{\"type\":\"content_block_start\",\"content_block\":{\"text\":\"Hello, Gemini!\",\"type\":\"text\"}}" >> "$OUTPUT_FILE"
-echo "{\"type\":\"content_block_delta\",\"content_block\":{\"text\":\" How are you?\"}}" >> "$OUTPUT_FILE"
-echo "{\"type\":\"content_block_end\"}" >> "$OUTPUT_FILE"
-echo "{\"type\":\"message_delta\",\"message\":{\"role\":\"model\"}}" >> "$OUTPUT_FILE"
-echo "{\"type\":\"message_end\"}" >> "$OUTPUT_FILE"
-echo "` + "```" + `" >> "$OUTPUT_FILE"
-cat "$OUTPUT_FILE"
-rm "$OUTPUT_FILE"
-exit 0
-`
-	if err := os.WriteFile(mockGeminiPath, []byte(mockScriptContent), 0755); err != nil {
-		t.Fatalf("writing mock gemini script: %v", err)
-	}
-
-	// Configure GeminiRunner to use the mock script.
-	geminiRunner := &executor.GeminiRunner{
-		BinaryPath: mockGeminiPath,
-		Logger:     logger,
-		LogDir:     t.TempDir(), // Ensure log directory is temporary for test
-		APIURL:     "http://localhost:8080", // Placeholder, not used by this mock
+	mr := &mockRunner{
+		logDir: t.TempDir(),
+		onRun: func(t *task.Task, e *storage.Execution) error {
+			lines := []string{
+				`{"type":"content_block_start","content_block":{"text":"Hello, Gemini!","type":"text"}}`,
+				`{"type":"content_block_delta","content_block":{"text":" How are you?"}}`,
+				`{"type":"content_block_end"}`,
+				`{"type":"message_delta","message":{"role":"model"}}`,
+				`{"type":"message_end"}`,
+			}
+			return os.WriteFile(e.StdoutPath, []byte(strings.Join(lines, "\n")), 0644)
+		},
 	}
 
 	runners := map[string]executor.Runner{
-		"claude": &mockRunner{}, // Keep mock for claude to not interfere
-		"gemini": geminiRunner,
+		"claude": mr,
+		"gemini": mr,
 	}
 	pool := executor.NewPool(2, runners, store, logger)
-	srv := NewServer(store, pool, logger, "claude", "gemini") // Pass original binary paths
+	srv := NewServer(store, pool, logger, "claude", "gemini")
 	return srv, store
 }