Skip to content

Commit 97149f0

Browse files
lpcoxCopilot
andcommitted
feat: make HTTP keepalive interval configurable
Add keepalive_interval field to GatewayConfig (seconds). Default 1500 (25 min) preserves current behavior. Set to -1 to disable keepalive pings entirely when higher-level timeouts manage session lifecycle. Thread the configurable duration through NewHTTPConnection → trySDKTransport → newMCPClient instead of hardcoding DefaultHTTPKeepaliveInterval. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 66d9146 commit 97149f0

11 files changed

Lines changed: 81 additions & 57 deletions

internal/config/config_core.go

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,18 @@ import (
2929
"io"
3030
"log"
3131
"os"
32+
"time"
3233

3334
"github.com/BurntSushi/toml"
3435
"github.com/github/gh-aw-mcpg/internal/logger"
3536
)
3637

3738
// Core constants for configuration defaults
3839
const (
39-
DefaultPort = 3000
40-
DefaultStartupTimeout = 60 // seconds
41-
DefaultToolTimeout = 120 // seconds
40+
DefaultPort = 3000
41+
DefaultStartupTimeout = 60 // seconds
42+
DefaultToolTimeout = 120 // seconds
43+
DefaultKeepaliveInterval = 1500 // seconds (25 minutes) — keeps HTTP backend sessions alive
4244
)
4345

