Skip to content

Commit b67207b

Browse files
authored
fix: align gateway timeout defaults with spec §4.1.3 (#3592)
`DefaultStartupTimeout` and `DefaultToolTimeout` were 2× the spec-documented values (60s vs 30s, 120s vs 60s), causing observable gateway behavior to diverge from §4.1.3. ## Changes - **`internal/config/config_core.go`**: Align constants with spec: ```go // Before DefaultStartupTimeout = 60 // seconds DefaultToolTimeout = 120 // seconds // After DefaultStartupTimeout = 30 // seconds (per spec §4.1.3) DefaultToolTimeout = 60 // seconds (per spec §4.1.3) ``` - **`internal/launcher/launcher_test.go`** / **`getorlaunch_timeout_test.go`**: Update hardcoded `"1m0s"` / `"60 seconds"` references to match the new default (`"30s"`). Tests that referenced the constants directly required no changes. > [!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-build3874128028/b514/launcher.test /tmp/go-build3874128028/b514/launcher.test -test.testlogfile=/tmp/go-build3874128028/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3874128028/b277/vet.cfg ify@v1.11.1/asse-errorsas om/tetratelabs/w-ifaceassert x_amd64/vet -I rs/otlp/otlptrac-atomic -I cpN1F_WSlTV5 -W .cfg 67475494/b288//_-ifaceassert x_amd64/vet . --gdwarf2 475494/b288/ x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build2871554986/b514/launcher.test /tmp/go-build2871554986/b514/launcher.test -test.testlogfile=/tmp/go-build2871554986/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool 87c5eecb91833e7a--log-format ache/go/1.25.8/xjson 87c5eecb91833e7a/usr/lib/open-iscsi/net-interface-handler` (dns block) > - `invalid-host-that-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build3874128028/b496/config.test /tmp/go-build3874128028/b496/config.test -test.testlogfile=/tmp/go-build3874128028/b496/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3874128028/b379/vet.cfg 5.0/deviceauth.ggo1.25.8 5.0/oauth2.go x_amd64/vet --gdwarf-5 nal/encoding/tex-atomic -o x_amd64/vet -I _.a -I x_amd64/vet --gdwarf-5 go-sdk/auth -o x_amd64/vet` (dns block) > - `nonexistent.local` > - Triggering command: `/tmp/go-build3874128028/b514/launcher.test /tmp/go-build3874128028/b514/launcher.test -test.testlogfile=/tmp/go-build3874128028/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3874128028/b277/vet.cfg ify@v1.11.1/asse-errorsas om/tetratelabs/w-ifaceassert x_amd64/vet -I rs/otlp/otlptrac-atomic -I cpN1F_WSlTV5 -W .cfg 67475494/b288//_-ifaceassert x_amd64/vet . --gdwarf2 475494/b288/ x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build2871554986/b514/launcher.test /tmp/go-build2871554986/b514/launcher.test -test.testlogfile=/tmp/go-build2871554986/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool 87c5eecb91833e7a--log-format ache/go/1.25.8/xjson 87c5eecb91833e7a/usr/lib/open-iscsi/net-interface-handler` (dns block) > - `slow.example.com` > - Triggering command: `/tmp/go-build3874128028/b514/launcher.test /tmp/go-build3874128028/b514/launcher.test -test.testlogfile=/tmp/go-build3874128028/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build3874128028/b277/vet.cfg ify@v1.11.1/asse-errorsas om/tetratelabs/w-ifaceassert x_amd64/vet -I rs/otlp/otlptrac-atomic -I cpN1F_WSlTV5 -W .cfg 67475494/b288//_-ifaceassert x_amd64/vet . --gdwarf2 475494/b288/ x_amd64/vet` (dns block) > - Triggering command: `/tmp/go-build2871554986/b514/launcher.test /tmp/go-build2871554986/b514/launcher.test -test.testlogfile=/tmp/go-build2871554986/b514/testlog.txt -test.paniconexit0 -test.timeout=10m0s -ato�� -bool 87c5eecb91833e7a--log-format ache/go/1.25.8/xjson 87c5eecb91833e7a/usr/lib/open-iscsi/net-interface-handler` (dns block) > - `this-host-does-not-exist-12345.com` > - Triggering command: `/tmp/go-build3874128028/b523/mcp.test /tmp/go-build3874128028/b523/mcp.test -test.testlogfile=/tmp/go-build3874128028/b523/testlog.txt -test.paniconexit0 -test.timeout=10m0s -I .cfg pkg/mod/go.opentelemetry.io/otel@v1.43.0/semconv/v1.40.0/doc.go x_amd64/vet --gdwarf-5 g/grpc/connectiv--version -o x_amd64/vet .cfg�� /auth/apikey.go /auth/header.go x_amd64/vet -p g/grpc -lang=go1.25 x_amd64/vet` (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 853b350 + 0a23da1 commit b67207b

File tree

3 files changed

+11
-7
lines changed

3 files changed

+11
-7
lines changed

internal/config/config_core.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ import (
3838
// Core constants for configuration defaults
3939
const (
4040
DefaultPort = 3000
41-
DefaultStartupTimeout = 60 // seconds
42-
DefaultToolTimeout = 120 // seconds
41+
DefaultStartupTimeout = 30 // seconds (per spec §4.1.3)
42+
DefaultToolTimeout = 60 // seconds (per spec §4.1.3)
4343
DefaultKeepaliveInterval = 1500 // seconds (25 minutes) — keeps HTTP backend sessions alive
4444
)
4545

internal/launcher/getorlaunch_timeout_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func TestGetOrLaunch_TimeoutWithDefaultTimeout(t *testing.T) {
7575
expectedDefault := time.Duration(config.DefaultStartupTimeout) * time.Second
7676
assert.Equal(t, expectedDefault, l.startupTimeout, "Should use default startup timeout")
7777

78-
// We won't actually wait for default timeout in test (60s), just verify it's set
78+
// We won't actually wait for default timeout in test (30s), just verify it's set
7979
// The test would take too long to actually timeout
8080
}
8181

internal/launcher/launcher_test.go

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import (
88
"net/http/httptest"
99
"os"
1010
"testing"
11+
"time"
1112

1213
"github.com/stretchr/testify/assert"
1314
"github.com/stretchr/testify/require"
@@ -552,16 +553,18 @@ func TestGetOrLaunchForSession_InvalidServer(t *testing.T) {
552553
}
553554

554555
func TestLauncher_StartupTimeout(t *testing.T) {
556+
defaultTimeout := (time.Duration(config.DefaultStartupTimeout) * time.Second).String()
557+
555558
// Test that launcher respects the startup timeout from config
556559
tests := []struct {
557560
name string
558561
configTimeout int
559562
expectedTimeout string
560563
}{
561564
{
562-
name: "default timeout (60 seconds)",
565+
name: "default timeout",
563566
configTimeout: 0, // 0 means use default
564-
expectedTimeout: "1m0s",
567+
expectedTimeout: defaultTimeout,
565568
},
566569
{
567570
name: "custom timeout (30 seconds)",
@@ -623,8 +626,9 @@ func TestLauncher_TimeoutWithNilGateway(t *testing.T) {
623626
l := New(ctx, cfg)
624627
defer l.Close()
625628

626-
// Should use default timeout (60 seconds)
627-
assert.Equal(t, "1m0s", l.startupTimeout.String())
629+
// Should use default timeout
630+
expectedDefault := (time.Duration(config.DefaultStartupTimeout) * time.Second).String()
631+
assert.Equal(t, expectedDefault, l.startupTimeout.String())
628632
}
629633

630634
func TestLauncher_OIDCProviderInitialization(t *testing.T) {

0 commit comments

Comments
 (0)