From 4111929890fe4b980e1d28f28bcf5bdcc77c9d10 Mon Sep 17 00:00:00 2001 From: paul <423357+toothbrush@users.noreply.github.com> Date: Fri, 20 Feb 2026 13:26:38 +0930 Subject: [PATCH 1/8] Auto-delete stale session state files during load/list Session state files (.git/entire-sessions/) accumulate over time. Sessions that ended more than 24 hours ago are no longer useful. This transparently deletes them during StateStore.List() and LoadSessionState() so stale sessions are invisible to all callers. Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: 126847879a7e --- cmd/entire/cli/session/state.go | 20 +++++ cmd/entire/cli/session/state_test.go | 78 +++++++++++++++++++ cmd/entire/cli/strategy/session_state.go | 10 +++ cmd/entire/cli/strategy/session_state_test.go | 50 ++++++++++++ 4 files changed, 158 insertions(+) diff --git a/cmd/entire/cli/session/state.go b/cmd/entire/cli/session/state.go index 7ce4cbc02..76f749475 100644 --- a/cmd/entire/cli/session/state.go +++ b/cmd/entire/cli/session/state.go @@ -21,6 +21,10 @@ import ( const ( // SessionStateDirName is the directory name for session state files within git common dir. SessionStateDirName = "entire-sessions" + + // StaleSessionThreshold is the duration after which an ended session is considered stale + // and will be automatically deleted during load/list operations. + StaleSessionThreshold = 24 * time.Hour ) // State represents the state of an active session. @@ -205,6 +209,12 @@ func (s *State) NormalizeAfterLoad() { } } +// IsStale returns true when the session has ended and the time since it ended +// exceeds StaleSessionThreshold. Active sessions (EndedAt == nil) are never stale. +func (s *State) IsStale() bool { + return s.EndedAt != nil && time.Since(*s.EndedAt) > StaleSessionThreshold +} + // StateStore provides low-level operations for managing session state files. // // StateStore is a primitive for session state persistence. It is NOT the same as @@ -336,6 +346,8 @@ func (s *StateStore) List(ctx context.Context) ([]*State, error) { return nil, fmt.Errorf("failed to read session state directory: %w", err) } + logCtx := logging.WithComponent(ctx, "session") + var states []*State for _, entry := range entries { if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".json") { @@ -354,6 +366,14 @@ func (s *StateStore) List(ctx context.Context) ([]*State, error) { continue } + if state.IsStale() { + logging.Info(logCtx, "deleting stale session state", + slog.String("session_id", sessionID), + ) + _ = s.Clear(ctx, sessionID) //nolint:errcheck // best-effort cleanup of stale session + continue + } + states = append(states, state) } return states, nil diff --git a/cmd/entire/cli/session/state_test.go b/cmd/entire/cli/session/state_test.go index 99fe48428..1465fb184 100644 --- a/cmd/entire/cli/session/state_test.go +++ b/cmd/entire/cli/session/state_test.go @@ -1,8 +1,12 @@ package session import ( + "context" "encoding/json" + "os" + "path/filepath" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -118,3 +122,77 @@ func TestState_NormalizeAfterLoad_JSONRoundTrip(t *testing.T) { }) } } + +func TestState_IsStale(t *testing.T) { + t.Parallel() + + t.Run("nil_EndedAt_is_not_stale", func(t *testing.T) { + t.Parallel() + state := &State{EndedAt: nil} + assert.False(t, state.IsStale()) + }) + + t.Run("recently_ended_is_not_stale", func(t *testing.T) { + t.Parallel() + recent := time.Now().Add(-1 * time.Hour) + state := &State{EndedAt: &recent} + assert.False(t, state.IsStale()) + }) + + t.Run("ended_over_24h_ago_is_stale", func(t *testing.T) { + t.Parallel() + old := time.Now().Add(-25 * time.Hour) + state := &State{EndedAt: &old} + assert.True(t, state.IsStale()) + }) + + t.Run("ended_exactly_at_threshold_is_not_stale", func(t *testing.T) { + t.Parallel() + // time.Since will be slightly over threshold due to execution time, + // but a value well within threshold should not be stale + recent := time.Now().Add(-23 * time.Hour) + state := &State{EndedAt: &recent} + assert.False(t, state.IsStale()) + }) +} + +func TestStateStore_List_DeletesStaleSession(t *testing.T) { + t.Parallel() + + stateDir := filepath.Join(t.TempDir(), "entire-sessions") + require.NoError(t, os.MkdirAll(stateDir, 0o750)) + store := NewStateStoreWithDir(stateDir) + ctx := context.Background() + + // Create an active session (no EndedAt) + active := &State{ + SessionID: "active-session", + BaseCommit: "abc123", + StartedAt: time.Now(), + } + require.NoError(t, store.Save(ctx, active)) + + // Create a stale session (ended >24h ago) + staleEnded := time.Now().Add(-48 * time.Hour) + stale := &State{ + SessionID: "stale-session", + BaseCommit: "def456", + StartedAt: time.Now().Add(-72 * time.Hour), + EndedAt: &staleEnded, + } + require.NoError(t, store.Save(ctx, stale)) + + // List should return only the active session + states, err := store.List(ctx) + require.NoError(t, err) + require.Len(t, states, 1) + assert.Equal(t, "active-session", states[0].SessionID) + + // Stale session file should be deleted from disk + _, err = os.Stat(filepath.Join(stateDir, "stale-session.json")) + assert.True(t, os.IsNotExist(err), "stale session file should be deleted") + + // Active session file should still exist + _, err = os.Stat(filepath.Join(stateDir, "active-session.json")) + assert.NoError(t, err, "active session file should still exist") +} diff --git a/cmd/entire/cli/strategy/session_state.go b/cmd/entire/cli/strategy/session_state.go index b159bd561..afd76a678 100644 --- a/cmd/entire/cli/strategy/session_state.go +++ b/cmd/entire/cli/strategy/session_state.go @@ -62,6 +62,16 @@ func LoadSessionState(sessionID string) (*SessionState, error) { return nil, fmt.Errorf("failed to unmarshal session state: %w", err) } state.NormalizeAfterLoad() + + if state.IsStale() { + logCtx := logging.WithComponent(context.Background(), "session") + logging.Info(logCtx, "deleting stale session state", + slog.String("session_id", sessionID), + ) + _ = ClearSessionState(sessionID) //nolint:errcheck // best-effort cleanup of stale session + return nil, nil //nolint:nilnil // stale session treated as not found + } + return &state, nil } diff --git a/cmd/entire/cli/strategy/session_state_test.go b/cmd/entire/cli/strategy/session_state_test.go index 5c598fa97..8bfd51403 100644 --- a/cmd/entire/cli/strategy/session_state_test.go +++ b/cmd/entire/cli/strategy/session_state_test.go @@ -415,3 +415,53 @@ func TestTransitionAndLog_ReturnsHandlerError(t *testing.T) { t.Error("TransitionAndLog() should return handler error") } } + +// TestLoadSessionState_DeletesStaleSession tests that LoadSessionState returns (nil, nil) +// for a stale session and deletes the file from disk. +func TestLoadSessionState_DeletesStaleSession(t *testing.T) { + dir := t.TempDir() + _, err := git.PlainInit(dir, false) + if err != nil { + t.Fatalf("failed to init git repo: %v", err) + } + + t.Chdir(dir) + + // Create a stale session (ended >24h ago) + staleEnded := time.Now().Add(-48 * time.Hour) + state := &SessionState{ + SessionID: "stale-load-test", + BaseCommit: "abc123def456", + StartedAt: time.Now().Add(-72 * time.Hour), + EndedAt: &staleEnded, + StepCount: 5, + } + + err = SaveSessionState(state) + if err != nil { + t.Fatalf("SaveSessionState() error = %v", err) + } + + // Verify file exists before load + stateFile, err := sessionStateFile("stale-load-test") + if err != nil { + t.Fatalf("sessionStateFile() error = %v", err) + } + if _, err := os.Stat(stateFile); err != nil { + t.Fatalf("state file should exist before load: %v", err) + } + + // Load should return (nil, nil) for stale session + loaded, err := LoadSessionState("stale-load-test") + if err != nil { + t.Errorf("LoadSessionState() error = %v, want nil for stale session", err) + } + if loaded != nil { + t.Error("LoadSessionState() returned non-nil for stale session") + } + + // File should be deleted from disk + if _, err := os.Stat(stateFile); !os.IsNotExist(err) { + t.Error("stale session file should be deleted after LoadSessionState()") + } +} From 738a41c8fcc9e5bcd4d908483c9c02370953e01b Mon Sep 17 00:00:00 2001 From: paul <423357+toothbrush@users.noreply.github.com> Date: Fri, 20 Feb 2026 13:28:34 +0930 Subject: [PATCH 2/8] Let's make it 12h. --- cmd/entire/cli/session/state.go | 2 +- cmd/entire/cli/session/state_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/session/state.go b/cmd/entire/cli/session/state.go index 76f749475..a305fa0dc 100644 --- a/cmd/entire/cli/session/state.go +++ b/cmd/entire/cli/session/state.go @@ -24,7 +24,7 @@ const ( // StaleSessionThreshold is the duration after which an ended session is considered stale // and will be automatically deleted during load/list operations. - StaleSessionThreshold = 24 * time.Hour + StaleSessionThreshold = 12 * time.Hour ) // State represents the state of an active session. diff --git a/cmd/entire/cli/session/state_test.go b/cmd/entire/cli/session/state_test.go index 1465fb184..a8f92a9cc 100644 --- a/cmd/entire/cli/session/state_test.go +++ b/cmd/entire/cli/session/state_test.go @@ -139,7 +139,7 @@ func TestState_IsStale(t *testing.T) { assert.False(t, state.IsStale()) }) - t.Run("ended_over_24h_ago_is_stale", func(t *testing.T) { + t.Run("ended_25h_ago_is_stale", func(t *testing.T) { t.Parallel() old := time.Now().Add(-25 * time.Hour) state := &State{EndedAt: &old} From 89de9757abddd674edad01ca020ae44507acde9b Mon Sep 17 00:00:00 2001 From: paul <423357+toothbrush@users.noreply.github.com> Date: Fri, 20 Feb 2026 13:41:57 +0930 Subject: [PATCH 3/8] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- cmd/entire/cli/session/state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/entire/cli/session/state.go b/cmd/entire/cli/session/state.go index a305fa0dc..f7ce8a61f 100644 --- a/cmd/entire/cli/session/state.go +++ b/cmd/entire/cli/session/state.go @@ -367,7 +367,7 @@ func (s *StateStore) List(ctx context.Context) ([]*State, error) { } if state.IsStale() { - logging.Info(logCtx, "deleting stale session state", + logging.Debug(logCtx, "deleting stale session state", slog.String("session_id", sessionID), ) _ = s.Clear(ctx, sessionID) //nolint:errcheck // best-effort cleanup of stale session From de817d3e761fd63ccefe6c7d87d8f5808ef9d564 Mon Sep 17 00:00:00 2001 From: paul <423357+toothbrush@users.noreply.github.com> Date: Fri, 20 Feb 2026 13:42:07 +0930 Subject: [PATCH 4/8] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- cmd/entire/cli/strategy/session_state.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/entire/cli/strategy/session_state.go b/cmd/entire/cli/strategy/session_state.go index afd76a678..7386852ff 100644 --- a/cmd/entire/cli/strategy/session_state.go +++ b/cmd/entire/cli/strategy/session_state.go @@ -65,7 +65,7 @@ func LoadSessionState(sessionID string) (*SessionState, error) { if state.IsStale() { logCtx := logging.WithComponent(context.Background(), "session") - logging.Info(logCtx, "deleting stale session state", + logging.Debug(logCtx, "deleting stale session state", slog.String("session_id", sessionID), ) _ = ClearSessionState(sessionID) //nolint:errcheck // best-effort cleanup of stale session From 398a6e420cd4c8373454b67b5e479a3f64081de4 Mon Sep 17 00:00:00 2001 From: paul <423357+toothbrush@users.noreply.github.com> Date: Fri, 20 Feb 2026 13:42:26 +0930 Subject: [PATCH 5/8] Revert "Let's make it 12h." This reverts commit 738a41c8fcc9e5bcd4d908483c9c02370953e01b. --- cmd/entire/cli/session/state.go | 2 +- cmd/entire/cli/session/state_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/session/state.go b/cmd/entire/cli/session/state.go index f7ce8a61f..91424067b 100644 --- a/cmd/entire/cli/session/state.go +++ b/cmd/entire/cli/session/state.go @@ -24,7 +24,7 @@ const ( // StaleSessionThreshold is the duration after which an ended session is considered stale // and will be automatically deleted during load/list operations. - StaleSessionThreshold = 12 * time.Hour + StaleSessionThreshold = 24 * time.Hour ) // State represents the state of an active session. diff --git a/cmd/entire/cli/session/state_test.go b/cmd/entire/cli/session/state_test.go index a8f92a9cc..1465fb184 100644 --- a/cmd/entire/cli/session/state_test.go +++ b/cmd/entire/cli/session/state_test.go @@ -139,7 +139,7 @@ func TestState_IsStale(t *testing.T) { assert.False(t, state.IsStale()) }) - t.Run("ended_25h_ago_is_stale", func(t *testing.T) { + t.Run("ended_over_24h_ago_is_stale", func(t *testing.T) { t.Parallel() old := time.Now().Add(-25 * time.Hour) state := &State{EndedAt: &old} From 8f9fadec675e8ac8b0b2e058611e37527e50e9ea Mon Sep 17 00:00:00 2001 From: paul <423357+toothbrush@users.noreply.github.com> Date: Fri, 20 Feb 2026 13:44:55 +0930 Subject: [PATCH 6/8] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- cmd/entire/cli/session/state_test.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/cmd/entire/cli/session/state_test.go b/cmd/entire/cli/session/state_test.go index 1465fb184..75fd96e4b 100644 --- a/cmd/entire/cli/session/state_test.go +++ b/cmd/entire/cli/session/state_test.go @@ -146,11 +146,12 @@ func TestState_IsStale(t *testing.T) { assert.True(t, state.IsStale()) }) - t.Run("ended_exactly_at_threshold_is_not_stale", func(t *testing.T) { + t.Run("ended_just_under_threshold_is_not_stale", func(t *testing.T) { t.Parallel() - // time.Since will be slightly over threshold due to execution time, - // but a value well within threshold should not be stale - recent := time.Now().Add(-23 * time.Hour) + // A session that ended just under the staleness threshold should not be stale. + // Use StaleSessionThreshold rather than a magic number so the test stays in sync + // if the threshold changes. + recent := time.Now().Add(-1 * (StaleSessionThreshold - time.Hour)) state := &State{EndedAt: &recent} assert.False(t, state.IsStale()) }) From 37ea9662f2366387a2610f861a7cc922e6435a56 Mon Sep 17 00:00:00 2001 From: paul <423357+toothbrush@users.noreply.github.com> Date: Fri, 20 Feb 2026 13:43:55 +0930 Subject: [PATCH 7/8] Context is now used. --- cmd/entire/cli/session/state.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/cmd/entire/cli/session/state.go b/cmd/entire/cli/session/state.go index 91424067b..f7b883b80 100644 --- a/cmd/entire/cli/session/state.go +++ b/cmd/entire/cli/session/state.go @@ -336,8 +336,6 @@ func (s *StateStore) RemoveAll() error { // List returns all session states. func (s *StateStore) List(ctx context.Context) ([]*State, error) { - _ = ctx // Reserved for future use - entries, err := os.ReadDir(s.stateDir) if os.IsNotExist(err) { return nil, nil From 0338a155965724b552052c1c297b9427101b6ade Mon Sep 17 00:00:00 2001 From: paul <423357+toothbrush@users.noreply.github.com> Date: Fri, 20 Feb 2026 14:23:27 +0930 Subject: [PATCH 8/8] Move stale session cleanup from callers into StateStore.Load() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, stale session detection was inconsistent: the package-level LoadSessionState() in strategy/ had its own stale check + file deletion, and StateStore.List() had a separate stale check, but StateStore.Load() did not. This meant ManualCommitStrategy.loadSessionState() — which calls StateStore.Load() directly and is used extensively in hooks, condensation, rewind, and git operations — never cleaned up stale sessions. Fix by adding the stale session check to StateStore.Load() itself, so all callers automatically benefit. Then: - Remove the duplicate stale check from StateStore.List() (it calls Load, which now handles it) - Simplify strategy.LoadSessionState() to delegate to StateStore instead of duplicating file I/O, unmarshaling, normalization, and stale checks Co-Authored-By: Claude Opus 4.6 Entire-Checkpoint: aebb716f655b --- cmd/entire/cli/session/state.go | 27 +++++++------- cmd/entire/cli/session/state_test.go | 46 ++++++++++++++++++++++++ cmd/entire/cli/strategy/session_state.go | 38 ++++---------------- 3 files changed, 66 insertions(+), 45 deletions(-) diff --git a/cmd/entire/cli/session/state.go b/cmd/entire/cli/session/state.go index f7b883b80..3a1cbcbc5 100644 --- a/cmd/entire/cli/session/state.go +++ b/cmd/entire/cli/session/state.go @@ -247,10 +247,9 @@ func NewStateStoreWithDir(stateDir string) *StateStore { } // Load loads the session state for the given session ID. -// Returns (nil, nil) when session file doesn't exist (not an error condition). +// Returns (nil, nil) when session file doesn't exist or session is stale (not an error condition). +// Stale sessions (ended longer than StaleSessionThreshold ago) are automatically deleted. func (s *StateStore) Load(ctx context.Context, sessionID string) (*State, error) { - _ = ctx // Reserved for future use - // Validate session ID to prevent path traversal if err := validation.ValidateSessionID(sessionID); err != nil { return nil, fmt.Errorf("invalid session ID: %w", err) @@ -271,6 +270,16 @@ func (s *StateStore) Load(ctx context.Context, sessionID string) (*State, error) return nil, fmt.Errorf("failed to unmarshal session state: %w", err) } state.NormalizeAfterLoad() + + if state.IsStale() { + logCtx := logging.WithComponent(ctx, "session") + logging.Debug(logCtx, "deleting stale session state", + slog.String("session_id", sessionID), + ) + _ = s.Clear(ctx, sessionID) //nolint:errcheck // best-effort cleanup of stale session + return nil, nil //nolint:nilnil // stale session treated as not found + } + return &state, nil } @@ -344,8 +353,6 @@ func (s *StateStore) List(ctx context.Context) ([]*State, error) { return nil, fmt.Errorf("failed to read session state directory: %w", err) } - logCtx := logging.WithComponent(ctx, "session") - var states []*State for _, entry := range entries { if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".json") { @@ -361,15 +368,7 @@ func (s *StateStore) List(ctx context.Context) ([]*State, error) { continue // Skip corrupted state files } if state == nil { - continue - } - - if state.IsStale() { - logging.Debug(logCtx, "deleting stale session state", - slog.String("session_id", sessionID), - ) - _ = s.Clear(ctx, sessionID) //nolint:errcheck // best-effort cleanup of stale session - continue + continue // Not found or stale (Load handles cleanup) } states = append(states, state) diff --git a/cmd/entire/cli/session/state_test.go b/cmd/entire/cli/session/state_test.go index 75fd96e4b..80bc31693 100644 --- a/cmd/entire/cli/session/state_test.go +++ b/cmd/entire/cli/session/state_test.go @@ -157,6 +157,52 @@ func TestState_IsStale(t *testing.T) { }) } +func TestStateStore_Load_DeletesStaleSession(t *testing.T) { + t.Parallel() + + stateDir := filepath.Join(t.TempDir(), "entire-sessions") + require.NoError(t, os.MkdirAll(stateDir, 0o750)) + store := NewStateStoreWithDir(stateDir) + ctx := context.Background() + + // Create a stale session (ended >24h ago) + staleEnded := time.Now().Add(-48 * time.Hour) + stale := &State{ + SessionID: "stale-session", + BaseCommit: "def456", + StartedAt: time.Now().Add(-72 * time.Hour), + EndedAt: &staleEnded, + } + require.NoError(t, store.Save(ctx, stale)) + + // Verify file exists before load + stateFile := filepath.Join(stateDir, "stale-session.json") + _, err := os.Stat(stateFile) + require.NoError(t, err, "state file should exist before load") + + // Load should return (nil, nil) for stale session + loaded, err := store.Load(ctx, "stale-session") + require.NoError(t, err, "Load should not return error for stale session") + assert.Nil(t, loaded, "Load should return nil for stale session") + + // File should be deleted from disk + _, err = os.Stat(stateFile) + assert.True(t, os.IsNotExist(err), "stale session file should be deleted after Load") + + // Create an active session (no EndedAt) to verify non-stale sessions still work + active := &State{ + SessionID: "active-session", + BaseCommit: "abc123", + StartedAt: time.Now(), + } + require.NoError(t, store.Save(ctx, active)) + + loaded, err = store.Load(ctx, "active-session") + require.NoError(t, err) + assert.NotNil(t, loaded, "Load should return state for active session") + assert.Equal(t, "active-session", loaded.SessionID) +} + func TestStateStore_List_DeletesStaleSession(t *testing.T) { t.Parallel() diff --git a/cmd/entire/cli/strategy/session_state.go b/cmd/entire/cli/strategy/session_state.go index 7386852ff..707fd490c 100644 --- a/cmd/entire/cli/strategy/session_state.go +++ b/cmd/entire/cli/strategy/session_state.go @@ -2,7 +2,6 @@ package strategy import ( "context" - "encoding/json" "fmt" "log/slog" "os" @@ -37,42 +36,19 @@ func sessionStateFile(sessionID string) (string, error) { } // LoadSessionState loads the session state for the given session ID. -// Returns (nil, nil) when session file doesn't exist (not an error condition). +// Returns (nil, nil) when session file doesn't exist or session is stale (not an error condition). +// Stale sessions are automatically deleted by the underlying StateStore. func LoadSessionState(sessionID string) (*SessionState, error) { - // Validate session ID to prevent path traversal - if err := validation.ValidateSessionID(sessionID); err != nil { - return nil, fmt.Errorf("invalid session ID: %w", err) - } - - stateFile, err := sessionStateFile(sessionID) + store, err := session.NewStateStore() if err != nil { - return nil, fmt.Errorf("failed to get session state file path: %w", err) + return nil, fmt.Errorf("failed to create state store: %w", err) } - data, err := os.ReadFile(stateFile) //nolint:gosec // stateFile is derived from sessionID, not user input - if os.IsNotExist(err) { - return nil, nil //nolint:nilnil // nil,nil indicates session not found (expected case) - } + state, err := store.Load(context.Background(), sessionID) if err != nil { - return nil, fmt.Errorf("failed to read session state: %w", err) - } - - var state SessionState - if err := json.Unmarshal(data, &state); err != nil { - return nil, fmt.Errorf("failed to unmarshal session state: %w", err) + return nil, fmt.Errorf("failed to load session state: %w", err) } - state.NormalizeAfterLoad() - - if state.IsStale() { - logCtx := logging.WithComponent(context.Background(), "session") - logging.Debug(logCtx, "deleting stale session state", - slog.String("session_id", sessionID), - ) - _ = ClearSessionState(sessionID) //nolint:errcheck // best-effort cleanup of stale session - return nil, nil //nolint:nilnil // stale session treated as not found - } - - return &state, nil + return state, nil } // SaveSessionState saves the session state atomically.