diff --git a/internal/mcp/transport.go b/internal/mcp/transport.go index f8f92702..4266d28c 100644 --- a/internal/mcp/transport.go +++ b/internal/mcp/transport.go @@ -20,12 +20,34 @@ import ( // the advertisement is the promise a client acts on, and nothing here has // been read against the modern revision, let alone tested. // -// It is also not merely a documentation gap. pad_set_workspace pins a session -// default workspace, and the stateless era has no sessions for that pin to -// live in — so the modern era is not something pad happens not to advertise, -// it is something pad is not known to be able to serve. Establishing what it -// would take is a separate unit; until then the honest advertisement is the -// era pad was built against and is tested against. +// It is also not merely a documentation gap: something pad relies on is deleted +// by that era. internal/server/middleware_mcp_session.go keys the +// mcp-active-sessions gauge on the Mcp-Session-Id header, and the generate-only +// session-id manager at this transport's call site exists SO THAT the header is +// always minted and the gauge stays observable. SEP-2567 removes session IDs in +// 2026-07-28 — a server serving that version never mints or echoes one — so in +// that era nothing pad mints is available to key on. The tracker does fall back +// to a client-supplied REQUEST header, so the honest claim is under-counting by +// a margin nobody controls rather than a flat zero; either way it is missing +// numbers rather than wrong ones, in the direction that reads as quiet. The +// full statement is at that header's declaration. Whoever opens that era +// re-keys the gauge first; the cost is recorded at the metric's definition. +// +// AN EARLIER VERSION OF THIS COMMENT GAVE A DIFFERENT AND FALSE REASON, and it +// is worth the four lines because the false one is the plausible one. It said +// pad_set_workspace pins a session default workspace that the stateless era has +// nowhere to keep. That is true of the LOCAL stdio transport and false of this +// one: cmd/pad builds the cloud dispatcher with a SHARED workspace state whose +// ResolveDefault() returns "" by construction (BUG-1865, the cross-user +// workspace bleed), so the pin is recorded and never consulted here, and +// resolution is already per-request — the explicit workspace argument, else a +// default derived from the caller's own OAuth identity and token allow-list. +// This transport has therefore been stateless with respect to workspace +// resolution since that bug was fixed, and the fix for a cross-user bug turns +// out to be most of the work a stateless era would need. +// +// Until the gauge is re-keyed, the honest advertisement is the era pad was +// built against and is tested against. // // DERIVED, NOT LISTED, and that is load-bearing. mcp.LegacyProtocolVersions() // is the SDK's own answer to "which revisions use the handshake", so a future diff --git a/internal/server/middleware_mcp_session.go b/internal/server/middleware_mcp_session.go index f2f52a68..a80d9337 100644 --- a/internal/server/middleware_mcp_session.go +++ b/internal/server/middleware_mcp_session.go @@ -269,6 +269,32 @@ func (s *Server) stopMCPSessionTracker() { // the audit row's status, so this matches the audit row's view. // // No-op when the tracker isn't wired (selfhost / tests). +// +// ACCEPTED COST, RECORDED HERE BECAUSE THIS IS WHERE THE KEY IS CHOSEN +// (TASK-2977, ruled day 62). This gauge is keyed on a header the MCP protocol +// DELETES: SEP-2567 removes session IDs in revision 2026-07-28, and a server +// serving that revision never mints or echoes one. So in that era there is no +// PROTOCOL-PROVIDED id for this to key on. +// +// Read trackMCPSession below before believing the stronger claim, which an +// earlier draft of this comment made: the id falls back to the REQUEST header, +// so a modern-era client that volunteers an Mcp-Session-Id anyway is still +// tracked. The accurate statement is therefore not "that era is invisible" but +// "the gauge stops depending on anything pad mints and starts depending on +// whether clients keep sending a header the spec removed" — under-counting by a +// margin nobody controls, in the direction that reads as quiet rather than as +// breakage. +// +// It costs nothing today because pad's remote transport does not advertise that +// revision: ServedProtocolVersions in internal/mcp restricts it to the +// handshake era, and the gauge's observability is one of the two reasons stated +// there. Named by path rather than as a symbol because it is not callable from +// here — internal/mcp imports this package, not the other way round, which is +// also why the transport is handed to the router as a plain http.Handler. The +// obligation is on whoever opens that era — RE-KEY THIS GAUGE FIRST, on +// something the modern era carries (the caller's identity plus a per-connection +// value), because the era's arrival is exactly the moment a silently-flat gauge +// would be read as "no MCP traffic" instead of "no measurement". const mcpSessionIDHeader = "Mcp-Session-Id" func (s *Server) trackMCPSession(reqHeader, respHeader func(string) string, method string, httpStatus int) {