Skip to content

Conversation

@ByteBaker
Copy link
Contributor

@ByteBaker ByteBaker commented Oct 22, 2025

User description

Summary

  • Fix .well-known/oauth-authorization-server endpoint to use /mcp prefix
  • Add MCP endpoints to OpenAPI specification
  • Add rate limiting configuration for all MCP endpoints with module name 'mcp'

Changes

  • Updated OAuth authorization server metadata endpoint path from /{org_id}/.well-known/oauth-authorization-server to /{org_id}/mcp/.well-known/oauth-authorization-server
  • Added handle_mcp_post, handle_mcp_get, and oauth_authorization_server_metadata to OpenAPI paths
  • Added x-o2-ratelimit extensions to all MCP endpoint definitions (both enterprise and non-enterprise versions)

PR Type

Enhancement, Documentation


Description

  • Add MCP rate limit OpenAPI extensions

  • Expose MCP handlers in OpenAPI router

  • Fix OAuth metadata endpoint path under MCP


Diagram Walkthrough

flowchart LR
  A["MCP handlers"] -- "annotate with x-o2-ratelimit" --> B["OpenAPI docs"]
  A -- "register in router" --> C["openapi.rs imports"]
  D["OAuth metadata route"] -- "move under /mcp" --> A
Loading

File Walkthrough

Relevant files
Enhancement
mod.rs
Rate limit annotations and MCP OAuth path fix                       

src/handler/http/request/mcp/mod.rs

  • Add x-o2-ratelimit extensions to MCP endpoints.
  • Update OAuth metadata route to /mcp/.well-known/....
  • Apply rate limit annotations for GET and POST variants.
  • Ensure enterprise-only variants include rate limit extensions.
+20/-2   
Documentation
openapi.rs
Register MCP endpoints in OpenAPI router                                 

src/handler/http/router/openapi.rs

  • Import handle_mcp_post, handle_mcp_get, and OAuth metadata into
    OpenAPI.
  • Ensure MCP endpoints are included in generated spec.
+3/-0     

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

OpenAPI Consistency

The request_body for non-enterprise MCP handlers uses generic Object while enterprise uses MCPRequest. Verify this is intentional and that schemas align across editions to avoid divergent API docs/clients.

    request_body(content = Object, description = "MCP request payload", content_type = "application/json"),
    responses(
        (status = 404, description = "Not Found - MCP server is only available in enterprise edition", content_type = "application/json"),
    ),
    extensions(
        ("x-o2-ratelimit" = json!({"module": "mcp", "operation": "post"}))
    ),
)]
#[post("/{org_id}/mcp")]
pub async fn handle_mcp_post(
    _org_id: web::Path<String>,
    _body: web::Bytes,
) -> actix_web::Result<HttpResponse> {
    mcp_only_in_enterprise()
}

#[cfg(not(feature = "enterprise"))]
#[utoipa::path(
    context_path = "/api",
    tag = "MCP",
    operation_id = "MCPRequestStream",
    security(
        ("Authorization" = [])
    ),
    params(
        ("org_id" = String, Path, description = "Organization name"),
    ),
    request_body(content = Object, description = "MCP request payload", content_type = "application/json"),
    responses(
        (status = 404, description = "Not Found - MCP server is only available in enterprise edition", content_type = "application/json"),
    ),
    extensions(
Rate Limit Operation Names

The x-o2-ratelimit extension uses operation values "post" and "get". Confirm these strings match rate limiter configuration expectations (case and naming) and that streaming GET uses the correct operation identifier.

    extensions(
        ("x-o2-ratelimit" = json!({"module": "mcp", "operation": "get"}))
    ),
)]
#[get("/{org_id}/mcp")]
pub async fn handle_mcp_get(
    _org_id: web::Path<String>,
    // This is `Bytes` because body in HTTP GET is not defined,
    // and actix-web tends to drop JSON body.
    // This is a workaround to prevent that.
    body: web::Bytes,
    req: HttpRequest,
) -> actix_web::Result<HttpResponse> {
    // Extract auth token from Authorization header
    let auth_token = req
        .headers()
        .get("Authorization")
        .and_then(|v| v.to_str().ok())
        .map(|s| s.to_string());

    // MCP Streamable HTTP: GET with empty body is optional
    // for server-initiated notifications. This is deprecated
    // and therefore not supported by O2
    if body.is_empty() {
        return Ok(HttpResponse::MethodNotAllowed()
            .insert_header(("Allow", "POST"))
            .json(serde_json::json!({
                "error": "GET requests must include a JSON-RPC request body. Use POST for standard requests."
            })));
    }

    // Parse JSON from body manually (GET with body is non-standard but MCP spec allows it)
    let mcp_request: MCPRequest = serde_json::from_slice(&body)
        .map_err(|e| actix_web::error::ErrorBadRequest(format!("Invalid JSON: {e}")))?;

    // Handle the request with streaming (returns SSE format)
    let stream = handle_mcp_request_stream(mcp_request, auth_token)
        .await
        .map_err(|e| actix_web::error::ErrorInternalServerError(format!("MCP error: {e}")))?;

    // Convert the stream error type to actix_web::Error
    let actix_stream = stream.map(|result| {
        result.map_err(|e| actix_web::error::ErrorInternalServerError(format!("Stream error: {e}")))
    });

    Ok(HttpResponse::Ok()
        .content_type("text/event-stream")
        .insert_header(("Cache-Control", "no-cache"))
        .insert_header(("X-Accel-Buffering", "no"))
        .streaming(actix_stream))
}