4446
// Config represents the internal gateway configuration.
@@ -87,6 +89,13 @@ type GatewayConfig struct {
8789
// ToolTimeout is the maximum time (seconds) to wait for tool execution
8890
ToolTimeout int `toml:"tool_timeout" json:"tool_timeout,omitempty"`
8991

92+
// KeepaliveInterval is the interval (seconds) for sending keepalive pings to HTTP
93+
// backends. This prevents long-running sessions from being expired by the remote
94+
// server's idle timeout (typically 30 minutes). Set to -1 to disable keepalive
95+
// pings entirely (useful when higher-level timeouts manage session lifecycle).
96+
// Default: 1500 (25 minutes)
97+
KeepaliveInterval int `toml:"keepalive_interval" json:"keepalive_interval,omitempty"`
98+
9099
// PayloadDir is the directory for storing large payloads
91100
PayloadDir string `toml:"payload_dir" json:"payload_dir,omitempty"`
92101

@@ -110,6 +119,15 @@ type GatewayConfig struct {
110119
TrustedBots []string `toml:"trusted_bots" json:"trusted_bots,omitempty"`
111120
}
112121

122+
// HTTPKeepaliveInterval returns the keepalive interval as a time.Duration.
123+
// A negative KeepaliveInterval disables keepalive (returns 0).
124+
func (g *GatewayConfig) HTTPKeepaliveInterval() time.Duration {
125+
if g.KeepaliveInterval < 0 {
126+
return 0
127+
}
128+
return time.Duration(g.KeepaliveInterval) * time.Second
129+
}
130+
113131
// GetAPIKey returns the gateway API key, handling a nil Gateway safely.
114132
func (c *Config) GetAPIKey() string {
115133
if c.Gateway == nil {
@@ -196,6 +214,9 @@ func applyGatewayDefaults(cfg *GatewayConfig) {
196214
if cfg.ToolTimeout == 0 {
197215
cfg.ToolTimeout = DefaultToolTimeout
198216
}
217+
if cfg.KeepaliveInterval == 0 {
218+
cfg.KeepaliveInterval = DefaultKeepaliveInterval
219+
}
199220
}
200221

201222
// EnsureGatewayDefaults guarantees that cfg.Gateway is non-nil and that all

internal/launcher/launcher.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,7 @@ func GetOrLaunch(l *Launcher, serverID string) (*mcp.Connection, error) {
139139
}
140140

141141
// Create an HTTP connection
142-
conn, err := mcp.NewHTTPConnection(l.ctx, serverID, serverCfg.URL, serverCfg.Headers, oidcProvider, oidcAudience)
142+
conn, err := mcp.NewHTTPConnection(l.ctx, serverID, serverCfg.URL, serverCfg.Headers, oidcProvider, oidcAudience, l.config.Gateway.HTTPKeepaliveInterval())
143143
if err != nil {
144144
logger.LogErrorWithServer(serverID, "backend", "Failed to create HTTP connection: %s, error=%v", serverID, err)
145145
log.Printf("[LAUNCHER] ❌ FAILED to create HTTP connection for '%s'", serverID)

internal/mcp/connection.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -197,7 +197,7 @@ func NewConnection(ctx context.Context, serverID, command string, args []string,
197197
// Authorization header from the headers map.
198198
//
199199
// This ensures compatibility with all types of HTTP MCP servers.
200-
func NewHTTPConnection(ctx context.Context, serverID, url string, headers map[string]string, oidcProvider *oidc.Provider, oidcAudience string) (*Connection, error) {
200+
func NewHTTPConnection(ctx context.Context, serverID, url string, headers map[string]string, oidcProvider *oidc.Provider, oidcAudience string, keepAlive time.Duration) (*Connection, error) {
201201
logger.LogInfo("backend", "Creating HTTP MCP connection with transport fallback, url=%s", url)
202202
ctx, cancel := context.WithCancel(ctx)
203203

@@ -235,7 +235,7 @@ func NewHTTPConnection(ctx context.Context, serverID, url string, headers map[st
235235

236236
// Try 1: Streamable HTTP (2025-03-26 spec)
237237
logConn.Printf("Attempting streamable HTTP transport for %s", url)
238-
conn, err := tryStreamableHTTPTransport(ctx, cancel, serverID, url, headers, headerClient)
238+
conn, err := tryStreamableHTTPTransport(ctx, cancel, serverID, url, headers, headerClient, keepAlive)
239239
if err == nil {
240240
logger.LogInfo("backend", "Successfully connected using streamable HTTP transport, url=%s", url)
241241
log.Printf("Configured HTTP MCP server with streamable transport: %s", url)
@@ -245,7 +245,7 @@ func NewHTTPConnection(ctx context.Context, serverID, url string, headers map[st
245245

246246
// Try 2: SSE (2024-11-05 spec)
247247
logConn.Printf("Attempting SSE transport for %s", url)
248-
conn, err = trySSETransport(ctx, cancel, serverID, url, headers, headerClient)
248+
conn, err = trySSETransport(ctx, cancel, serverID, url, headers, headerClient, keepAlive)
249249
if err == nil {
250250
logger.LogWarn("backend", "⚠️ MCP over SSE has been deprecated. Connected using SSE transport for url=%s. Please migrate to streamable HTTP transport (2025-03-26 spec).", url)
251251
log.Printf("⚠️ WARNING: MCP over SSE (2024-11-05 spec) has been DEPRECATED")

internal/mcp/connection_arguments_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ func TestCallTool_ArgumentsPassed(t *testing.T) {
159159
// Create connection
160160
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
161161
"Authorization": "test-token",
162-
}, nil, "")
162+
}, nil, "", 0)
163163
require.NoError(t, err, "Failed to create HTTP connection")
164164
defer conn.Close()
165165

@@ -224,7 +224,7 @@ func TestCallTool_MissingArguments(t *testing.T) {
224224

225225
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
226226
"Authorization": "test-token",
227-
}, nil, "")
227+
}, nil, "", 0)
228228
require.NoError(t, err)
229229
defer conn.Close()
230230

internal/mcp/connection_stderr_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func TestConnection_SendRequest(t *testing.T) {
3737

3838
conn, err := NewHTTPConnection(context.Background(), "test-server", srv.URL, map[string]string{
3939
"Authorization": "test-token",
40-
}, nil, "")
40+
}, nil, "", 0)
4141
require.NoError(t, err)
4242
defer conn.Close()
4343

internal/mcp/connection_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ func TestHTTPRequest_SessionIDHeader(t *testing.T) {
4242
// Create an HTTP connection
4343
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
4444
"Authorization": "test-auth-token",
45-
}, nil, "")
45+
}, nil, "", 0)
4646
require.NoError(t, err, "Failed to create HTTP connection")
4747

4848
// Create a context with session ID
@@ -79,7 +79,7 @@ func TestHTTPRequest_NoSessionID(t *testing.T) {
7979
// Create an HTTP connection
8080
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
8181
"Authorization": "test-auth-token",
82-
}, nil, "")
82+
}, nil, "", 0)
8383
require.NoError(t, err, "Failed to create HTTP connection")
8484

