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
6 changes: 6 additions & 0 deletions internal/config/config_core.go
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,12 @@ func LoadFromFile(path string) (*Config, error) {
// Apply feature-specific defaults
applyDefaults(&cfg)

// Validate payload_size_threshold per spec §4.1.3.3: must be positive integer.
// applyDefaults replaces 0 with the default, so only negative values remain to catch.
if cfg.Gateway.PayloadSizeThreshold < 0 {
return nil, fmt.Errorf("gateway.payload_size_threshold must be a positive integer, got %d (spec §4.1.3.3)", cfg.Gateway.PayloadSizeThreshold)
}

if err := validateGuardPolicies(&cfg); err != nil {
return nil, err
}
Expand Down
17 changes: 17 additions & 0 deletions internal/config/config_core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,3 +335,20 @@ func TestGetAPIKey_ReturnsKey(t *testing.T) {
cfg := &Config{Gateway: &GatewayConfig{APIKey: "super-secret-key"}}
assert.Equal(t, "super-secret-key", cfg.GetAPIKey())
}

// TestLoadFromFile_NegativePayloadSizeThresholdRejected verifies that TOML configs with
// a negative payload_size_threshold are rejected per spec §4.1.3.3.
func TestLoadFromFile_NegativePayloadSizeThresholdRejected(t *testing.T) {
path := writeTempTOML(t, `
[gateway]
payload_size_threshold = -1

[servers.github]
command = "docker"
args = ["run", "--rm", "-i", "ghcr.io/github/github-mcp-server:latest"]
`)
cfg, err := LoadFromFile(path)
require.Error(t, err)
assert.Nil(t, cfg)
assert.Contains(t, err.Error(), "payload_size_threshold must be a positive integer")
}
10 changes: 7 additions & 3 deletions internal/config/config_stdin.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ type StdinGatewayConfig struct {
Domain string `json:"domain,omitempty"`
StartupTimeout *int `json:"startupTimeout,omitempty"`
ToolTimeout *int `json:"toolTimeout,omitempty"`
KeepaliveInterval *int `json:"keepaliveInterval,omitempty"`
PayloadDir string `json:"payloadDir,omitempty"`
TrustedBots []string `json:"trustedBots,omitempty"`
KeepaliveInterval *int `json:"keepaliveInterval,omitempty"`
PayloadDir string `json:"payloadDir,omitempty"`
PayloadSizeThreshold *int `json:"payloadSizeThreshold,omitempty"`
TrustedBots []string `json:"trustedBots,omitempty"`
Comment on lines +40 to +43
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

This change makes payloadSizeThreshold configurable via JSON stdin, but docs/CONFIGURATION.md currently lists “Payload size threshold” under “TOML-only / CLI-only options (not available in JSON stdin)”. Please update the documentation (and any related references) so users know the field is now supported in JSON stdin and what its JSON name is (payloadSizeThreshold).

See below for a potential fix:

	Port           *int   `json:"port,omitempty"`
	APIKey         string `json:"apiKey,omitempty"`
	Domain         string `json:"domain,omitempty"`
	StartupTimeout *int   `json:"startupTimeout,omitempty"`
	ToolTimeout    *int   `json:"toolTimeout,omitempty"`

	// KeepaliveInterval configures the keepalive interval in JSON stdin as `keepaliveInterval`.
	KeepaliveInterval *int `json:"keepaliveInterval,omitempty"`

	// PayloadDir configures the payload directory in JSON stdin as `payloadDir`.
	PayloadDir string `json:"payloadDir,omitempty"`

	// PayloadSizeThreshold configures the payload size threshold in JSON stdin as
	// `payloadSizeThreshold`.
	PayloadSizeThreshold *int `json:"payloadSizeThreshold,omitempty"`

	// TrustedBots configures trusted bot identifiers in JSON stdin as `trustedBots`.
	TrustedBots []string `json:"trustedBots,omitempty"`

	OpenTelemetry *StdinOpenTelemetryConfig `json:"opentelemetry,omitempty"`

Copilot uses AI. Check for mistakes.
OpenTelemetry *StdinOpenTelemetryConfig `json:"opentelemetry,omitempty"`
}

Expand Down Expand Up @@ -308,6 +309,9 @@ func convertStdinConfig(stdinCfg *StdinConfig) (*Config, error) {
if stdinCfg.Gateway.PayloadDir != "" {
cfg.Gateway.PayloadDir = stdinCfg.Gateway.PayloadDir
}
if stdinCfg.Gateway.PayloadSizeThreshold != nil {
cfg.Gateway.PayloadSizeThreshold = *stdinCfg.Gateway.PayloadSizeThreshold
}
if stdinCfg.Gateway.TrustedBots != nil {
if err := validateTrustedBots(stdinCfg.Gateway.TrustedBots); err != nil {
return nil, err
Expand Down
62 changes: 62 additions & 0 deletions internal/config/config_stdin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1134,3 +1134,65 @@ func TestStdinServerConfig_AuthJSON(t *testing.T) {
assert.False(t, exists, "auth should be a known field, not an additional property")
})
}

// TestConvertStdinConfig_PayloadSizeThreshold verifies that payloadSizeThreshold is
// correctly wired from the JSON stdin format to the internal config (spec §4.1.3.3).
func TestConvertStdinConfig_PayloadSizeThreshold(t *testing.T) {
intPtr := func(v int) *int { return &v }

t.Run("payloadSizeThreshold wired from stdin gateway config", func(t *testing.T) {
stdinCfg := &StdinConfig{
MCPServers: map[string]*StdinServerConfig{},
Gateway: &StdinGatewayConfig{
PayloadSizeThreshold: intPtr(1048576),
},
}

cfg, err := convertStdinConfig(stdinCfg)
require.NoError(t, err)
require.NotNil(t, cfg.Gateway)
assert.Equal(t, 1048576, cfg.Gateway.PayloadSizeThreshold)
})

t.Run("payloadSizeThreshold nil leaves default applied", func(t *testing.T) {
stdinCfg := &StdinConfig{
MCPServers: map[string]*StdinServerConfig{},
Gateway: &StdinGatewayConfig{},
}

cfg, err := convertStdinConfig(stdinCfg)
require.NoError(t, err)
require.NotNil(t, cfg.Gateway)
// applyDefaults should have set the default value
assert.Equal(t, DefaultPayloadSizeThreshold, cfg.Gateway.PayloadSizeThreshold)
})

t.Run("payloadSizeThreshold zero rejected per spec §4.1.3.3", func(t *testing.T) {
gateway := &StdinGatewayConfig{
PayloadSizeThreshold: intPtr(0),
}

err := validateGatewayConfig(gateway)
require.Error(t, err)
assert.Contains(t, err.Error(), "payloadSizeThreshold must be a positive integer")
})

t.Run("payloadSizeThreshold negative rejected per spec §4.1.3.3", func(t *testing.T) {
gateway := &StdinGatewayConfig{
PayloadSizeThreshold: intPtr(-1),
}

err := validateGatewayConfig(gateway)
require.Error(t, err)
assert.Contains(t, err.Error(), "payloadSizeThreshold must be a positive integer")
})

t.Run("payloadSizeThreshold one accepted", func(t *testing.T) {
gateway := &StdinGatewayConfig{
PayloadSizeThreshold: intPtr(1),
}

err := validateGatewayConfig(gateway)
assert.NoError(t, err)
})
}
5 changes: 5 additions & 0 deletions internal/config/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,11 @@ func validateGatewayConfig(gateway *StdinGatewayConfig) error {
}
}

// Validate payloadSizeThreshold per spec §4.1.3.3: must be a positive integer when present.
if gateway.PayloadSizeThreshold != nil && *gateway.PayloadSizeThreshold < 1 {
return fmt.Errorf("gateway.payloadSizeThreshold must be a positive integer, got %d (spec §4.1.3.3)", *gateway.PayloadSizeThreshold)
}

// Validate trustedBots per spec §4.1.3.4: must be non-empty array when present
if err := validateTrustedBots(gateway.TrustedBots); err != nil {
return err
Expand Down
10 changes: 10 additions & 0 deletions internal/config/validation_schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,16 @@ func fixSchemaBytes(schemaBytes []byte) ([]byte, error) {
},
"minItems": 1,
}

// Add keepaliveInterval to gatewayConfig.
// Spec §4.1.3.5: optional integer keepalive ping interval in seconds for HTTP
// MCP backends. The Go struct (StdinGatewayConfig.KeepaliveInterval) supports
// this field, but the embedded schema (v0.64.4) omits it, causing schema
// validation to reject the field when additionalProperties is false.
props["keepaliveInterval"] = map[string]interface{}{
"type": "integer",
"description": "Keepalive ping interval in seconds for HTTP MCP backends. Use -1 to disable, 0 or unset for gateway default (1500s), or a positive integer for a custom interval.",
Copy link

Copilot AI Apr 6, 2026

Choose a reason for hiding this comment

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

The newly added schema description says “0 or unset for gateway default (1500s)”, but in the JSON stdin path a user-provided keepaliveInterval=0 will be preserved (intPtrOrDefault returns 0) and GatewayConfig.HTTPKeepaliveInterval() converts 0 to a 0s duration (effectively disabling keepalive). This makes the schema/docs misleading and creates inconsistent behavior vs TOML (where 0 is normalized to the default). Consider either (a) treating 0 as unset/default during stdin conversion or validation, or (b) updating the schema description (and any related docs) to reflect that 0 disables keepalive and only nil uses the default.

Suggested change
"description": "Keepalive ping interval in seconds for HTTP MCP backends. Use -1 to disable, 0 or unset for gateway default (1500s), or a positive integer for a custom interval.",
"description": "Keepalive ping interval in seconds for HTTP MCP backends. Use -1 to disable, unset for the gateway default (1500s), 0 to disable HTTP keepalive, or a positive integer for a custom interval.",

Copilot uses AI. Check for mistakes.
}
}
}
}
Expand Down
43 changes: 43 additions & 0 deletions internal/config/validation_schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -686,3 +686,46 @@ func TestSchemaConfiguration(t *testing.T) {

t.Logf("Embedded schema size: %d bytes", len(embeddedSchemaBytes))
}

