From f9a495e7fce99fb87fc6738d65d233ac012e0c8a Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Thu, 13 Aug 2026 15:42:10 -0400 Subject: [PATCH] feat(api): invite a player by handle into a lobby seat The third resolver: Lobbies::invite_handle, alongside claim_seat and assign_bot. Resolves the same way plan_players' player control does (Atproto::resolve_player, a real DNS-over-HTTPS round trip), seats through the same finish_claim/Db::claim_match_player atomic write the other two resolvers use, and refuses a resolved DID already seated elsewhere the same way a self-claim's duplicate check does. The socket's request loop spawns the resolution rather than awaiting it inline - a slow lookup would otherwise block that connection from reading further messages or relaying broadcasts until it finished. The result reaches every connection the same way any other claim's does: the next seats snapshot. The happy path (a handle that actually resolves) has no test seam, same gap plan_players' own duplicate-seat guard already has and for the same reason - resolve_player holds concrete resolver fields, not a trait a fake can stand in for. What's covered without network: the fast-fail slot check ahead of the resolve call, and the write path shared verbatim with claim_seat's own tests via finish_claim. Change-Id: I9ce92de933b0ef276c668405c7704dce05d30650 --- services/api/src/matches/lobby.rs | 121 +++++++++++++++++++++++++++++- services/api/src/routes.rs | 99 ++++++++++++++++++++++-- 2 files changed, 210 insertions(+), 10 deletions(-) diff --git a/services/api/src/matches/lobby.rs b/services/api/src/matches/lobby.rs index d1969b1..00d83ad 100644 --- a/services/api/src/matches/lobby.rs +++ b/services/api/src/matches/lobby.rs @@ -11,10 +11,19 @@ //! (`Db::claim_match_player`, `Db::delete_match_player`) in place of //! `insert_match_players`'s one-shot slate. //! -//! Two resolvers only: a connected player claiming a named slot as -//! themselves (`claim_seat`), and assigning a slot to the bot -//! (`assign_bot`). Inviting a handle who is not connected, `OpenInvite` and -//! `Matchmaking` are not here; see plan/invitations.md. Turning a full lobby into a +//! Three resolvers: a connected player claiming a named slot as themselves +//! (`claim_seat`), assigning a slot to the bot (`assign_bot`), and inviting a +//! handle who need not be connected at all (`invite_handle`) — the one piece +//! `plan/lobby.md` had scoped out of the first slice. It leans on the same +//! `Atproto::resolve_player` lookup `routes.rs`'s `plan_players` uses for its +//! own `player` control, and the same atomic `Db::claim_match_player` write +//! the other two resolvers use, so a resolved handle gets the identical +//! first-write-wins guarantee a self-claim does — no second, weaker seating +//! path for the one resolver that happens to need a network round trip +//! first. Nothing here asks the invited player first, same as `plan_players` +//! never has — see `plan/invitations.md` for why that is the wrong default, +//! not a feature. `OpenInvite` and `Matchmaking` are still not here. Turning +//! a full lobby into a //! launched match ("Deploy") *is* here in spirit — `broadcast_deployed` and //! the `LobbyEvent::Deployed` it sends — but the resolver itself, //! `deploy_lobby`, lives in `routes.rs`, not this module: it calls @@ -403,6 +412,58 @@ impl Lobbies { self.finish_claim(match_id, player).await } + /// Invites `handle` — who need not be connected to this lobby, or to + /// anything, at the moment this is called — into `slot`. + /// + /// Resolved the same way `plan_players`'s `player` control resolves an + /// invited seat: `Atproto::resolve_player`, a real DNS-over-HTTPS round + /// trip and the one thing that makes this resolver different from + /// `claim_seat`/`assign_bot`, which never leave the process. Callers run + /// this off a connection's own `tokio::select!` loop (see routes.rs's + /// `lobby_socket`) precisely because of that round trip: awaiting it + /// inline would leave that connection unable to read further messages or + /// relay broadcasts until the resolution finished. + /// + /// `slot` is checked against the lobby's current scenario twice — once + /// before the network call, to fail fast on a typo without ever + /// resolving anything, and once after, because the scenario can change + /// while a resolution is in flight and a stale check would let a slot + /// the new scenario dropped get claimed anyway. Seating itself is + /// `finish_claim`, the exact write `claim_seat` and `assign_bot` use — + /// `Db::claim_match_player`'s atomic `INSERT ... WHERE NOT EXISTS` + /// refuses a resolved DID already seated elsewhere in this lobby, the + /// same guard that already stops a caller claiming two slots for + /// themselves. + /// + /// Silently a no-op on failure — an unknown slot, an unresolvable + /// handle, or a resolved DID already seated — the same "cosmetic, not + /// fatal" posture `claim_seat` and `assign_bot` already take. Returns + /// whether it took, for the caller to log. + pub async fn invite_handle( + &self, + atproto: &crate::atproto::Atproto, + match_id: &str, + slot: &str, + handle: &str, + ) -> bool { + if !self.slot_exists(match_id, slot).await { + return false; + } + let Some((invited_did, canonical_handle)) = atproto.resolve_player(handle).await else { + return false; + }; + if !self.slot_exists(match_id, slot).await { + return false; + } + let player = MatchPlayer { + slot: slot.to_owned(), + control: "human".into(), + did: Some(invited_did), + handle: Some(canonical_handle), + }; + self.finish_claim(match_id, player).await + } + /// Whether `slot` belongs to the scenario this lobby currently holds. /// A lobby row with no scenario at all (should not happen; `open` /// always sets one) or a scenario path the catalog no longer has both @@ -890,6 +951,58 @@ mod tests { assert_eq!(lobbies.join(&id).1.len(), 1); } + // --- invite_handle --------------------------------------------------- + // + // `invite_handle`'s happy path (a handle resolves, and gets seated) and + // its "resolves to a DID already seated elsewhere" refusal both depend + // on `Atproto::resolve_player` actually resolving something, which means + // a real DNS-over-HTTPS round trip — the identical gap `plan/lobby.md` + // already documents for `plan_players`'s duplicate-seat guard, and for + // the same reason: `resolve_player` holds concrete resolver fields, not + // a trait `cargo test` can hand a fake to, so exercising those two paths + // here would mean `cargo test` depending on live network egress. Not + // attempted, honestly, rather than faked. + // + // What *is* tested here without any network: the fast-fail slot check + // that runs before `resolve_player` is ever called, and — indirectly, + // through `claim_seat`'s own tests and `db.rs`'s + // `claim_match_player_refuses_a_second_slot_for_the_same_did` — the + // write path `invite_handle` shares verbatim with the other two + // resolvers via `finish_claim`. `invite_handle` builds the same + // `MatchPlayer` shape `claim_seat` does and hands it to the same + // `finish_claim`, so a resolved handle gets the same atomic + // first-write-wins guarantee those tests already prove, without this + // module reimplementing (and needing to separately test) the guard. + + fn unresolvable_atproto() -> crate::atproto::Atproto { + // A real Atproto, built the same way routes.rs's own tests build + // one (`loopback_state`) — its resolvers are never exercised by the + // test below, which returns before `resolve_player` is called. + let dir = tempfile::tempdir().unwrap(); + let db = Db::open(&dir.path().join("atproto.sqlite")).unwrap(); + let config = crate::config::Config::from_lookup(|_| None).unwrap(); + crate::atproto::Atproto::new(&config, db).expect("test config builds a valid Atproto") + } + + #[tokio::test] + async fn invite_handle_rejects_an_unknown_slot_without_resolving_anything() { + let (_dir, lobbies) = temp_lobbies().await; + let id = lobbies + .open("did:plc:abc", "TrainingScenarios/1-FirstRun.mms") + .await + .unwrap(); + let atproto = unresolvable_atproto(); + + // "NotARealSlot" fails `slot_exists` before `resolve_player` is ever + // reached, so this needs no network to run or to pass. + assert!( + !lobbies + .invite_handle(&atproto, &id, "NotARealSlot", "quire.example") + .await + ); + assert_eq!(lobbies.seats(&id).await, Vec::new()); + } + /// The Lobbies-level version of the race db.rs's own test proves at the /// SQL layer: two real concurrent claims for the same slot, exactly one /// wins. diff --git a/services/api/src/routes.rs b/services/api/src/routes.rs index 30b555e..236a2ca 100644 --- a/services/api/src/routes.rs +++ b/services/api/src/routes.rs @@ -1099,11 +1099,15 @@ async fn create_match( // Claiming a seat is narrower than connecting: `claim_seat` only ever seats // the caller's own session DID — there is no "claim for someone else" on // this channel, the same way `plan_players`'s `self` control only ever -// resolves to the caller. Inviting a handle who is not connected needs the -// same async `Atproto::resolve_player` lookup `plan_players` does for its -// `player` control, and real thought about a socket left waiting mid-flow -// on a DNS-over-HTTPS round trip; deliberately not attempted here, see -// plan/invitations.md. +// resolves to the caller. Inviting a handle who is not connected is the +// third resolver, `invite_handle` (`matches::lobby`), and it does use the +// same async `Atproto::resolve_player` lookup `plan_players`'s `player` +// control does — which is exactly why `lobby_socket` spawns it rather than +// awaiting it inline: a DNS-over-HTTPS round trip on the connection's own +// `tokio::select!` loop would leave that connection unable to read further +// messages or relay broadcasts until the resolution finished. It seats the +// resolved DID the moment resolution succeeds, same as `plan_players` +// always has — see plan/invitations.md for why neither of those asks first. #[derive(serde::Deserialize)] struct OpenLobbyBody { @@ -1161,7 +1165,8 @@ async fn lobby_live( } }; let lobbies = state.lobbies.clone(); - ws.on_upgrade(move |socket| lobby_socket(socket, lobbies, id, row.scenario, did)) + let atproto = state.atproto.clone(); + ws.on_upgrade(move |socket| lobby_socket(socket, lobbies, atproto, id, row.scenario, did)) } /// What a client may send on a lobby's live channel. Tagged so this @@ -1189,6 +1194,15 @@ enum LobbyRequest { Chat { text: String, }, + /// Invites `handle` — who need not be connected to this lobby, or to + /// anything, right now — into `slot`. See `matches::lobby::Lobbies:: + /// invite_handle` for the resolution and seating this triggers, and the + /// note above `lobby_socket` for why it runs off this loop rather than + /// on it. + Invite { + slot: String, + handle: String, + }, } /// One connection's whole life: the lobby's current scenario and seats @@ -1205,6 +1219,7 @@ enum LobbyRequest { async fn lobby_socket( mut socket: WebSocket, lobbies: Arc, + atproto: Arc, match_id: String, initial_scenario: Option, did: String, @@ -1267,6 +1282,36 @@ async fn lobby_socket( ); } } + Ok(LobbyRequest::Invite { slot, handle }) => { + // Spawned rather than awaited: resolving a + // handle is a real DNS-over-HTTPS round + // trip, and awaiting it inline here would + // leave this one connection unable to read + // further messages or relay broadcasts + // until it finished. The result reaches + // every connection the same way a + // synchronous claim's does — the next + // `seats` snapshot — so there is nothing for + // the spawned task to report back to this + // socket specifically. + let lobbies = lobbies.clone(); + let atproto = atproto.clone(); + let match_id = match_id.clone(); + let inviter = did.clone(); + tokio::spawn(async move { + let took = lobbies + .invite_handle(&atproto, &match_id, &slot, &handle) + .await; + tracing::debug!( + match_id, + inviter, + slot, + handle, + took, + "lobby: invite resolved" + ); + }); + } // A bad shape or an unknown scenario/slot is // ignored rather than dropping the socket: a // typo in flight should not end the session. @@ -3455,6 +3500,48 @@ mod tests { ); } + /// An invite to a slot the current scenario does not have is a no-op — + /// the same "cosmetic, not fatal" refusal `claimSeat`/`botSeat` give a + /// bad slot. Provable without any network: `Lobbies::invite_handle` + /// checks the slot before it ever calls `Atproto::resolve_player`, so + /// this fails fast and the socket keeps answering right after — shown + /// here by a real claim landing on the same connection immediately + /// after the refused invite. The happy path (a handle that actually + /// resolves) is not covered over the wire for the reason + /// `matches::lobby`'s own test module documents: `resolve_player` has + /// no seam for a fake resolver, so proving it here would mean `cargo + /// test` depending on live network egress. + #[tokio::test] + async fn inviting_an_unknown_slot_is_a_silent_no_op() { + use tokio_tungstenite::tungstenite::Message as WsMessage; + + let (_dir, addr, match_id) = + serving_lobby("TrainingScenarios/1-FirstRun.mms", "did:plc:abc").await; + let cookie = test_cookie("did:plc:abc"); + let mut socket = connect_to_lobby(addr, &match_id, &cookie).await; + next_scenario(&mut socket).await; + next_seats(&mut socket).await; + + futures_util::SinkExt::send( + &mut socket, + WsMessage::text(r#"{"type":"invite","slot":"NotARealSlot","handle":"quire.example"}"#), + ) + .await + .unwrap(); + futures_util::SinkExt::send( + &mut socket, + WsMessage::text(r#"{"type":"botSeat","slot":"TraineeB"}"#), + ) + .await + .unwrap(); + + assert_eq!( + next_seats(&mut socket).await, + vec![("TraineeB".to_owned(), None)], + "the socket is still alive and answering after the refused invite" + ); + } + /// Two real, separately-connected clients racing to claim the same /// slot: exactly one claim takes, proven end to end over real sockets /// rather than by calling `Lobbies` directly. -- 2.51.2