Skip to content

Commit d98458b

Browse files
Dumbrisclaude
Dumbris
and
claude
authored
feat: add reconnect_on_use for on-demand upstream reconnection (#363)
* feat: add reconnect_on_use option for on-demand upstream reconnection When enabled per-server, tool calls targeting a disconnected upstream will trigger a bounded immediate reconnect attempt (15s timeout) before failing. If reconnection succeeds, the tool call is retried automatically. This improves UX for intermittent servers like remote devtools, SSH tunnels, and OAuth-backed services. Key implementation details: - New `reconnect_on_use` field in ServerConfig (opt-in, default false) - New `TryReconnectSync()` method on managed.Client for synchronous reconnection (vs ForceReconnect which is async) - Manager.CallTool() releases m.mu.RLock during reconnect to avoid blocking other operations - Reconnect is skipped for user-logged-out, quarantined, or already-connecting servers - Uses existing reconnectInProgress flag to prevent storms - Concurrent callers wait for in-progress reconnect via polling Fixes #354 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: regenerate OpenAPI spec for reconnect_on_use field Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: wire reconnect_on_use through REST API, storage, and server list The reconnect_on_use config field was only added to the config struct and manager logic but was missing from the full persistence chain: - REST API AddServerRequest and PatchServer handler - BBolt storage (UpstreamRecord, Save/Get/List) - Runtime server map for StateView - Management service ListServers converter - Contract types for API response - CLI client AddServerRequest - UpdateServer method in server.go Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: regenerate OpenAPI spec for reconnect_on_use field Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Code <noreply@anthropic.com>
1 parent 3770ece commit d98458b

17 files changed

Lines changed: 649 additions & 96 deletions

File tree

‎docs/configuration.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ MCPProxy looks for configuration in these locations (in order):
153153
| `isolation` | object | No | Per-server Docker isolation settings (see [Docker Isolation](#docker-isolation)) |
154154
| `enabled` | boolean | No | Enable/disable server (default: `true`) |
155155
| `quarantined` | boolean | No | Security quarantine status (default: `false` for manually added servers, `true` for LLM-added servers) |
156+
| `reconnect_on_use` | boolean | No | When `true`, tool calls to a disconnected server trigger an immediate reconnect attempt (15s timeout) before failing (default: `false`) |
156157
| `created` | string | No | ISO 8601 timestamp (auto-generated) |
157158
| `updated` | string | No | ISO 8601 timestamp (auto-updated) |
158159

‎internal/cliclient/client.go‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -911,16 +911,17 @@ func (c *Client) TriggerOAuthLogout(ctx context.Context, serverName string) erro
911911

912912
// AddServerRequest represents the request body for adding a server.
913913
type AddServerRequest struct {
914-
Name string `json:"name"`
915-
URL string `json:"url,omitempty"`
916-
Command string `json:"command,omitempty"`
917-
Args []string `json:"args,omitempty"`
918-
Env map[string]string `json:"env,omitempty"`
919-
Headers map[string]string `json:"headers,omitempty"`
920-
WorkingDir string `json:"working_dir,omitempty"`
921-
Protocol string `json:"protocol,omitempty"`
922-
Enabled *bool `json:"enabled,omitempty"`
923-
Quarantined *bool `json:"quarantined,omitempty"`
914+
Name string `json:"name"`
915+
URL string `json:"url,omitempty"`
916+
Command string `json:"command,omitempty"`
917+
Args []string `json:"args,omitempty"`
918+
Env map[string]string `json:"env,omitempty"`
919+
Headers map[string]string `json:"headers,omitempty"`
920+
WorkingDir string `json:"working_dir,omitempty"`
921+
Protocol string `json:"protocol,omitempty"`
922+
Enabled *bool `json:"enabled,omitempty"`
923+
Quarantined *bool `json:"quarantined,omitempty"`
924+
ReconnectOnUse *bool `json:"reconnect_on_use,omitempty"`
924925
}
925926

926927
// AddServerResult represents the result of adding a server.

‎internal/config/config.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,8 @@ type ServerConfig struct {
193193
Shared bool `json:"shared,omitempty" mapstructure:"shared"` // Server edition: shared with all users
194194
Created time.Time `json:"created" mapstructure:"created"`
195195
Updated time.Time `json:"updated,omitempty" mapstructure:"updated"`
196-
Isolation *IsolationConfig `json:"isolation,omitempty" mapstructure:"isolation"` // Per-server isolation settings
196+
Isolation *IsolationConfig `json:"isolation,omitempty" mapstructure:"isolation"` // Per-server isolation settings
197+
ReconnectOnUse bool `json:"reconnect_on_use,omitempty" mapstructure:"reconnect-on-use"` // Attempt reconnection when a tool call targets a disconnected server
197198
}
198199

199200
// OAuthConfig represents OAuth configuration for a server

‎internal/config/config_test.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,3 +1260,69 @@ func TestTelemetryConfig_OmittedWhenNil(t *testing.T) {
12601260
// telemetry should not appear in JSON when nil
12611261
assert.NotContains(t, string(data), "telemetry")
12621262
}
1263+
1264+
func TestServerConfig_ReconnectOnUse(t *testing.T) {
1265+
t.Run("defaults to false", func(t *testing.T) {
1266+
server := &ServerConfig{
1267+
Name: "test-server",
1268+
Enabled: true,
1269+
}
1270+
assert.False(t, server.ReconnectOnUse)
1271+
})
1272+
1273+
t.Run("parses from JSON when true", func(t *testing.T) {
1274+
jsonStr := `{"name":"test","enabled":true,"reconnect_on_use":true}`
1275+
var server ServerConfig
1276+
err := json.Unmarshal([]byte(jsonStr), &server)
1277+
require.NoError(t, err)
1278+
assert.True(t, server.ReconnectOnUse)
1279+
})
1280+
1281+
t.Run("parses from JSON when false", func(t *testing.T) {
1282+
jsonStr := `{"name":"test","enabled":true,"reconnect_on_use":false}`
1283+
var server ServerConfig
1284+
err := json.Unmarshal([]byte(jsonStr), &server)
1285+
require.NoError(t, err)
1286+
assert.False(t, server.ReconnectOnUse)
1287+
})
1288+
1289+
t.Run("omitted from JSON when false", func(t *testing.T) {
1290+
server := &ServerConfig{
1291+
Name: "test",
1292+
Enabled: true,
1293+
ReconnectOnUse: false,
1294+
}
1295+
data, err := json.Marshal(server)
1296+
require.NoError(t, err)
1297+
assert.NotContains(t, string(data), "reconnect_on_use")
1298+
})
1299+
1300+
t.Run("present in JSON when true", func(t *testing.T) {
1301+
server := &ServerConfig{
1302+
Name: "test",
1303+
Enabled: true,
1304+
ReconnectOnUse: true,
1305+
}
1306+
data, err := json.Marshal(server)
1307+
require.NoError(t, err)
1308+
assert.Contains(t, string(data), `"reconnect_on_use":true`)
1309+
})
1310+
1311+
t.Run("round-trip serialization", func(t *testing.T) {
1312+
server := &ServerConfig{
1313+
Name: "reconnect-test",
1314+
URL: "http://localhost:3000",
1315+
Protocol: "http",
1316+
Enabled: true,
1317+
ReconnectOnUse: true,
1318+
Created: time.Now(),
1319+
}
1320+
data, err := json.Marshal(server)
1321+
require.NoError(t, err)
1322+
1323+
var restored ServerConfig
1324+
err = json.Unmarshal(data, &restored)
1325+
require.NoError(t, err)
1326+
assert.Equal(t, server.ReconnectOnUse, restored.ReconnectOnUse)
1327+
})
1328+
}

‎internal/contracts/types.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,10 @@ type Server struct {
4545
ShouldRetry bool `json:"should_retry,omitempty"`
4646
RetryCount int `json:"retry_count,omitempty"`
4747
LastRetryTime *time.Time `json:"last_retry_time,omitempty"`
48-
UserLoggedOut bool `json:"user_logged_out,omitempty"` // True if user explicitly logged out (prevents auto-reconnection)
49-
Health *HealthStatus `json:"health,omitempty"` // Unified health status calculated by the backend
50-
Quarantine *QuarantineStats `json:"quarantine,omitempty"` // Tool quarantine metrics for this server
48+
UserLoggedOut bool `json:"user_logged_out,omitempty"` // True if user explicitly logged out (prevents auto-reconnection)
49+
Health *HealthStatus `json:"health,omitempty"` // Unified health status calculated by the backend
50+
Quarantine *QuarantineStats `json:"quarantine,omitempty"` // Tool quarantine metrics for this server
51+
ReconnectOnUse bool `json:"reconnect_on_use,omitempty"` // Attempt reconnection when a tool call targets this disconnected server
5152
}
5253

5354
// QuarantineStats represents tool quarantine metrics for a server.

‎internal/httpapi/server.go‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -987,16 +987,17 @@ func (s *Server) enrichServersWithQuarantineStats(servers []contracts.Server) {
987987

988988
// AddServerRequest represents a request to add a new server
989989
type AddServerRequest struct {
990-
Name string `json:"name"`
991-
URL string `json:"url,omitempty"`
992-
Command string `json:"command,omitempty"`
993-
Args []string `json:"args,omitempty"`
994-
Env map[string]string `json:"env,omitempty"`
995-
Headers map[string]string `json:"headers,omitempty"`
996-
WorkingDir string `json:"working_dir,omitempty"`
997-
Protocol string `json:"protocol,omitempty"`
998-
Enabled *bool `json:"enabled,omitempty"`
999-
Quarantined *bool `json:"quarantined,omitempty"`
990+
Name string `json:"name"`
991+
URL string `json:"url,omitempty"`
992+
Command string `json:"command,omitempty"`
993+
Args []string `json:"args,omitempty"`
994+
Env map[string]string `json:"env,omitempty"`
995+
Headers map[string]string `json:"headers,omitempty"`
996+
WorkingDir string `json:"working_dir,omitempty"`
997+
Protocol string `json:"protocol,omitempty"`
998+
Enabled *bool `json:"enabled,omitempty"`
999+
Quarantined *bool `json:"quarantined,omitempty"`
1000+
ReconnectOnUse *bool `json:"reconnect_on_use,omitempty"`
10001001
}
10011002

10021003
// handleAddServer godoc
@@ -1064,6 +1065,9 @@ func (s *Server) handleAddServer(w http.ResponseWriter, r *http.Request) {
10641065
Enabled: enabled,
10651066
Quarantined: quarantined,
10661067
}
1068+
if req.ReconnectOnUse != nil {
1069+
serverConfig.ReconnectOnUse = *req.ReconnectOnUse
1070+
}
10671071

10681072
// Add server via controller
10691073
logger := s.getRequestLogger(r) // T019: Use request-scoped logger
@@ -1196,6 +1200,10 @@ func (s *Server) handlePatchServer(w http.ResponseWriter, r *http.Request) {
11961200
updates.Quarantined = *req.Quarantined
11971201
hasUpdates = true
11981202
}
1203+
if req.ReconnectOnUse != nil {
1204+
updates.ReconnectOnUse = *req.ReconnectOnUse
1205+
hasUpdates = true
1206+
}
11991207

12001208
if !hasUpdates {
12011209
s.writeError(w, r, http.StatusBadRequest, "No fields to update")

‎internal/management/service.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -321,6 +321,11 @@ func (s *service) ListServers(ctx context.Context) ([]*contracts.Server, *contra
321321
srv.Updated = updated
322322
}
323323

324+
// Extract reconnect_on_use
325+
if reconnectOnUse, ok := srvRaw["reconnect_on_use"].(bool); ok {
326+
srv.ReconnectOnUse = reconnectOnUse
327+
}
328+
324329
// Extract unified health status
325330
if health, ok := srvRaw["health"].(*contracts.HealthStatus); ok {
326331
srv.Health = health

‎internal/runtime/runtime.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1710,6 +1710,11 @@ func (r *Runtime) GetAllServers() ([]map[string]interface{}, error) {
17101710
"authenticated": authenticated,
17111711
}
17121712

1713+
// Add reconnect_on_use from config
1714+
if serverStatus.Config != nil && serverStatus.Config.ReconnectOnUse {
1715+
serverMap["reconnect_on_use"] = true
1716+
}
1717+
17131718
// Add OAuth status fields if available
17141719
if oauthStatus != "" {
17151720
serverMap["oauth_status"] = oauthStatus

‎internal/server/server.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1113,6 +1113,7 @@ func (s *Server) UpdateServer(ctx context.Context, serverName string, updates *c
11131113
// when the caller explicitly provided these fields
11141114
existing.Enabled = updates.Enabled
11151115
existing.Quarantined = updates.Quarantined
1116+
existing.ReconnectOnUse = updates.ReconnectOnUse
11161117

11171118
// Save to storage
11181119
if err := storageManager.SaveUpstreamServer(existing); err != nil {

‎internal/storage/async_ops_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ func TestSaveServerSyncFieldCoverage(t *testing.T) {
244244
"Isolation": true,
245245
"Shared": true, // Teams-only: persisted in JSON config, not in BBolt
246246
"SkipQuarantine": true, // Spec 032: runtime-only field, not persisted to BBolt
247+
"ReconnectOnUse": true, // Spec 354: persisted to BBolt for on-demand reconnection
247248
}
248249

249250
// Get all fields from ServerConfig

0 commit comments

Comments
 (0)