8585
// Send a request without session ID in context
@@ -117,7 +117,7 @@ func TestHTTPRequest_ConfiguredHeaders(t *testing.T) {
117117
authToken := "configured-auth-token"
118118
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
119119
"Authorization": authToken,
120-
}, nil, "")
120+
}, nil, "", 0)
121121
require.NoError(t, err, "Failed to create HTTP connection")
122122

123123
// Create a context with session ID
@@ -376,7 +376,7 @@ func TestHTTPRequest_ErrorResponses(t *testing.T) {
376376
// Create connection with custom headers to use plain JSON transport
377377
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
378378
"Authorization": "test-token",
379-
}, nil, "")
379+
}, nil, "", 0)
380380
if err != nil && tt.expectError {
381381
// Error during initialization is expected for some error conditions
382382
if tt.errorSubstring != "" && !containsSubstring(err.Error(), tt.errorSubstring) {
@@ -428,7 +428,7 @@ func TestConnection_IsHTTP(t *testing.T) {
428428
"X-Custom": "custom-value",
429429
}
430430

431-
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, headers, nil, "")
431+
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, headers, nil, "", 0)
432432
require.NoError(t, err, "Failed to create HTTP connection")
433433
defer conn.Close()
434434

@@ -475,7 +475,7 @@ func TestHTTPConnection_InvalidURL(t *testing.T) {
475475

476476
for _, tt := range tests {
477477
t.Run(tt.name, func(t *testing.T) {
478-
_, err := NewHTTPConnection(context.Background(), "test-server", tt.url, tt.headers, nil, "")
478+
_, err := NewHTTPConnection(context.Background(), "test-server", tt.url, tt.headers, nil, "", 0)
479479

480480
if tt.expectError {
481481
if err == nil {

internal/mcp/http_connection_test.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ func TestNewHTTPConnection_WithCustomHeaders(t *testing.T) {
5454
"X-Custom-Header": "custom-value",
5555
}
5656

57-
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, customHeaders, nil, "")
57+
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, customHeaders, nil, "", 0)
5858
require.NoError(err, "Failed to create HTTP connection with custom headers")
5959
require.NotNil(conn, "Connection should not be nil")
6060
defer conn.Close()
@@ -112,7 +112,7 @@ func TestNewHTTPConnection_WithoutHeaders_FallbackSequence(t *testing.T) {
112112
defer testServer.Close()
113113

114114
// Create connection without custom headers - streamable transport should succeed first
115-
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, nil, nil, "")
115+
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, nil, nil, "", 0)
116116
require.NoError(err, "Connection should succeed")
117117
require.NotNil(conn)
118118
defer conn.Close()
@@ -134,7 +134,7 @@ func TestNewHTTPConnection_AllTransportsFail(t *testing.T) {
134134
defer testServer.Close()
135135

136136
// Try to create connection without custom headers (will try all transports)
137-
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, nil, nil, "")
137+
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, nil, nil, "", 0)
138138

139139
// Should fail after trying all transports
140140
require.Error(err, "Should fail when all transports fail")
@@ -161,7 +161,7 @@ func TestNewHTTPConnection_ContextCancellation(t *testing.T) {
161161
cancel()
162162

163163
// Try to create connection with cancelled context
164-
conn, err := NewHTTPConnection(ctx, "test-server", testServer.URL, map[string]string{"Auth": "token"}, nil, "")
164+
conn, err := NewHTTPConnection(ctx, "test-server", testServer.URL, map[string]string{"Auth": "token"}, nil, "", 0)
165165

166166
// Should fail due to context cancellation
167167
require.Error(err, "Should fail with cancelled context")
@@ -198,7 +198,7 @@ func TestNewHTTPConnection_InvalidURL(t *testing.T) {
198198

199199
for _, tt := range tests {
200200
t.Run(tt.name, func(t *testing.T) {
201-
conn, err := NewHTTPConnection(context.Background(), "test-server", tt.url, tt.headers, nil, "")
201+
conn, err := NewHTTPConnection(context.Background(), "test-server", tt.url, tt.headers, nil, "", 0)
202202

203203
if tt.expectError {
204204
assert.Error(t, err, "Expected error for invalid URL")
@@ -273,7 +273,7 @@ func TestTryPlainJSONTransport_InitializeFailure(t *testing.T) {
273273
// Try to create connection
274274
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
275275
"Authorization": "test-token",
276-
}, nil, "")
276+
}, nil, "", 0)
277277

278278
// Should fail with appropriate error
279279
require.Error(t, err, "Should fail on initialization error")
@@ -306,7 +306,7 @@ data: {"jsonrpc":"2.0","id":1,"result":{"protocolVersion":"2024-11-05","serverIn
306306
// Create connection
307307
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
308308
"Authorization": "test-token",
309-
}, nil, "")
309+
}, nil, "", 0)
310310

311311
require.NoError(err, "Should successfully parse SSE-formatted initialize response")
312312
require.NotNil(conn)
@@ -347,7 +347,7 @@ func TestHTTPConnection_NoSessionIDInResponse(t *testing.T) {
347347
// Create connection
348348
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
349349
"Authorization": "test-token",
350-
}, nil, "")
350+
}, nil, "", 0)
351351

