MCP Streamable HTTP: add stateless Mcp-Session-Id support

[Copilot]

Summary

Add stateless Mcp-Session-Id support for the MCP Streamable HTTP transport (spec 2025-03-26).

Fixes #21999 — Codex fails during MCP initialization because Netdata did not return Mcp-Session-Id in the initialize response.

Approach

The session ID is a stateless opaque token — no server-side session state is kept.

• Parse incoming Mcp-Session-Id request header into web_client.mcp_session_id (as nd_uuid_t — 16 bytes, no heap)
• If no session ID received (first request): generate a random UUID
• Emit Mcp-Session-Id response header on every MCP HTTP response

Changes

File	Change
src/web/server/web_client.h	Add nd_uuid_t mcp_session_id field
src/web/server/web_client.c	Clear on request reset
src/web/server/web_client_cache.c	Clear on web_client reuse from cache
src/web/api/http_header.c	Parse incoming Mcp-Session-Id header (with validation)
src/web/mcp/adapters/mcp-http.c	Generate if absent, emit as response header

[cubic-dev-ai]

1 issue found across 4 files

Confidence score: 3/5

• There is a concrete regression risk in src/web/mcp/adapters/mcp-http.c: batch JSON requests (json_type_array) appear to skip session handling because method is not extracted for arrays, despite the comment indicating otherwise.
• Given the 6/10 severity with high confidence (9/10), this is likely user-impacting for clients that send batch requests, even if non-batch flows remain unaffected.
• Pay close attention to src/web/mcp/adapters/mcp-http.c - batch-request parsing needs to enforce the same session logic as object-based requests.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/web/mcp/adapters/mcp-http.c">

<violation number="1" location="src/web/mcp/adapters/mcp-http.c:310">
P2: Batch requests (`json_type_array`) bypass all session logic because `method` is never extracted from the array. The comment claims the first array element is inspected, but the code only handles `json_type_object`. Add an `else if (json_object_is_type(root, json_type_array))` branch to extract the method from the first item.</violation>
</file>

Architecture diagram

sequenceDiagram
    participant Client as MCP Client (Codex)
    participant Web as Web Server (web_client)
    participant Adapter as MCP HTTP Adapter
    participant Store as Session Store (Dictionary)
    participant Core as MCP Core Logic

    Note over Client,Store: Initial Handshake (First Request)

    Client->>Web: POST /mcp (method: "initialize")
    Web->>Web: Parse standard HTTP headers
    Web->>Adapter: mcp_http_handle_request()
    Adapter->>Core: mcp_create_client() (transient)
    Core-->>Adapter: mcpc object
    Adapter->>Adapter: Process "initialize" logic
    
    Adapter->>Store: NEW: mcp_http_session_create(mcpc)
    Store->>Store: Generate UUID & Store state (TTL 1h)
    Store-->>Adapter: session_id
    
    Adapter->>Web: NEW: Add "Mcp-Session-Id" to response headers
    Web-->>Client: 200 OK + Mcp-Session-Id

    Note over Client,Store: Subsequent Request (e.g. notifications/initialized)

    Client->>Web: POST /mcp (method: "...", Mcp-Session-Id: UUID)
    Web->>Web: NEW: http_header_mcp_session_id() (Parse header)
    Web->>Adapter: mcp_http_handle_request()
    
    alt NEW: Session lookup
        Adapter->>Store: mcp_http_session_exists(UUID)
        Store-->>Adapter: false (Expired/Invalid)
        Adapter-->>Client: 404 Not Found (Per Spec)
    else Session Valid
        Store-->>Adapter: true
        Adapter->>Store: NEW: mcp_http_session_restore(UUID, mcpc)
        Store-->>Adapter: Restore protocol_version, ready, client_info
        
        Adapter->>Core: Process request (e.g. set mcpc->ready = true)
        
        Adapter->>Store: NEW: mcp_http_session_update(UUID, mcpc)
        Note right of Store: Persist state changes (e.g. ready flag)
        
        Adapter-->>Client: 200 OK / 202 Accepted
    end

    opt Background Cleanup
        Store->>Store: NEW: mcp_http_sessions_cleanup_expired_nolock()
    end

