coilyco-flight-deck/mcp-beaver

Watch

Upstream mode re-probes the upstream on every tool call, and that second session breaks against real Node MCP servers #67

New issue

Closed

opened 2026-08-15 15:12:17 +00:00 by coilyco-ops · 1 comment

coilyco-ops commented

2026-08-15 15:12:17 +00:00

Member

Filed by Olaf (ops, claude seat). Live: every serve-upstream deployment in the fleet is non-functional at call time. Three pods, two capabilities, both Sirens lanes.

Symptom

Ground truth from the tool result, not a summary:

refresh upstream tools: sending 'notifications/initialized': Bad Request

Reproduced twice per server in one turn, on trace 7ea1e319b92b0357d3e2ac71b802a66a, 2026-08-15 14:46Z. Affected: sirens-echo-playwright-mcp, sirens-deep-playwright-mcp, sirens-deep-bluesky-mcp. In the same turn nine spec-mode servers answered with real data, so this is specific to upstream mode rather than to the fleet or the network.

The startup handshake succeeds. Both pods log serving upstream proxy ... (8 MCP and HTTP tools) and (2 MCP and HTTP tools), so connectivity, allowlisting and tool discovery are all fine. Only the per-call path fails.

Where it comes from

internal/mcpserver/upstream.go:112 calls ensureFresh on every tool call. ensureFresh calls probeTools, and probeTools opens a brand-new MCP session:

client := mcp.NewClient(&mcp.Implementation{Name: "ward-mcp-probe", ...}, nil)
session, err := client.Connect(ctx, &mcp.StreamableClientTransport{...})
defer func() { _ = session.Close() }()
return session.ListTools(ctx, nil)

So each tool call is: open a second session beside the long-lived one, initialize, notifications/initialized, tools/list, close. The Node upstreams reject that notification with HTTP 400.

Both failing upstreams are Node servers, mcr.microsoft.com/playwright/mcp and coilyco-flight-deck/bluesky-mcp. My local reproduction used ward-mcp as its own upstream and passed, which is exactly why this did not show up in testing: ward-mcp accepts a second session and the real upstreams do not.

I did not determine whether the 400 is a session-id requirement, a single-session limit, or a protocol-version difference. That is the one open question and it wants a real Node upstream to answer, which is also the fixture this needs.

The design question underneath

Even fixed, re-probing the entire tool list on every call is worth reconsidering, because the deployment model already rules out what it defends against.

Every serve-upstream deployment here co-locates its upstream as a sidecar in the same pod, pinned by image digest. The rosters state the consequence as a guarantee:

Every server named here exposes a tool surface fixed at its image build, so a client may discover it once and cache it for the life of its process. A server that gains a runtime-variable tool list breaks that assumption and must land with the client-side cache in the same change.

A sidecar pinned by digest cannot change its tools while the pod lives, and a rollout replaces the pod. So the drift ensureFresh detects is not reachable in this topology, and the cost is real: every call pays a second round trip, and every call now depends on the upstream tolerating repeated session setup.

Options, roughly in order of how much I would trust them:

Reuse the long-lived session for the drift check rather than dialling a new one. Keeps the guarantee, removes the second session entirely.
Make the check opt-in or interval-based rather than per-call, so a digest-pinned sidecar can skip it.
Fix only the 400 and keep per-call probing. Restores service and leaves the round trip and the fragility.

There is currently no flag to disable it, so operators have no mitigation short of reverting the image.

Impact

Echo loses her browser. Deep loses its browser and the Bluesky reader that shipped this morning. All three report healthy: pods are 2/2 Running, probes pass, startup logs are clean, and nothing is logged at failure time on either container. The only signal is the tool result reaching the model, which is how this was found rather than by monitoring.

Acceptance

A serve-upstream tool call against a Node MCP upstream succeeds.
Covered by a test using a real non-ward MCP server, since ward-fronting-ward is what hid this.
Whatever lands, a digest-pinned sidecar does not pay a second session per call.

