refactor: narrow auth classification API

Author: DaleSeo

crates/rmcp/src/transport.rs

@@ -272,12 +272,7 @@ impl DynamicTransportError {
         }
     }
 
-    /// Returns whether this transport error means the client must authorize again.
-    ///
-    /// This recognizes both an exhausted local OAuth authorization and an HTTP
-    /// authorization challenge without requiring the caller to know or downcast
-    /// the concrete transport error type.
-    pub fn is_authorization_required(&self) -> bool {
+    pub(crate) fn is_authorization_required(&self) -> bool {
         let mut error = Some(self.error.as_ref() as &(dyn std::error::Error + 'static));
         while let Some(current) = error {
             #[cfg(feature = "auth")]

crates/rmcp/tests/test_auth_error_classification.rs

@@ -15,13 +15,13 @@ type TestHttpError = StreamableHttpError<std::io::Error>;
 #[error("outer transport wrapper")]
 struct OuterError(#[source] TestHttpError);
 
-fn dynamic(error: impl Error + Send + Sync + 'static) -> DynamicTransportError {
-    DynamicTransportError::from_parts("test transport", TypeId::of::<()>(), Box::new(error))
-}
-
-fn initialization_error(error: TestHttpError) -> ClientInitializeError {
+fn initialization_error(error: impl Error + Send + Sync + 'static) -> ClientInitializeError {
     ClientInitializeError::TransportError {
-        error: dynamic(error),
+        error: DynamicTransportError::from_parts(
+            "test transport",
+            TypeId::of::<()>(),
+            Box::new(error),
+        ),
         context: "initialize".into(),
     }
 }
@@ -30,33 +30,22 @@ fn initialization_error(error: TestHttpError) -> ClientInitializeError {
 fn classifies_local_authorization_required() {
     let error = TestHttpError::Auth(AuthError::AuthorizationRequired);
 
-    assert!(dynamic(error).is_authorization_required());
+    assert!(initialization_error(error).is_authorization_required());
 }
 
 #[test]
 fn classifies_http_authorization_challenge() {
     let error =
         TestHttpError::AuthRequired(AuthRequiredError::new("Bearer realm=\"mcp\"".to_owned()));
 
-    assert!(dynamic(error).is_authorization_required());
+    assert!(initialization_error(error).is_authorization_required());
 }
 
 #[test]
 fn classifies_authorization_required_through_multiple_sources() {
     let error = OuterError(TestHttpError::Auth(AuthError::AuthorizationRequired));
 
-    assert!(dynamic(error).is_authorization_required());
-}
-
-#[test]
-fn client_initialization_error_delegates_to_transport_classification() {
-    let local = initialization_error(TestHttpError::Auth(AuthError::AuthorizationRequired));
-    let challenge = initialization_error(TestHttpError::AuthRequired(AuthRequiredError::new(
-        "Bearer realm=\"mcp\"".to_owned(),
-    )));
-
-    assert!(local.is_authorization_required());
-    assert!(challenge.is_authorization_required());
+    assert!(initialization_error(error).is_authorization_required());
 }
 
 #[test]
@@ -68,9 +57,9 @@ fn does_not_classify_unrelated_transport_errors() {
         Some("admin".to_owned()),
     ));
 
-    assert!(!dynamic(closed).is_authorization_required());
-    assert!(!dynamic(refresh).is_authorization_required());
-    assert!(!dynamic(scope).is_authorization_required());
+    assert!(!initialization_error(closed).is_authorization_required());
+    assert!(!initialization_error(refresh).is_authorization_required());
+    assert!(!initialization_error(scope).is_authorization_required());
 }
 
 #[test]
@@ -93,5 +82,4 @@ fn http_challenge_remains_available_as_an_error_source() {
         .expect("source should retain the challenge type");
 
     assert_eq!(challenge.www_authenticate_header, "Bearer realm=\"mcp\"");
-    assert_eq!(source.to_string(), "authorization required");
 }

docs/plans/2026-07-27-issue-795-auth-error-classification.md

@@ -16,19 +16,15 @@ An authorized streamable HTTP client can currently report the same caller action
 
 The variants should remain distinct internally. The HTTP form carries a `WWW-Authenticate` header, while the OAuth-manager form describes local credential state and is only available with the `auth` feature. Merging them would either lose challenge data or make the streamable HTTP transport depend on OAuth even when OAuth support is disabled.
 
-The proposed additive API is:
+The public API addition is:
 
 ```rust
-impl DynamicTransportError {
-    pub fn is_authorization_required(&self) -> bool;
-}
-
 impl ClientInitializeError {
     pub fn is_authorization_required(&self) -> bool;
 }
 ```
 
-`DynamicTransportError` will inspect the standard `std::error::Error::source` chain. `ClientInitializeError` will delegate for its `TransportError` variant and return `false` for all other variants.
+Internally, `DynamicTransportError` inspects the standard `std::error::Error::source` chain. `ClientInitializeError` delegates for its `TransportError` variant and returns `false` for all other variants. The transport-level helper remains crate-internal until another public use case requires it.
 
 `AuthRequiredError` must participate in that source chain. It should implement `std::error::Error`, and `StreamableHttpError::AuthRequired` should expose it as its source. `StreamableHttpError::Auth(AuthError)` already exposes `AuthError` through `thiserror`.
 
@@ -51,7 +47,7 @@ impl ClientInitializeError {
 
 | File | Role | Status |
 | --- | --- | --- |
-| `crates/rmcp/src/transport/streamable_http_client.rs` | Make the HTTP 401 detail error visible through the standard source chain | Complete |
+| `crates/rmcp/src/transport/streamable_http_client.rs` | Verify the HTTP 401 detail error is visible through the standard source chain | Complete upstream |
 | `crates/rmcp/src/transport.rs` | Add transport-independent authorization-required classification | Complete |
 | `crates/rmcp/src/service/client.rs` | Add the initialization-error convenience predicate | Complete |
 | `crates/rmcp/tests/test_auth_error_classification.rs` | Exercise the public API and guard false positives | Complete |
@@ -76,33 +72,33 @@ impl ClientInitializeError {
 - [x] Non-transport `ClientInitializeError` variants classify as false.
 - [x] The test requires `auth`, `client`, and `transport-streamable-http-client`, but not a concrete reqwest or Unix-socket backend.
 
-### 2. Expose the HTTP challenge through the error source chain
+### 2. Verify the HTTP challenge source chain
 
 **Files:** `crates/rmcp/src/transport/streamable_http_client.rs`
 
-**Description:** Derive or implement `std::error::Error` for `AuthRequiredError` with a concise display message, then mark the value inside `StreamableHttpError::AuthRequired` as its source.
+**Description:** Confirm that `AuthRequiredError` implements `std::error::Error` and that `StreamableHttpError::AuthRequired` exposes it as its source. This prerequisite is already present on the latest `origin/main`.
 
 **Acceptance criteria:**
 
 - [x] `AuthRequiredError` remains constructible through `AuthRequiredError::new`.
 - [x] `www_authenticate_header` remains available and unchanged.
 - [x] `StreamableHttpError::source()` returns the contained `AuthRequiredError`.
-- [x] Error display does not print credentials or the challenge header.
+- [x] Existing error display behavior remains unchanged.
 - [x] No existing enum variants or fields are removed or renamed.
 
 ### 3. Add transport-independent classification
 
 **Files:** `crates/rmcp/src/transport.rs`
 
-**Description:** Add `DynamicTransportError::is_authorization_required()`. Walk the inner error and its sources, recognizing `AuthError::AuthorizationRequired` when `auth` is enabled and `AuthRequiredError` when the streamable HTTP client is enabled.
+**Description:** Add a crate-internal `DynamicTransportError::is_authorization_required()`. Walk the inner error and its sources, recognizing `AuthError::AuthorizationRequired` when `auth` is enabled and `AuthRequiredError` when the streamable HTTP client is enabled. Keep the transport-level classifier internal until another public use case requires it.
 
 **Acceptance criteria:**
 
-- [x] Callers do not need the transport type, backend error type, or a downcast.
+- [x] Client callers do not need the transport type, backend error type, or a downcast.
 - [x] Direct and multiply wrapped matching errors are recognized.
 - [x] Feature-disabled builds compile; feature-specific checks are guarded locally.
 - [x] Unknown custom transport errors return false.
-- [x] The method is documented as a classification predicate, not an authorization action.
+- [x] The helper name describes classification rather than an authorization action.
 
 ### 4. Add the initialization-error convenience API