// Dummy handlers for non-enterprise builds
#[cfg(not(feature = "enterprise"))]
#[utoipa::path(
    context_path = "/api",
    tag = "MCP",
    operation_id = "MCPRequest",
    security(
        ("Authorization" = [])
    ),
    params(
        ("org_id" = String, Path, description = "Organization ID"),
    ),
    request_body(content = Object, description = "MCP request payload", content_type = "application/json"),
    responses(
        (status = 404, description = "Not Found - MCP server is only available in enterprise edition", content_type = "application/json"),
    ),
    extensions(
        ("x-o2-ratelimit" = json!({"module": "mcp", "operation": "post"}))
    ),
)]
#[post("/{org_id}/mcp")]
pub async fn handle_mcp_post(
    _org_id: web::Path<String>,
    _body: web::Bytes,
) -> actix_web::Result<HttpResponse> {
    mcp_only_in_enterprise()
}

#[cfg(not(feature = "enterprise"))]
#[utoipa::path(
    context_path = "/api",
    tag = "MCP",
    operation_id = "MCPRequestStream",
    security(
        ("Authorization" = [])
    ),
    params(
        ("org_id" = String, Path, description = "Organization name"),
    ),
    request_body(content = Object, description = "MCP request payload", content_type = "application/json"),
    responses(
        (status = 404, description = "Not Found - MCP server is only available in enterprise edition", content_type = "application/json"),
    ),
    extensions(
        ("x-o2-ratelimit" = json!({"module": "mcp", "operation": "get"}))
    ),
Router Exposure

Newly added MCP handlers are now exported to the OpenAPI router; ensure the module is guarded correctly for non-enterprise builds to prevent exposing endpoints that should be enterprise-only in docs.

request::mcp::handle_mcp_post,
request::mcp::handle_mcp_get,
request::mcp::oauth_authorization_server_metadata,

@github-actions
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Preserve backward-compatible well-known path

Changing the well-known path adds a "/mcp" segment, which may break existing clients
relying on the standard .well-known location. If this change is intended, consider
keeping a backward-compatible route alias to the previous path to prevent
regressions.

src/handler/http/request/mcp/mod.rs [249]

 #[get("/{org_id}/mcp/.well-known/oauth-authorization-server")]
+#[get("/{org_id}/.well-known/oauth-authorization-server")]
Suggestion importance[1-10]: 7

__

Why: The route was moved from /{org_id}/.well-known/... to /{org_id}/mcp/.well-known/...; adding a backward-compatible alias can prevent client breakage, making it a valuable but optional compatibility improvement.

Medium
Standardize ratelimit operation casing

Ensure the operation value matches your ratelimiter’s expected case/convention
across all MCP endpoints. Using mixed "post"/"get" strings can lead to separate
buckets; standardize to uppercase HTTP verbs for both enterprise and non-enterprise
variants.

src/handler/http/request/mcp/mod.rs [192-194]

 extensions(
-    ("x-o2-ratelimit" = json!({"module": "mcp", "operation": "post"}))
+    ("x-o2-ratelimit" = json!({"module": "mcp", "operation": "POST"}))
 ),
Suggestion importance[1-10]: 5

__

Why: The snippet at 192-194 exists and the advice to standardize casing could avoid separate rate limit buckets; however, correctness depends on the ratelimiter’s expected convention, so impact is moderate.

Low
Possible issue
Normalize ratelimit operation names

The operation value for the GET MCP handler is set to "get", while the POST handler
uses "post". If your rate limiter expects canonical operation names, ensure
consistency to avoid misclassification. Align the operation values with the actual
HTTP verbs used by the handlers.

