fix(core): keep an absent manager field distinct from an explicit removal

PUT /club/manager took `owned_card_id: Option<String>`, so serde collapsed
"field absent" and "field explicitly null" into the same None, and the route
treated both as a clear. A caller that simply had nothing to say about the
manager therefore DELETED the assignment.

That is the second destructive squad-save path. WAL forensics on the staging
DB pin it to commit frame 468, squad_managers 1 row -> 0, in a transaction
touching only squad_managers and its indexes - disjoint from the player wipe
at frame 465, which touched squads/squad_players/game_entity_ext. The two
wipes came from two different writes, and only the first was guarded.

The three states are now distinct:

  {}                            leave the manager exactly as it is
  {"owned_card_id": null}       explicitly remove it (still supported)
  {"owned_card_id": "<id>"}     assign that owned card

A deliberate removal is a legitimate operation and is preserved; only the
"absent means delete" reading is gone.

set_squad_manager_for_squad now runs its two existence checks and the insert
in ONE transaction. Validating on the pool and then inserting left a window in
which the squad or the card could disappear between check and write.

Tests cover assign, reassign, idempotent re-assign, absent-is-a-no-op,
explicit-null-still-removes, unowned-manager-refused, absent-against-no-manager
not over-guarded, and that no manager write disturbs player assignments. A
malformed body is asserted to be a parser rejection, distinguishable from the
guard. With the fix reverted the absent-field test fails.

src/routes/club.rs

@@ -139,15 +139,36 @@ pub async fn get_squad_manager(
     Ok(Json(json!({ "manager": manager })))
 }
 
+/// A manager write. The three states are DISTINCT and must stay that way:
+///
+/// | body | meaning |
+/// | --- | --- |
+/// | `{}` — field absent | say nothing about the manager; leave it as it is |
+/// | `{"owned_card_id": null}` | explicitly remove the current manager |
+/// | `{"owned_card_id": "<id>"}` | assign that owned card |
+///
+/// A plain `Option<String>` collapsed the first two into `None`, so a caller
+/// that simply had nothing to say silently deleted the assignment. That is how a
+/// FIFA 17 client with a destroyed squad model wiped a real manager row. The
+/// double option keeps "absent" and "null" apart.
 #[derive(Deserialize)]
 pub struct SetManagerRequest {
-    /// The owned card to assign as manager, or `null`/absent to clear it.
-    pub owned_card_id: Option<String>,
+    #[serde(default, deserialize_with = "deserialize_present_option")]
+    pub owned_card_id: Option<Option<String>>,
 }
 
-/// Assign (or, with a null/absent `owned_card_id`, clear) the active squad's
-/// manager. Fail-closed: the card must be owned by this club and the club must
-/// have a squad. Returns the resulting assignment.
+/// Deserialize a field that is present-but-null into `Some(None)`, leaving an
+/// absent field as `None` (supplied by `#[serde(default)]`).
+fn deserialize_present_option<'de, D>(d: D) -> Result<Option<Option<String>>, D::Error>
+where
+    D: serde::Deserializer<'de>,
+{
+    Option::<String>::deserialize(d).map(Some)
+}
+
+/// Assign, explicitly remove, or leave unchanged the active squad's manager.
+/// Fail-closed: the card must be owned by this club and the club must have a
+/// squad. Returns the resulting assignment.
 pub async fn put_squad_manager(
     State(state): State<AppState>,
     game: GameId,
@@ -156,10 +177,13 @@ pub async fn put_squad_manager(
     let profile = profile_svc::get_active_profile(&state.pool, game.as_str()).await?;
     let club = club_svc::get_club_by_profile(&state.pool, &profile.id).await?;
     match req.owned_card_id {
-        Some(owned_card_id) => {
+        Some(Some(owned_card_id)) => {
             club_svc::set_squad_manager(&state.pool, &club.id, &owned_card_id).await?
         }
-        None => club_svc::clear_squad_manager(&state.pool, &club.id).await?,
+        // Explicit null: a deliberate removal, which is a legitimate operation.
+        Some(None) => club_svc::clear_squad_manager(&state.pool, &club.id).await?,
+        // Absent: this request expresses no manager decision. Touch nothing.
+        None => {}
     }
     let manager = club_svc::get_squad_manager(&state.pool, &club.id).await?;
     Ok(Json(json!({ "manager": manager })))

src/services/club.rs

@@ -195,11 +195,16 @@ pub async fn set_squad_manager_for_squad(
     squad_id: &str,
     owned_card_id: &str,
 ) -> AppResult<()> {
+    // One transaction: both existence checks and the write. Validating on the
+    // pool and then inserting left a window in which the squad or the card could
+    // be removed between the check and the write, persisting an assignment whose
+    // preconditions no longer held.
+    let mut tx = pool.begin().await?;
     let squad_ok =
         sqlx::query_scalar::<_, String>("SELECT id FROM squads WHERE id = ? AND club_id = ?")
             .bind(squad_id)
             .bind(club_id)
-            .fetch_optional(pool)
+            .fetch_optional(&mut *tx)
             .await?;
     if squad_ok.is_none() {
         return Err(AppError::NotFound(format!("squad '{squad_id}' not found")));
@@ -208,7 +213,7 @@ pub async fn set_squad_manager_for_squad(
         sqlx::query_scalar::<_, String>("SELECT id FROM owned_cards WHERE id = ? AND club_id = ?")
             .bind(owned_card_id)
             .bind(club_id)
-            .fetch_optional(pool)
+            .fetch_optional(&mut *tx)
             .await?;
     if card_ok.is_none() {
         return Err(AppError::NotFound(format!(
@@ -223,8 +228,9 @@ pub async fn set_squad_manager_for_squad(
     .bind(squad_id)
     .bind(owned_card_id)
     .bind(&now)
-    .execute(pool)
+    .execute(&mut *tx)
     .await?;
+    tx.commit().await?;
     Ok(())
 }