352352
require.NoError(err, "Should succeed even without session ID from server")
353353
require.NotNil(conn)
@@ -393,7 +393,7 @@ func TestNewHTTPConnection_HeadersPropagation(t *testing.T) {
393393
"X-Custom-2": "value2",
394394
}
395395

396-
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, customHeaders, nil, "")
396+
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, customHeaders, nil, "", 0)
397397
require.NoError(err)
398398
require.NotNil(conn)
399399
defer conn.Close()
@@ -440,7 +440,7 @@ func TestNewHTTPConnection_EmptyHeaders(t *testing.T) {
440440
defer testServer.Close()
441441

442442
// Create connection with empty headers - should try SDK transports first
443-
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{}, nil, "")
443+
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{}, nil, "", 0)
444444
require.NoError(err, "Should succeed with empty headers")
445445
require.NotNil(conn)
446446
defer conn.Close()
@@ -473,7 +473,7 @@ func TestNewHTTPConnection_NilHeaders(t *testing.T) {
473473
defer testServer.Close()
474474

475475
// Create connection with nil headers (should try SDK transports first)
476-
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, nil, nil, "")
476+
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, nil, nil, "", 0)
477477
require.NoError(err, "Should succeed with nil headers")
478478
require.NotNil(conn)
479479
defer conn.Close()
@@ -509,7 +509,7 @@ func TestNewHTTPConnection_HTTPClientTimeout(t *testing.T) {
509509
// Create connection
510510
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
511511
"Authorization": "test",
512-
}, nil, "")
512+
}, nil, "", 0)
513513

514514
// Should succeed (delay is within timeout)
515515
require.NoError(err, "Should succeed within timeout")
@@ -530,7 +530,7 @@ func TestNewHTTPConnection_ConnectionRefused(t *testing.T) {
530530
// Try to create connection
531531
conn, err := NewHTTPConnection(context.Background(), "test-server", unreachableURL, map[string]string{
532532
"Authorization": "test",
533-
}, nil, "")
533+
}, nil, "", 0)
534534

535535
// Should fail with connection error
536536
require.Error(err, "Should fail with connection refused")
@@ -563,7 +563,7 @@ func TestNewHTTPConnection_GettersAfterCreation(t *testing.T) {
563563
"X-Custom": "custom-value",
564564
}
565565

566-
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, customHeaders, nil, "")
566+
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, customHeaders, nil, "", 0)
567567
require.NoError(err)
568568
require.NotNil(conn)
569569
defer conn.Close()

internal/mcp/http_error_propagation_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ func TestHTTPErrorPropagation_Non200Status(t *testing.T) {
119119
// Create connection with custom headers to use plain JSON transport
120120
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
121121
"Authorization": "test-token",
122-
}, nil, "")
122+
}, nil, "", 0)
123123
require.NoError(t, err, "Failed to create connection")
124124
defer conn.Close()
125125

@@ -192,7 +192,7 @@ func TestHTTPErrorPropagation_JSONRPCError(t *testing.T) {
192192

193193
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
194194
"Authorization": "test-token",
195-
}, nil, "")
195+
}, nil, "", 0)
196196
require.NoError(t, err, "Failed to create connection")
197197
defer conn.Close()
198198

