Skip to content

Commit b299564

Browse files
authored
feat: MCP_GATEWAY_TOOL_TIMEOUT env var + minimum 10s bound for toolTimeout (no upper limit) (#4967)
Slow MCP backends (e.g. Repo Mind full-text search) fail with the hardcoded 60s tool timeout and had no way to override it per-workflow. The `toolTimeout` field already existed in the stdin JSON schema but lacked an env var fallback. ## Changes **Env var fallback (`MCP_GATEWAY_TOOL_TIMEOUT`)** - Priority chain: stdin `gateway.toolTimeout` → `MCP_GATEWAY_TOOL_TIMEOUT` env var → built-in default (60s) - Applied in `convertStdinConfig` for both "gateway section present" and "no gateway section" paths - Public `GetGatewayToolTimeoutFromEnv()` + private `toolTimeoutEnvOrDefault()` in `config_env.go` - Env var is only evaluated when stdin omits `gateway.toolTimeout`, avoiding spurious log noise **Bounds enforcement (minimum 10s, no upper limit)** - New `rules.TimeoutMinimum(timeout, min, ...)` function for minimum-only validation - `validateGatewayConfig` uses `TimeoutMinimum(10)` for `toolTimeout` — any value ≥ 10 seconds is accepted - JSON schema updated: `minimum: 10` (no `maximum`) - Constant `ToolTimeoutMin = 10` exported from `config_env.go` **Example usage** ```json { "gateway": { "toolTimeout": 3600 }, "mcpServers": { "repo-mind": { "type": "http", "url": "..." } } } ``` Or without touching the config: ```bash MCP_GATEWAY_TOOL_TIMEOUT=3600 ./awmg --config-stdin ... ``` **Test updates** - New table-driven tests for `GetGatewayToolTimeoutFromEnv`, `toolTimeoutEnvOrDefault`, and the three-level priority in `convertStdinConfig` - Tests cover large values (3600 = 1 hour, 86400 = 24 hours) to confirm no upper bound - Existing tests updated to reflect the minimum-only constraint
2 parents f662a30 + 6580406 commit b299564

10 files changed

Lines changed: 353 additions & 8 deletions

AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,7 @@ DEBUG_COLORS=0 DEBUG=* ./awmg --config config.toml
386386
- `MCP_GATEWAY_PAYLOAD_PATH_PREFIX` - Path prefix for remapping payloadPath returned to clients (sets default for `--payload-path-prefix` flag, default: empty)
387387
- `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD` - Size threshold in bytes for payload storage; payloads larger than this are stored to disk (sets default for `--payload-size-threshold` flag, default: `524288`)
388388
- `MCP_GATEWAY_SESSION_TIMEOUT` - Session timeout for stateful sessions in both unified mode (`/mcp`) and routed mode (`/mcp/<server>`). Accepts Go duration strings (e.g., `30m`, `1h`, `2h30m`). (default: `6h`)
389+
- `MCP_GATEWAY_TOOL_TIMEOUT` - Tool invocation timeout in seconds. Fallback when `gateway.toolTimeout` is not set in stdin JSON config. Accepts any integer ≥ 10 (no upper bound). Priority: stdin config > env var > built-in default. (default: `60`)
389390
- `DOCKER_HOST` - Docker daemon socket path (default: `/var/run/docker.sock`)
390391
- `MCP_GATEWAY_GUARDS_SINK_SERVER_IDS` - Comma-separated server IDs whose RPC JSONL logs should include agent secrecy/integrity tag snapshots (sets default for `--guards-sink-server-ids`)
391392
- `MCP_GATEWAY_GUARDS_MODE` - Guards enforcement mode: `strict` (deny violations), `filter` (remove denied tools), `propagate` (auto-adjust agent labels) (sets default for `--guards-mode`, default: `strict`)

docs/ENVIRONMENT_VARIABLES.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ When running locally (`run.sh`), these variables are optional (warnings shown if
2626
| `MCP_GATEWAY_PAYLOAD_PATH_PREFIX` | Path prefix for remapping payloadPath returned to clients (sets default for `--payload-path-prefix` flag) | (empty - use actual filesystem path) |
2727
| `MCP_GATEWAY_PAYLOAD_SIZE_THRESHOLD` | Size threshold in bytes for payload storage (sets default for `--payload-size-threshold` flag) | `524288` |
2828
| `MCP_GATEWAY_SESSION_TIMEOUT` | Session timeout for stateful sessions in both unified (`/mcp`) and routed (`/mcp/<server>`) modes. Accepts Go duration strings (e.g., `30m`, `1h`). Default is 6 hours to match the GitHub Actions default timeout. | `6h` |
29+
| `MCP_GATEWAY_TOOL_TIMEOUT` | Tool invocation timeout in seconds. Used as fallback when `gateway.toolTimeout` is not set in the stdin JSON config. Accepts any integer ≥ 10 (no upper bound). Priority: stdin `gateway.toolTimeout` > this env var > built-in default. | `60` |
2930
| `MCP_GATEWAY_WASM_GUARDS_DIR` | Root directory for per-server WASM guards (`<root>/<serverID>/*.wasm`, first match is loaded) | (disabled) |
3031
| `MCP_GATEWAY_GUARDS_MODE` | Guards enforcement mode: `strict` (deny violations), `filter` (remove denied tools), `propagate` (auto-adjust agent labels) (sets default for `--guards-mode`) | `strict` |
3132
| `MCP_GATEWAY_GUARDS_SINK_SERVER_IDS` | Comma-separated sink server IDs for JSONL guards tag enrichment (sets default for `--guards-sink-server-ids`) | (disabled) |

internal/config/config_env.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import (
88
"github.com/github/gh-aw-mcpg/internal/envutil"
99
)
1010

11+
// ToolTimeoutMin is the minimum allowed value for toolTimeout (seconds).
12+
const ToolTimeoutMin = 10
13+
1114
// GetGatewayPortFromEnv returns the MCP_GATEWAY_PORT value, parsed as int
1215
func GetGatewayPortFromEnv() (int, error) {
1316
portStr := envutil.GetEnvString("MCP_GATEWAY_PORT", "")
@@ -50,3 +53,42 @@ func GetGatewayAPIKeyFromEnv() string {
5053
}
5154
return key
5255
}
56+
57+
// GetGatewayToolTimeoutFromEnv returns the MCP_GATEWAY_TOOL_TIMEOUT value, parsed as int.
58+
// Returns (0, false) when the environment variable is not set or empty.
59+
// Returns an error when the variable is set but invalid (non-integer or below minimum of 10).
60+
func GetGatewayToolTimeoutFromEnv() (int, bool, error) {
61+
timeoutStr := envutil.GetEnvString("MCP_GATEWAY_TOOL_TIMEOUT", "")
62+
if timeoutStr == "" {
63+
logConfig.Print("MCP_GATEWAY_TOOL_TIMEOUT not set in environment")
64+
return 0, false, nil
65+
}
66+
67+
timeout, err := strconv.Atoi(timeoutStr)
68+
if err != nil {
69+
logConfig.Printf("MCP_GATEWAY_TOOL_TIMEOUT=%q is not a valid integer: %v", timeoutStr, err)
70+
return 0, false, fmt.Errorf("invalid MCP_GATEWAY_TOOL_TIMEOUT value: %s", timeoutStr)
71+
}
72+
73+
if validationErr := rules.TimeoutMinimum(timeout, ToolTimeoutMin, "MCP_GATEWAY_TOOL_TIMEOUT", "MCP_GATEWAY_TOOL_TIMEOUT"); validationErr != nil {
74+
logConfig.Printf("MCP_GATEWAY_TOOL_TIMEOUT=%d is below minimum %d: %s", timeout, ToolTimeoutMin, validationErr.Message)
75+
return 0, false, fmt.Errorf("%s", validationErr.Message)
76+
}
77+
78+
logConfig.Printf("MCP_GATEWAY_TOOL_TIMEOUT resolved to %d", timeout)
79+
return timeout, true, nil
80+
}
81+
82+
// toolTimeoutEnvOrDefault returns the value of MCP_GATEWAY_TOOL_TIMEOUT when set and valid,
83+
// otherwise DefaultToolTimeout. Invalid env var values are logged and ignored (fallback to default).
84+
func toolTimeoutEnvOrDefault() int {
85+
timeout, ok, err := GetGatewayToolTimeoutFromEnv()
86+
if err != nil {
87+
logConfig.Printf("MCP_GATEWAY_TOOL_TIMEOUT is invalid, falling back to default %d: %v", DefaultToolTimeout, err)
88+
return DefaultToolTimeout
89+
}
90+
if !ok {
91+
return DefaultToolTimeout
92+
}
93+
return timeout
94+
}

internal/config/config_stdin.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -309,9 +309,13 @@ func convertStdinConfig(stdinCfg *StdinConfig) (*Config, error) {
309309
APIKey: stdinCfg.Gateway.APIKey,
310310
Domain: stdinCfg.Gateway.Domain,
311311
StartupTimeout: intPtrOrDefault(stdinCfg.Gateway.StartupTimeout, DefaultStartupTimeout),
312-
ToolTimeout: intPtrOrDefault(stdinCfg.Gateway.ToolTimeout, DefaultToolTimeout),
313312
KeepaliveInterval: intPtrOrDefault(stdinCfg.Gateway.KeepaliveInterval, DefaultKeepaliveInterval),
314313
}
314+
if stdinCfg.Gateway.ToolTimeout != nil {
315+
cfg.Gateway.ToolTimeout = *stdinCfg.Gateway.ToolTimeout
316+
} else {
317+
cfg.Gateway.ToolTimeout = toolTimeoutEnvOrDefault()
318+
}
315319
if stdinCfg.Gateway.PayloadDir != "" {
316320
cfg.Gateway.PayloadDir = stdinCfg.Gateway.PayloadDir
317321
}
@@ -328,6 +332,10 @@ func convertStdinConfig(stdinCfg *StdinConfig) (*Config, error) {
328332
logStdin.Print("No gateway config in stdin, applying defaults")
329333
cfg.Gateway = &GatewayConfig{}
330334
applyGatewayDefaults(cfg.Gateway)
335+
// Apply MCP_GATEWAY_TOOL_TIMEOUT env var if set. toolTimeoutEnvOrDefault() returns
336+
// the env var value when valid and present, otherwise DefaultToolTimeout (which
337+
// applyGatewayDefaults already wrote). This is a no-op when the env var is absent.
338+
cfg.Gateway.ToolTimeout = toolTimeoutEnvOrDefault()
331339
}
332340

333341
// Apply feature-specific defaults

internal/config/docker_helpers_and_env_test.go

Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,3 +108,234 @@ func TestGetGatewayPortFromEnv_Comprehensive(t *testing.T) {
108108
})
109109
}
110110
}
111+
112+
// TestGetGatewayToolTimeoutFromEnv tests the env-based tool timeout parsing.
113+
func TestGetGatewayToolTimeoutFromEnv(t *testing.T) {
114+
tests := []struct {
115+
name string
116+
envValue string
117+
envSet bool
118+
wantTimeout int
119+
wantOK bool
120+
wantErr bool
121+
errMsg string
122+
}{
123+
{
124+
name: "env var not set",
125+
envSet: false,
126+
wantOK: false,
127+
wantErr: false,
128+
},
129+
{
130+
name: "env var set to empty string",
131+
envValue: "",
132+
envSet: true,
133+
wantOK: false,
134+
wantErr: false,
135+
},
136+
{
137+
name: "invalid integer value",
138+
envValue: "not-a-number",
139+
envSet: true,
140+
wantOK: false,
141+
wantErr: true,
142+
errMsg: "invalid MCP_GATEWAY_TOOL_TIMEOUT value",
143+
},
144+
{
145+
name: "below minimum (9)",
146+
envValue: "9",
147+
envSet: true,
148+
wantOK: false,
149+
wantErr: true,
150+
errMsg: "must be at least 10",
151+
},
152+
{
153+
name: "zero (out of range)",
154+
envValue: "0",
155+
envSet: true,
156+
wantOK: false,
157+
wantErr: true,
158+
errMsg: "must be at least 10",
159+
},
160+
{
161+
name: "negative value",
162+
envValue: "-1",
163+
envSet: true,
164+
wantOK: false,
165+
wantErr: true,
166+
errMsg: "must be at least 10",
167+
},
168+
{
169+
name: "valid minimum boundary (10)",
170+
envValue: "10",
171+
envSet: true,
172+
wantTimeout: 10,
173+
wantOK: true,
174+
wantErr: false,
175+
},
176+
{
177+
name: "valid default value (60)",
178+
envValue: "60",
179+
envSet: true,
180+
wantTimeout: 60,
181+
wantOK: true,
182+
wantErr: false,
183+
},
184+
{
185+
name: "valid high value (120)",
186+
envValue: "120",
187+
envSet: true,
188+
wantTimeout: 120,
189+
wantOK: true,
190+
wantErr: false,
191+
},
192+
{
193+
name: "valid large value (3600 = 1 hour)",
194+
envValue: "3600",
195+
envSet: true,
196+
wantTimeout: 3600,
197+
wantOK: true,
198+
wantErr: false,
199+
},
200+
{
201+
name: "valid very large value (86400 = 24 hours)",
202+
envValue: "86400",
203+
envSet: true,
204+
wantTimeout: 86400,
205+
wantOK: true,
206+
wantErr: false,
207+
},
208+
}
209+
210+
for _, tt := range tests {
211+
t.Run(tt.name, func(t *testing.T) {
212+
if tt.envSet {
213+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", tt.envValue)
214+
} else {
215+
os.Unsetenv("MCP_GATEWAY_TOOL_TIMEOUT")
216+
}
217+
218+
timeout, ok, err := GetGatewayToolTimeoutFromEnv()
219+
if tt.wantErr {
220+
require.Error(t, err)
221+
assert.False(t, ok)
222+
if tt.errMsg != "" {
223+
assert.Contains(t, err.Error(), tt.errMsg)
224+
}
225+
assert.Equal(t, 0, timeout)
226+
} else {
227+
require.NoError(t, err)
228+
assert.Equal(t, tt.wantOK, ok)
229+
if tt.wantOK {
230+
assert.Equal(t, tt.wantTimeout, timeout)
231+
} else {
232+
assert.Equal(t, 0, timeout)
233+
}
234+
}
235+
})
236+
}
237+
}
238+
239+
// TestToolTimeoutEnvOrDefault tests that toolTimeoutEnvOrDefault uses the env var when set.
240+
func TestToolTimeoutEnvOrDefault(t *testing.T) {
241+
t.Run("returns DefaultToolTimeout when env var is not set", func(t *testing.T) {
242+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "")
243+
got := toolTimeoutEnvOrDefault()
244+
assert.Equal(t, DefaultToolTimeout, got, "should return DefaultToolTimeout when env var is empty")
245+
})
246+
247+
t.Run("returns env var value when set to valid value", func(t *testing.T) {
248+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "120")
249+
got := toolTimeoutEnvOrDefault()
250+
assert.Equal(t, 120, got, "should return 120 from env var")
251+
})
252+
253+
t.Run("returns DefaultToolTimeout when env var has invalid value", func(t *testing.T) {
254+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "not-a-number")
255+
got := toolTimeoutEnvOrDefault()
256+
assert.Equal(t, DefaultToolTimeout, got, "should fall back to default on invalid env var")
257+
})
258+
259+
t.Run("returns env var value at minimum boundary (10)", func(t *testing.T) {
260+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "10")
261+
got := toolTimeoutEnvOrDefault()
262+
assert.Equal(t, 10, got, "should return 10 at minimum boundary")
263+
})
264+
265+
t.Run("returns env var large value (3600 = 1 hour)", func(t *testing.T) {
266+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "3600")
267+
got := toolTimeoutEnvOrDefault()
268+
assert.Equal(t, 3600, got, "should return 3600 (1 hour)")
269+
})
270+
}
271+
272+
// TestConvertStdinConfig_ToolTimeoutEnvFallback verifies the stdin config priority:
273+
// stdin config value > MCP_GATEWAY_TOOL_TIMEOUT env var > built-in default.
274+
func TestConvertStdinConfig_ToolTimeoutEnvFallback(t *testing.T) {
275+
t.Run("stdin value takes priority over env var", func(t *testing.T) {
276+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "120")
277+
stdinCfg := &StdinConfig{
278+
MCPServers: map[string]*StdinServerConfig{
279+
"test": {Type: "stdio", Container: "test/server:latest"},
280+
},
281+
Gateway: &StdinGatewayConfig{
282+
ToolTimeout: intPtr(300),
283+
},
284+
}
285+
cfg, err := convertStdinConfig(stdinCfg)
286+
require.NoError(t, err)
287+
assert.Equal(t, 300, cfg.Gateway.ToolTimeout, "stdin value (300) should override env var (120)")
288+
})
289+
290+
t.Run("env var used as fallback when stdin omits toolTimeout", func(t *testing.T) {
291+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "120")
292+
stdinCfg := &StdinConfig{
293+
MCPServers: map[string]*StdinServerConfig{
294+
"test": {Type: "stdio", Container: "test/server:latest"},
295+
},
296+
Gateway: &StdinGatewayConfig{}, // toolTimeout not set
297+
}
298+
cfg, err := convertStdinConfig(stdinCfg)
299+
require.NoError(t, err)
300+
assert.Equal(t, 120, cfg.Gateway.ToolTimeout, "env var (120) should be used when stdin omits toolTimeout")
301+
})
302+
303+
t.Run("built-in default used when both stdin and env var are absent", func(t *testing.T) {
304+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "")
305+
stdinCfg := &StdinConfig{
306+
MCPServers: map[string]*StdinServerConfig{
307+
"test": {Type: "stdio", Container: "test/server:latest"},
308+
},
309+
Gateway: &StdinGatewayConfig{}, // toolTimeout not set
310+
}
311+
cfg, err := convertStdinConfig(stdinCfg)
312+
require.NoError(t, err)
313+
assert.Equal(t, DefaultToolTimeout, cfg.Gateway.ToolTimeout, "built-in default should be used when both stdin and env var are absent")
314+
})
315+
316+
t.Run("env var used when no gateway section in stdin", func(t *testing.T) {
317+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "180")
318+
stdinCfg := &StdinConfig{
319+
MCPServers: map[string]*StdinServerConfig{
320+
"test": {Type: "stdio", Container: "test/server:latest"},
321+
},
322+
// no Gateway section
323+
}
324+
cfg, err := convertStdinConfig(stdinCfg)
325+
require.NoError(t, err)
326+
assert.Equal(t, 180, cfg.Gateway.ToolTimeout, "env var (180) should be used when no gateway section present")
327+
})
328+
329+
t.Run("built-in default used when no gateway section and no env var", func(t *testing.T) {
330+
t.Setenv("MCP_GATEWAY_TOOL_TIMEOUT", "")
331+
stdinCfg := &StdinConfig{
332+
MCPServers: map[string]*StdinServerConfig{
333+
"test": {Type: "stdio", Container: "test/server:latest"},
334+
},
335+
// no Gateway section
336+
}
337+
cfg, err := convertStdinConfig(stdinCfg)
338+
require.NoError(t, err)
339+
assert.Equal(t, DefaultToolTimeout, cfg.Gateway.ToolTimeout, "built-in default should be used when no gateway section and no env var")
340+
})
341+
}

