Skip to content

Commit 056b712

Browse files
authored
Deduplicate MCP_GATEWAY_SESSION_TIMEOUT env-var lookup into shared getSessionTimeout() (#5100)
`envutil.GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 6*time.Hour)` was inlined in `transport.go` despite `routed.go` already owning an identical wrapper — two places to update if the env-var key or default ever changes. ## Changes - **`routed.go`** — Rename `getRoutedSessionTimeout()` → `getSessionTimeout()`; update its doc comment to reflect shared ownership - **`transport.go`** — Replace inline literal with `getSessionTimeout()`; drop now-unused `envutil` and `time` imports - **`routed_test.go`** — Update `TestRoutedModeSessionTimeout_ReadsEnvVar` call sites to the new name; test coverage is unchanged ```go // Before — transport.go inlined the literal independently SessionTimeout: envutil.GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 6*time.Hour), // After — both callers share one function SessionTimeout: getSessionTimeout(), ``` > [!WARNING] > > <details> > <summary>Firewall rules blocked me from connecting to one or more addresses (expand for details)</summary> > > #### I tried to connect to the following addresses, but was blocked by firewall rules: > > - `example.com` > - Triggering command: `/tmp/go-build602771692/b513/launcher.test /tmp/go-build602771692/b513/launcher.test -test.testlogfile=/tmp/go-build602771692/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build602771692/b428/vet.cfg ache/go/1.25.9/x64/src/runtime/cgo /tmp/go-build3583632305/b314/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet go_.�� 64/src/net -I x_amd64/vet --gdwarf-5 ions =0 x_amd64/vet` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build602771692/b495/config.test /tmp/go-build602771692/b495/config.test -test.testlogfile=/tmp/go-build602771692/b495/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build602771692/b365/vet.cfg Ua_e/htfOW-jx_xLgo1.25.9 ache/go/1.25.9/x-c=4 x_amd64/vet 0276810/b172/ go.opentelemetry-atomic` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build602771692/b513/launcher.test /tmp/go-build602771692/b513/launcher.test -test.testlogfile=/tmp/go-build602771692/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build602771692/b428/vet.cfg ache/go/1.25.9/x64/src/runtime/cgo /tmp/go-build3583632305/b314/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet go_.�� 64/src/net -I x_amd64/vet --gdwarf-5 ions =0 x_amd64/vet` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build602771692/b513/launcher.test /tmp/go-build602771692/b513/launcher.test -test.testlogfile=/tmp/go-build602771692/b513/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build602771692/b428/vet.cfg ache/go/1.25.9/x64/src/runtime/cgo /tmp/go-build3583632305/b314/ x_amd64/vet . --gdwarf2 --64 x_amd64/vet go_.�� 64/src/net -I x_amd64/vet --gdwarf-5 ions =0 x_amd64/vet` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build602771692/b522/mcp.test /tmp/go-build602771692/b522/mcp.test -test.testlogfile=/tmp/go-build602771692/b522/testlog.txt -test.paniconexit0 -test.timeout=10m0s` (dns block) > > If you need me to access, download, or install something from one of these locations, you can either: > > - Configure [Actions setup steps](https://gh.io/copilot/actions-setup-steps) to set up my environment, which run before the firewall is enabled > - Add the appropriate URLs or hosts to the custom allowlist in this repository's [Copilot coding agent settings](https://github.com/github/gh-aw-mcpg/settings/copilot/coding_agent) (admins only) > > </details>
2 parents 91a2aa2 + 7df1064 commit 056b712

3 files changed

Lines changed: 13 additions & 15 deletions

File tree

internal/server/routed.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,10 @@ func (c *filteredServerCache) getOrCreate(backendID, sessionID string, creator f
121121
return server
122122
}
123123

124-
// getRoutedSessionTimeout returns the session timeout for routed mode by reading
125-
// MCP_GATEWAY_SESSION_TIMEOUT with a 6-hour default, matching unified mode behaviour.
126-
// It is extracted as a package-level function so tests can assert the env-var wiring directly.
127-
func getRoutedSessionTimeout() time.Duration {
124+
// getSessionTimeout returns the session timeout by reading MCP_GATEWAY_SESSION_TIMEOUT
125+
// with a 6-hour default. It is shared by both routed mode and unified (transport) mode
126+
// and extracted as a package-level function so tests can assert the env-var wiring directly.
127+
func getSessionTimeout() time.Duration {
128128
return envutil.GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 6*time.Hour)
129129
}
130130

@@ -150,7 +150,7 @@ func CreateHTTPServerForRoutedMode(addr string, unifiedServer *UnifiedServer, ap
150150
// TTL matches the SDK SessionTimeout so cache entries expire with sessions.
151151
// Long-running agentic workflows (e.g. >30 min GitHub Actions jobs) need this
152152
// to be at least as long as the workflow to avoid spurious "session not found" errors.
153-
routedSessionTimeout := getRoutedSessionTimeout()
153+
routedSessionTimeout := getSessionTimeout()
154154
serverCache := newFilteredServerCache(routedSessionTimeout)
155155

156156
// Create a proxy for each backend server

internal/server/routed_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -816,19 +816,19 @@ func TestCreateHTTPServerForRoutedMode_OAuth(t *testing.T) {
816816
func TestRoutedModeSessionTimeout_ReadsEnvVar(t *testing.T) {
817817
t.Run("custom timeout from env", func(t *testing.T) {
818818
t.Setenv("MCP_GATEWAY_SESSION_TIMEOUT", "2h")
819-
got := getRoutedSessionTimeout()
820-
assert.Equal(t, 2*time.Hour, got, "getRoutedSessionTimeout should return the value from MCP_GATEWAY_SESSION_TIMEOUT")
819+
got := getSessionTimeout()
820+
assert.Equal(t, 2*time.Hour, got, "getSessionTimeout should return the value from MCP_GATEWAY_SESSION_TIMEOUT")
821821
})
822822

823823
t.Run("default 6h timeout when env var is empty", func(t *testing.T) {
824824
t.Setenv("MCP_GATEWAY_SESSION_TIMEOUT", "")
825-
got := getRoutedSessionTimeout()
826-
assert.Equal(t, 6*time.Hour, got, "getRoutedSessionTimeout should default to 6h when MCP_GATEWAY_SESSION_TIMEOUT is empty")
825+
got := getSessionTimeout()
826+
assert.Equal(t, 6*time.Hour, got, "getSessionTimeout should default to 6h when MCP_GATEWAY_SESSION_TIMEOUT is empty")
827827
})
828828

829829
t.Run("default 6h timeout is not the old 30min value", func(t *testing.T) {
830830
t.Setenv("MCP_GATEWAY_SESSION_TIMEOUT", "")
831-
got := getRoutedSessionTimeout()
831+
got := getSessionTimeout()
832832
assert.Greater(t, got, 30*time.Minute, "default timeout must exceed the old hardcoded 30-minute value")
833833
})
834834
}

internal/server/transport.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@ package server
22

33
import (
44
"net/http"
5-
"time"
65

7-
"github.com/github/gh-aw-mcpg/internal/envutil"
86
"github.com/github/gh-aw-mcpg/internal/logger"
97
sdk "github.com/modelcontextprotocol/go-sdk/mcp"
108
)
@@ -38,9 +36,9 @@ func CreateHTTPServerForMCP(addr string, unifiedServer *UnifiedServer, apiKey, h
3836

3937
return unifiedServer.server
4038
}, &sdk.StreamableHTTPOptions{
41-
Stateless: false, // Support stateful sessions
42-
Logger: logger.NewSlogLoggerWithHandler(logTransport), // Integrate SDK logging with project logger
43-
SessionTimeout: envutil.GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 6*time.Hour), // Configurable; 6h default matches GitHub Actions default timeout
39+
Stateless: false, // Support stateful sessions
40+
Logger: logger.NewSlogLoggerWithHandler(logTransport), // Integrate SDK logging with project logger
41+
SessionTimeout: getSessionTimeout(), // Configurable; 6h default matches GitHub Actions default timeout
4442
})
4543

4644
// Wrap with session auto-init to handle clients (e.g. Gemini CLI v0.37.x) that send

0 commit comments

Comments
 (0)