@@ -275,7 +275,7 @@ func TestHTTPErrorPropagation_MixedContent(t *testing.T) {
275275

276276
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
277277
"Authorization": "test-token",
278-
}, nil, "")
278+
}, nil, "", 0)
279279
require.NoError(t, err, "Failed to create connection")
280280
defer conn.Close()
281281

@@ -344,7 +344,7 @@ func TestHTTPErrorPropagation_PreservesDetails(t *testing.T) {
344344

345345
conn, err := NewHTTPConnection(context.Background(), "test-server", testServer.URL, map[string]string{
346346
"Authorization": "test-token",
347-
}, nil, "")
347+
}, nil, "", 0)
348348
require.NoError(t, err, "Failed to create connection")
349349
defer conn.Close()
350350

internal/mcp/http_transport.go

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -403,11 +403,12 @@ func trySDKTransport(
403403
transportType HTTPTransportType,
404404
transportName string,
405405
createTransport transportConnector,
406+
keepAlive time.Duration,
406407
) (*Connection, error) {
407-
// Create MCP client with logger and keepalive enabled.
408-
// DefaultHTTPKeepaliveInterval sends periodic pings to prevent session expiry
409-
// on backends (e.g. safeoutputs) that have a 30-minute idle timeout.
410-
client := newMCPClient(logConn, DefaultHTTPKeepaliveInterval)
408+
// Create MCP client with logger and optional keepalive.
409+
// When keepAlive > 0, periodic pings prevent session expiry on backends
410+
// (e.g. safeoutputs) that have an idle timeout.
411+
client := newMCPClient(logConn, keepAlive)
411412

412413
// Create transport using the provided connector
413414
transport := createTransport(url, httpClient)
@@ -422,15 +423,15 @@ func trySDKTransport(
422423
return nil, fmt.Errorf("%s transport connect failed: %w", transportName, err)
423424
}
424425

425-
conn := newHTTPConnection(ctx, cancel, client, session, url, headers, httpClient, transportType, serverID, DefaultHTTPKeepaliveInterval)
426+
conn := newHTTPConnection(ctx, cancel, client, session, url, headers, httpClient, transportType, serverID, keepAlive)
426427

427428
logger.LogInfo("backend", "%s transport connected successfully", transportName)
428429
logConn.Printf("Connected with %s transport", transportName)
429430
return conn, nil
430431
}
431432

432433
// tryStreamableHTTPTransport attempts to connect using the streamable HTTP transport (2025-03-26 spec)
433-
func tryStreamableHTTPTransport(ctx context.Context, cancel context.CancelFunc, serverID, url string, headers map[string]string, httpClient *http.Client) (*Connection, error) {
434+
func tryStreamableHTTPTransport(ctx context.Context, cancel context.CancelFunc, serverID, url string, headers map[string]string, httpClient *http.Client, keepAlive time.Duration) (*Connection, error) {
434435
return trySDKTransport(
435436
ctx, cancel, serverID, url, headers, httpClient,
436437
HTTPTransportStreamable,
@@ -442,11 +443,12 @@ func tryStreamableHTTPTransport(ctx context.Context, cancel context.CancelFunc,
442443
MaxRetries: 0, // Don't retry on failure - we'll try other transports
443444
}
444445
},
446+
keepAlive,
445447
)
446448
}
447449

448450
// trySSETransport attempts to connect using the SSE transport (2024-11-05 spec)
449-
func trySSETransport(ctx context.Context, cancel context.CancelFunc, serverID, url string, headers map[string]string, httpClient *http.Client) (*Connection, error) {
451+
func trySSETransport(ctx context.Context, cancel context.CancelFunc, serverID, url string, headers map[string]string, httpClient *http.Client, keepAlive time.Duration) (*Connection, error) {
450452
return trySDKTransport(
451453
ctx, cancel, serverID, url, headers, httpClient,
452454
HTTPTransportSSE,
@@ -457,6 +459,7 @@ func trySSETransport(ctx context.Context, cancel context.CancelFunc, serverID, u
457459
HTTPClient: httpClient,
458460
}
459461
},
462+
keepAlive,
460463
)
461464
}
462465

0 commit comments

Comments
 (0)