Skip to content

Commit b0991b8

Browse files
committed
fix: external session ids in sse
1 parent 96d5157 commit b0991b8

File tree

3 files changed

+124
-9
lines changed

3 files changed

+124
-9
lines changed

kc/manager_test.go

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,6 +493,44 @@ func TestNewConfigConstructor(t *testing.T) {
493493
})
494494
}
495495

496+
// TestExternalSessionIDFromErrorLog tests the exact session ID from the error log
497+
func TestExternalSessionIDFromErrorLog(t *testing.T) {
498+
manager, err := newTestManager("test_key", "test_secret")
499+
if err != nil {
500+
t.Fatalf("Expected no error creating manager, got: %v", err)
501+
}
502+
503+
// This is the exact session ID from the error log that was failing
504+
externalSessionID := "6f615000-2644-45a7-a27c-f579e20b5992"
505+
506+
// Should be able to get or create session with external session ID
507+
kiteSession, isNew, err := manager.GetOrCreateSession(externalSessionID)
508+
if err != nil {
509+
t.Errorf("Expected no error for external session ID from error log, got: %v", err)
510+
}
511+
if !isNew {
512+
t.Error("Expected new session to be created for external session ID")
513+
}
514+
if kiteSession == nil {
515+
t.Error("Expected non-nil Kite session data")
516+
}
517+
if kiteSession.Kite == nil {
518+
t.Error("Expected Kite client to be initialized")
519+
}
520+
521+
// Subsequent call should reuse existing session
522+
kiteSession2, isNew2, err2 := manager.GetOrCreateSession(externalSessionID)
523+
if err2 != nil {
524+
t.Errorf("Expected no error on second call, got: %v", err2)
525+
}
526+
if isNew2 {
527+
t.Error("Expected existing session to be reused")
528+
}
529+
if kiteSession2 != kiteSession {
530+
t.Error("Expected same session instance to be returned")
531+
}
532+
}
533+
496534
// Helper function to check if string contains substring
497535
func managerContains(s, substr string) bool {
498536
if len(substr) > len(s) {

kc/session.go

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -100,11 +100,19 @@ func (sm *SessionRegistry) GenerateWithData(data any) string {
100100
}
101101

102102
// should be a valid uuid and start with the correct prefix.
103+
// checkSessionID validates the format of a MCP session ID
104+
// Accepts both internal format (kitemcp-<uuid>) and external format (plain uuid)
103105
func checkSessionID(sessionID string) error {
104-
if !strings.HasPrefix(sessionID, mcpSessionPrefix) {
105-
return fmt.Errorf("%s", errInvalidSessionIDFormat)
106+
// Handle internal format with prefix
107+
if strings.HasPrefix(sessionID, mcpSessionPrefix) {
108+
if _, err := uuid.Parse(sessionID[len(mcpSessionPrefix):]); err != nil {
109+
return fmt.Errorf("%s: %w", errInvalidSessionIDFormat, err)
110+
}
111+
return nil
106112
}
107-
if _, err := uuid.Parse(sessionID[len(mcpSessionPrefix):]); err != nil {
113+
114+
// Handle external format (plain UUID from SSE/stdio modes)
115+
if _, err := uuid.Parse(sessionID); err != nil {
108116
return fmt.Errorf("%s: %w", errInvalidSessionIDFormat, err)
109117
}
110118
return nil
@@ -350,8 +358,20 @@ func (sm *SessionRegistry) GetOrCreateSessionData(sessionID string, createDataFn
350358

351359
session, exists := sm.sessions[sessionID]
352360
if !exists {
353-
sm.logger.Warn("Session not found for ID", "session_id", sessionID)
354-
return nil, false, errors.New(errSessionNotFound)
361+
// Create a new session for external session IDs (from SSE/stdio modes)
362+
sm.logger.Info("Creating new session for external session ID", "session_id", sessionID)
363+
now := time.Now()
364+
expiresAt := now.Add(sm.sessionDuration)
365+
366+
session = &MCPSession{
367+
ID: sessionID,
368+
Terminated: false,
369+
CreatedAt: now,
370+
ExpiresAt: expiresAt,
371+
Data: nil,
372+
}
373+
sm.sessions[sessionID] = session
374+
exists = true
355375
}
356376

357377
now := time.Now()

kc/session_test.go

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -599,18 +599,75 @@ func TestSessionExpiration(t *testing.T) {
599599
t.Errorf("Expected no error for new session, got: %v", err)
600600
}
601601
if isTerminated {
602-
t.Error("Expected new session to not be terminated")
602+
t.Error("Expected session to not be terminated initially")
603603
}
604604

605-
// Wait for expiration
606-
time.Sleep(2 * time.Millisecond)
605+
// Wait for session to expire
606+
time.Sleep(5 * time.Millisecond)
607607

608608
// Session should now be expired
609609
isTerminated, err = manager.Validate(sessionID)
610610
if err != nil {
611611
t.Errorf("Expected no error for expired session validation, got: %v", err)
612612
}
613613
if !isTerminated {
614-
t.Error("Expected expired session to be terminated")
614+
t.Error("Expected session to be terminated after expiry")
615+
}
616+
}
617+
618+
func TestExternalSessionIDFormat(t *testing.T) {
619+
manager := NewSessionRegistry(testLogger())
620+
621+
// Test external session ID (plain UUID format from SSE/stdio modes)
622+
externalSessionID := "6f615000-2644-45a7-a27c-f579e20b5992"
623+
624+
// Should be able to create session data with external session ID
625+
testData := map[string]string{"test": "data"}
626+
data, isNew, err := manager.GetOrCreateSessionData(externalSessionID, func() any {
627+
return testData
628+
})
629+
630+
if err != nil {
631+
t.Errorf("Expected no error for external session ID, got: %v", err)
632+
}
633+
if !isNew {
634+
t.Error("Expected new session to be created")
635+
}
636+
retrievedData, ok := data.(map[string]string)
637+
if !ok {
638+
t.Errorf("Expected data to be map[string]string, got: %T", data)
639+
}
640+
if retrievedData["test"] != "data" {
641+
t.Errorf("Expected data['test'] to be 'data', got: %v", retrievedData["test"])
642+
}
643+
644+
// Should be able to validate external session ID
645+
isTerminated, err := manager.Validate(externalSessionID)
646+
if err != nil {
647+
t.Errorf("Expected no error validating external session ID, got: %v", err)
648+
}
649+
if isTerminated {
650+
t.Error("Expected external session to not be terminated")
651+
}
652+
653+
// Test internal session ID format still works
654+
internalSessionID := manager.Generate()
655+
656+
data2, isNew2, err2 := manager.GetOrCreateSessionData(internalSessionID, func() any {
657+
return testData
658+
})
659+
660+
if err2 != nil {
661+
t.Errorf("Expected no error for internal session ID, got: %v", err2)
662+
}
663+
if !isNew2 {
664+
t.Error("Expected new internal session to be created")
665+
}
666+
retrievedData2, ok2 := data2.(map[string]string)
667+
if !ok2 {
668+
t.Errorf("Expected internal data to be map[string]string, got: %T", data2)
669+
}
670+
if retrievedData2["test"] != "data" {
671+
t.Errorf("Expected internal data['test'] to be 'data', got: %v", retrievedData2["test"])
615672
}
616673
}

0 commit comments

Comments
 (0)