fix(channels/discord): convert upstream 401/403 to domain-scoped error so card click can't sign user out (#2285) (#2376)

src/core/jsonrpc_tests.rs

@@ -693,6 +693,36 @@ fn is_session_expired_error_does_not_match_unrelated_errors() {
     assert!(!is_session_expired_error(""));
 }
 
+#[test]
+fn is_session_expired_error_skips_discord_rewrap_for_2285() {
+    // Cross-module regression guard for #2285: the Discord domain
+    // controller intentionally formats its upstream-auth failures so
+    // they do NOT match this dispatch-time classifier. If anyone
+    // changes the wording on either side back into a string that
+    // contains both "401" and "unauthorized", a connected-Discord
+    // card click would once again log the user out of OpenHuman.
+    //
+    // We pin the exact substrings the Discord rewrap was designed
+    // to avoid, plus the canonical post-rewrap message body, so
+    // either-side drift fails loudly.
+    let canonical_rewrap = "Discord API error: Discord list_guilds: bot token was rejected \
+         (upstream HTTP four-oh-one). Open Settings → Channels → Discord \
+         and rotate / reconnect the bot token.";
+    assert!(
+        !is_session_expired_error(canonical_rewrap),
+        "Discord rewrap must NOT trip the session-expired classifier: {canonical_rewrap}"
+    );
+    // Defensive: also pin the 403 variant. Same rewrap path, same
+    // requirement — neither '403' nor 'forbidden' is part of the
+    // session classifier today, but locking the message in keeps a
+    // future regression visible.
+    let canonical_rewrap_403 =
+        "Discord API error: Discord list_channels: bot token lacks required Discord permissions \
+         (upstream HTTP four-oh-three). Open Settings → Channels → Discord \
+         and rotate / reconnect the bot token.";
+    assert!(!is_session_expired_error(canonical_rewrap_403));
+}
+
 #[test]
 fn is_param_validation_error_matches_the_three_validator_shapes() {
     // Regression guard for OPENHUMAN-TAURI-20: pre-#1467 cores rejected

src/openhuman/channels/providers/discord/api.rs

@@ -48,6 +48,66 @@ fn auth_header(token: &str) -> String {
     format!("Bot {token}")
 }
 
+/// Format a non-2xx response from the Discord REST API as a string
+/// suitable for a JSON-RPC error result.
+///
+/// **Load-bearing for issue #2285**: the global JSON-RPC dispatcher
+/// at `src/core/jsonrpc.rs::is_session_expired_error` matches any
+/// error string that contains `"401"` AND `"unauthorized"` as a
+/// signal that the OpenHuman backend session has expired, and
+/// publishes a `DomainEvent::SessionExpired` event that signs the
+/// user out. A raw upstream Discord 401 (revoked bot token) would
+/// previously trip that classifier — opening the connected-Discord
+/// card on the Channels page logged the user out of OpenHuman over
+/// a *Discord* credentials problem.
+///
+/// The fix is to convert auth failures here into a Discord-domain
+/// message that:
+///
+///  1. Does NOT contain both `"401"` and `"unauthorized"` as a pair
+///     (so the global classifier ignores it).
+///  2. Tells the user the actual remediation: rotate the bot token
+///     at `Settings → Channels → Discord`.
+///  3. Preserves the originating endpoint identifier in the message
+///     so triage can still see WHICH Discord call failed without
+///     plumbing a separate error code.
+///
+/// Other non-2xx statuses (400 / 404 / 5xx) pass through with a
+/// `Discord API error` prefix — they don't match the
+/// `is_session_expired_error` predicate even when verbatim.
+fn format_discord_http_error(endpoint: &str, status: reqwest::StatusCode, body: &str) -> String {
+    if status == reqwest::StatusCode::UNAUTHORIZED || status == reqwest::StatusCode::FORBIDDEN {
+        let kind = if status == reqwest::StatusCode::UNAUTHORIZED {
+            "bot token was rejected"
+        } else {
+            "bot token lacks required Discord permissions"
+        };
+        // Spell out the HTTP code so `lower.contains("401")` does NOT
+        // match — see #2285 rationale on this helper. Also avoid the
+        // word `unauthorized` for the same reason; "rejected"/"forbidden"
+        // are the user-visible equivalents.
+        let code_word = if status == reqwest::StatusCode::UNAUTHORIZED {
+            "four-oh-one"
+        } else {
+            "four-oh-three"
+        };
+        // Deliberately do NOT splice the upstream body into this
+        // user-facing message — Discord's auth-error bodies often
+        // include the literal words "401" and "Unauthorized", which
+        // would smuggle the cascade trigger back in. The body is
+        // still in `tracing::debug!` logs above the call site for
+        // triage; the user-facing message only needs the remediation.
+        let _ = body;
+        format!(
+            "Discord {endpoint}: {kind} (upstream HTTP {code_word}). \
+             Open Settings → Channels → Discord and rotate / reconnect the bot \
+             token."
+        )
+    } else {
+        format!("Discord {endpoint} failed ({status}): {body}")
+    }
+}
+
 /// List all guilds (servers) the bot is a member of.
 pub async fn list_bot_guilds(token: &str) -> anyhow::Result<Vec<DiscordGuild>> {
     list_bot_guilds_at_base(DISCORD_API_BASE, token).await
@@ -77,7 +137,10 @@ async fn list_bot_guilds_at_base(base: &str, token: &str) -> anyhow::Result<Vec<
             body = %body,
             "[discord-api] non-success response"
         );
-        anyhow::bail!("Discord list guilds failed ({status}): {body}");
+        anyhow::bail!(
+            "{}",
+            format_discord_http_error("list_guilds", status, &body)
+        );
     }
 
     let guilds: Vec<DiscordGuild> = resp.json().await?;
@@ -120,7 +183,10 @@ async fn list_guild_channels_at_base(
             body = %body,
             "[discord-api] non-success response"
         );
-        anyhow::bail!("Discord list channels failed ({status}): {body}");
+        anyhow::bail!(
+            "{}",
+            format_discord_http_error("list_channels", status, &body)
+        );
     }
 
     let all_channels: Vec<DiscordTextChannel> = resp.json().await?;
@@ -183,7 +249,10 @@ async fn check_channel_permissions_at_base(
             body = %body,
             "[discord-api] non-success response"
         );
-        anyhow::bail!("Discord get bot user failed ({status}): {body}");
+        anyhow::bail!(
+            "{}",
+            format_discord_http_error("get_bot_user", status, &body)
+        );
     }
     let me: serde_json::Value = me_resp.json().await?;
     let bot_user_id = me.get("id").and_then(|i| i.as_str()).unwrap_or("").trim();
@@ -212,7 +281,10 @@ async fn check_channel_permissions_at_base(
             body = %body,
             "[discord-api] non-success response"
         );
-        anyhow::bail!("Discord get member info failed ({status}): {body}");
+        anyhow::bail!(
+            "{}",
+            format_discord_http_error("get_member_info", status, &body)
+        );
     }
 
     let member: serde_json::Value = member_resp.json().await?;
@@ -237,7 +309,10 @@ async fn check_channel_permissions_at_base(
             body = %body,
             "[discord-api] non-success response"
         );
-        anyhow::bail!("Discord get guild roles failed ({status}): {body}");
+        anyhow::bail!(
+            "{}",
+            format_discord_http_error("get_guild_roles", status, &body)
+        );
     }
     let guild_roles: Vec<serde_json::Value> = roles_resp.json().await?;
 
@@ -295,7 +370,10 @@ async fn check_channel_permissions_at_base(
             body = %body,
             "[discord-api] non-success response"
         );
-        anyhow::bail!("Discord get channel failed ({status}): {body}");
+        anyhow::bail!(
+            "{}",
+            format_discord_http_error("get_channel", status, &body)
+        );
     }
     let channel_data: serde_json::Value = ch_resp.json().await?;
     if let Some(overwrites) = channel_data

src/openhuman/channels/providers/discord/api_tests.rs

@@ -151,18 +151,72 @@ async fn list_bot_guilds_parses_discord_response() {
 }
 
 #[tokio::test]
-async fn list_bot_guilds_errors_on_non_success_status() {
+async fn list_bot_guilds_rewraps_401_so_global_session_cascade_does_not_fire() {
+    // Upstream returns 401 with the canonical Discord auth-error body.
+    // BEFORE #2285 the error string flowed up to JSON-RPC as
+    // "Discord list guilds failed (401 Unauthorized): {\"message\":
+    // \"401: Unauthorized\",\"code\":0}" — that pair tripped
+    // `jsonrpc::is_session_expired_error` ("401" + "unauthorized")
+    // and logged the user out of OpenHuman over a *Discord*
+    // credentials problem.
+    //
+    // After the fix the user-facing message:
+    //   - does NOT contain "401" or "unauthorized" as substrings
+    //     (so `is_session_expired_error` returns false), AND
+    //   - names the endpoint + the actionable Settings → Channels →
+    //     Discord remediation path.
     let app = Router::new().route(
         "/users/@me/guilds",
-        get(|| async { (StatusCode::UNAUTHORIZED, "bad token") }),
+        get(|| async {
+            (
+                StatusCode::UNAUTHORIZED,
+                r#"{"message":"401: Unauthorized","code":0}"#,
+            )
+        }),
     );
     let base = spawn_mock(app).await;
     let err = list_bot_guilds_at_base(&base, "t")
         .await
         .unwrap_err()
         .to_string();
-    assert!(err.contains("list guilds failed"));
-    assert!(err.contains("401"));
+    let lower = err.to_ascii_lowercase();
+    assert!(
+        !lower.contains("401"),
+        "must NOT contain '401' substring: {err}"
+    );
+    assert!(
+        !lower.contains("unauthorized"),
+        "must NOT contain 'unauthorized' substring: {err}"
+    );
+    assert!(
+        err.contains("list_guilds"),
+        "endpoint identifier preserved for triage: {err}"
+    );
+    assert!(
+        err.contains("Settings → Channels → Discord"),
+        "remediation path present: {err}"
+    );
+}
+
+#[tokio::test]
+async fn list_bot_guilds_5xx_still_carries_raw_status() {
+    // Non-auth errors fall through to the legacy verbose format —
+    // those don't match `is_session_expired_error` even verbatim, so
+    // surfacing the raw status code helps the user / triage.
+    let app = Router::new().route(
+        "/users/@me/guilds",
+        get(|| async { (StatusCode::INTERNAL_SERVER_ERROR, "discord melting") }),
+    );
+    let base = spawn_mock(app).await;
+    let err = list_bot_guilds_at_base(&base, "t")
+        .await
+        .unwrap_err()
+        .to_string();
+    assert!(
+        err.contains("500"),
+        "5xx must surface verbatim status: {err}"
+    );
+    assert!(err.contains("list_guilds"));
 }
 
 #[tokio::test]
@@ -188,18 +242,36 @@ async fn list_guild_channels_filters_text_channels_and_sorts_by_position() {
 }
 
 #[tokio::test]
-async fn list_guild_channels_errors_on_non_success_status() {
+async fn list_guild_channels_rewraps_403_with_remediation_and_no_session_keywords() {
+    // 403 follows the same rewrap path as 401 (#2285) — both can
+    // happen on a stale/disabled bot token AND both share enough
+    // substrings with `is_session_expired_error` to be a problem if
+    // raw upstream text reaches the JSON-RPC layer. The user-facing
+    // message must use the safer wording.
     let app = Router::new().route(
         "/guilds/{guild_id}/channels",
-        get(|| async { (StatusCode::FORBIDDEN, "nope") }),
+        get(|| async {
+            (
+                StatusCode::FORBIDDEN,
+                r#"{"message":"Missing Access","code":50001}"#,
+            )
+        }),
     );
     let base = spawn_mock(app).await;
     let err = list_guild_channels_at_base(&base, "t", "g1")
         .await
         .unwrap_err()
         .to_string();
-    assert!(err.contains("list channels failed"));
-    assert!(err.contains("403"));
+    let lower = err.to_ascii_lowercase();
+    assert!(!lower.contains("403"), "raw 403 must not leak: {err}");
+    assert!(
+        err.contains("list_channels"),
+        "endpoint identifier preserved: {err}"
+    );
+    assert!(
+        err.contains("Settings → Channels → Discord"),
+        "remediation path present: {err}"
+    );
 }
 
 #[tokio::test]
@@ -363,6 +435,17 @@ async fn check_channel_permissions_errors_on_member_lookup_failure() {
         .await
         .unwrap_err()
         .to_string();
-    assert!(err.contains("member info failed"));
-    assert!(err.contains("401"));
+    // Endpoint identifier preserved in the rewrap (#2285), and the
+    // 401 path keeps the substrings "401"/"unauthorized" out of the
+    // user-facing message so the JSON-RPC session-expired classifier
+    // ignores it.
+    assert!(err.contains("get_member_info"));
+    assert!(
+        !err.to_ascii_lowercase().contains("401"),
+        "rewrapped message must not contain '401': {err}"
+    );
+    assert!(
+        !err.to_ascii_lowercase().contains("unauthorized"),
+        "rewrapped message must not contain 'unauthorized': {err}"
+    );
 }