Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions internal/envutil/envutil.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"os"
"strconv"
"strings"
"time"
)

// GetEnvString returns the value of the environment variable specified by envKey.
Expand All @@ -28,6 +29,18 @@ func GetEnvInt(envKey string, defaultValue int) int {
return defaultValue
}

// GetEnvDuration returns the time.Duration value of the environment variable specified by envKey.
// If the environment variable is not set, is empty, or cannot be parsed by time.ParseDuration,
// it returns the defaultValue. Accepts any string valid for time.ParseDuration (e.g. "2h", "30m", "90s").
func GetEnvDuration(envKey string, defaultValue time.Duration) time.Duration {
if envValue := os.Getenv(envKey); envValue != "" {
if d, err := time.ParseDuration(envValue); err == nil && d > 0 {
return d
Comment on lines +32 to +38
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring says this falls back only when the env var is unset/empty or cannot be parsed, but the implementation also falls back when the parsed duration is non-positive (d <= 0). Please update the comment to mention the > 0 requirement (similar to GetEnvInt) so behavior and docs match.

Copilot uses AI. Check for mistakes.
}
}
return defaultValue
}

// GetEnvBool returns the boolean value of the environment variable specified by envKey.
// If the environment variable is not set or is empty, it returns the defaultValue.
// Truthy values (case-insensitive): "1", "true", "yes", "on"
Expand Down
120 changes: 120 additions & 0 deletions internal/envutil/envutil_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,130 @@ package envutil
import (
"os"
"testing"
"time"

"github.com/stretchr/testify/assert"
)

func TestGetEnvDuration(t *testing.T) {
tests := []struct {
name string
envKey string
envValue string
setEnv bool
defaultValue time.Duration
expected time.Duration
}{
{
name: "env var set to '2h' - returns 2 hours",
envKey: "TEST_DURATION_VAR",
envValue: "2h",
setEnv: true,
defaultValue: 30 * time.Minute,
expected: 2 * time.Hour,
},
{
name: "env var set to '30m' - returns 30 minutes",
envKey: "TEST_DURATION_VAR",
envValue: "30m",
setEnv: true,
defaultValue: 2 * time.Hour,
expected: 30 * time.Minute,
},
{
name: "env var set to '90s' - returns 90 seconds",
envKey: "TEST_DURATION_VAR",
envValue: "90s",
setEnv: true,
defaultValue: 2 * time.Hour,
expected: 90 * time.Second,
},
{
name: "env var not set - returns default",
envKey: "TEST_DURATION_VAR",
setEnv: false,
defaultValue: 2 * time.Hour,
expected: 2 * time.Hour,
},
{
name: "env var empty string - returns default",
envKey: "TEST_DURATION_VAR",
envValue: "",
setEnv: true,
defaultValue: 2 * time.Hour,
expected: 2 * time.Hour,
},
{
name: "env var with invalid value - returns default",
envKey: "TEST_DURATION_VAR",
envValue: "invalid",
setEnv: true,
defaultValue: 2 * time.Hour,
expected: 2 * time.Hour,
},
{
name: "env var with zero duration - returns default",
envKey: "TEST_DURATION_VAR",
envValue: "0s",
setEnv: true,
defaultValue: 2 * time.Hour,
expected: 2 * time.Hour,
},
{
name: "env var with negative duration - returns default",
envKey: "TEST_DURATION_VAR",
envValue: "-1h",
setEnv: true,
defaultValue: 2 * time.Hour,
expected: 2 * time.Hour,
},
{
name: "env var with mixed units - returns parsed duration",
envKey: "TEST_DURATION_VAR",
envValue: "1h30m",
setEnv: true,
defaultValue: 2 * time.Hour,
expected: 90 * time.Minute,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
os.Unsetenv(tt.envKey)
defer os.Unsetenv(tt.envKey)

if tt.setEnv {
os.Setenv(tt.envKey, tt.envValue)
}

Comment on lines +94 to +101
Copy link

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are manually setting/unsetting environment variables via os.Unsetenv/os.Setenv. The repo frequently uses testing.T.Setenv for this (e.g., internal/sys/container_test.go:12-68), which automatically restores env vars via t.Cleanup and is safer if tests ever become parallelized. Consider switching these cases to t.Setenv (and avoiding manual Unsetenv/defer).

Copilot uses AI. Check for mistakes.
result := GetEnvDuration(tt.envKey, tt.defaultValue)
assert.Equal(t, tt.expected, result)
})
}
}

// TestGetEnvDurationRealWorldScenarios tests realistic usage scenarios
func TestGetEnvDurationRealWorldScenarios(t *testing.T) {
t.Run("session timeout configuration", func(t *testing.T) {
os.Unsetenv("MCP_GATEWAY_SESSION_TIMEOUT")
defer os.Unsetenv("MCP_GATEWAY_SESSION_TIMEOUT")

// Default case
result := GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 2*time.Hour)
assert.Equal(t, 2*time.Hour, result)

// Override with shorter timeout
os.Setenv("MCP_GATEWAY_SESSION_TIMEOUT", "30m")
result = GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 2*time.Hour)
assert.Equal(t, 30*time.Minute, result)

// Override with longer timeout
os.Setenv("MCP_GATEWAY_SESSION_TIMEOUT", "4h")
result = GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 2*time.Hour)
assert.Equal(t, 4*time.Hour, result)
})
}

func TestGetEnvString(t *testing.T) {
tests := []struct {
name string
Expand Down
7 changes: 4 additions & 3 deletions internal/server/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"net/http"
"time"

"github.com/github/gh-aw-mcpg/internal/envutil"
"github.com/github/gh-aw-mcpg/internal/logger"
sdk "github.com/modelcontextprotocol/go-sdk/mcp"
)
Expand Down Expand Up @@ -35,9 +36,9 @@ func CreateHTTPServerForMCP(addr string, unifiedServer *UnifiedServer, apiKey st

return unifiedServer.server
}, &sdk.StreamableHTTPOptions{
Stateless: false, // Support stateful sessions
Logger: logger.NewSlogLoggerWithHandler(logTransport), // Integrate SDK logging with project logger
SessionTimeout: 2 * time.Hour, // 2h accommodates long-running workflows with idle periods
Stateless: false, // Support stateful sessions
Logger: logger.NewSlogLoggerWithHandler(logTransport), // Integrate SDK logging with project logger
SessionTimeout: envutil.GetEnvDuration("MCP_GATEWAY_SESSION_TIMEOUT", 2*time.Hour), // Configurable; 2h default accommodates long-running workflows with idle periods
})

// Apply standard middleware stack (SDK logging → shutdown check → auth)
Expand Down
Loading