From e52a15f60026ca76fa9b66eb6280006a75437650 Mon Sep 17 00:00:00 2001
From: Gwendal Leclerc <gwendal.leclerc@gmail.com>
Date: Sun, 19 Jul 2026 20:42:12 +0200
Subject: [PATCH] fix(server): create the default session atomically (no
 duplicate "Session #1")
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

GetLastSession auto-created a session when the list was empty but released the
lock between the empty-check and the creation (and NewSession computed the
"Session #N" name outside the lock). Concurrent requests against an empty
session list — e.g. the History page's parallel GETs right after "Reset
Sessions" — could each create one, yielding two "Session #1" (the flaky
"rename a session" e2e test).

Move the append into an unlocked newSessionLocked helper used within a single
critical section by both NewSession and GetLastSession. Add a -race regression
test hammering GetLastSession on an empty service.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
---
 server/services/mocks.go      | 25 ++++++++++++++-----------
 server/services/mocks_test.go | 26 ++++++++++++++++++++++++++
 2 files changed, 40 insertions(+), 11 deletions(-)

diff --git a/server/services/mocks.go b/server/services/mocks.go
index c762cf5..fd590ac 100644
--- a/server/services/mocks.go
+++ b/server/services/mocks.go
@@ -264,6 +264,16 @@ func (s *mocks) GetHistoryByPath(sessionID, filterPath string) (types.History, e
 }
 
 func (s *mocks) NewSession(name string) *types.Session {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.newSessionLocked(name)
+}
+
+// newSessionLocked appends a new session and returns it. The caller MUST hold s.mu: the default
+// name and the carried-over locked mocks are both derived from s.sessions, so the whole thing has
+// to happen in one critical section — otherwise two concurrent creators can read the same length
+// and both mint e.g. "Session #1" (the bug behind the flaky rename e2e test).
+func (s *mocks) newSessionLocked(name string) *types.Session {
 	if strings.TrimSpace(name) == "" {
 		name = fmt.Sprintf("Session #%d", len(s.sessions)+1)
 	}
@@ -275,21 +285,16 @@ func (s *mocks) NewSession(name string) *types.Session {
 		history = types.History{}
 	}
 
+	// Carry over the locked mocks from the current last session, reset for the new one.
 	mocks := types.Mocks{}
 	if len(s.sessions) > 0 {
-		session := s.GetLastSession()
-		s.mu.Lock()
-		for _, mock := range session.Mocks {
+		for _, mock := range s.sessions[len(s.sessions)-1].Mocks {
 			if mock.State.Locked {
 				mocks = append(mocks, mock.CloneAndReset())
 			}
 		}
-		s.mu.Unlock()
 	}
 
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
 	session := &types.Session{
 		ID:      types.NewID(),
 		Name:    name,
@@ -346,12 +351,10 @@ func (s *mocks) DeleteSession(id string) error {
 
 func (s *mocks) GetLastSession() *types.Session {
 	s.mu.Lock()
+	defer s.mu.Unlock()
 	if len(s.sessions) == 0 {
-		s.mu.Unlock()
-		s.NewSession("")
-		s.mu.Lock()
+		return s.newSessionLocked("").Clone()
 	}
-	defer s.mu.Unlock()
 	return s.sessions[len(s.sessions)-1].Clone()
 }
 
diff --git a/server/services/mocks_test.go b/server/services/mocks_test.go
index 55d4bd7..dc5f79f 100644
--- a/server/services/mocks_test.go
+++ b/server/services/mocks_test.go
@@ -1,12 +1,38 @@
 package services
 
 import (
+	"sync"
 	"testing"
 
 	"github.com/smocker-dev/smocker/server/types"
 	"gopkg.in/yaml.v3"
 )
 
+// TestGetLastSessionConcurrent guards the TOCTOU race that let concurrent callers each create a
+// default session (two "Session #1") when the list was empty — the source of the flaky "rename a
+// session" e2e test. Run under -race.
+func TestGetLastSessionConcurrent(t *testing.T) {
+	svc := NewMocks(nil, 0, NewPersistence(""))
+
+	const n = 50
+	start := make(chan struct{})
+	var wg sync.WaitGroup
+	wg.Add(n)
+	for i := 0; i < n; i++ {
+		go func() {
+			defer wg.Done()
+			<-start
+			_ = svc.GetLastSession()
+		}()
+	}
+	close(start) // release all goroutines at once to maximize contention
+	wg.Wait()
+
+	if got := len(svc.GetSessions()); got != 1 {
+		t.Fatalf("concurrent GetLastSession on an empty service created %d sessions, want 1", got)
+	}
+}
+
 func mockFromYAML(t *testing.T, s string) *types.Mock {
 	t.Helper()
 	var m types.Mock