From 7115e7e00c4de2760e658b98ec31a8652be6a50d Mon Sep 17 00:00:00 2001
From: Supra4E8C <tan13318527080@163.com>
Date: Fri, 10 Jul 2026 16:18:41 +0800
Subject: [PATCH] fix(oauth): make session completion idempotent

---
 .../api/handlers/management/oauth_sessions.go |  4 +-
 .../management/oauth_sessions_test.go         | 47 +++++++++++++++++++
 2 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/internal/api/handlers/management/oauth_sessions.go b/internal/api/handlers/management/oauth_sessions.go
index 32c4463e..f24eb4eb 100644
--- a/internal/api/handlers/management/oauth_sessions.go
+++ b/internal/api/handlers/management/oauth_sessions.go
@@ -155,7 +155,7 @@ func (s *oauthSessionStore) Complete(state string) {
 
 	s.purgeExpiredLocked(now)
 	session, ok := s.sessions[state]
-	if !ok {
+	if !ok || session.Completed {
 		return
 	}
 	session.Status = ""
@@ -179,7 +179,7 @@ func (s *oauthSessionStore) CompleteProvider(provider string, source string) int
 	s.purgeExpiredLocked(now)
 	removed := 0
 	for state, session := range s.sessions {
-		if strings.EqualFold(session.Provider, provider) && (source == "" || session.Source == source) {
+		if !session.Completed && strings.EqualFold(session.Provider, provider) && (source == "" || session.Source == source) {
 			session.Status = ""
 			session.Metadata = nil
 			session.Completed = true
diff --git a/internal/api/handlers/management/oauth_sessions_test.go b/internal/api/handlers/management/oauth_sessions_test.go
index 17ebdab1..3e70541b 100644
--- a/internal/api/handlers/management/oauth_sessions_test.go
+++ b/internal/api/handlers/management/oauth_sessions_test.go
@@ -26,6 +26,53 @@ func TestOAuthSessionStoreCompleteKeepsShortLivedSession(t *testing.T) {
 	}
 }
 
+func TestOAuthSessionStoreCompleteDoesNotExtendCompletedSession(t *testing.T) {
+	store := newOAuthSessionStore(time.Minute)
+	store.Register("completed-state", "codex")
+	store.Complete("completed-state")
+	before, ok := store.Get("completed-state")
+	if !ok {
+		t.Fatal("completed OAuth session tombstone is missing")
+	}
+
+	store.completedTTL = 2 * time.Minute
+	store.Complete("completed-state")
+	after, ok := store.Get("completed-state")
+	if !ok {
+		t.Fatal("completed OAuth session tombstone is missing after repeated completion")
+	}
+	if !after.ExpiresAt.Equal(before.ExpiresAt) {
+		t.Fatalf("repeated completion extended expiry from %s to %s", before.ExpiresAt, after.ExpiresAt)
+	}
+}
+
+func TestOAuthSessionStoreCompleteProviderSkipsCompletedSessions(t *testing.T) {
+	store := newOAuthSessionStore(time.Minute)
+	store.Register("completed-state", "codex")
+	store.Register("pending-state", "codex")
+	store.Complete("completed-state")
+	completedBefore, ok := store.Get("completed-state")
+	if !ok {
+		t.Fatal("completed OAuth session tombstone is missing")
+	}
+
+	store.completedTTL = 2 * time.Minute
+	if got := store.CompleteProvider("codex", oauthSessionSourceBuiltin); got != 1 {
+		t.Fatalf("CompleteProvider() = %d, want 1 newly completed session", got)
+	}
+	completedAfter, ok := store.Get("completed-state")
+	if !ok {
+		t.Fatal("completed OAuth session tombstone is missing after provider completion")
+	}
+	if !completedAfter.ExpiresAt.Equal(completedBefore.ExpiresAt) {
+		t.Fatalf("provider completion extended existing tombstone from %s to %s", completedBefore.ExpiresAt, completedAfter.ExpiresAt)
+	}
+	pendingAfter, ok := store.Get("pending-state")
+	if !ok || !pendingAfter.Completed {
+		t.Fatalf("pending session completed/ok = %t/%t, want true/true", pendingAfter.Completed, ok)
+	}
+}
+
 func TestGetOAuthSessionHidesCompletedSession(t *testing.T) {
 	store := newOAuthSessionStore(time.Minute)
 	replaceOAuthSessionStoreForTest(t, store)