From 45a6acf77ea2cf0e388f037441cd0297201566fd Mon Sep 17 00:00:00 2001
From: Claudomator Agent <agent@claudomator>
Date: Mon, 9 Mar 2026 07:37:53 +0000
Subject: executor: log errors from all unchecked
 UpdateTaskState/UpdateTaskQuestion calls

All previously ignored errors from p.store.UpdateTaskState() and
p.store.UpdateTaskQuestion() in execute() and executeResume() now log
with structured context (taskID, state, error).

Introduces a Store interface so tests can inject a failing mock store.
Adds TestPool_UpdateTaskState_DBError_IsLoggedAndResultDelivered to
verify that a DB write failure is logged and the result is still
delivered to resultCh.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 internal/executor/executor_test.go | 76 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 76 insertions(+)

(limited to 'internal/executor/executor_test.go')

diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go
index c0f6d66..1adba7e 100644
--- a/internal/executor/executor_test.go
+++ b/internal/executor/executor_test.go
@@ -15,6 +15,43 @@ import (
 	"github.com/thepeterstone/claudomator/internal/task"
 )
 
+// capturingHandler is a slog.Handler that records log records for assertions.
+type capturingHandler struct {
+	mu      sync.Mutex
+	records []slog.Record
+}
+
+func (h *capturingHandler) Enabled(_ context.Context, _ slog.Level) bool { return true }
+func (h *capturingHandler) Handle(_ context.Context, r slog.Record) error {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	h.records = append(h.records, r)
+	return nil
+}
+func (h *capturingHandler) WithAttrs(attrs []slog.Attr) slog.Handler { return h }
+func (h *capturingHandler) WithGroup(name string) slog.Handler       { return h }
+
+func (h *capturingHandler) hasMessageContaining(substr string) bool {
+	h.mu.Lock()
+	defer h.mu.Unlock()
+	for _, r := range h.records {
+		if strings.Contains(r.Message, substr) {
+			return true
+		}
+	}
+	return false
+}
+
+// failingStore wraps a real DB but returns an error for UpdateTaskState calls.
+type failingStore struct {
+	*storage.DB
+	updateStateErr error
+}
+
+func (f *failingStore) UpdateTaskState(id string, newState task.State) error {
+	return f.updateStateErr
+}
+
 // mockRunner implements Runner for testing.
 type mockRunner struct {
 	mu       sync.Mutex
@@ -156,6 +193,45 @@ func TestPool_Submit_Failure(t *testing.T) {
 	}
 }
 
+// TestPool_UpdateTaskState_DBError_IsLoggedAndResultDelivered verifies that
+// when UpdateTaskState returns an error, the error is logged with structured
+// context (taskID, state) and the execution result is still sent to resultCh.
+func TestPool_UpdateTaskState_DBError_IsLoggedAndResultDelivered(t *testing.T) {
+	db := testStore(t)
+	store := &failingStore{DB: db, updateStateErr: fmt.Errorf("db write failed")}
+
+	handler := &capturingHandler{}
+	logger := slog.New(handler)
+
+	runner := &mockRunner{err: fmt.Errorf("runner error")}
+	runners := map[string]Runner{"claude": runner}
+	pool := NewPool(2, runners, store, logger)
+
+	tk := makeTask("dberr-1")
+	db.CreateTask(tk)
+
+	if err := pool.Submit(context.Background(), tk); err != nil {
+		t.Fatalf("submit: %v", err)
+	}
+
+	select {
+	case result := <-pool.Results():
+		// Result must still arrive despite the DB error.
+		if result == nil {
+			t.Fatal("expected non-nil result")
+		}
+		if result.TaskID != tk.ID {
+			t.Errorf("taskID: want %q, got %q", tk.ID, result.TaskID)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("timed out waiting for result — result not delivered despite DB error")
+	}
+
+	if !handler.hasMessageContaining("failed to update task state") {
+		t.Error("expected 'failed to update task state' log entry, but none found")
+	}
+}
+
 func TestPool_Submit_Timeout(t *testing.T) {
 	store := testStore(t)
 	runner := &mockRunner{delay: 5 * time.Second}
-- 
cgit v1.2.3