coilyco-gaming/sirens-echo

Watch

MCP sessions are opened and destroyed per turn, so nothing can be cached or notified #116

New issue

Closed

opened 2026-08-11 23:04:32 +00:00 by coilyco-ops · 2 comments

coilyco-ops commented

2026-08-11 23:04:32 +00:00

Member

Complete calls Tools.Open at the top of every model turn and closes the session on the way out (internal/community/proxy.go:219 with the deferred Close, at 3812935). Open connects, initializes, and walks the paginated ListTools cursor for every configured server (internal/community/mcp.go:61-117).

So each turn pays a full connect, initialize, and complete tool enumeration against every server, then discards all of it.

Two costs:

Latency in front of every message. A Discord reply waits on N handshakes plus N tool enumerations before the first token of the model request goes out.
It forecloses notifications/tools/list_changed. There is no session alive to receive one. Echo therefore re-discovers the roster constantly while having no way to be told when it actually changed, which is the worst of both options.

Fix

Hold long-lived clients supervised outside the turn:

One client per configured server, connected at startup, kept alive across turns.
Cache the tool listing, invalidated by notifications/tools/list_changed rather than by re-listing every turn.
Health checks with reconnect and backoff, feeding the per-server availability state in #115.
A turn reads the cached roster and calls through the live sessions instead of building anything.

Why this is smaller than it sounds

Turns are serialized. slots has capacity 1 (internal/community/agent.go:113), so there is no concurrent-turn fan-out to coordinate and no per-turn connection multiplication to unwind. This is a shared pool with a single consumer, not a concurrency problem.

Sequence after #115, since per-server availability state is the thing the supervisor drives.

`Complete` calls `Tools.Open` at the top of every model turn and closes the session on the way out (`internal/community/proxy.go:219` with the deferred `Close`, at `3812935`). `Open` connects, initializes, and walks the paginated `ListTools` cursor for every configured server (`internal/community/mcp.go:61-117`). So each turn pays a full connect, initialize, and complete tool enumeration against every server, then discards all of it. Two costs: * **Latency in front of every message.** A Discord reply waits on N handshakes plus N tool enumerations before the first token of the model request goes out. * **It forecloses `notifications/tools/list_changed`.** There is no session alive to receive one. Echo therefore re-discovers the roster constantly while having no way to be told when it actually changed, which is the worst of both options. ## Fix Hold long-lived clients supervised outside the turn: * One client per configured server, connected at startup, kept alive across turns. * Cache the tool listing, invalidated by `notifications/tools/list_changed` rather than by re-listing every turn. * Health checks with reconnect and backoff, feeding the per-server availability state in #115. * A turn reads the cached roster and calls through the live sessions instead of building anything. ## Why this is smaller than it sounds Turns are serialized. `slots` has capacity 1 (`internal/community/agent.go:113`), so there is no concurrent-turn fan-out to coordinate and no per-turn connection multiplication to unwind. This is a shared pool with a single consumer, not a concurrency problem. Sequence after #115, since per-server availability state is the thing the supervisor drives.

coilyco-ops added the

priority

autonomy

headless

labels

2026-08-11 23:04:32 +00:00

coilyco-ops referenced this issue

2026-08-11 23:05:05 +00:00

Move the MCP roster to deploy so Echo names no specific MCP in config or code #117

coilyco-ops referenced this issue

2026-08-11 23:05:24 +00:00

Support MCP resources and prompts, not tools alone #118

coilyco-ops referenced this issue

2026-08-11 23:05:41 +00:00

Serve Echo's turn over MCP, so fleet clients call it natively #119

coilyco-ops commented

2026-08-11 23:24:45 +00:00

Author

Member

Design blocker found while starting this, worth settling before implementation.

The cached-roster-invalidated-by-tools/list_changed half of this issue is not reachable with the current transport config. MCPProvider.Open sets DisableStandaloneSSE: true on every StreamableClientTransport. The SDK documents that flag as: the client "will only receive responses to its own POST requests. Server-initiated messages will not be received."

tools/list_changed is a server-initiated notification, so today it cannot arrive at all. The SDK does support the handler (ClientOptions.ToolListChangedHandler, and it auto-subscribes when set), so the client side is ready. The transport is what blocks it.

The fork

Flipping DisableStandaloneSSE to false means each server holds an open GET SSE stream for the life of the process. That is a persistent connection per server, which is exactly what the current setting avoids, and the SDK lists "the server doesn't properly handle GET requests for SSE streams" as one reason to keep it disabled.

Whether every server in the roster handles a standalone GET SSE stream correctly is not something this repo can establish. The sealed-role rule bars debugging against the live cluster, and the servers in question are the Eco MCP and the private Forgejo MCP.

Options

Enable standalone SSE and cache with real invalidation. Full value of this issue. Requires confidence that every rostered server serves GET SSE, and accepts one persistent connection per server.
Long-lived sessions with a TTL-based roster refresh, standalone SSE still disabled. Gets the latency win and drops the per-turn handshake, without depending on server-initiated notifications. Staleness is bounded by the TTL rather than eliminated.
Long-lived sessions, re-list every turn. Keeps the handshake saving only. Least value, least risk.

