address review: extract mode resolution, add unit tests, drop Debug

- resolve_credential (+ token_credential / password_fields helpers)
extracted out of put_matrix_account, which was tripping
clippy::too_many_lines (101/100). Both helpers surface plain &str
errors rather than ProblemDetails to dodge clippy::result_large_err on
a private fn (put_matrix_account itself is exempt only via clippy's
avoid-breaking-exported-api default, which does not cover these);
resolve_credential boxes its own ProblemDetails Err for the same
reason, unboxed at its one call site.
- Added unit tests for the extracted logic: main-reserved, token-mode
missing-token / pass-through, password-mode missing-fields / trailing-
slash trim, and two async resolve_credential end-to-end checks
(token mode, unknown mode) that need no network access.
- Dropped PutMatrixAccountRequest's Debug derive to match
hive-c0re::dashboard::matrix_accounts::MatrixLoginForm's existing
precedent of not deriving Debug on a struct carrying a password field.

swarm-controller/src/matrix_account.rs

@@ -45,7 +45,11 @@ fn default_mode() -> String {
 }
 
 /// The credential to store for one agent's external matrix account.
-#[derive(Debug, Deserialize, ToSchema)]
+///
+/// No `Debug` derive — matching `hive-c0re::dashboard::matrix_accounts`'s
+/// analogous `MatrixLoginForm`, which also carries a password field and
+/// deliberately omits it so nothing can `{:?}`-log this by accident.
+#[derive(Deserialize, ToSchema)]
 pub struct PutMatrixAccountRequest {
     /// `"token"` (default, back-compat) or `"password"`. Token mode stores
     /// `token` as given; password mode logs into `homeserver` with `user_id`
@@ -140,7 +144,7 @@ pub async fn put_matrix_account(
     // but would confuse anything that lists accounts by name. Same guard
     // hive-c0re's own `/api/matrix-account-login` applies, checked before any
     // mode-specific work (including a network login) runs.
-    if account == "main" {
+    if is_reserved_account(&account) {
         return Err(error_problem(
             StatusCode::BAD_REQUEST,
             "'main' is the hive-internal account, synthesized per agent from \
@@ -148,43 +152,9 @@ pub async fn put_matrix_account(
         ));
     }
 
-    // Resolve the mode to a concrete (token, homeserver, resolved user id) —
+    // Password mode's network call happens here, before the store is
-    // password mode's network call happens here, before the store is
     // touched, so a failed login leaves no partial state behind.
-    let (token, homeserver, user_id) = match req.mode.as_str() {
+    let (token, homeserver, user_id) = resolve_credential(&req).await.map_err(|b| *b)?;
-        "token" => {
-            let Some(token) = req.token else {
-                return Err(error_problem(
-                    StatusCode::BAD_REQUEST,
-                    "token mode needs a token",
-                ));
-            };
-            (token, req.homeserver, None)
-        }
-        "password" => {
-            let (Some(homeserver), Some(user_id), Some(password)) = (
-                req.homeserver.as_deref(),
-                req.user_id.as_deref(),
-                req.password.as_deref(),
-            ) else {
-                return Err(error_problem(
-                    StatusCode::BAD_REQUEST,
-                    "password mode needs homeserver, user_id, and password",
-                ));
-            };
-            let homeserver = homeserver.trim_end_matches('/').to_owned();
-            let (token, resolved_user_id) = matrix_password_login(&homeserver, user_id, password)
-                .await
-                .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e))?;
-            (token, Some(homeserver), Some(resolved_user_id))
-        }
-        other => {
-            return Err(error_problem(
-                StatusCode::BAD_REQUEST,
-                &format!("unknown mode {other:?} (want token|password)"),
-            ));
-        }
-    };
 
     let store = SecretStore::from_env(CERT_ROLE).await.map_err(|e| {
         tracing::warn!(error = %e, "connecting to the swarm secret store failed");
@@ -240,6 +210,92 @@ pub async fn put_matrix_account(
     Ok(Json(PutMatrixAccountResponse { user_id }))
 }
 
+/// Whether `account` is the hive-internal name every hive synthesizes per
+/// agent (`nix/agent-modules/matrix.nix`) — see the call site's own comment
+/// for why this route must never write one.
+fn is_reserved_account(account: &str) -> bool {
+    account == "main"
+}
+
+/// Token-mode's only requirement: a token was actually given. Split out of
+/// `resolve_credential` (with `password_fields` below) so each mode's
+/// validation is unit-testable directly — only the actual network login in
+/// `resolve_credential` itself needs an async runtime to exercise. Returns a
+/// plain `&'static str` rather than a `ProblemDetails` — `clippy::result_large_err`
+/// (this workspace runs `pedantic = deny`) flags a private fn returning one of
+/// those directly; `put_matrix_account` itself is exempt only because it is
+/// `pub` (clippy's `avoid-breaking-exported-api` default), which these
+/// helpers are not.
+fn token_credential(
+    req: &PutMatrixAccountRequest,
+) -> Result<(String, Option<String>), &'static str> {
+    let Some(token) = req.token.clone() else {
+        return Err("token mode needs a token");
+    };
+    Ok((token, req.homeserver.clone()))
+}
+
+/// Password mode's required fields, validated present, with the
+/// homeserver's trailing slash already trimmed — this is what actually
+/// reaches `matrix_password_login`, not the caller's raw string, so the
+/// login URL below never ends up with a doubled `//`.
+struct PasswordFields<'a> {
+    homeserver: String,
+    user_id: &'a str,
+    password: &'a str,
+}
+
+fn password_fields(req: &PutMatrixAccountRequest) -> Result<PasswordFields<'_>, &'static str> {
+    let (Some(homeserver), Some(user_id), Some(password)) = (
+        req.homeserver.as_deref(),
+        req.user_id.as_deref(),
+        req.password.as_deref(),
+    ) else {
+        return Err("password mode needs homeserver, user_id, and password");
+    };
+    Ok(PasswordFields {
+        homeserver: homeserver.trim_end_matches('/').to_owned(),
+        user_id,
+        password,
+    })
+}
+
+/// Resolve a request's credential mode to a concrete `(token, homeserver,
+/// resolved user id)`. Extracted out of `put_matrix_account` to keep that
+/// function under `clippy::too_many_lines`, and so the mode branching has
+/// something to unit-test directly (via `token_credential`/`password_fields`
+/// above) instead of only through the full route.
+///
+/// `Box`ed error for the same `result_large_err` reason `token_credential`'s
+/// doc explains — this fn is private too, so it does not get `put_matrix_account`'s
+/// exported-API exemption. Unboxed at the one call site instead of changing
+/// `put_matrix_account`'s own (exempt, and part of the route's documented
+/// contract) return type.
+async fn resolve_credential(
+    req: &PutMatrixAccountRequest,
+) -> Result<(String, Option<String>, Option<String>), Box<problem_details::ProblemDetails>> {
+    match req.mode.as_str() {
+        "token" => {
+            let (token, homeserver) = token_credential(req)
+                .map_err(|e| Box::new(error_problem(StatusCode::BAD_REQUEST, e)))?;
+            Ok((token, homeserver, None))
+        }
+        "password" => {
+            let fields = password_fields(req)
+                .map_err(|e| Box::new(error_problem(StatusCode::BAD_REQUEST, e)))?;
+            let (token, resolved_user_id) =
+                matrix_password_login(&fields.homeserver, fields.user_id, fields.password)
+                    .await
+                    .map_err(|e| Box::new(error_problem(StatusCode::BAD_REQUEST, &e)))?;
+            Ok((token, Some(fields.homeserver), Some(resolved_user_id)))
+        }
+        other => Err(Box::new(error_problem(
+            StatusCode::BAD_REQUEST,
+            &format!("unknown mode {other:?} (want token|password)"),
+        ))),
+    }
+}
+
 /// POST `m.login.password` to `<homeserver>/_matrix/client/v3/login`.
 /// Returns `(access_token, user_id)`.
 ///
@@ -288,3 +344,101 @@ async fn matrix_password_login(
         .ok_or_else(|| "login response missing user_id".to_owned())?;
     Ok((token.to_owned(), uid.to_owned()))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{
+        PutMatrixAccountRequest, is_reserved_account, password_fields, resolve_credential,
+        token_credential,
+    };
+
+    fn request(mode: &str) -> PutMatrixAccountRequest {
+        PutMatrixAccountRequest {
+            mode: mode.to_owned(),
+            token: None,
+            user_id: None,
+            password: None,
+            homeserver: None,
+        }
+    }
+
+    #[test]
+    fn main_is_reserved_but_nothing_else_is() {
+        assert!(is_reserved_account("main"));
+        assert!(!is_reserved_account("ops-relay"));
+        // Case-sensitive on purpose: `matrixAccounts` is a nix attrset, so
+        // `Main` is a distinct, legal key from the reserved `main`.
+        assert!(!is_reserved_account("Main"));
+    }
+
+    #[test]
+    fn token_mode_needs_a_token() {
+        assert!(token_credential(&request("token")).is_err());
+    }
+
+    #[test]
+    fn token_mode_passes_through_token_and_homeserver_unchanged() {
+        let mut req = request("token");
+        req.token = Some("t0k3n".to_owned());
+        req.homeserver = Some("https://matrix.example.org".to_owned());
+        let (token, homeserver) = token_credential(&req).expect("token + homeserver both given");
+        assert_eq!(token, "t0k3n");
+        assert_eq!(homeserver.as_deref(), Some("https://matrix.example.org"));
+    }
+
+    #[test]
+    fn password_mode_needs_all_three_fields() {
+        let mut req = request("password");
+        assert!(password_fields(&req).is_err(), "none given");
+        req.homeserver = Some("https://matrix.example.org".to_owned());
+        assert!(
+            password_fields(&req).is_err(),
+            "still missing user_id + password"
+        );
+        req.user_id = Some("@a:matrix.example.org".to_owned());
+        assert!(password_fields(&req).is_err(), "still missing password");
+        req.password = Some("hunter2".to_owned());
+        assert!(password_fields(&req).is_ok(), "now all three given");
+    }
+
+    #[test]
+    fn password_mode_trims_a_trailing_slash_off_the_homeserver() {
+        let mut req = request("password");
+        req.homeserver = Some("https://matrix.example.org/".to_owned());
+        req.user_id = Some("@a:matrix.example.org".to_owned());
+        req.password = Some("hunter2".to_owned());
+        let fields = password_fields(&req).expect("all three fields given");
+        assert_eq!(fields.homeserver, "https://matrix.example.org");
+    }
+
+    #[test]
+    fn password_mode_leaves_a_homeserver_with_no_trailing_slash_unchanged() {
+        let mut req = request("password");
+        req.homeserver = Some("https://matrix.example.org".to_owned());
+        req.user_id = Some("@a:matrix.example.org".to_owned());
+        req.password = Some("hunter2".to_owned());
+        let fields = password_fields(&req).expect("all three fields given");
+        assert_eq!(fields.homeserver, "https://matrix.example.org");
+    }
+
+    #[tokio::test]
+    async fn resolve_credential_token_mode_end_to_end() {
+        let mut req = request("token");
+        req.token = Some("t0k3n".to_owned());
+        let (token, homeserver, user_id) = resolve_credential(&req)
+            .await
+            .expect("token mode with a token given");
+        assert_eq!(token, "t0k3n");
+        assert_eq!(homeserver, None);
+        assert_eq!(user_id, None);
+    }
+
+    #[tokio::test]
+    async fn resolve_credential_rejects_an_unknown_mode() {
+        assert!(
+            resolve_credential(&request("carrier-pigeon"))
+                .await
+                .is_err()
+        );
+    }
+}