.github-issue-comments.md

GitHub Issue Comments

Issue #82: Add HttpMiddleware trait for transport-level HTTP concerns

Status: RESOLVED ✅

---

Closing Comment

This issue has been fully resolved! 🎉

What Was Delivered

Core HttpMiddleware Infrastructure

✅ HttpMiddleware trait with comprehensive lifecycle hooks:

#[async_trait]
pub trait HttpMiddleware: Send + Sync {
    async fn on_request(&self, request: &mut HttpRequest, context: &HttpMiddlewareContext) -> Result<()>;
    async fn on_response(&self, response: &mut HttpResponse, context: &HttpMiddlewareContext) -> Result<()>;
    async fn on_error(&self, error: &Error, context: &HttpMiddlewareContext) -> Result<()>;
    fn priority(&self) -> i32;
    async fn should_execute(&self, context: &HttpMiddlewareContext) -> bool;
}

✅ HttpRequest and HttpResponse abstractions:

• Case-insensitive header operations (HTTP RFC 7230 compliant)
• Clean API: add_header(), get_header(), has_header(), remove_header()
• Works with HashMap<String, String> internally

✅ HttpMiddlewareContext:

• Request metadata (url, method, attempt number)
• Shareable metadata store for middleware coordination
• Request ID correlation support

✅ HttpMiddlewareChain:

• Priority-based execution ordering
• Short-circuit error semantics
• Reverse order for response processing
• Comprehensive error handling with on_error hooks

Integration with StreamableHttpTransport

✅ Automatic middleware invocation in StreamableHttpTransport:

• build_request_with_middleware() - Applied before HTTP send (lines 339-435)
• apply_response_middleware() - Applied after HTTP receive (lines 437-470)
• Fast path optimization: Skips middleware when none configured
• OAuth precedence: auth_provider > HttpMiddleware > extra_headers

✅ Configuration API:

let mut http_chain = HttpMiddlewareChain::new();
http_chain.add(Arc::new(OAuthClientMiddleware::new(token)));

let config = StreamableHttpTransportConfig {
    url: Url::parse("http://localhost:8080").unwrap(),
    http_middleware_chain: Some(Arc::new(http_chain)), // ← Integration point
    // ...
};

let transport = StreamableHttpTransport::new(config);
let client = ClientBuilder::new(transport).build();

// Middleware runs automatically - no manual calls needed!

Use Cases Demonstrated

1. Header Injection ✅

See examples/40_middleware_demo.rs - CorrelationHeaderMiddleware

2. Request/Response Logging ✅

Built-in with comprehensive header tracking

3. OAuth Token Injection ✅

