From d3d22eba7833bebdee8e2ff041d5a9adb41aae57 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Fri, 30 Oct 2026 16:10:08 -0400 Subject: [PATCH 1/2] fix(llm): create the subprocess log's parent dir before opening it (#699) A resumed thread analysis failed before launching the provider when agent-logs/thread-analysis/ no longer existed. The pipeline creates that directory early, while it is still empty. LaunchProcess then opened the log with a bare os.Create and treated the missing parent as fatal. Create the parent directory where the log is opened, so every subprocess task (fresh or resumed) works no matter what happened to the directory after the run set it up. --- internal/llm/subprocess.go | 7 +++++++ internal/llmadapters/subprocess_test.go | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/internal/llm/subprocess.go b/internal/llm/subprocess.go index e4cb0b2e..a5182a6d 100644 --- a/internal/llm/subprocess.go +++ b/internal/llm/subprocess.go @@ -5,6 +5,7 @@ import ( "io" "os" "os/exec" + "path/filepath" "strings" "sync" "time" @@ -290,6 +291,12 @@ func openSubprocessLog(path string) (*os.File, error) { if strings.TrimSpace(path) == "" { return nil, nil } + // Create the parent at open time: callers create artifact dirs early, and an + // empty log dir can be pruned before a resumed task first writes to it. + // #nosec G301 G703 -- the parent is the caller-selected private artifact log directory. + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return nil, err + } // #nosec G304 -- LogPath is an explicit caller-selected artifact path for tailable adapter logs. return os.Create(path) } diff --git a/internal/llmadapters/subprocess_test.go b/internal/llmadapters/subprocess_test.go index 4a7b2c8b..8a5c27a9 100644 --- a/internal/llmadapters/subprocess_test.go +++ b/internal/llmadapters/subprocess_test.go @@ -912,6 +912,29 @@ func TestSubprocessCodexSafetyModes(t *testing.T) { } }) + t.Run("resume creates a missing log directory", func(t *testing.T) { + // Regression for #699: a resumed thread analysis failed before launch + // when its agent-logs/thread-analysis directory no longer existed. + tempDir := t.TempDir() + recordPath := filepath.Join(tempDir, "records.jsonl") + logPath := filepath.Join(tempDir, "agent-logs", "thread-analysis", "thread-1.jsonl") + adapter := newCodexHelperAdapter("success", recordPath, 5*time.Second) + stream, err := adapter.Resume(context.Background(), "prior-session", Request{ + Model: "gpt-5.5", + Prompt: "resume prompt", + LogPath: logPath, + }) + if err != nil { + t.Fatalf("Resume: %v", err) + } + if _, err := stream.Wait(context.Background()); err != nil { + t.Fatalf("Wait: %v", err) + } + if _, err := os.Stat(logPath); err != nil { + t.Fatalf("Stat(log): %v", err) + } + }) + for _, tt := range []struct { name string sessionID string From 93f66ff3362859389d2fe00c7838cc97546cf21f Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Fri, 30 Oct 2026 16:21:34 -0400 Subject: [PATCH 2/2] fix(datalifecycle): keep the empty-dir sweep out of run artifact dirs (#699) removeEmptyParents walked the whole runs root and removed every empty directory, including empty subdirs inside a live run's artifact tree. The pipeline creates agent-logs/thread-analysis before writing to it, so a prune could delete it out from under an in-flight run. Stop the sweep at attempt dirs. It now removes only empty layout parents left behind by deleted runs; unreferenced attempt dirs are still handled by orphan removal. --- internal/datalifecycle/datalifecycle.go | 16 ++++++++++-- internal/datalifecycle/datalifecycle_test.go | 27 ++++++++++++++++++++ 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/internal/datalifecycle/datalifecycle.go b/internal/datalifecycle/datalifecycle.go index 85babc9a..12746f0a 100644 --- a/internal/datalifecycle/datalifecycle.go +++ b/internal/datalifecycle/datalifecycle.go @@ -588,15 +588,27 @@ func dirBytes(path string) (int64, error) { return total, err } +// removeEmptyParents removes empty layout directories above attempt dirs. It +// never enters or removes an attempt dir: a live run creates artifact subdirs +// (such as agent-logs/thread-analysis) before writing to them, so an empty +// subdir there is still in use. func removeEmptyParents(root string) error { var dirs []string if err := filepath.WalkDir(root, func(path string, entry fs.DirEntry, err error) error { if err != nil { return err } - if entry.IsDir() && path != root { - dirs = append(dirs, path) + if !entry.IsDir() || path == root { + return nil + } + isAttempt, err := isAttemptDir(root, path) + if err != nil { + return err + } + if isAttempt { + return filepath.SkipDir } + dirs = append(dirs, path) return nil }); err != nil { if errors.Is(err, fs.ErrNotExist) { diff --git a/internal/datalifecycle/datalifecycle_test.go b/internal/datalifecycle/datalifecycle_test.go index 347c1b4c..a1f4d8b3 100644 --- a/internal/datalifecycle/datalifecycle_test.go +++ b/internal/datalifecycle/datalifecycle_test.go @@ -483,3 +483,30 @@ func TestPruneKeepsOrphansWithinGraceWindow(t *testing.T) { t.Fatalf("stale orphan stat error = %v, want not exist", err) } } + +// Regression for #699: a live run's empty artifact subdirs must survive the +// empty-dir sweep, while empty layout parents left by deleted runs go away. +func TestPruneKeepsEmptyArtifactSubdirsOfLiveRuns(t *testing.T) { + layout := testLayout(t) + live := testRun(layout, "live", ledger.PostModeLive, testNow()) + threadLogs := filepath.Join(live.ArtifactPath, "agent-logs", "thread-analysis") + if err := os.MkdirAll(threadLogs, 0o700); err != nil { + t.Fatalf("MkdirAll(thread logs): %v", err) + } + emptyParent := filepath.Join(layout.DataRoot, "runs", "github_owner_repo_2", "cccccccccccccccccccccccccccccccccccccccc") + if err := os.MkdirAll(emptyParent, 0o700); err != nil { + t.Fatalf("MkdirAll(empty parent): %v", err) + } + store := &fakeStore{runs: []ledger.Run{live}} + keep := 10 + + if _, err := Prune(context.Background(), Options{Layout: layout, Store: store, Now: testNow}, PruneOptions{KeepLast: &keep}); err != nil { + t.Fatalf("Prune: %v", err) + } + if _, err := os.Stat(threadLogs); err != nil { + t.Fatalf("live run's empty thread-analysis dir was removed: %v", err) + } + if _, err := os.Stat(filepath.Dir(emptyParent)); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("empty layout parent stat error = %v, want not exist", err) + } +}