internal/config/rules/rules.go

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,39 @@ func TimeoutPositive(timeout int, fieldName, jsonPath string) *ValidationError {
120120
return nil
121121
}
122122

123+
// TimeoutMinimum validates that a timeout value is at least min.
124+
// Returns nil if valid, *ValidationError if below the minimum.
125+
func TimeoutMinimum(timeout, min int, fieldName, jsonPath string) *ValidationError {
126+
log.Printf("Validating timeout minimum: field=%s, value=%d, min=%d, jsonPath=%s", fieldName, timeout, min, jsonPath)
127+
if timeout < min {
128+
log.Printf("Timeout minimum validation failed: %s=%d is below minimum %d", fieldName, timeout, min)
129+
return &ValidationError{
130+
Field: fieldName,
131+
Message: fmt.Sprintf("%s must be at least %d, got %d", fieldName, min, timeout),
132+
JSONPath: jsonPath,
133+
Suggestion: fmt.Sprintf("Use a value of at least %d seconds", min),
134+
}
135+
}
136+
return nil
137+
}
138+
139+
// TimeoutRange validates that a timeout value is within [min, max] (inclusive).
140+
// Returns nil if valid, *ValidationError if outside the range.
141+
func TimeoutRange(timeout, min, max int, fieldName, jsonPath string) *ValidationError {
142+
log.Printf("Validating timeout range: field=%s, value=%d, min=%d, max=%d, jsonPath=%s", fieldName, timeout, min, max, jsonPath)
143+
if timeout < min || timeout > max {
144+
log.Printf("Timeout range validation failed: %s=%d is outside [%d, %d]", fieldName, timeout, min, max)
145+
suggestedTimeout := min + (max-min)/2
146+
return &ValidationError{
147+
Field: fieldName,
148+
Message: fmt.Sprintf("%s must be between %d and %d, got %d", fieldName, min, max, timeout),
149+
JSONPath: jsonPath,
150+
Suggestion: fmt.Sprintf("Use a value between %d and %d seconds (e.g., %d)", min, max, suggestedTimeout),
151+
}
152+
}
153+
return nil
154+
}
155+
123156
// MountFormat validates a mount specification in the format "source:dest:mode"
124157
// Returns nil if valid, *ValidationError if invalid
125158
// Per MCP Gateway specification v1.8.0 section 4.1.5:

internal/config/schema/mcp-gateway-config.schema.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@
251251
"toolTimeout": {
252252
"type": "integer",
253253
"description": "Tool invocation timeout in seconds. The gateway enforces this timeout for individual tool/method calls to MCP servers.",
254-
"minimum": 1,
254+
"minimum": 10,
255255
"default": 60
256256
},
257257
"payloadDir": {

internal/config/validation.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ func validateGatewayConfig(gateway *StdinGatewayConfig) error {
359359

360360
if gateway.ToolTimeout != nil {
361361
logValidation.Printf("Validating tool timeout: %d", *gateway.ToolTimeout)
362-
if err := rules.TimeoutPositive(*gateway.ToolTimeout, "toolTimeout", "gateway.toolTimeout"); err != nil {
362+
if err := rules.TimeoutMinimum(*gateway.ToolTimeout, ToolTimeoutMin, "toolTimeout", "gateway.toolTimeout"); err != nil {
363363
return err
364364
}
365365
}

0 commit comments

Comments
 (0)