From f8b5f2580e730a8affbccec8b5bde9b96b1f9fc2 Mon Sep 17 00:00:00 2001
From: Peter Stone <thepeterstone@gmail.com>
Date: Thu, 5 Mar 2026 23:03:02 +0000
Subject: executor: persist log paths at execution create time, not just at end

Add LogPather interface; ClaudeRunner implements it via ExecLogDir().
Pool pre-populates stdout_path/stderr_path/artifact_dir on the execution
record before CreateExecution, so paths are in the DB from the moment
a task starts running.

ClaudeRunner.Run() skips path assignment when already set by the pool.

Also update scripts/debug-execution to derive paths from the known
convention (<data-dir>/executions/<exec-id>/) as a fallback for
historical records that predate this change.

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

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

diff --git a/internal/executor/executor_test.go b/internal/executor/executor_test.go
index 18a79bb..b3e6dae 100644
--- a/internal/executor/executor_test.go
+++ b/internal/executor/executor_test.go
@@ -6,6 +6,7 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -206,6 +207,77 @@ func TestPool_AtCapacity(t *testing.T) {
 	<-pool.Results() // drain
 }
 
+// logPatherMockRunner is a mockRunner that also implements LogPather,
+// and captures the StdoutPath seen when Run() is called.
+type logPatherMockRunner struct {
+	mockRunner
+	logDir       string
+	capturedPath string
+}
+
+func (m *logPatherMockRunner) ExecLogDir(execID string) string {
+	return filepath.Join(m.logDir, execID)
+}
+
+func (m *logPatherMockRunner) Run(ctx context.Context, t *task.Task, e *storage.Execution) error {
+	m.mu.Lock()
+	m.capturedPath = e.StdoutPath
+	m.mu.Unlock()
+	return m.mockRunner.Run(ctx, t, e)
+}
+
+// TestPool_Execute_LogPathsPreSetBeforeRun verifies that when the runner
+// implements LogPather, log paths are set on the execution before Run() is
+// called — so they land in the DB at CreateExecution time, not just at
+// UpdateExecution time.
+func TestPool_Execute_LogPathsPreSetBeforeRun(t *testing.T) {
+	store := testStore(t)
+	runner := &logPatherMockRunner{logDir: t.TempDir()}
+	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError}))
+	pool := NewPool(2, runner, store, logger)
+
+	tk := makeTask("lp-1")
+	store.CreateTask(tk)
+	if err := pool.Submit(context.Background(), tk); err != nil {
+		t.Fatalf("submit: %v", err)
+	}
+	result := <-pool.Results()
+
+	runner.mu.Lock()
+	captured := runner.capturedPath
+	runner.mu.Unlock()
+
+	if captured == "" {
+		t.Fatal("StdoutPath was empty when Run() was called; expected pre-set path")
+	}
+	if !strings.HasSuffix(captured, "stdout.log") {
+		t.Errorf("expected stdout.log suffix, got: %s", captured)
+	}
+	// Path in the returned execution record should match.
+	if result.Execution.StdoutPath != captured {
+		t.Errorf("execution StdoutPath %q != captured %q", result.Execution.StdoutPath, captured)
+	}
+}
+
+// TestPool_Execute_NoLogPather_PathsEmptyBeforeRun verifies that a runner
+// without LogPather doesn't panic and paths remain empty until Run() sets them.
+func TestPool_Execute_NoLogPather_PathsEmptyBeforeRun(t *testing.T) {
+	store := testStore(t)
+	runner := &mockRunner{} // does NOT implement LogPather
+	logger := slog.New(slog.NewTextHandler(os.Stderr, &slog.HandlerOptions{Level: slog.LevelError}))
+	pool := NewPool(2, runner, store, logger)
+
+	tk := makeTask("nolp-1")
+	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.Fatalf("unexpected error: %v", result.Err)
+	}
+}
+
 func TestPool_ConcurrentExecution(t *testing.T) {
 	store := testStore(t)
 	runner := &mockRunner{delay: 50 * time.Millisecond}
-- 
cgit v1.2.3