Skip to content

Commit 32b8c76

Browse files
lpcoxCopilot
andcommitted
Wire toolTimeout as context deadline in callBackendTool
The ToolTimeout config value was never consumed to create a context deadline — removing http.Client.Timeout (120s) would leave HTTP backend calls unbounded. Fix by adding context.WithTimeout(ctx, toolTimeout) in callBackendTool as the primary enforcement point. - Add context.WithTimeout in callBackendTool using cfg.Gateway.ToolTimeout - Nil-check cfg.Gateway before accessing ToolTimeout (it's a pointer) - Add TestCallBackendTool_ToolTimeoutEnforcedViaContext proving deadline - Update CONFIGURATION.md to accurately describe enforcement scope Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent f52fd0b commit 32b8c76

3 files changed

Lines changed: 81 additions & 1 deletion

File tree

docs/CONFIGURATION.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,7 @@ The `customSchemas` top-level field allows you to define custom server types bey
372372
| `apiKey` | API key for authentication | (disabled) |
373373
| `domain` | Gateway domain (`"localhost"`, `"host.docker.internal"`, or `"${VAR}"`) | (unset) |
374374
| `startupTimeout` | Seconds to wait for backend startup | `30` |
375-
| `toolTimeout` | Seconds to wait for tool execution (applies to backend request execution via request context deadlines, including HTTP backends) | `60` |
375+
| `toolTimeout` | Maximum seconds for a single tool call, enforced as a context deadline on all backend requests (stdio and HTTP) | `60` |
376376
| `payloadDir` | Directory for large payload files | `/tmp/jq-payloads` |
377377
| `payloadSizeThreshold` (JSON) / `payload_size_threshold` (TOML) | Size threshold in bytes; responses larger than this are stored to disk and returned as a `payloadPath` reference | `524288` (512 KB) |
378378
| `trustedBots` (JSON) / `trusted_bots` (TOML) | Optional list of additional bot usernames to trust with "approved" integrity level. Additive to the built-in trusted bot list. When specified, must be a non-empty array with non-empty string entries (spec §4.1.3.4); omit the field entirely if not needed. Example: `["my-bot[bot]", "org-automation"]` | (disabled) |

internal/server/call_backend_tool_test.go

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net/http/httptest"
88
"strings"
99
"testing"
10+
"time"
1011

1112
"github.com/github/gh-aw-mcpg/internal/config"
1213
sdk "github.com/modelcontextprotocol/go-sdk/mcp"
@@ -484,3 +485,72 @@ func TestCallBackendTool_AllowedToolsError_MessageFormat(t *testing.T) {
484485
assert.True(t, strings.Contains(text, `"blocked"`), "error message should include tool name: %s", text)
485486
assert.True(t, strings.Contains(text, "allowed-tools"), "error message should mention allowed-tools: %s", text)
486487
}
488+
489+
// TestCallBackendTool_ToolTimeoutEnforcedViaContext verifies that the configured
490+
// toolTimeout is applied as a context deadline, causing slow backend calls to fail
491+
// with a deadline-exceeded error instead of hanging indefinitely.
492+
func TestCallBackendTool_ToolTimeoutEnforcedViaContext(t *testing.T) {
493+
require := require.New(t)
494+
495+
// Create a slow backend that delays longer than our tool timeout
496+
backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
497+
var req map[string]interface{}
498+
if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
499+
w.WriteHeader(http.StatusBadRequest)
500+
return
501+
}
502+
503+
method, _ := req["method"].(string)
504+
switch method {
505+
case "initialize":
506+
json.NewEncoder(w).Encode(map[string]interface{}{
507+
"jsonrpc": "2.0", "id": req["id"],
508+
"result": map[string]interface{}{
509+
"protocolVersion": "2024-11-05",
510+
"capabilities": map[string]interface{}{},
511+
"serverInfo": map[string]interface{}{"name": "slow-backend", "version": "1.0.0"},
512+
},
513+
})
514+
case "tools/list":
515+
json.NewEncoder(w).Encode(map[string]interface{}{
516+
"jsonrpc": "2.0", "id": req["id"],
517+
"result": map[string]interface{}{"tools": []map[string]interface{}{}},
518+
})
519+
case "tools/call":
520+
// Simulate a slow tool: sleep longer than the configured toolTimeout.
521+
// The actual tool call should return after ~1s (timeout), but the
522+
// httptest.Server cleanup waits for this goroutine to finish.
523+
time.Sleep(3 * time.Second)
524+
json.NewEncoder(w).Encode(map[string]interface{}{
525+
"jsonrpc": "2.0", "id": req["id"],
526+
"result": map[string]interface{}{
527+
"content": []map[string]interface{}{{"type": "text", "text": "should not reach here"}},
528+
},
529+
})
530+
}
531+
}))
532+
defer backend.Close()
533+
534+
// Configure with a very short toolTimeout (1 second)
535+
cfg := &config.Config{
536+
Gateway: &config.GatewayConfig{
537+
ToolTimeout: 1,
538+
},
539+
Servers: map[string]*config.ServerConfig{
540+
"slow": {Type: "http", URL: backend.URL},
541+
},
542+
}
543+
544+
us, err := NewUnified(context.Background(), cfg)
545+
require.NoError(err)
546+
defer us.Close()
547+
548+
ctx := context.WithValue(context.Background(), SessionIDContextKey, "timeout-test")
549+
result, _, callErr := us.callBackendTool(ctx, "slow", "slow_tool", map[string]interface{}{})
550+
551+
// The call should fail due to context deadline exceeded
552+
require.Error(callErr, "Tool call should fail due to timeout")
553+
require.NotNil(result, "Should return a CallToolResult even on timeout")
554+
require.True(result.IsError, "Result should be marked as error")
555+
t.Logf("Tool call correctly timed out: %v", callErr)
556+
}

internal/server/unified.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,16 @@ func (us *UnifiedServer) callBackendTool(ctx context.Context, serverID, toolName
466466
// The closure captures the request and validates before calling this method
467467
logUnified.Printf("callBackendTool: serverID=%s, toolName=%s, args=%+v", serverID, toolName, args)
468468

469+
// Apply the configured tool timeout as a context deadline so backend calls
470+
// (including HTTP backends) are bounded by toolTimeout rather than hanging
471+
// indefinitely. This is the primary enforcement point for the gateway's
472+
// tool execution budget.
473+
if us.cfg != nil && us.cfg.Gateway != nil && us.cfg.Gateway.ToolTimeout > 0 {
474+
var cancel context.CancelFunc
475+
ctx, cancel = context.WithTimeout(ctx, time.Duration(us.cfg.Gateway.ToolTimeout)*time.Second)
476+
defer cancel()
477+
}
478+
469479
// Start an OTEL span for the full tool call lifecycle (spans all phases 0–6)
470480
// Attribute names follow MCP Gateway Specification §4.1.3.6
471481
ctx, toolSpan := us.getTracer().Start(ctx, "mcp.tool_call",

0 commit comments

Comments
 (0)