Skip to content

Respect client retry backoff in supervisor reconciliation - #1014

Open
antongulin wants to merge 1 commit into
smart-mcp-proxy:mainfrom
antongulin:fix/supervisor-reconnect-backoff
Open

Respect client retry backoff in supervisor reconciliation#1014
antongulin wants to merge 1 commit into
smart-mcp-proxy:mainfrom
antongulin:fix/supervisor-reconnect-backoff

Conversation

@antongulin

Copy link
Copy Markdown

Fixes #1013.

Impact

The supervisor's periodic 30s reconciliation planned ActionConnect for any enabled, unconnected server regardless of failure history, bypassing the managed client's exponential backoff and MaxConnectionRetries give-up. The ConnectAll path in internal/upstream/manager.go already gates on StateError && !ShouldRetry() — the per-server reconcile path did not. A dead or 401-ing remote upstream was re-dialed every 30 seconds forever (~3 HTTP requests per attempt, ~9k requests/day). In our case this tripped a third-party MCP vendor's server alarms twice and got our account paused (full incident detail in #1013).

Changes

  • internal/upstream/types/types.go — extracted the backoff formula from StateManager.ShouldRetry into RetryBackoffDuration(retryCount) (behavior unchanged: 1s → 2s → 4s … capped at 5min), and added ConnectionInfo.ShouldAutoReconnect(now): false while the backoff window is open, after GaveUp/MaxConnectionRetries, and for StatePendingAuth (redialing cannot succeed until the user completes OAuth login; each attempt costs real requests against the upstream).
  • internal/runtime/supervisor/supervisor.gocomputeReconcilePlan consults actual.ConnectionInfo.ShouldAutoReconnect(...) before planning ActionConnect and plans ActionNone (with a debug log) when the client's retry policy says wait.

Manual reconnects, login flows, reconnect_on_use, and config-change ActionReconnect are unaffected — only the automatic periodic redial is gated. Once the backoff window elapses, the next reconcile tick reconnects as before.

Verification

go test ./internal/upstream/... ./internal/runtime/...   # all pass (incl. new tests)
go vet ./internal/upstream/... ./internal/runtime/supervisor/
gofmt -l <changed files>                                  # clean

New tests:

  • TestRetryBackoffDuration — schedule values and overflow caps
  • TestConnectionInfo_ShouldAutoReconnect — nil/fresh/ready/pending-auth/backoff-open/backoff-elapsed/gave-up cases
  • TestSupervisor_Reconcile_RespectsRetryBackoff — reconcile skips a failed server inside its backoff window, a gave-up server, and a PendingAuth server; reconnects after backoff elapses

Log excerpt of the failure mode this prevents (identical cycle every 30s for two weeks):

18:15:52.759 | INFO  | Starting connection attempt          | {"server": "press-ranger", ...}
18:15:53.809 | ERROR | MCP initialize JSON-RPC call failed  | {"error": "transport error: authorization required"}
18:15:58.214 | ERROR | Connection failed                    | {"error": "OAuth authentication required ..."}
18:16:22.758 | INFO  | Starting connection attempt          | ...

The periodic 30s reconciliation planned ActionConnect for any enabled,
unconnected server regardless of its failure history, bypassing the managed
client's exponential backoff and MaxConnectionRetries give-up (the ConnectAll
path already gates on ShouldRetry; the per-server reconcile path did not).
A dead or 401-ing remote upstream was therefore re-dialed every 30 seconds
forever - roughly 3 HTTP requests per tick, ~9k requests/day against a server
that cannot answer.

computeReconcilePlan now consults the client's ConnectionInfo and plans
ActionNone while the backoff window is open, after the client gave up, and for
servers parked in PendingAuth awaiting user OAuth login. The backoff formula
is extracted from StateManager.ShouldRetry into RetryBackoffDuration so both
paths share one schedule.

Fixes smart-mcp-proxy#1013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Upstream reconnect has no backoff — supervisor re-dials failed servers every 30s forever, hammering dead remotes (~9k req/day)

1 participant