// TestFixSchemaBytes_keepaliveInterval verifies that keepaliveInterval is accepted by
// the schema after fixSchemaBytes is applied (spec §4.1.3.5).
// Prior to this fix, the field was silently rejected with additionalProperties:false
// even though the Go struct already supported it.
func TestFixSchemaBytes_keepaliveInterval(t *testing.T) {
validConfig := `{
"mcpServers": {
"github": {
"container": "ghcr.io/github/github-mcp-server:latest"
}
},
"gateway": {
"port": 8080,
"domain": "localhost",
"apiKey": "test-key",
"keepaliveInterval": 300
}
}`

err := validateJSONSchema([]byte(validConfig))
assert.NoError(t, err, "keepaliveInterval should be accepted by the schema (spec §4.1.3.5)")
}

// TestFixSchemaBytes_keepaliveIntervalNegative verifies that -1 (disable) is accepted.
func TestFixSchemaBytes_keepaliveIntervalNegative(t *testing.T) {
validConfig := `{
"mcpServers": {
"github": {
"container": "ghcr.io/github/github-mcp-server:latest"
}
},
"gateway": {
"port": 8080,
"domain": "localhost",
"apiKey": "test-key",
"keepaliveInterval": -1
}
}`

err := validateJSONSchema([]byte(validConfig))
assert.NoError(t, err, "keepaliveInterval -1 (disable) should be accepted by the schema")
}
Loading