src/handler/http/request/mcp/mod.rs [125-126]

 extensions(
-    ("x-o2-ratelimit" = json!({"module": "mcp", "operation": "get"}))
+    ("x-o2-ratelimit" = json!({"module": "mcp", "operation": "GET"}))
 ),
Suggestion importance[1-10]: 5

__

Why: The snippet exists at lines 125-126 and suggests normalizing the operation value case; this can prevent bucket fragmentation but the PR consistently uses lowercase elsewhere, so impact is moderate and context-dependent.

Low

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Greptile Overview

Summary

Fixes MCP endpoint routing consistency and adds proper OpenAPI documentation.

Key Changes:

  • Updated OAuth authorization server metadata endpoint path from /{org_id}/.well-known/oauth-authorization-server to /{org_id}/mcp/.well-known/oauth-authorization-server to align with the /mcp prefix used by other MCP endpoints
  • Added rate limiting configuration to all MCP endpoints (both enterprise and non-enterprise versions) with x-o2-ratelimit extension using module name mcp
  • Added three MCP endpoints to OpenAPI specification: handle_mcp_post, handle_mcp_get, and oauth_authorization_server_metadata

Impact:

  • Ensures consistent URL structure for all MCP endpoints under the /mcp namespace
  • Enables proper API documentation generation for MCP endpoints
  • Applies rate limiting policy uniformly across MCP operations

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are straightforward and consist of: (1) a simple URL path update to add /mcp prefix for consistency, (2) adding rate limiting configuration to all MCP endpoints, and (3) registering endpoints in OpenAPI specification. All changes are configuration-level with no logic modifications. The path change aligns the OAuth discovery endpoint with the existing MCP endpoint namespace, improving API consistency.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/handler/http/request/mcp/mod.rs 5/5 Fixed OAuth endpoint path from /{org_id}/.well-known/oauth-authorization-server to /{org_id}/mcp/.well-known/oauth-authorization-server and added rate limiting configuration with module name 'mcp'
src/handler/http/router/openapi.rs 5/5 Added MCP endpoints (handle_mcp_post, handle_mcp_get, oauth_authorization_server_metadata) to OpenAPI specification for documentation generation

Sequence Diagram

sequenceDiagram
    participant Client as MCP Client
    participant API as API Gateway
    participant Handler as MCP Handler
    participant OAuth as OAuth Service
    participant RateLimit as Rate Limiter
    
    Note over Client,OAuth: OAuth Discovery
    Client->>API: GET OAuth metadata endpoint
    API->>RateLimit: Check rate limits (module: mcp)
    RateLimit-->>API: Allowed
    API->>OAuth: Get metadata
    OAuth-->>API: OAuth configuration
    API-->>Client: 200 OK
    
    Note over Client,Handler: MCP POST Request
    Client->>API: POST to MCP endpoint
    API->>RateLimit: Check rate limits (module: mcp, operation: post)
    RateLimit-->>API: Allowed
    API->>Handler: handle_mcp_post()
    Handler->>Handler: Validate auth token
    Handler->>Handler: Process request
    Handler-->>API: Response
    API-->>Client: 200 OK
    
    Note over Client,Handler: MCP GET Request
    Client->>API: GET to MCP endpoint with JSON body
    API->>RateLimit: Check rate limits (module: mcp, operation: get)
    RateLimit-->>API: Allowed
    API->>Handler: handle_mcp_get()
    Handler->>Handler: Validate auth token
    Handler->>Handler: Parse JSON and stream
    Handler-->>API: SSE stream
    API-->>Client: 200 OK (streaming)
Loading

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: ByteBaker | Branch: chore/mcp | Commit: 42a018c

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 365 342 0 19 4 94% 5m 13s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: ByteBaker | Branch: chore/mcp | Commit: 1a73007

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 365 343 0 19 3 94% 5m 13s

View Detailed Results

- Fix .well-known/oauth-authorization-server endpoint to use /mcp prefix
- Add MCP endpoints to OpenAPI spec
- Add rate limiting configuration for all MCP endpoints with module name 'mcp'
@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: ByteBaker | Branch: chore/mcp | Commit: 8742843

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 365 343 0 19 3 94% 5m 14s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: ByteBaker | Branch: chore/mcp | Commit: 8742843

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 365 344 0 19 2 94% 5m 23s

View Detailed Results

@ByteBaker ByteBaker merged commit ca1ca8e into main Oct 22, 2025
32 checks passed
@ByteBaker ByteBaker deleted the chore/mcp branch October 22, 2025 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants