fix(sdk): preserve transient/terminal distinction in refresh errors

The refresh single-flight collapsed both `RefreshError::Transient` and
`RefreshError::Terminal` into a stringified `SdkError::Auth`, so consumers
(CLI, TUI, future language bindings) had no machine-readable way to tell a
retryable IdP blip from a dead session that needs re-authentication.

Carry the `RefreshError` through the shared outcome (kept `Clone` for
`Shared`) instead of its rendered text, and map it at the await site to a
new `retryable` flag on `SdkError::Auth`. Add `SdkError::auth_retryable`
and a `SdkError::retryable()` accessor; transient refresh failures report
`true`, every other error `false`. Add classification tests.

Signed-off-by: Max Dubrinsky <mdubrinsky@nvidia.com>

Author: maxdubrinsky

crates/openshell-sdk/src/error.rs

@@ -45,11 +45,18 @@ pub enum SdkError {
 
     /// Auth-related failure: OIDC discovery / refresh, token format invalid
     /// for header injection.
+    ///
+    /// `retryable` mirrors the refresh contract's transient/terminal split: a
+    /// transient failure (network or `IdP` blip) may succeed on retry, while a
+    /// terminal one (session revoked, refresh token expired) requires
+    /// re-authentication. All non-refresh auth failures are non-retryable.
     #[error("auth error: {message}")]
     #[diagnostic(code(openshell::sdk::auth))]
     Auth {
         /// Error message.
         message: String,
+        /// Whether retrying the same operation may succeed.
+        retryable: bool,
     },
 
     /// Local IO failure (file read, listener bind, socket).
@@ -110,10 +117,19 @@ impl SdkError {
         }
     }
 
-    /// Create an `Auth` error.
+    /// Create a non-retryable `Auth` error.
     pub fn auth(message: impl Into<String>) -> Self {
         Self::Auth {
             message: message.into(),
+            retryable: false,
+        }
+    }
+
+    /// Create an `Auth` error, tagging whether retrying may succeed.
+    pub fn auth_retryable(message: impl Into<String>, retryable: bool) -> Self {
+        Self::Auth {
+            message: message.into(),
+            retryable,
         }
     }
 
@@ -123,6 +139,22 @@ impl SdkError {
     /// `not_found`, `already_exists`, `rpc`. Phase 3 (napi binding) will
     /// surface this as the JS error's `code` field for discriminated-union
     /// ergonomics.
+    /// Whether retrying the same operation may succeed.
+    ///
+    /// Only a transient [`SdkError::Auth`] failure reports `true`; every other
+    /// error is non-retryable and the caller should surface it rather than
+    /// loop. Consumers use this to distinguish a retryable refresh blip from a
+    /// dead session that needs re-authentication.
+    pub const fn retryable(&self) -> bool {
+        matches!(
+            self,
+            Self::Auth {
+                retryable: true,
+                ..
+            }
+        )
+    }
+
     pub const fn code(&self) -> &'static str {
         match self {
             Self::InvalidConfig { .. } => "invalid_config",

crates/openshell-sdk/src/refresh.rs

@@ -35,7 +35,7 @@ use tokio::sync::{Mutex, RwLock};
 /// Domain-specific, deliberately not coupled to `tonic`, `napi`, or any
 /// FFI-facing error type. The SDK maps these into [`SdkError::Auth`] before
 /// surfacing to callers.
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 #[non_exhaustive]
 pub enum RefreshError {
     /// Refresh failed but a retry might succeed (network blip, transient
@@ -60,7 +60,8 @@ impl std::error::Error for RefreshError {}
 
 impl From<RefreshError> for SdkError {
     fn from(value: RefreshError) -> Self {
-        Self::auth(value.to_string())
+        let retryable = matches!(value, RefreshError::Transient(_));
+        Self::auth_retryable(value.to_string(), retryable)
     }
 }
 
@@ -127,9 +128,10 @@ struct TokenState {
 }
 
 /// Cloneable outcome of a single refresh attempt, shared across all waiters
-/// that joined it. `Err` carries the rendered message (so the type stays
-/// `Clone` for [`Shared`]); the SDK remaps it to [`SdkError::Auth`].
-type RefreshOutcome = std::result::Result<String, String>;
+/// that joined it. `Err` carries the [`RefreshError`] itself (kept `Clone`
+/// for [`Shared`]) so its Transient/Terminal kind survives to the await site,
+/// where it maps to [`SdkError::Auth`] with `retryable` set accordingly.
+type RefreshOutcome = std::result::Result<String, RefreshError>;
 type RefreshFuture = Shared<Pin<Box<dyn Future<Output = RefreshOutcome> + Send>>>;
 
 /// In-flight refresh attempt, if any. `epoch` lets the leader that started an
@@ -257,10 +259,10 @@ impl TokenSource {
                                 Ok(state.token.clone())
                             }
                         }
-                        // Store the bare refresh-error text; the single
-                        // `SdkError::auth` wrap happens where the shared
-                        // result is awaited below, so it isn't double-wrapped.
-                        Err(err) => Err(err.to_string()),
+                        // Carry the `RefreshError` itself so its
+                        // Transient/Terminal kind survives; the single
+                        // `SdkError` wrap happens at the await site below.
+                        Err(err) => Err(err),
                     };
                     // Clear this attempt's slot so the next caller starts a
                     // fresh refresh. Epoch-guarded so a newer attempt is never
@@ -283,7 +285,7 @@ impl TokenSource {
             }
         };
 
-        shared.await.map_err(SdkError::auth)
+        shared.await.map_err(SdkError::from)
     }
 
     /// Replace the current token without invoking the refresher.
@@ -635,4 +637,45 @@ mod tests {
             assert!(outcome.is_err(), "every waiter should observe the failure");
         }
     }
+
+    #[test]
+    fn refresh_error_maps_to_classified_auth() {
+        // The Transient/Terminal split must survive conversion into `SdkError`
+        // so consumers can tell a retryable blip from a dead session, and the
+        // message must be wrapped exactly once.
+        let transient = SdkError::from(RefreshError::Transient("blip".into()));
+        assert!(transient.retryable(), "transient refresh is retryable");
+        assert_eq!(
+            transient.to_string(),
+            "auth error: transient refresh error: blip",
+            "single wrap, no doubled `auth error:` prefix"
+        );
+
+        let terminal = SdkError::from(RefreshError::Terminal("revoked".into()));
+        assert!(!terminal.retryable(), "terminal refresh needs re-auth");
+        assert_eq!(terminal.code(), "auth");
+    }
+
+    #[tokio::test]
+    async fn terminal_refresh_surfaces_non_retryable() {
+        // End-to-end: a Terminal error from the refresher reaches the caller as
+        // a non-retryable auth error through the shared single-flight path.
+        struct TerminalRefresher;
+        #[async_trait::async_trait]
+        impl Refresh for TerminalRefresher {
+            async fn refresh(&self) -> std::result::Result<RefreshedToken, RefreshError> {
+                Err(RefreshError::Terminal("session revoked".into()))
+            }
+        }
+        let source = TokenSource::new(
+            RefreshedToken::new("initial").with_expires_at(0),
+            Arc::new(TerminalRefresher),
+        );
+        let err = source.refresh_now().await.expect_err("refresh must fail");
+        assert!(!err.retryable(), "terminal failure is not retryable");
+        assert!(
+            err.to_string()
+                .contains("terminal refresh error: session revoked")
+        );
+    }
 }