Loading

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[ilyam8]

@cubic-dev-ai review this PR

[cubic-dev-ai]

@cubic-dev-ai review this PR

@ilyam8 I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

1 issue found across 4 files

Confidence score: 3/5

• There is a concrete session-management risk in src/web/mcp/adapters/mcp-http.c: validation checks Mcp-Session-Id existence but not TTL expiry, so expired sessions may remain usable.
• Given the high severity/confidence (7/10, 9/10) and direct user-facing auth impact, this is more than a minor issue and adds meaningful regression/security risk before merge.
• Pay close attention to src/web/mcp/adapters/mcp-http.c - ensure session validation enforces TTL expiration so stale session IDs are rejected.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/web/mcp/adapters/mcp-http.c">

<violation number="1" location="src/web/mcp/adapters/mcp-http.c:171">
P1: Session validation ignores TTL, so expired `Mcp-Session-Id` values can continue to work until another session is created.</violation>
</file>

Architecture diagram

sequenceDiagram
    participant C as Client (Codex)
    participant W as Web Server (Headers)
    participant A as MCP Adapter (mcp-http.c)
    participant S as Session Store (Dictionary)
    participant L as MCP Logic (Core)

    Note over C, L: Session Creation (Handshake)

    C->>W: POST /mcp (method: initialize)
    W->>A: mcp_http_handle_request()
    A->>L: mcp_create_client()
    L-->>A: mcpc instance (default state)
    A->>L: Process initialize params
    A->>S: NEW: mcp_http_session_create(mcpc)
    S->>S: Generate UUID & store state (TTL 1h)
    S-->>A: session_id
    A-->>C: 200 OK + NEW: Mcp-Session-Id header

    Note over C, L: Subsequent Request (State Resumption)

    C->>W: POST /mcp (Mcp-Session-Id: uuid, method: ...)
    W->>W: CHANGED: Parse Mcp-Session-Id into web_client
    W->>A: mcp_http_handle_request()
    
    alt Session exists
        A->>S: NEW: mcp_http_session_restore(uuid, mcpc)
        S-->>A: success (state loaded into mcpc)
        A->>L: Execute method (e.g. notifications/initialized)
        L->>L: Update mcpc (e.g. ready = true)
        A->>S: NEW: mcp_http_session_update(uuid, mcpc)
        A-->>C: 200/202 OK + Echo Mcp-Session-Id
    else Session missing or expired
        A->>S: NEW: mcp_http_session_exists(uuid)
        S-->>A: false
        A-->>C: NEW: 404 Not Found (per spec)
    end

    Note over C, L: Session Termination

    C->>W: NEW: DELETE /mcp (Mcp-Session-Id: uuid)
    W->>A: mcp_http_handle_request()
    A->>S: NEW: mcp_http_session_delete(uuid)
    alt Found
        S-->>A: true
        A-->>C: 200 OK
    else Not Found
        S-->>A: false
        A-->>C: 404 Not Found
    end

Loading

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[ktsaou]

@copilot-swe-agent We reviewed this PR in depth and have a fundamental question about the design.

The session store persists 5 fields (protocol_version, ready, client_name, client_version, logging_level) across HTTP requests — but we traced every one of them through the codebase and none are actually needed:

Persisted field	Actually used after restore?
protocol_version	Only read during initialize itself (for capabilities gating in the response). Never read for any behavioral decision in subsequent requests.
ready	mcp.c only logs a debug message when not ready, and auto-sets ready = true on the first non-init method call anyway. No request is ever rejected.
client_name / client_version	Appear only in debug/info log messages. Cosmetic.
logging_level	Set but never read by any code path. Dead state.