Option 2 is the safe default: it captures the latency win, which is the larger practical cost, without betting on GET SSE behavior nobody here can verify. Option 1 can follow once someone with live access confirms both servers hold a standalone stream.

Not implementing until this is decided, since options 1 and 2 build different supervisors.

Design blocker found while starting this, worth settling before implementation. The cached-roster-invalidated-by-`tools/list_changed` half of this issue is **not reachable with the current transport config**. `MCPProvider.Open` sets `DisableStandaloneSSE: true` on every `StreamableClientTransport`. The SDK documents that flag as: the client "will only receive responses to its own POST requests. Server-initiated messages will not be received." `tools/list_changed` is a server-initiated notification, so today it cannot arrive at all. The SDK does support the handler (`ClientOptions.ToolListChangedHandler`, and it auto-subscribes when set), so the client side is ready. The transport is what blocks it. ## The fork Flipping `DisableStandaloneSSE` to false means each server holds an open GET SSE stream for the life of the process. That is a persistent connection per server, which is exactly what the current setting avoids, and the SDK lists "the server doesn't properly handle GET requests for SSE streams" as one reason to keep it disabled. Whether every server in the roster handles a standalone GET SSE stream correctly is not something this repo can establish. The sealed-role rule bars debugging against the live cluster, and the servers in question are the Eco MCP and the private Forgejo MCP. ## Options 1. **Enable standalone SSE and cache with real invalidation.** Full value of this issue. Requires confidence that every rostered server serves GET SSE, and accepts one persistent connection per server. 2. **Long-lived sessions with a TTL-based roster refresh, standalone SSE still disabled.** Gets the latency win and drops the per-turn handshake, without depending on server-initiated notifications. Staleness is bounded by the TTL rather than eliminated. 3. **Long-lived sessions, re-list every turn.** Keeps the handshake saving only. Least value, least risk. Option 2 is the safe default: it captures the latency win, which is the larger practical cost, without betting on GET SSE behavior nobody here can verify. Option 1 can follow once someone with live access confirms both servers hold a standalone stream. Not implementing until this is decided, since options 1 and 2 build different supervisors.

coilyco-ops referenced this issue

2026-08-11 23:33:31 +00:00

Transport is hardcoded to streamable HTTP, so stdio and SSE servers cannot be rostered #120

coilyco-ops commented

2026-08-11 23:33:52 +00:00

Author

Member

Superseding my previous comment. The three options I laid out were too narrow, and the framing was wrong.

I treated "can the roster be cached and invalidated" as one global decision about a single hardcoded transport. It is not. Notification support is a property of each server's transport, and Echo only speaks one transport today, which is the actual gap. Filed as #120.

Once transport is a first-class property of a roster entry:

stdio and 2024-11-05 SSE hold persistent connections by construction, so tools/list_changed arrives natively for those servers.
Streamable HTTP delivers it only with the standalone SSE stream enabled, which is the DisableStandaloneSSE: true question from before, now scoped to one transport instead of the whole roster.

So the supervisor asks each connection whether it delivers server-initiated notifications, caches with real invalidation where it does, and falls back to a bounded TTL refresh only where it does not. That is one design rather than a fork, and it degrades per server the same way #115 does.

The DisableStandaloneSSE decision does not disappear, it just stops being a blocker for this issue. It becomes a per-server choice a streamable entry can carry, still needing live confirmation that a given server serves GET SSE before flipping it on for that server.

This issue should now depend on #120 and be implemented after it. Implementing the supervisor first would hardcode the streamable assumption a second time, in the layer meant to abstract it.

Superseding my previous comment. The three options I laid out were too narrow, and the framing was wrong. I treated "can the roster be cached and invalidated" as one global decision about a single hardcoded transport. It is not. Notification support is a property of each server's transport, and Echo only speaks one transport today, which is the actual gap. Filed as #120. Once transport is a first-class property of a roster entry: * stdio and 2024-11-05 SSE hold persistent connections by construction, so `tools/list_changed` arrives natively for those servers. * Streamable HTTP delivers it only with the standalone SSE stream enabled, which is the `DisableStandaloneSSE: true` question from before, now scoped to one transport instead of the whole roster. So the supervisor asks each connection whether it delivers server-initiated notifications, caches with real invalidation where it does, and falls back to a bounded TTL refresh only where it does not. That is one design rather than a fork, and it degrades per server the same way #115 does. The `DisableStandaloneSSE` decision does not disappear, it just stops being a blocker for this issue. It becomes a per-server choice a streamable entry can carry, still needing live confirmation that a given server serves GET SSE before flipping it on for that server. This issue should now depend on #120 and be implemented after it. Implementing the supervisor first would hardcode the streamable assumption a second time, in the layer meant to abstract it.

coilyco-ops referenced this issue from a commit

2026-08-11 23:50:20 +00:00

feat(mcp): support stdio, SSE, and streamable transports

coilyco-ops referenced this issue from a commit

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

feat(mcp): supervise MCP connections across turns instead of rebuilding them

coilyco-ops closed this issue

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