OAuthClientMiddleware (see issue #83)

4. Status Code Handling ✅

on_response hook with access to status codes

Testing

✅ 23 comprehensive integration tests:

• 14 HTTP middleware integration tests (tests/http_middleware_integration.rs)

  • Middleware ordering/priority
  • OAuth flows (no provider, expired token, duplicate headers)
  • Concurrency and state isolation
  • Error handling and short-circuit behavior
  • Header case-insensitivity
  • Double-retry protection

• 4 SSE middleware tests (tests/sse_middleware_integration.rs)

  • Middleware on SSE GET requests
  • Last-Event-ID header tracking
  • Multiple HTTP methods
  • Response status tracking

• 5 OAuth integration tests (tests/streamable_http_oauth_integration.rs)

  • Real client-server interaction
  • Token injection and precedence
  • Token expiry error handling

Documentation

✅ Chapter 11: Middleware in pmcp-book

• HTTP-Level Middleware section with architecture diagram
• Two-layer middleware system (Protocol vs HTTP)
• API examples and use cases

✅ Chapter 10.3: Streamable HTTP

• HTTP middleware configuration examples
• OAuth middleware integration guide

✅ API documentation

• Complete rustdoc with examples
• Type-level documentation for all public APIs

Extra Features Beyond Original Request

🎁 Fast path optimization: Avoids overhead when no middleware configured 🎁 OAuth precedence policy: Ensures auth_provider takes precedence 🎁 Metadata propagation: Allows middleware coordination (e.g., retry protection) 🎁 Case-insensitive headers: HTTP standard compliance 🎁 Error hooks: Comprehensive error handling with on_error

Files Changed

Core Implementation:

• src/client/http_middleware.rs - HttpMiddleware trait, chain, abstractions (356 lines)
• src/shared/streamable_http.rs - Integration with transport (lines 333-470)
• src/client/oauth_middleware.rs - OAuth client middleware (244 lines)

Tests:

• tests/http_middleware_integration.rs (764 lines)
• tests/sse_middleware_integration.rs (421 lines)
• tests/streamable_http_oauth_integration.rs (361 lines)

Documentation:

• pmcp-book/src/ch11-middleware.md - HTTP middleware section
• pmcp-book/src/ch10-03-streamable-http.md - Integration guide

Examples:

• examples/40_middleware_demo.rs - Full demonstration

Commits

• Phase 1 Foundation: d024af8
• OAuth Integration Tests: 57349e4
• Phase 2 Complete: 9036e5a
• Quality Gate Fixes: 4dc71df

Related Issues

• ✅ Resolves #82 (this issue)
• ✅ Resolves #83 (OAuth client middleware - see separate comment)
• ⚠️ Partial progress on #80 (HTTP middleware integration complete, ClientBuilder API still pending)

---

This provides a solid foundation for HTTP-level concerns in the PMCP SDK and achieves feature parity with the TypeScript SDK's fetch-wrapping middleware! 🚀

---

Issue #83: Add OAuth client-side middleware with token injection and auto-refresh

Status: CORE FEATURES COMPLETE ✅ (Future enhancements noted)

---

Closing Comment

The core OAuth client middleware features have been implemented! 🎉

What Was Delivered

OAuthClientMiddleware Implementation

✅ Automatic bearer token injection:

pub struct OAuthClientMiddleware {
    token: Arc<RwLock<BearerToken>>,
}

pub struct BearerToken {
    pub token: String,
    pub token_type: String,
    pub expires_at: Option<SystemTime>,
}

impl BearerToken {
    pub fn new(token: String) -> Self;
    pub fn with_expiry(token: String, duration: Duration) -> Self;
    pub fn is_expired(&self) -> bool;
    pub fn to_header_value(&self) -> String;
}

✅ Token lifecycle features:

• Expiry tracking with expires_at field
• Proactive expiry checking in on_request
• Returns Error::Authentication when token expired
• is_expired() method for manual checking

✅ 401/403 detection and metadata:

• Detects authentication failures in on_response
• Sets auth_failure: true metadata for retry coordination
• Sets status_code metadata for debugging
• Logs warnings on auth failure after retry

✅ Smart precedence handling:

• Skips injection if auth_provider present (set via metadata)
• Skips if Authorization header already exists (case-insensitive)
• Prevents duplicate authentication headers
• Clean integration with transport-level auth

✅ Retry coordination:

• Sets oauth.retry_used metadata for double-retry protection
• Prevents infinite retry loops
• Coordinates with external retry middleware
• Uses on_error hook for proper error-chain handling

Integration

✅ Works with StreamableHttpTransport:

use pmcp::client::http_middleware::HttpMiddlewareChain;
use pmcp::client::oauth_middleware::{OAuthClientMiddleware, BearerToken};
use std::time::Duration;

// Create OAuth middleware
let token = BearerToken::with_expiry(
    "api-token-12345".to_string(),
    Duration::from_secs(3600)
);
let mut http_chain = HttpMiddlewareChain::new();
http_chain.add(Arc::new(OAuthClientMiddleware::new(token)));

// Configure transport
let config = StreamableHttpTransportConfig {
    url: Url::parse("http://localhost:8080").unwrap(),
    http_middleware_chain: Some(Arc::new(http_chain)),
    // ...
};

let transport = StreamableHttpTransport::new(config);
let mut client = ClientBuilder::new(transport).build();

// Token automatically injected into all requests!
let info = client.initialize(ClientCapabilities::minimal()).await?;

Testing

✅ 5 OAuth integration tests (tests/streamable_http_oauth_integration.rs):

• test_oauth_middleware_injects_token - Verifies automatic token injection
• test_auth_provider_takes_precedence_over_oauth - Validates precedence policy
• test_oauth_token_expiry_triggers_error - Token expiry handling
• test_multiple_requests_with_oauth - Sequential requests with same token
• test_oauth_with_case_insensitive_header_check - Duplicate detection

✅ 3 OAuth retry coordination tests (tests/http_middleware_integration.rs):

• test_oauth_retry_coordination_with_metadata - Metadata-based coordination
• test_double_retry_protection - Prevents infinite loops
• test_oauth_error_hook_logging - Error hook integration

✅ Real client-server integration:

• Tests use actual StreamableHttpServer instances
• Validates end-to-end token flow
• Confirms server receives injected headers

Documentation

✅ Chapter 11: Middleware - OAuth middleware section ✅ Chapter 10.3: Streamable HTTP - OAuth configuration examples ✅ API documentation - Complete rustdoc with examples

What's Not Implemented (Future Enhancements)

The following features from the original issue are not yet implemented but can be added in future PRs:

1. Automatic Token Refresh ⚠️

Current behavior: Returns Error::Authentication when token expires Desired behavior: Automatically refresh token and retry request

Future implementation approach:

pub trait TokenProvider: Send + Sync {
    async fn get_token(&self) -> Result<AccessToken>;
    async fn refresh_token(&self, current: &AccessToken) -> Result<AccessToken>;
}

pub struct OAuthClientMiddleware {
    token_provider: Arc<dyn TokenProvider>,
    // Automatically refreshes on expiry or 401
}

2. Multiple OAuth Flows ⚠️

Current support: Bearer token only Future flows:

• Client credentials (client_id + client_secret → token)
• Refresh token (refresh_token → new access_token)
• Device code flow
• Custom token providers

3. Automatic Retry After 401 ⚠️

Current behavior: Sets metadata, app handles retry Desired behavior: Middleware automatically retries after refresh

Workaround: Use external retry middleware that checks auth_failure metadata

4. Token Storage Strategies ⚠️

Current support: In-memory only Future strategies:

• Persistent storage (file, keychain)
• Encrypted token storage
• Shared token cache across clients

5. Proactive Token Refresh ⚠️

Partial support: Can check expiry before request Enhancement: Automatically refresh N seconds before expiry

Why Close This Issue?

The core value proposition of this issue has been delivered:

• ✅ OAuth tokens are automatically injected
• ✅ Token expiry is tracked and validated
• ✅ 401/403 responses are detected
• ✅ Retry coordination infrastructure is in place

The missing features (auto-refresh, multiple flows) are valuable enhancements but can be added incrementally without blocking other work. The current implementation provides a solid foundation that users can build upon.

Recommendation

Close this issue and open separate enhancement issues for:

1. "Add automatic token refresh to OAuthClientMiddleware" (#TODO)
2. "Support multiple OAuth flows (client credentials, refresh token)" (#TODO)
3. "Add TokenProvider trait for custom OAuth flows" (#TODO)

This allows tracking specific enhancements independently while acknowledging the core feature is complete.

Files Changed

Implementation:

• src/client/oauth_middleware.rs (244 lines)
• src/client/http_middleware.rs (trait definition)

Tests:

• tests/streamable_http_oauth_integration.rs (361 lines)
• tests/http_middleware_integration.rs (OAuth sections)

Documentation:

• pmcp-book/src/ch11-middleware.md
• pmcp-book/src/ch10-03-streamable-http.md

Commits

• OAuth Middleware Implementation: 57349e4
• Phase 2 Integration Tests: 9036e5a
• Quality Gate Fixes: 4dc71df

Related Issues

• ✅ Resolves #83 (this issue - core features)
• ✅ Resolves #82 (HttpMiddleware trait)
• ⚠️ Partial progress on #80 (HTTP middleware integration)

---

Great work positioning PMCP as the best MCP SDK! The OAuth middleware provides essential client-side authentication capabilities. 🚀

---

Issue #80: Add first-class middleware integration to Client/Protocol/Transport

Status: IN PROGRESS ⚠️ (HTTP layer complete, Client/Protocol layer pending)

---

Progress Update Comment

Significant progress has been made on this issue! The HTTP transport layer now has full first-class middleware integration.

✅ What's Been Completed

HTTP-Level Middleware Integration (COMPLETE)

✅ Transport-level configuration:

use pmcp::client::http_middleware::HttpMiddlewareChain;
use pmcp::shared::streamable_http::{StreamableHttpTransport, StreamableHttpTransportConfig};

let mut http_chain = HttpMiddlewareChain::new();
http_chain.add(Arc::new(LoggingMiddleware::default()));
http_chain.add(Arc::new(OAuthClientMiddleware::new(token)));

let config = StreamableHttpTransportConfig {
    url: Url::parse("http://localhost:8080").unwrap(),
    http_middleware_chain: Some(Arc::new(http_chain)),  // ← First-class integration!
    // ...
};

let transport = StreamableHttpTransport::new(config);
let mut client = ClientBuilder::new(transport).build();

// Middleware runs automatically on ALL operations - zero manual calls!
let tools = client.list_tools(None).await?;  // Middleware runs transparently

✅ Automatic invocation - No manual chain.process_request() calls needed:

• build_request_with_middleware() - Called automatically before HTTP send
• apply_response_middleware() - Called automatically after HTTP receive
• Short-circuit error handling with on_error hooks
• Priority-based execution ordering

✅ Clean architecture:

• Configuration via StreamableHttpTransportConfig::http_middleware_chain
• Middleware chain is Option<Arc<HttpMiddlewareChain>> - zero overhead when unused
• Fast path optimization when no middleware configured

✅ Production-ready features:

• Error handling with on_error lifecycle
• Context metadata for middleware coordination
• Priority-based ordering (lower runs first)
• Conditional execution via should_execute()

Test Coverage

✅ 23 integration tests demonstrating automatic middleware invocation:

• Real client-server interactions
• Multiple middleware chaining
• Error scenarios
• Retry coordination

Documentation

✅ Chapter 11: Middleware - Complete HTTP middleware section ✅ Chapter 10.3: Streamable HTTP - Integration guide ✅ examples/40_middleware_demo.rs - Full demonstration

⚠️ What's Still Pending

1. ClientBuilder API (HIGH PRIORITY)

Desired API:

let client = Client::builder()
    .capabilities(ClientCapabilities::default())
    .with_middleware_chain(chain)  // ← Doesn't exist yet
    .streamable_http_transport("http://localhost:8080")
    .await?
    .build()
    .await?;

Current workaround: Configure via transport config (works but less ergonomic)

Implementation needed:

• Add middleware_chain: Option<Arc<MiddlewareChain>> field to ClientBuilder
• Add .with_middleware_chain() builder method
• Pass middleware to transport during client construction
• Update all transport builders to accept middleware

2. Protocol-Level Middleware Auto-Invocation (MEDIUM PRIORITY)

Current state: Protocol middleware (AdvancedMiddleware) requires manual calls:

// Manual invocation required
let mut request = create_request("tools/list", None);
middleware_chain.process_request(&mut request).await?;
let response = client.call_method(request).await?;
middleware_chain.process_response(&mut response).await?;

Desired state: Automatic invocation like HTTP middleware

Implementation needed:

• Add middleware chain to Protocol struct
• Wrap all request/response paths with automatic middleware invocation
• Ensure backward compatibility

3. Unified Middleware Configuration (LOW PRIORITY)

Desired: Single configuration point for both HTTP and Protocol middleware

let client = Client::builder()
    .with_http_middleware(http_chain)      // HTTP layer
    .with_protocol_middleware(proto_chain) // Protocol layer
    .build();

Benefit: Clear separation of concerns, easier to understand

Progress Summary

Component	Status	Notes
HTTP middleware integration	✅ COMPLETE	Full automatic invocation
HTTP middleware config API	✅ COMPLETE	Via transport config
HTTP middleware tests	✅ COMPLETE	23 integration tests
HTTP middleware docs	✅ COMPLETE	Chapter 11 + examples
ClientBuilder API	❌ TODO	No .with_middleware_chain()
Protocol middleware auto-invoke	❌ TODO	Still requires manual calls
Unified middleware config	❌ TODO	Separate HTTP/Protocol APIs

Recommendation

Keep this issue open but update the title or description to reflect progress:

Option 1: Update title to focus on remaining work: "Add ClientBuilder middleware API and Protocol-level auto-invocation"

Option 2: Create separate issues:

• New Issue: "Add ClientBuilder::with_middleware_chain() API" (#TODO)
• New Issue: "Add automatic Protocol middleware invocation" (#TODO)
• Close #80 noting HTTP layer is complete

Option 3: Use milestones:

• Milestone 1: HTTP-Level Integration ✅ COMPLETE
• Milestone 2: ClientBuilder API ⚠️ In Progress
• Milestone 3: Protocol-Level Integration ⚠️ Pending

What Works Today

Users can immediately use HTTP middleware with this pattern:

use pmcp::client::http_middleware::HttpMiddlewareChain;
use pmcp::client::oauth_middleware::{OAuthClientMiddleware, BearerToken};
use pmcp::shared::streamable_http::{StreamableHttpTransport, StreamableHttpTransportConfig};
use std::sync::Arc;

let mut http_chain = HttpMiddlewareChain::new();
http_chain.add(Arc::new(OAuthClientMiddleware::new(token)));

let config = StreamableHttpTransportConfig {
    url: Url::parse("http://localhost:8080")?,
    http_middleware_chain: Some(Arc::new(http_chain)),
    // ... other config
};

let transport = StreamableHttpTransport::new(config);
let mut client = ClientBuilder::new(transport).build();

// All HTTP operations now have middleware applied automatically!
client.initialize(ClientCapabilities::minimal()).await?;
client.list_tools(None).await?;

This provides immediate value while we continue improving the ergonomics.

Files Changed (HTTP Layer)

• src/client/http_middleware.rs (356 lines)
• src/shared/streamable_http.rs (integration code)
• tests/http_middleware_integration.rs (764 lines)
• tests/sse_middleware_integration.rs (421 lines)
• tests/streamable_http_oauth_integration.rs (361 lines)

Next Steps

1. Add ClientBuilder::with_middleware_chain() - Improves ergonomics
2. Add Protocol middleware auto-invocation - Parity with HTTP layer
3. Update documentation - Show both patterns

Related Issues

• ✅ #82 - HttpMiddleware trait (RESOLVED)
• ✅ #83 - OAuth client middleware (RESOLVED)
• ⚠️ #80 - This issue (IN PROGRESS - HTTP complete, Client/Protocol pending)

---

The HTTP middleware integration is production-ready and provides immediate value. The remaining work (ClientBuilder API, Protocol auto-invocation) will further improve ergonomics. 🚀

---

Summary

Issues ready to close:

• ✅ #82 - HttpMiddleware trait (fully implemented)
• ✅ #83 - OAuth client middleware (core features complete)

Issues to keep open:

• ⚠️ #80 - First-class integration (HTTP done, ClientBuilder/Protocol pending)

New issues to create (optional):

1. "Add automatic token refresh to OAuthClientMiddleware"
2. "Support multiple OAuth flows (client credentials, refresh token)"
3. "Add ClientBuilder::with_middleware_chain() API"
4. "Add automatic Protocol middleware invocation"