Additionally, auth is re-derived from the Authorization header on every single HTTP request — it's orthogonal to the session store. The client must send the bearer token every time regardless.

The MCP Streamable HTTP spec only requires:

1. Return Mcp-Session-Id header on initialize response
2. Accept it on subsequent requests
3. Optionally return 404 to force re-initialization

So: why does Netdata need to keep state for something it does not need? If Codex needs an Mcp-Session-Id, we can give it one, but we don't have to do anything with it. A simpler implementation would just return a UUID on initialize and accept any session ID on subsequent requests — no DICTIONARY, no spinlock, no STRING lifecycle management, no TTL cleanup.

Can you explain what breaks if we don't persist any server-side state? What specific client behavior requires the server to remember these fields?

[Copilot]

@copilot-swe-agent We reviewed this PR in depth and have a fundamental question about the design.

The session store persists 5 fields (protocol_version, ready, client_name, client_version...

You are correct on every point — I traced all five fields and confirmed none require server-side persistence:

Field	Why not needed
protocol_version	Only read inside mcp-initialize.c during the initialize call itself
ready	Every non-logging handler auto-sets it to true; nothing rejects a request based on it
client_name / client_version	Debug/info log messages only
logging_level	Written in mcp-logging.c but never read to filter anything

Fixed in d559008. The session store now tracks only UUIDs with a last_accessed timestamp:

• Removed the MCP_HTTP_SESSION struct fields and the delete_callback
• Replaced mcp_http_session_restore() (which copied state into MCP_CLIENT) with mcp_http_session_validate() (existence + TTL check only)
• Removed mcp_http_session_update() entirely
• mcp_http_session_create() no longer takes an MCP_CLIENT * argument

[ktsaou]

@ilyam8 I rewrote this PR from scratch. Here's the reasoning:

Why stateless session IDs

The original implementation (by Copilot) introduced a server-side session store: a dictionary with spinlock, TTL expiry, cleanup sweeps, and session validation on every request. I went through several iterations before realizing the fundamental problem: the session store itself is unnecessary and creates attack surface.

The MCP spec (2025-03-26) says:

• Server MAY return Mcp-Session-Id on initialize (it's optional)
• Client MUST send it back on subsequent requests
• Server SHOULD echo it in responses
• No MCP_CLIENT state actually needs to persist — every field is re-derived per request

A server-side session store would let an attacker exhaust server memory by opening sessions, with no benefit — we don't use the session state for anything.

What this PR does instead

1. On initialize: generate a random UUID, store it in web_client.mcp_session_id (as nd_uuid_t — 16 bytes on stack, no heap allocation)
2. On subsequent requests: the HTTP header parser stores the incoming Mcp-Session-Id into the same field via uuid_parse
3. On every MCP response: emit the header back if set

No session store, no validation, no TTL, no tracking, no spinlock. The web_client struct is the only home for the session ID — it lives and dies with the HTTP request lifecycle.

This fixes the Codex startup failure (#21999) while keeping the implementation minimal and safe.

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds stateless Mcp-Session-Id support for MCP Streamable HTTP by storing a per-request UUID on web_client, parsing incoming session IDs, and emitting the header on responses (including generating one on successful initialize).

Changes:

• Add mcp_session_id field to web_client and clear it on request/client reset.
• Parse incoming Mcp-Session-Id header into web_client.
• Generate and emit Mcp-Session-Id on successful initialize, and emit on subsequent responses.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/web/server/web_client_cache.c	Clears mcp_session_id when reusing web_client from cache.
src/web/server/web_client.h	Adds nd_uuid_t mcp_session_id to persist session ID per request.
src/web/server/web_client.c	Clears mcp_session_id during allocation reset.
src/web/mcp/adapters/mcp-http.c	Generates UUID on initialize success and emits Mcp-Session-Id header; changes batch response allocation.
src/web/api/http_header.c	Parses Mcp-Session-Id request header into web_client.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.