Skip to content

Commit 9717d7d

Browse files
feat(config): ping liveness + configurable discovery/health-check intervals
Replaces the heavyweight tools/list liveness probe with the MCP-standard lightweight `ping`, and makes both background cadences configurable globally and per-server. Ping liveness (US1, FR-001/002): - Add Client.Ping wrapper to internal/upstream/core/client.go (mcp-go Ping). - Rewrite managed performHealthCheck to probe via a narrow livenessProber interface (the coreClient in prod; a fake in tests) instead of ListTools, preserving the existing connection-error classification and flap resistance. An idle proxy no longer emits recurring tools/list to upstreams (#608). Configurable intervals (US2/US3, FR-004..009): - *Duration tri-state (nil=inherit, "0s"=disabled, positive=interval) on both Config (global) and ServerConfig (per-server): health_check_interval (30s default) and tool_discovery_interval (5m default). Defaults live only in the resolvers so unset keys behave exactly as before (SC-005). - ResolveHealthCheckInterval / ResolveToolDiscoveryInterval: per-server -> global -> default; resolved <=0 disables the loop. - Validate() enforces health-check {0} U [5s,1h] and tool-discovery {0} U [30s,24h] for every global and per-server pointer, with clear messages. - backgroundHealthCheck and backgroundToolIndexing now use resettable timers that re-resolve each cycle (hot-reload aware) and skip when disabled. - Per-server overrides round-trip through UpstreamRecord (storage canary). UI + docs: - Web settings "Tool discovery & health checks" accordion (fields.ts) and the mirrored macOS ConfigSection (SettingsCatalog.swift), duration controls with 0s=disabled help text. - docs/configuration.md documents both keys, defaults, ranges, 0s semantics, and the per-server override; swagger regenerated. Related #608 Co-Authored-By: Paperclip <noreply@paperclip.ing>
1 parent 89f8f0c commit 9717d7d

17 files changed

Lines changed: 634 additions & 27 deletions

File tree

docs/configuration.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,54 @@ MCPProxy looks for configuration in these locations (in order):
9696
| `tool_response_limit` | integer | `20000` | Maximum characters in tool responses (0 = unlimited) |
9797
| `call_tool_timeout` | string | `"2m"` | Timeout for tool calls (e.g., `"30s"`, `"2m"`, `"5m"`). **Note**: When using agents like Codex or Claude as MCP servers, you may need to increase this timeout significantly, even up to 10 minutes (`"10m"`), as these agents may require longer processing times for complex operations |
9898

99+
### Discovery & Health Checks
100+
101+
mcpproxy keeps each upstream connection alive and its tool index fresh with two
102+
background loops. Both intervals are tunable globally and per server, so you can
103+
quiet a chatty upstream that returns a large tool catalog.
104+
105+
```json
106+
{
107+
"health_check_interval": "30s",
108+
"tool_discovery_interval": "5m"
109+
}
110+
```
111+
112+
| Field | Type | Default | Description |
113+
|-------|------|---------|-------------|
114+
| `health_check_interval` | duration | `"30s"` | How often to probe each connected server for liveness with a lightweight MCP `ping`. `"0s"` disables the periodic probe. Range: `5s``1h`. |
115+
| `tool_discovery_interval` | duration | `"5m"` | How often to re-list every server's tools to rebuild the search index. `"0s"` disables the periodic sweep. Range: `30s``24h`. |
116+
117+
**Liveness uses `ping`, not `tools/list`.** The health-check loop issues the
118+
MCP-standard `ping` request rather than re-listing every tool, so an idle proxy
119+
no longer generates large recurring `tools/list` traffic to upstream servers
120+
(GitHub [#608](https://github.com/smart-mcp-proxy/mcpproxy-go/issues/608)). Tool
121+
changes are still picked up reactively whenever a server pushes
122+
`notifications/tools/list_changed`.
123+
124+
**Disabling a loop (`"0s"`).** Set either key to `"0s"` to turn the
125+
corresponding loop off:
126+
127+
- `health_check_interval: "0s"` — no periodic liveness probe. A dead transport
128+
is then detected lazily, on the next real tool call or discovery sweep, rather
129+
than proactively.
130+
- `tool_discovery_interval: "0s"` — no periodic index rebuild. Tools are still
131+
discovered at connect time and whenever a server pushes
132+
`notifications/tools/list_changed`. **Trade-off:** a server that does *not*
133+
support `list_changed` will not have new/removed tools reflected until it
134+
reconnects or you trigger a manual refresh.
135+
136+
An unset key behaves exactly as before this feature (the built-in default), and
137+
a change to either interval takes effect on the next cycle without restarting
138+
the proxy.
139+
140+
> **Per-server override.** Both keys can also be set on an individual server
141+
> entry under `mcpServers[]` (see [Server Fields](#server-fields)) to override
142+
> the global value for just that server; the per-server value wins, and `"0s"`
143+
> disables the loop for that server only. A dedicated per-server form control in
144+
> the Web UI / macOS app is planned; for now set per-server overrides via the
145+
> Raw JSON editor or the REST API.
146+
99147
### Debug & Development
100148

101149
```json
@@ -150,6 +198,8 @@ MCPProxy looks for configuration in these locations (in order):
150198
| `working_dir` | string | No | Working directory for stdio servers, or for the locally-launched child of an HTTP/SSE server (default: current directory) |
151199
| `env` | object | No | Environment variables for stdio servers, or for the locally-launched child of an HTTP/SSE server |
152200
| `launcher_wait_timeout` | duration | No | When `command` is set together with an HTTP/SSE `url`, how long mcpproxy waits for that URL to become reachable after spawning the child (e.g. `"15s"`, default `"30s"`) |
201+
| `health_check_interval` | duration | No | Per-server override for the global [`health_check_interval`](#discovery--health-checks). `"0s"` disables the liveness probe for this server only. Range: `5s``1h`. Omit to inherit the global value. |
202+
| `tool_discovery_interval` | duration | No | Per-server override for the global [`tool_discovery_interval`](#discovery--health-checks). Accepted and persisted for forward-compatibility; the periodic index rebuild currently runs on the global cadence. Range: `30s``24h`. Omit to inherit the global value. |
153203
| `oauth` | object | No | OAuth configuration (see [OAuth Configuration](#oauth-configuration)) |
154204
| `isolation` | object | No | Per-server Docker isolation settings (see [Docker Isolation](#docker-isolation)) |
155205
| `enabled` | boolean | No | Enable/disable server (default: `true`) |

frontend/src/views/settings/fields.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,15 @@ export const ADVANCED_ACCORDIONS: SettingsAccordion[] = [
331331
{ key: 'activity_cleanup_interval_min', label: 'Cleanup runs every (minutes)', control: 'number', min: 1 },
332332
],
333333
},
334+
{
335+
id: 'discovery',
336+
title: 'Tool discovery & health checks',
337+
description: 'How often mcpproxy probes upstream servers for liveness and re-discovers their tools. Lower these to reduce background traffic to chatty servers.',
338+
fields: [
339+
{ key: 'health_check_interval', label: 'Health-check interval', help: 'How often to send a lightweight liveness ping to each connected server. "0s" disables the periodic probe (a dead server is then detected lazily on the next tool call). Range: 5s–1h. Default 30s.', control: 'duration', placeholder: '30s', optional: true },
340+
{ key: 'tool_discovery_interval', label: 'Tool-discovery interval', help: 'How often to re-list every server’s tools to rebuild the search index. "0s" disables the periodic sweep — tool changes are then picked up only at connect time and via tools/list_changed push notifications. Range: 30s–24h. Default 5m.', control: 'duration', placeholder: '5m', optional: true },
341+
],
342+
},
334343
{
335344
id: 'logging',
336345
title: 'Logging',

internal/config/config.go

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,51 @@ func (d Duration) Duration() time.Duration {
5252
return time.Duration(d)
5353
}
5454

55+
// Built-in defaults for the discovery/health-check loops (spec 074). They live
56+
// here (not in DefaultConfig) so an unset key resolves to the historical
57+
// behaviour and existing configs are unchanged (SC-005).
58+
const (
59+
defaultHealthCheckInterval = 30 * time.Second
60+
defaultToolDiscoveryInterval = 5 * time.Minute
61+
)
62+
63+
// resolveInterval applies the per-server → global → default precedence for an
64+
// optional *Duration. A non-nil pointer wins at each level, including a pointer
65+
// to 0 ("disabled"). Returns the resolved duration; a value <= 0 means the
66+
// caller should disable the corresponding loop.
67+
func resolveInterval(server, global *Duration, def time.Duration) time.Duration {
68+
if server != nil {
69+
return server.Duration()
70+
}
71+
if global != nil {
72+
return global.Duration()
73+
}
74+
return def
75+
}
76+
77+
// ResolveHealthCheckInterval resolves the effective health-check probe interval
78+
// for a server: per-server override → global → 30s default. A resolved value
79+
// <= 0 disables the periodic probe for that server (spec 074, FR-006).
80+
func (c *Config) ResolveHealthCheckInterval(sc *ServerConfig) time.Duration {
81+
var server *Duration
82+
if sc != nil {
83+
server = sc.HealthCheckInterval
84+
}
85+
return resolveInterval(server, c.HealthCheckInterval, defaultHealthCheckInterval)
86+
}
87+
88+
// ResolveToolDiscoveryInterval resolves the effective tool-discovery sweep
89+
// interval: per-server override → global → 5m default. A resolved value <= 0
90+
// disables the periodic sweep (connect-time + reactive list_changed discovery
91+
// still run). The periodic index rebuild is global; pass nil for the sweep.
92+
func (c *Config) ResolveToolDiscoveryInterval(sc *ServerConfig) time.Duration {
93+
var server *Duration
94+
if sc != nil {
95+
server = sc.ToolDiscoveryInterval
96+
}
97+
return resolveInterval(server, c.ToolDiscoveryInterval, defaultToolDiscoveryInterval)
98+
}
99+
55100
// Config represents the main configuration structure
56101
type Config struct {
57102
Listen string `json:"listen" mapstructure:"listen"`
@@ -69,6 +114,15 @@ type Config struct {
69114
CallToolTimeout Duration `json:"call_tool_timeout" mapstructure:"call-tool-timeout" swaggertype:"string"`
70115
MaxResultSizeChars int `json:"max_result_size_chars,omitempty" mapstructure:"max-result-size-chars"` // Advertised on every tool as `_meta.anthropic/maxResultSizeChars`; raises Claude Code's inline-response ceiling from 50k to up to 500k chars. Set to 0 to disable.
71116

117+
// Discovery & health-check cadence (spec 074, #608). Both are *Duration
118+
// tri-state pointers: nil = inherit the built-in default; a pointer to 0s =
119+
// the loop is disabled; a positive value = that interval. Defaults live only
120+
// in the resolvers (ResolveHealthCheckInterval / ResolveToolDiscoveryInterval)
121+
// so an unset key behaves exactly as before this feature (SC-005). Validated
122+
// in Validate(): health-check ∈ {0} ∪ [5s,1h]; tool-discovery ∈ {0} ∪ [30s,24h].
123+
HealthCheckInterval *Duration `json:"health_check_interval,omitempty" mapstructure:"health-check-interval" swaggertype:"string"`
124+
ToolDiscoveryInterval *Duration `json:"tool_discovery_interval,omitempty" mapstructure:"tool-discovery-interval" swaggertype:"string"`
125+
72126
// Environment configuration for secure variable filtering
73127
Environment *secureenv.EnvConfig `json:"environment,omitempty" mapstructure:"environment"`
74128

@@ -257,8 +311,19 @@ type ServerConfig struct {
257311
// mcpproxy starts the process AND connects via network. Stdio servers ignore
258312
// this field. Zero or unset → 30s default.
259313
LauncherWaitTimeout Duration `json:"launcher_wait_timeout,omitempty" mapstructure:"launcher_wait_timeout" swaggertype:"string"`
260-
EnabledTools []string `json:"enabled_tools,omitempty" mapstructure:"enabled_tools"` // Allowlist: only these tools are exposed; mutually exclusive with disabled_tools
261-
DisabledTools []string `json:"disabled_tools,omitempty" mapstructure:"disabled_tools"` // Denylist: these tools are hidden; mutually exclusive with enabled_tools
314+
315+
// Per-server discovery & health-check overrides (spec 074). Same *Duration
316+
// tri-state as the global keys: nil = inherit the global value (or default),
317+
// pointer to 0s = disabled for this server, positive = that interval.
318+
// HealthCheckInterval is fully wired into the per-server health loop;
319+
// ToolDiscoveryInterval is accepted/validated and round-trips for
320+
// forward-compat, but the periodic index sweep is governed by the global
321+
// cadence in this iteration (see spec 074 plan §C).
322+
HealthCheckInterval *Duration `json:"health_check_interval,omitempty" mapstructure:"health_check_interval" swaggertype:"string"`
323+
ToolDiscoveryInterval *Duration `json:"tool_discovery_interval,omitempty" mapstructure:"tool_discovery_interval" swaggertype:"string"`
324+
325+
EnabledTools []string `json:"enabled_tools,omitempty" mapstructure:"enabled_tools"` // Allowlist: only these tools are exposed; mutually exclusive with disabled_tools
326+
DisabledTools []string `json:"disabled_tools,omitempty" mapstructure:"disabled_tools"` // Denylist: these tools are hidden; mutually exclusive with enabled_tools
262327

263328
// SourceRegistryID records which registry this server was added from (empty
264329
// for manually-configured servers). MCP-866: surfaced in the approval /
@@ -1257,6 +1322,14 @@ func (c *Config) ValidateDetailed() []ValidationError {
12571322
})
12581323
}
12591324

1325+
// Validate global discovery/health-check intervals (spec 074, FR-008).
1326+
if e := validateIntervalBound("health_check_interval", c.HealthCheckInterval, 5*time.Second, time.Hour); e != nil {
1327+
errors = append(errors, *e)
1328+
}
1329+
if e := validateIntervalBound("tool_discovery_interval", c.ToolDiscoveryInterval, 30*time.Second, 24*time.Hour); e != nil {
1330+
errors = append(errors, *e)
1331+
}
1332+
12601333
// Validate code execution configuration (0 means use default)
12611334
if c.CodeExecutionTimeoutMs != 0 && (c.CodeExecutionTimeoutMs < 1 || c.CodeExecutionTimeoutMs > 600000) {
12621335
errors = append(errors, ValidationError{
@@ -1365,6 +1438,14 @@ func (c *Config) ValidateDetailed() []ValidationError {
13651438
// Spec 074: per-upstream auth_broker validation + default application.
13661439
// No-op in the personal edition (stub); enforced in the server edition.
13671440
errors = append(errors, validateServerAuthBroker(server, fieldPrefix)...)
1441+
1442+
// Spec 074: per-server discovery/health-check interval overrides.
1443+
if e := validateIntervalBound(fieldPrefix+".health_check_interval", server.HealthCheckInterval, 5*time.Second, time.Hour); e != nil {
1444+
errors = append(errors, *e)
1445+
}
1446+
if e := validateIntervalBound(fieldPrefix+".tool_discovery_interval", server.ToolDiscoveryInterval, 30*time.Second, 24*time.Hour); e != nil {
1447+
errors = append(errors, *e)
1448+
}
13681449
}
13691450

13701451
// Validate DataDir exists (if specified and not empty).
@@ -1405,6 +1486,27 @@ func (c *Config) ValidateDetailed() []ValidationError {
14051486
return errors
14061487
}
14071488

1489+
// validateIntervalBound enforces the tri-state interval contract (spec 074,
1490+
// FR-008): nil is fine (inherit), 0s is fine (disabled), and any other value
1491+
// must fall within [min, max]. Returns nil when valid, or a ValidationError
1492+
// with a human-readable, actionable message.
1493+
func validateIntervalBound(field string, d *Duration, minVal, maxVal time.Duration) *ValidationError {
1494+
if d == nil {
1495+
return nil
1496+
}
1497+
v := d.Duration()
1498+
if v == 0 {
1499+
return nil // explicit "disabled"
1500+
}
1501+
if v < minVal || v > maxVal {
1502+
return &ValidationError{
1503+
Field: field,
1504+
Message: fmt.Sprintf("must be 0s (disabled) or between %s and %s, got %s", minVal, maxVal, v),
1505+
}
1506+
}
1507+
return nil
1508+
}
1509+
14081510
// isValidListenAddr checks if the listen address format is valid
14091511
func isValidListenAddr(addr string) bool {
14101512
// Allow :port format
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
package config
2+
3+
import (
4+
"testing"
5+
"time"
6+
)
7+
8+
func durPtr(d time.Duration) *Duration {
9+
v := Duration(d)
10+
return &v
11+
}
12+
13+
// TestResolveHealthCheckInterval covers the per-server → global → default
14+
// precedence and the pointer-to-zero "disabled" semantics (spec 074, FR-006/FR-007).
15+
func TestResolveHealthCheckInterval(t *testing.T) {
16+
cases := []struct {
17+
name string
18+
global *Duration
19+
server *Duration
20+
want time.Duration
21+
}{
22+
{"both unset → default", nil, nil, 30 * time.Second},
23+
{"global set, server unset → global", durPtr(45 * time.Second), nil, 45 * time.Second},
24+
{"server override wins", durPtr(45 * time.Second), durPtr(10 * time.Second), 10 * time.Second},
25+
{"server override wins over default when global unset", nil, durPtr(15 * time.Second), 15 * time.Second},
26+
{"global disabled (0s)", durPtr(0), nil, 0},
27+
{"server disabled (0s) wins over global interval", durPtr(45 * time.Second), durPtr(0), 0},
28+
{"server interval wins over global disabled", durPtr(0), durPtr(20 * time.Second), 20 * time.Second},
29+
}
30+
for _, tc := range cases {
31+
t.Run(tc.name, func(t *testing.T) {
32+
c := &Config{HealthCheckInterval: tc.global}
33+
sc := &ServerConfig{HealthCheckInterval: tc.server}
34+
got := c.ResolveHealthCheckInterval(sc)
35+
if got != tc.want {
36+
t.Errorf("ResolveHealthCheckInterval = %v, want %v", got, tc.want)
37+
}
38+
})
39+
}
40+
41+
// A nil server config must still resolve via global → default.
42+
c := &Config{HealthCheckInterval: durPtr(45 * time.Second)}
43+
if got := c.ResolveHealthCheckInterval(nil); got != 45*time.Second {
44+
t.Errorf("nil server: got %v, want 45s", got)
45+
}
46+
}
47+
48+
// TestResolveToolDiscoveryInterval mirrors the health-check resolver with the
49+
// 5m default.
50+
func TestResolveToolDiscoveryInterval(t *testing.T) {
51+
cases := []struct {
52+
name string
53+
global *Duration
54+
server *Duration
55+
want time.Duration
56+
}{
57+
{"both unset → default", nil, nil, 5 * time.Minute},
58+
{"global set", durPtr(10 * time.Minute), nil, 10 * time.Minute},
59+
{"server override wins", durPtr(10 * time.Minute), durPtr(2 * time.Minute), 2 * time.Minute},
60+
{"global disabled", durPtr(0), nil, 0},
61+
{"server disabled wins", durPtr(10 * time.Minute), durPtr(0), 0},
62+
}
63+
for _, tc := range cases {
64+
t.Run(tc.name, func(t *testing.T) {
65+
c := &Config{ToolDiscoveryInterval: tc.global}
66+
sc := &ServerConfig{ToolDiscoveryInterval: tc.server}
67+
got := c.ResolveToolDiscoveryInterval(sc)
68+
if got != tc.want {
69+
t.Errorf("ResolveToolDiscoveryInterval = %v, want %v", got, tc.want)
70+
}
71+
})
72+
}
73+
}
74+
75+
// TestValidateDiscoveryIntervalBounds covers FR-008: 0s accepted (disabled),
76+
// in-range accepted, out-of-range rejected — for both the global and per-server
77+
// pointers, across both keys.
78+
func TestValidateDiscoveryIntervalBounds(t *testing.T) {
79+
cases := []struct {
80+
name string
81+
mutate func(*Config)
82+
wantError bool
83+
}{
84+
// Health-check: 0s or [5s, 1h]
85+
{"health 0s accepted (disabled)", func(c *Config) { c.HealthCheckInterval = durPtr(0) }, false},
86+
{"health 5s accepted (min)", func(c *Config) { c.HealthCheckInterval = durPtr(5 * time.Second) }, false},
87+
{"health 1h accepted (max)", func(c *Config) { c.HealthCheckInterval = durPtr(time.Hour) }, false},
88+
{"health 2s rejected (below min)", func(c *Config) { c.HealthCheckInterval = durPtr(2 * time.Second) }, true},
89+
{"health 2h rejected (above max)", func(c *Config) { c.HealthCheckInterval = durPtr(2 * time.Hour) }, true},
90+
{"health negative rejected", func(c *Config) { c.HealthCheckInterval = durPtr(-1 * time.Second) }, true},
91+
// Tool-discovery: 0s or [30s, 24h]
92+
{"discovery 0s accepted (disabled)", func(c *Config) { c.ToolDiscoveryInterval = durPtr(0) }, false},
93+
{"discovery 30s accepted (min)", func(c *Config) { c.ToolDiscoveryInterval = durPtr(30 * time.Second) }, false},
94+
{"discovery 24h accepted (max)", func(c *Config) { c.ToolDiscoveryInterval = durPtr(24 * time.Hour) }, false},
95+
{"discovery 10s rejected (below min)", func(c *Config) { c.ToolDiscoveryInterval = durPtr(10 * time.Second) }, true},
96+
{"discovery 48h rejected (above max)", func(c *Config) { c.ToolDiscoveryInterval = durPtr(48 * time.Hour) }, true},
97+
// Per-server pointers validated too.
98+
{"per-server health 2s rejected", func(c *Config) {
99+
c.Servers = []*ServerConfig{{Name: "s", HealthCheckInterval: durPtr(2 * time.Second)}}
100+
}, true},
101+
{"per-server discovery 30s accepted", func(c *Config) {
102+
c.Servers = []*ServerConfig{{Name: "s", ToolDiscoveryInterval: durPtr(30 * time.Second)}}
103+
}, false},
104+
{"per-server discovery 5s rejected", func(c *Config) {
105+
c.Servers = []*ServerConfig{{Name: "s", ToolDiscoveryInterval: durPtr(5 * time.Second)}}
106+
}, true},
107+
}
108+
for _, tc := range cases {
109+
t.Run(tc.name, func(t *testing.T) {
110+
c := DefaultConfig()
111+
tc.mutate(c)
112+
errs := c.ValidateDetailed()
113+
hasIntervalErr := false
114+
for _, e := range errs {
115+
if containsAny(e.Field, "health_check_interval", "tool_discovery_interval") {
116+
hasIntervalErr = true
117+
}
118+
}
119+
if hasIntervalErr != tc.wantError {
120+
t.Errorf("validation interval error = %v, want %v (errors: %+v)", hasIntervalErr, tc.wantError, errs)
121+
}
122+
})
123+
}
124+
}
125+
126+
func containsAny(s string, subs ...string) bool {
127+
for _, sub := range subs {
128+
for i := 0; i+len(sub) <= len(s); i++ {
129+
if s[i:i+len(sub)] == sub {
130+
return true
131+
}
132+
}
133+
}
134+
return false
135+
}

0 commit comments

Comments
 (0)