From ab438d453c5164550d04aca137f3e4d99bfe4c13 Mon Sep 17 00:00:00 2001 From: mingmxren Date: Sun, 1 Mar 2026 12:12:51 +0800 Subject: [PATCH] fix(session): persist new session before index mutation --- pkg/session/manager.go | 52 +++++++++++++++++++++++++++++++------ pkg/session/manager_test.go | 27 +++++++++++++++++++ 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/pkg/session/manager.go b/pkg/session/manager.go index ef69c530c..974ce3fb6 100644 --- a/pkg/session/manager.go +++ b/pkg/session/manager.go @@ -89,10 +89,27 @@ func (sm *SessionManager) StartNew(scopeKey string) (string, error) { defer sm.mu.Unlock() now := time.Now() - scope, _ := sm.ensureScopeLocked(scopeKey, now) + prevScopeEntryExists := false + prevScopeEntryWasNil := false + var prevScopeSnapshot *scopeIndex + if sm.index.Scopes != nil { + if existingScope, ok := sm.index.Scopes[scopeKey]; ok { + prevScopeEntryExists = true + if existingScope == nil { + prevScopeEntryWasNil = true + } else { + prevScopeSnapshot = cloneScopeIndex(existingScope) + } + } + } + + orderedForOrdinal := []string{scopeKey} + if prevScopeSnapshot != nil && len(prevScopeSnapshot.OrderedSessions) > 0 { + orderedForOrdinal = prevScopeSnapshot.OrderedSessions + } newOrdinal := 2 - for _, existing := range scope.OrderedSessions { + for _, existing := range orderedForOrdinal { ordinal, ok := sessionOrdinal(scopeKey, existing) if !ok { continue @@ -121,16 +138,23 @@ func (sm *SessionManager) StartNew(scopeKey string) (string, error) { return "", err } - prevActive := scope.ActiveSessionKey - prevOrdered := append([]string(nil), scope.OrderedSessions...) - prevUpdated := scope.UpdatedAt + scope, _ := sm.ensureScopeLocked(scopeKey, now) scope.ActiveSessionKey = newSessionKey scope.OrderedSessions = prependSessionUnique(scope.OrderedSessions, newSessionKey) scope.UpdatedAt = now if err := sm.saveIndexLocked(); err != nil { - scope.ActiveSessionKey = prevActive - scope.OrderedSessions = prevOrdered - scope.UpdatedAt = prevUpdated + if prevScopeEntryExists { + if sm.index.Scopes == nil { + sm.index.Scopes = make(map[string]*scopeIndex) + } + if prevScopeEntryWasNil { + sm.index.Scopes[scopeKey] = nil + } else { + sm.index.Scopes[scopeKey] = cloneScopeIndex(prevScopeSnapshot) + } + } else if sm.index.Scopes != nil { + delete(sm.index.Scopes, scopeKey) + } if created { delete(sm.sessions, newSessionKey) _ = sm.deleteSessionFile(newSessionKey) @@ -625,6 +649,18 @@ func prependSessionUnique(ordered []string, sessionKey string) []string { return next } +func cloneScopeIndex(scope *scopeIndex) *scopeIndex { + if scope == nil { + return nil + } + cloned := &scopeIndex{ + ActiveSessionKey: scope.ActiveSessionKey, + UpdatedAt: scope.UpdatedAt, + } + cloned.OrderedSessions = append([]string(nil), scope.OrderedSessions...) + return cloned +} + func cloneSession(stored *Session) Session { snapshot := Session{ Key: stored.Key, diff --git a/pkg/session/manager_test.go b/pkg/session/manager_test.go index 9df61cae8..60d87214d 100644 --- a/pkg/session/manager_test.go +++ b/pkg/session/manager_test.go @@ -175,6 +175,33 @@ func TestStartNew_PersistsSessionFileWithoutManualSave(t *testing.T) { } } +func TestStartNew_DoesNotMutateIndexWhenSessionPersistFails(t *testing.T) { + dir := t.TempDir() + sm := NewSessionManager(dir) + scope := "../invalid/scope" + + _, err := sm.StartNew(scope) + if err == nil { + t.Fatalf("expected StartNew to fail for invalid persisted session key") + } + + if _, exists := sm.index.Scopes[scope]; exists { + t.Fatalf("scope %q should not be added to index on session persist failure", scope) + } + + if _, exists := sm.sessions[scope+"#2"]; exists { + t.Fatalf("session %q should not remain in memory on session persist failure", scope+"#2") + } + + files, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read dir %q: %v", dir, err) + } + if len(files) != 0 { + t.Fatalf("storage should stay untouched, found files: %v", files) + } +} + func TestListAndResume_ByScopeOrdinal(t *testing.T) { sm := NewSessionManager(t.TempDir()) scope := "agent:main:telegram:direct:user1"