|
9 | 9 | "os" |
10 | 10 | "strings" |
11 | 11 | "testing" |
| 12 | + "time" |
12 | 13 |
|
13 | 14 | "github.com/github/gh-aw-mcpg/internal/config" |
14 | 15 | "github.com/github/gh-aw-mcpg/internal/difc" |
@@ -41,7 +42,7 @@ func TestHTTPRequest_SessionIDHeader(t *testing.T) { |
41 | 42 | // Create an HTTP connection |
42 | 43 | conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{ |
43 | 44 | "Authorization": "test-auth-token", |
44 | | - }, nil, "") |
| 45 | + }, nil, "", 0) |
45 | 46 | require.NoError(t, err, "Failed to create HTTP connection") |
46 | 47 |
|
47 | 48 | // Create a context with session ID |
@@ -78,7 +79,7 @@ func TestHTTPRequest_NoSessionID(t *testing.T) { |
78 | 79 | // Create an HTTP connection |
79 | 80 | conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{ |
80 | 81 | "Authorization": "test-auth-token", |
81 | | - }, nil, "") |
| 82 | + }, nil, "", 0) |
82 | 83 | require.NoError(t, err, "Failed to create HTTP connection") |
83 | 84 |
|
84 | 85 | // Send a request without session ID in context |
@@ -116,7 +117,7 @@ func TestHTTPRequest_ConfiguredHeaders(t *testing.T) { |
116 | 117 | authToken := "configured-auth-token" |
117 | 118 | conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{ |
118 | 119 | "Authorization": authToken, |
119 | | - }, nil, "") |
| 120 | + }, nil, "", 0) |
120 | 121 | require.NoError(t, err, "Failed to create HTTP connection") |
121 | 122 |
|
122 | 123 | // Create a context with session ID |
@@ -375,7 +376,7 @@ func TestHTTPRequest_ErrorResponses(t *testing.T) { |
375 | 376 | // Create connection with custom headers to use plain JSON transport |
376 | 377 | conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{ |
377 | 378 | "Authorization": "test-token", |
378 | | - }, nil, "") |
| 379 | + }, nil, "", 0) |
379 | 380 | if err != nil && tt.expectError { |
380 | 381 | // Error during initialization is expected for some error conditions |
381 | 382 | if tt.errorSubstring != "" && !containsSubstring(err.Error(), tt.errorSubstring) { |
@@ -427,7 +428,7 @@ func TestConnection_IsHTTP(t *testing.T) { |
427 | 428 | "X-Custom": "custom-value", |
428 | 429 | } |
429 | 430 |
|
430 | | - conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, headers, nil, "") |
| 431 | + conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, headers, nil, "", 0) |
431 | 432 | require.NoError(t, err, "Failed to create HTTP connection") |
432 | 433 | defer conn.Close() |
433 | 434 |
|
@@ -474,7 +475,7 @@ func TestHTTPConnection_InvalidURL(t *testing.T) { |
474 | 475 |
|
475 | 476 | for _, tt := range tests { |
476 | 477 | t.Run(tt.name, func(t *testing.T) { |
477 | | - _, err := NewHTTPConnection(context.Background(), "test-server", tt.url, tt.headers, nil, "") |
| 478 | + _, err := NewHTTPConnection(context.Background(), "test-server", tt.url, tt.headers, nil, "", 0) |
478 | 479 |
|
479 | 480 | if tt.expectError { |
480 | 481 | if err == nil { |
@@ -507,17 +508,54 @@ func stringContains(s, substr string) bool { |
507 | 508 |
|
508 | 509 | // TestNewMCPClient tests the newMCPClient helper function |
509 | 510 | func TestNewMCPClient(t *testing.T) { |
510 | | - client := newMCPClient(nil) |
| 511 | + client := newMCPClient(nil, 0) |
511 | 512 | require.NotNil(t, client, "newMCPClient should return a non-nil client") |
512 | 513 | } |
513 | 514 |
|
514 | 515 | // TestNewMCPClientWithLogger tests that newMCPClient accepts a logger |
515 | 516 | func TestNewMCPClientWithLogger(t *testing.T) { |
516 | 517 | log := logger.New("test:client") |
517 | | - client := newMCPClient(log) |
| 518 | + client := newMCPClient(log, 0) |
518 | 519 | require.NotNil(t, client, "newMCPClient should return a non-nil client with logger") |
519 | 520 | } |
520 | 521 |
|
| 522 | +// TestNewMCPClientWithKeepalive tests that newMCPClient accepts a keepalive interval |
| 523 | +func TestNewMCPClientWithKeepalive(t *testing.T) { |
| 524 | + keepAlive := time.Duration(config.DefaultKeepaliveInterval) * time.Second |
| 525 | + client := newMCPClient(nil, keepAlive) |
| 526 | + require.NotNil(t, client, "newMCPClient should return a non-nil client with keepalive") |
| 527 | +} |
| 528 | + |
| 529 | +// TestDefaultKeepaliveInterval verifies the config keepalive default is less than a typical |
| 530 | +// backend session timeout (30 minutes) to prevent session expiry during long agent runs. |
| 531 | +func TestDefaultKeepaliveInterval(t *testing.T) { |
| 532 | + const typicalBackendTimeout = 30 * time.Minute |
| 533 | + keepAlive := time.Duration(config.DefaultKeepaliveInterval) * time.Second |
| 534 | + assert.Less(t, keepAlive, typicalBackendTimeout, |
| 535 | + "DefaultKeepaliveInterval must be less than the typical backend session timeout to prevent expiry") |
| 536 | + assert.Greater(t, keepAlive, time.Duration(0), |
| 537 | + "DefaultKeepaliveInterval must be positive") |
| 538 | +} |
| 539 | + |
| 540 | +// TestNewHTTPConnectionStoresKeepalive verifies that the keepalive interval is stored on |
| 541 | +// the connection struct so that reconnectSDKTransport can recreate the session with the same setting. |
| 542 | +func TestNewHTTPConnectionStoresKeepalive(t *testing.T) { |
| 543 | + ctx, cancel := context.WithCancel(context.Background()) |
| 544 | + defer cancel() |
| 545 | + |
| 546 | + keepAlive := time.Duration(config.DefaultKeepaliveInterval) * time.Second |
| 547 | + client := newMCPClient(nil, keepAlive) |
| 548 | + url := "http://example.com/mcp" |
| 549 | + headers := map[string]string{} |
| 550 | + httpClient := &http.Client{} |
| 551 | + |
| 552 | + conn := newHTTPConnection(ctx, cancel, client, nil, url, headers, httpClient, HTTPTransportStreamable, "test-server", keepAlive) |
| 553 | + |
| 554 | + require.NotNil(t, conn) |
| 555 | + assert.Equal(t, keepAlive, conn.keepAliveInterval, |
| 556 | + "keepAliveInterval should be stored on the connection for use during reconnection") |
| 557 | +} |
| 558 | + |
521 | 559 | // TestSetupHTTPRequest tests the setupHTTPRequest helper function |
522 | 560 | func TestSetupHTTPRequest(t *testing.T) { |
523 | 561 | tests := []struct { |
@@ -593,12 +631,12 @@ func TestNewHTTPConnection(t *testing.T) { |
593 | 631 | ctx, cancel := context.WithCancel(context.Background()) |
594 | 632 | defer cancel() |
595 | 633 |
|
596 | | - client := newMCPClient(nil) |
| 634 | + client := newMCPClient(nil, 0) |
597 | 635 | url := "http://example.com/mcp" |
598 | 636 | headers := map[string]string{"Authorization": "test"} |
599 | 637 | httpClient := &http.Client{} |
600 | 638 |
|
601 | | - conn := newHTTPConnection(ctx, cancel, client, nil, url, headers, httpClient, HTTPTransportStreamable, "test-server") |
| 639 | + conn := newHTTPConnection(ctx, cancel, client, nil, url, headers, httpClient, HTTPTransportStreamable, "test-server", 0) |
602 | 640 |
|
603 | 641 | require.NotNil(t, conn, "Connection should not be nil") |
604 | 642 | assert.Equal(t, client, conn.client, "Client should match") |
|
0 commit comments