**Filed by Olaf (ops, claude seat).** Live: **every `serve-upstream` deployment in the fleet is non-functional at call time.** Three pods, two capabilities, both Sirens lanes. ## Symptom Ground truth from the tool result, not a summary: ``` refresh upstream tools: sending 'notifications/initialized': Bad Request ``` Reproduced twice per server in one turn, on trace `7ea1e319b92b0357d3e2ac71b802a66a`, 2026-08-15 14:46Z. Affected: `sirens-echo-playwright-mcp`, `sirens-deep-playwright-mcp`, `sirens-deep-bluesky-mcp`. In the same turn **nine spec-mode servers answered with real data**, so this is specific to upstream mode rather than to the fleet or the network. The startup handshake succeeds. Both pods log `serving upstream proxy ... (8 MCP and HTTP tools)` and `(2 MCP and HTTP tools)`, so connectivity, allowlisting and tool discovery are all fine. Only the per-call path fails. ## Where it comes from `internal/mcpserver/upstream.go:112` calls `ensureFresh` on **every** tool call. `ensureFresh` calls `probeTools`, and `probeTools` opens a **brand-new MCP session**: ```go client := mcp.NewClient(&mcp.Implementation{Name: "ward-mcp-probe", ...}, nil) session, err := client.Connect(ctx, &mcp.StreamableClientTransport{...}) defer func() { _ = session.Close() }() return session.ListTools(ctx, nil) ``` So each tool call is: open a second session beside the long-lived one, `initialize`, `notifications/initialized`, `tools/list`, close. The Node upstreams reject that notification with HTTP 400. Both failing upstreams are Node servers, `mcr.microsoft.com/playwright/mcp` and `coilyco-flight-deck/bluesky-mcp`. My local reproduction used ward-mcp as its own upstream and passed, which is exactly why this did not show up in testing: **ward-mcp accepts a second session and the real upstreams do not.** I did not determine whether the 400 is a session-id requirement, a single-session limit, or a protocol-version difference. That is the one open question and it wants a real Node upstream to answer, which is also the fixture this needs. ## The design question underneath Even fixed, re-probing the entire tool list on every call is worth reconsidering, because the deployment model already rules out what it defends against. Every `serve-upstream` deployment here co-locates its upstream as a sidecar in the same pod, pinned by image digest. The rosters state the consequence as a guarantee: > Every server named here exposes a tool surface fixed at its image build, so a client may discover it once and cache it for the life of its process. A server that gains a runtime-variable tool list breaks that assumption and must land with the client-side cache in the same change. A sidecar pinned by digest **cannot** change its tools while the pod lives, and a rollout replaces the pod. So the drift `ensureFresh` detects is not reachable in this topology, and the cost is real: every call pays a second round trip, and every call now depends on the upstream tolerating repeated session setup. Options, roughly in order of how much I would trust them: 1. **Reuse the long-lived session** for the drift check rather than dialling a new one. Keeps the guarantee, removes the second session entirely. 2. **Make the check opt-in or interval-based** rather than per-call, so a digest-pinned sidecar can skip it. 3. Fix only the 400 and keep per-call probing. Restores service and leaves the round trip and the fragility. There is currently **no flag to disable it**, so operators have no mitigation short of reverting the image. ## Impact Echo loses her browser. Deep loses its browser and the Bluesky reader that shipped this morning. All three report healthy: pods are `2/2 Running`, probes pass, startup logs are clean, and nothing is logged at failure time on either container. The only signal is the tool result reaching the model, which is how this was found rather than by monitoring. ## Acceptance * A `serve-upstream` tool call against a Node MCP upstream succeeds. * Covered by a test using a real non-ward MCP server, since ward-fronting-ward is what hid this. * Whatever lands, a digest-pinned sidecar does not pay a second session per call.

coilyco-ops added the

labels

2026-08-17 01:14:30 +00:00

coilyco-ops referenced this issue from a commit

2026-08-17 02:20:30 +00:00

fix(upstream): reuse the one session for the drift check (#67)

coilyco-ops closed this issue

2026-08-17 02:20:31 +00:00

coilyco-ops commented

2026-08-17 02:26:04 +00:00

Author

Member

Fixed on main in 8cc94bd.

probeTools now lists over the long-lived session opened at startup instead of dialling a second one, so a tool call is tools/list plus tools/call on one session. The endpoint, HTTP client and telemetry hook left the proxyBackend struct with it - once the session is up nothing redials, and holding the dial inputs is only an invitation to.

Option 1 from the issue. The drift guarantee is unchanged, and the second round trip a digest-pinned sidecar never needed goes away with the second session.

On the test. TestUpstreamProxyReusesOneSessionPerCall fronts a fixture that serves one handshake and answers every later initialize with 400, which is the constraint the real Node upstreams enforce. Borrowing another Go MCP server would have reproduced exactly the hole this fell through. Verified the fixture catches it: restoring the per-call dial fails at startup with list upstream tools: calling "initialize": sending "initialize": Bad Request.

TestUpstreamProxySchemaDriftFailsClosed had to change shape. It used to swap the whole *mcp.Server behind the endpoint, which an established session cannot see - and in production cannot happen either, since replacing the served binary replaces the pod and the session with it. It now mutates the tool on the same upstream instance, which is the drift a live session can actually observe.

The open question about why the 400 (session-id requirement, single-session limit, protocol-version difference) is not answered here and no longer blocks anything, since nothing opens a second session to find out.

Recorded in docs/DESIGN.md under "One upstream session, reused".

Fixed on `main` in `8cc94bd`. `probeTools` now lists over the long-lived session opened at startup instead of dialling a second one, so a tool call is `tools/list` plus `tools/call` on one session. The endpoint, HTTP client and telemetry hook left the `proxyBackend` struct with it - once the session is up nothing redials, and holding the dial inputs is only an invitation to. Option 1 from the issue. The drift guarantee is unchanged, and the second round trip a digest-pinned sidecar never needed goes away with the second session. **On the test.** `TestUpstreamProxyReusesOneSessionPerCall` fronts a fixture that serves one handshake and answers every later `initialize` with 400, which is the constraint the real Node upstreams enforce. Borrowing another Go MCP server would have reproduced exactly the hole this fell through. Verified the fixture catches it: restoring the per-call dial fails at startup with `list upstream tools: calling "initialize": sending "initialize": Bad Request`. `TestUpstreamProxySchemaDriftFailsClosed` had to change shape. It used to swap the whole `*mcp.Server` behind the endpoint, which an established session cannot see - and in production cannot happen either, since replacing the served binary replaces the pod and the session with it. It now mutates the tool on the same upstream instance, which is the drift a live session can actually observe. The open question about *why* the 400 (session-id requirement, single-session limit, protocol-version difference) is not answered here and no longer blocks anything, since nothing opens a second session to find out. Recorded in `docs/DESIGN.md` under "One upstream session, reused".

coilyco-ops referenced this issue

2026-08-17 03:32:53 +00:00

upstream passthrough: every tools/call terminates without response, 100% failure against a healthy upstream #79

coilyco-ops referenced this issue from a commit

2026-08-17 03:49:09 +00:00

fix(upstream): stop a slow call from killing the session for good (#79)

coilyco-ops referenced this issue

2026-08-17 03:49:32 +00:00

upstream passthrough: every tools/call terminates without response, 100% failure against a healthy upstream #79

coilyco-ops referenced this issue

2026-08-17 06:18:17 +00:00

upstream passthrough: session recovery reconnects into #67's Bad Request, so a lost session is still permanent #80