Skip to content

Commit 878264a

Browse files
committed
fix: make session termination idempotent to prevent retry failures
1 parent a0f4e56 commit 878264a

File tree

2 files changed

+31
-7
lines changed

2 files changed

+31
-7
lines changed

server/streamable_http.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1048,8 +1048,11 @@ func (s *InsecureStatefulSessionIdManager) Validate(sessionID string) (isTermina
10481048
}
10491049

10501050
func (s *InsecureStatefulSessionIdManager) Terminate(sessionID string) (isNotAllowed bool, err error) {
1051+
if _, exists := s.terminated.Load(sessionID); exists {
1052+
return false, nil
1053+
}
10511054
if _, exists := s.sessions.Load(sessionID); !exists {
1052-
return false, fmt.Errorf("session not found: %s", sessionID)
1055+
return false, nil
10531056
}
10541057
s.terminated.Store(sessionID, true)
10551058
s.sessions.Delete(sessionID)

server/streamable_http_test.go

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1254,16 +1254,37 @@ func TestInsecureStatefulSessionIdManager(t *testing.T) {
12541254
}
12551255
})
12561256

1257-
t.Run("Terminate rejects non-existent session ID", func(t *testing.T) {
1257+
t.Run("Terminate is idempotent for non-existent session ID", func(t *testing.T) {
12581258
manager := &InsecureStatefulSessionIdManager{}
12591259
fakeSessionID := "mcp-session-ffffffff-ffff-ffff-ffff-ffffffffffff"
12601260

1261-
_, err := manager.Terminate(fakeSessionID)
1262-
if err == nil {
1263-
t.Error("Expected error when terminating non-existent session")
1261+
isNotAllowed, err := manager.Terminate(fakeSessionID)
1262+
if err != nil {
1263+
t.Errorf("Expected no error when terminating non-existent session, got: %v", err)
12641264
}
1265-
if !strings.Contains(err.Error(), "session not found") {
1266-
t.Errorf("Expected 'session not found' error, got: %v", err)
1265+
if isNotAllowed {
1266+
t.Error("Expected isNotAllowed to be false")
1267+
}
1268+
})
1269+
1270+
t.Run("Terminate is idempotent for already-terminated session", func(t *testing.T) {
1271+
manager := &InsecureStatefulSessionIdManager{}
1272+
sessionID := manager.Generate()
1273+
1274+
isNotAllowed, err := manager.Terminate(sessionID)
1275+
if err != nil {
1276+
t.Errorf("Expected no error on first termination, got: %v", err)
1277+
}
1278+
if isNotAllowed {
1279+
t.Error("Expected termination to be allowed")
1280+
}
1281+
1282+
isNotAllowed, err = manager.Terminate(sessionID)
1283+
if err != nil {
1284+
t.Errorf("Expected no error on second termination (idempotent), got: %v", err)
1285+
}
1286+
if isNotAllowed {
1287+
t.Error("Expected termination to be allowed on retry")
12671288
}
12681289
})
12691290

0 commit comments

Comments
 (0)