Clean stale session files

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 = 7 * 24 * time.Hour
 )
 
 // State represents the state of an active session.
@@ -205,6 +209,13 @@ func (s *State) NormalizeAfterLoad() {
 	}
 }
 
+// IsStale returns true when the last time a session saw interaction exceeds StaleSessionThreshold.
+// If LastInteractionTime isn't set, we don't consider a session stale to avoid aggressively
+// deleting things.
+func (s *State) IsStale() bool {
+	return s.LastInteractionTime != nil && time.Since(*s.LastInteractionTime) > 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
@@ -237,10 +248,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)
@@ -261,6 +271,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
 }
 
@@ -326,8 +346,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
@@ -351,7 +369,7 @@ func (s *StateStore) List(ctx context.Context) ([]*State, error) {
 			continue // Skip corrupted state files
 		}
 		if state == nil {
-			continue
+			continue // Not found or stale (Load handles cleanup)
 		}
 
 		states = append(states, state)

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,124 @@ func TestState_NormalizeAfterLoad_JSONRoundTrip(t *testing.T) {
 		})
 	}
 }
+
+func TestState_IsStale(t *testing.T) {
+	t.Parallel()
+
+	t.Run("nil_LastInteractionTime_is_not_stale", func(t *testing.T) {
+		t.Parallel()
+		state := &State{LastInteractionTime: nil}
+		assert.False(t, state.IsStale())
+	})
+
+	t.Run("recently_interacted_is_not_stale", func(t *testing.T) {
+		t.Parallel()
+		recent := time.Now().Add(-1 * time.Hour)
+		state := &State{LastInteractionTime: &recent}
+		assert.False(t, state.IsStale())
+	})
+
+	t.Run("ended_over_2wk_ago_is_stale", func(t *testing.T) {
+		t.Parallel()
+		old := time.Now().Add(-14 * 24 * time.Hour)
+		state := &State{LastInteractionTime: &old}
+		assert.True(t, state.IsStale())
+	})
+
+	t.Run("ended_just_under_threshold_is_not_stale", func(t *testing.T) {
+		t.Parallel()
+		// 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{LastInteractionTime: &recent}
+		assert.False(t, state.IsStale())
+	})
+}
+
+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 >1wk ago)
+	staleInteracted := time.Now().Add(-2 * 7 * 24 * time.Hour)
+	stale := &State{
+		SessionID:           "stale-session",
+		BaseCommit:          "def456",
+		StartedAt:           time.Now().Add(-3 * 7 * 24 * time.Hour),
+		LastInteractionTime: &staleInteracted,
+	}
+	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 LastInteractionTime) 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()
+
+	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 LastInteractionTime)
+	active := &State{
+		SessionID:  "active-session",
+		BaseCommit: "abc123",
+		StartedAt:  time.Now(),
+	}
+	require.NoError(t, store.Save(ctx, active))
+
+	// Create a stale session (ended >2wk ago)
+	staleInteracted := time.Now().Add(-2 * 7 * 24 * time.Hour)
+	stale := &State{
+		SessionID:           "stale-session",
+		BaseCommit:          "def456",
+		StartedAt:           time.Now().Add(-3 * 7 * 24 * time.Hour),
+		LastInteractionTime: &staleInteracted,
+	}
+	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")
+}

cmd/entire/cli/strategy/session_state.go

@@ -2,7 +2,6 @@ package strategy
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"log/slog"
 	"os"
@@ -37,32 +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()
-	return &state, nil
+	return state, nil
 }
 
 // SaveSessionState saves the session state atomically.

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 >2wk ago)
+	staleInteracted := time.Now().Add(-2 * 7 * 24 * time.Hour)
+	state := &SessionState{
+		SessionID:           "stale-load-test",
+		BaseCommit:          "abc123def456",
+		StartedAt:           time.Now().Add(-3 * 7 * 24 * time.Hour),
+		LastInteractionTime: &staleInteracted,
+		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()")
+	}
+}