From 7d6943c5f9f4124c652377148a35bea5f61be4bf Mon Sep 17 00:00:00 2001 From: Claudomator Agent Date: Tue, 10 Mar 2026 00:28:33 +0000 Subject: executor: extract handleRunResult to deduplicate error-classification logic Both execute() and executeResume() shared ~80% identical post-run logic: error classification (BLOCKED, TIMED_OUT, CANCELLED, BUDGET_EXCEEDED, FAILED), state transitions, result emission, and UpdateExecution. Extract this into handleRunResult(ctx, t, exec, err, agentType) on *Pool. Both functions now call it after runner.Run() returns. Also adds TestHandleRunResult_SharedPath which directly exercises the new function via a minimalMockStore, covering FAILED, READY, COMPLETED, and TIMED_OUT classification paths. Co-Authored-By: Claude Sonnet 4.6 --- internal/executor/executor_test.go | 190 +++++++++++++++++++++++++++++++++++++ 1 file changed, 190 insertions(+) (limited to 'internal/executor/executor_test.go') diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go index 9896ba1..0935545 100644 --- a/internal/executor/executor_test.go +++ b/internal/executor/executor_test.go @@ -805,6 +805,196 @@ func TestPool_Submit_ParentNotBlocked_NoTransition(t *testing.T) { } } +// minimalMockStore is a standalone Store implementation for unit-testing Pool +// methods that do not require a real SQLite database. +type minimalMockStore struct { + mu sync.Mutex + tasks map[string]*task.Task + executions map[string]*storage.Execution + stateUpdates []struct{ id string; state task.State } + questionUpdates []string + subtasksFunc func(parentID string) ([]*task.Task, error) + updateExecErr error + updateStateErr error +} + +func newMinimalMockStore() *minimalMockStore { + return &minimalMockStore{ + tasks: make(map[string]*task.Task), + executions: make(map[string]*storage.Execution), + } +} + +func (m *minimalMockStore) GetTask(id string) (*task.Task, error) { + m.mu.Lock() + defer m.mu.Unlock() + t, ok := m.tasks[id] + if !ok { + return nil, fmt.Errorf("task %q not found", id) + } + return t, nil +} +func (m *minimalMockStore) ListTasks(_ storage.TaskFilter) ([]*task.Task, error) { return nil, nil } +func (m *minimalMockStore) ListSubtasks(parentID string) ([]*task.Task, error) { + if m.subtasksFunc != nil { + return m.subtasksFunc(parentID) + } + return nil, nil +} +func (m *minimalMockStore) ListExecutions(_ string) ([]*storage.Execution, error) { return nil, nil } +func (m *minimalMockStore) CreateExecution(e *storage.Execution) error { return nil } +func (m *minimalMockStore) UpdateExecution(e *storage.Execution) error { + return m.updateExecErr +} +func (m *minimalMockStore) UpdateTaskState(id string, newState task.State) error { + if m.updateStateErr != nil { + return m.updateStateErr + } + m.mu.Lock() + m.stateUpdates = append(m.stateUpdates, struct{ id string; state task.State }{id, newState}) + if t, ok := m.tasks[id]; ok { + t.State = newState + } + m.mu.Unlock() + return nil +} +func (m *minimalMockStore) UpdateTaskQuestion(taskID, questionJSON string) error { + m.mu.Lock() + m.questionUpdates = append(m.questionUpdates, questionJSON) + m.mu.Unlock() + return nil +} + +func (m *minimalMockStore) lastStateUpdate() (string, task.State, bool) { + m.mu.Lock() + defer m.mu.Unlock() + if len(m.stateUpdates) == 0 { + return "", "", false + } + u := m.stateUpdates[len(m.stateUpdates)-1] + return u.id, u.state, true +} + +func newPoolWithMockStore(store Store) *Pool { + logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError})) + return &Pool{ + maxConcurrent: 2, + runners: map[string]Runner{"claude": &mockRunner{}}, + store: store, + logger: logger, + activePerAgent: make(map[string]int), + rateLimited: make(map[string]time.Time), + cancels: make(map[string]context.CancelFunc), + resultCh: make(chan *Result, 4), + workCh: make(chan workItem, 4), + doneCh: make(chan struct{}, 2), + Questions: NewQuestionRegistry(), + } +} + +// TestHandleRunResult_SharedPath verifies that handleRunResult correctly +// classifies runner errors and transitions task state via the store. +func TestHandleRunResult_SharedPath(t *testing.T) { + t.Run("generic error sets FAILED", func(t *testing.T) { + store := newMinimalMockStore() + pool := newPoolWithMockStore(store) + tk := makeTask("hrr-fail") + store.tasks[tk.ID] = tk + + exec := &storage.Execution{ID: "e1", TaskID: tk.ID, Status: "RUNNING"} + ctx := context.Background() + + pool.handleRunResult(ctx, tk, exec, fmt.Errorf("something broke"), "claude") + + if exec.Status != "FAILED" { + t.Errorf("exec.Status: want FAILED, got %q", exec.Status) + } + if exec.ErrorMsg != "something broke" { + t.Errorf("exec.ErrorMsg: want %q, got %q", "something broke", exec.ErrorMsg) + } + _, state, ok := store.lastStateUpdate() + if !ok || state != task.StateFailed { + t.Errorf("expected UpdateTaskState(FAILED), got state=%v ok=%v", state, ok) + } + result := <-pool.resultCh + if result.Err == nil || result.Execution.Status != "FAILED" { + t.Errorf("unexpected result: %+v", result) + } + }) + + t.Run("nil error top-level no subtasks sets READY", func(t *testing.T) { + store := newMinimalMockStore() + pool := newPoolWithMockStore(store) + tk := makeTask("hrr-ready") + store.tasks[tk.ID] = tk + + exec := &storage.Execution{ID: "e2", TaskID: tk.ID, Status: "RUNNING"} + ctx := context.Background() + + pool.handleRunResult(ctx, tk, exec, nil, "claude") + + if exec.Status != "READY" { + t.Errorf("exec.Status: want READY, got %q", exec.Status) + } + _, state, ok := store.lastStateUpdate() + if !ok || state != task.StateReady { + t.Errorf("expected UpdateTaskState(READY), got state=%v ok=%v", state, ok) + } + result := <-pool.resultCh + if result.Err != nil || result.Execution.Status != "READY" { + t.Errorf("unexpected result: %+v", result) + } + }) + + t.Run("nil error subtask sets COMPLETED", func(t *testing.T) { + store := newMinimalMockStore() + pool := newPoolWithMockStore(store) + parent := makeTask("hrr-parent") + parent.State = task.StateBlocked + store.tasks[parent.ID] = parent + + tk := makeTask("hrr-sub") + tk.ParentTaskID = parent.ID + store.tasks[tk.ID] = tk + + exec := &storage.Execution{ID: "e3", TaskID: tk.ID, Status: "RUNNING"} + ctx := context.Background() + + pool.handleRunResult(ctx, tk, exec, nil, "claude") + + if exec.Status != "COMPLETED" { + t.Errorf("exec.Status: want COMPLETED, got %q", exec.Status) + } + result := <-pool.resultCh + if result.Err != nil || result.Execution.Status != "COMPLETED" { + t.Errorf("unexpected result: %+v", result) + } + }) + + t.Run("timeout sets TIMED_OUT", func(t *testing.T) { + store := newMinimalMockStore() + pool := newPoolWithMockStore(store) + tk := makeTask("hrr-timeout") + store.tasks[tk.ID] = tk + + exec := &storage.Execution{ID: "e4", TaskID: tk.ID, Status: "RUNNING"} + ctx, cancel := context.WithCancel(context.Background()) + cancel() // make ctx.Err() == context.Canceled + + // Simulate deadline exceeded by using a deadline-exceeded context. + dctx, dcancel := context.WithDeadline(context.Background(), time.Now().Add(-time.Second)) + defer dcancel() + + pool.handleRunResult(dctx, tk, exec, context.DeadlineExceeded, "claude") + + if exec.Status != "TIMED_OUT" { + t.Errorf("exec.Status: want TIMED_OUT, got %q", exec.Status) + } + _ = ctx + <-pool.resultCh + }) +} + func TestPool_UnsupportedAgent(t *testing.T) { store := testStore(t) runners := map[string]Runner{"claude": &mockRunner{}} -- cgit v1.2.3