Repository navigation
Replay review requests from the client, via the api and RabbitMQ #1181
Description
Goal in one sentence: a player presses "request replay review" in the client, and the request appears as a post in the training Discord's replay review forum channel, attributed to a FAF account the server vouched for.
This writes up Brutus5000's suggestion from Discord: lobby command → RabbitMQ → faf-qai consumes and posts. It replaces two earlier ideas that were rejected for good reasons — a webhook in the client, which cannot verify who is asking, and an HTTP write endpoint on faf-qai, which adds inbound attack surface to a service that is not closely maintained.
Why this shape
- Identity is free. The client is already authenticated on the lobby socket. The server knows who is sending, so nothing has to trust a client-supplied player id.
- No new inbound surface. faf-qai consumes from the broker. It opens no port. This was the blocking objection to the HTTP variant.
- Kill switch. Unbind the queue and the feature is off — no client release, no code change, no restart.
- Durability. A qai restart does not lose requests; they wait in the queue.
Step 1 — Lobby server (this repo)
A command_request_replay_review handler on LobbyConnection, following the convention of command_invite_to_party and command_set_player_vetoes. It:
- Requires an authenticated connection (the existing
ensure_authenticatedgate covers this — no new mechanism). - Validates the payload against a fixed schema and rejects anything else.
- Stamps
player_idandloginfrom the connection, not from the body. The client supplies content fields only. This is the property the whole design rests on; without it the impersonation hole would be back by another route. - Rate limits per player and rejects over-limit requests before publishing, so abuse never reaches the bus.
- Publishes to
MQ_EXCHANGE_NAMEwith routing keyrequest.replay_review.create.
Payload:
| field | source | notes |
|---|---|---|
player_id, login |
server | from the connection |
replay_id |
client | id only, never a url — see below |
map, game_mode, faction, rating, played_at |
client | prefilled by the client, editable by the player |
goal |
client | required, free text |
struggle |
client | optional, free text |
requested_at |
server | ISO 8601 UTC |
Two deliberate choices in that table:
Structured fields, not rendered Markdown. Formatting belongs at the destination, so changing how a review post reads does not need a client release, and a second consumer can use the same request differently.
A replay is named by id only. The client also knows how to name a replay by link or by a local file. Neither belongs on the bus: a link would let the client decide what a review post points at, and a file nobody else can fetch is not a review request. Those two cases keep using the client's existing copy-and-paste path.
Routing key follows the request.* / success.* convention already used by client_message_queue_service.py and avatar_change_queue_service.py.
Step 2 — Rate limit
There is no per-player rate limiter in the lobby server today, so this is new code. Proposal: one request per player per 24h, as a config constant next to the other tunables, held in memory in a small service.
In-memory means it does not survive a restart and each instance counts on its own — a spam guard rather than a quota. That looks like the right trade: the cost of a duplicate is a duplicate post, and the alternative is a schema migration plus a database write on a path that has no other reason to touch the database. Happy to be argued out of it.
Step 3 — Broker
A queue for faf-qai bound to request.replay_review.create on faf-rabbitmq. Ops task, not a code change.
Step 4 — faf-qai
A consumer on that queue that opens a forum post per request. DSharpPlus supports forum posts natively, so no REST workarounds. It renders the structured payload and writes the verified player name, so the author shown in Discord is one the server vouched for.
If the requesting player has linked their Discord account (AccountLinkService), a later version can mention them so the answer reaches them. Not in the first version.
Step 5 — Client
One frame alongside the existing lobby commands. The existing copy-and-paste path stays as the fallback for a player who is offline, for a replay that has no id, and for the case where any part of this is switched off — so nothing regresses if the feature is disabled.
Step 6 — Rollout
Ship the lobby side first with no consumer bound: the messages go nowhere and cost nothing. Bind qai's queue when both sides are deployed. The only hard ordering is lobby-before-client.
Decisions needed
- Routing key name — is
request.replay_review.createright? - Rate limit — one per player per day, and is the in-memory spam guard acceptable, or does this want to be durable?
- Who creates the queue and binding?
- Is faf-qai's maintainer on board, and does the bot have permission to create posts in that channel? This one is outside both repos and it is what killed the Dostya attempt, so I would rather ask now than find out later.
A fifth, smaller one: the lobby currently sends nothing back on success and answers a rejection with the existing notice path, the same as command_invite_to_party. That means the client shows success optimistically. If an explicit acknowledgement is wanted instead, that is a new server→client command and worth deciding before clients are written against it.
Not in scope
The generic passthrough — a mechanism letting any client message reach the bus without a per-command change. Worth building, but it is the harder problem: it puts unvalidated client input on a bus whose producers are trusted precisely because it is internal only (see the docstring in client_message_queue_service.py). One command with a fixed routing key and a validated schema is the safe increment, and it is a concrete first candidate to migrate once the general mechanism exists.
Fan-out itself already works: the topic exchange lets any number of services bind their own queue to the same key, which ClientMessageQueueService and AvatarChangeQueueService already do side by side. So "multiple services consuming" needs nothing new.
What I am offering
I have written the lobby server side with tests, and the qai consumer. I need review and a deploy, not implementation time.
Activity
TimMasalme commentedon Sep 6, 2026
Both sides are now up:
Neither depends on the other to be safe to merge. With no queue bound to the routing key the lobby publishes into nothing, and the qai side stays dormant until it is given a broker and a channel id. So the four decisions above are still the only thing this is waiting on, and the fourth one — whether a consumer in faf-qai is wanted at all, and whether the bot may post in that channel — is the one I would most like an answer to before anything is deployed.
Sheikah45 commentedon Sep 6, 2026
Why is this added to the lobby server as a command instead of the api? I would think that would be the better place for something that doesn't really have state.
Brutus5000 commentedon Sep 6, 2026
This issue doesn't really uphold to what I said. I said I want a dynamic handling of all kind of messages that the lobby-server itself does not act on.
There are multiple reasons I would do it this way:
But the dynamic binding and relying of messages to RabbitMQ is a key component I guess.
Sheikah45 commentedon Sep 6, 2026
What is the difference between an API endpoint and a lobby command message? They are pretty similar in terms of lift to develop. I agree service communication is by rabbitmq so seems just as simple in the API. Not sure what the async communication would be.
I mainly just see the lobby server protocol as a bit clunky and harder to maintain vs a dedicated API endpoint. It also is confusing then why we moved the avatar selection to the api instead of continuing to use the lobby server command.
Sheikah45 commentedon Sep 6, 2026
It being an api endpoint would also allow other actors to request replay reviews without being required to implement the full lobby protocol and have thee lobby scope on the auth token.
TimMasalme commentedon Sep 6, 2026
The reason why i didn't do the dynamic handling is that you get unvalidated client input on a bus whose producer is considered trusted
Sheikah45 commentedon Sep 6, 2026
The api would be authenticated. I am talking about an endpoint in FAForever/java-api
TimMasalme commentedon Sep 6, 2026
I think you two should make the decision on how it should look like in the end. im just the executor of the plan. I think thats better than me deciding on my own and doing smth and in the end that doesn't fit in your plans/vision
Brutus5000 commentedon Sep 7, 2026
@Sheikah45 Avatar is different. It was already part of the domain model of the API and therefore it makes sense to use it. It was used by the moderator client too and there is no reason to have it in both places.
I am looking for a generic way that other services can message asynchronously. It may not always be needed, but it may be sometimes needed and I don't want to build a solution for both. If I build a generic endpoint to the API there isn't much gained. What I want to avoid is building specific endpoints for other services into the API. It also shouldn't be in the lobby server, but until we extract the logic from the lobby server into a dedicated message broker I see no better way?
Also important: we need to have some dynamic binding maybe even with message spec validation. This could be somewhere in mysql where we can dynamically route messages into rabbitmq and validate the payload.
But feel free to give me counterarguments why it should be in the API.
Sheikah45 commentedon Sep 7, 2026
I think in this case it is about what this action is doing and what the entry point is ultimately for.
I view the websocket based communication as being for two-way communication that is focused on actions that take place mainly for players who are currently online. The messages in that domain should be focused on actions the client takes with regards to other players and games that are currently live. This would also hold in my mind to any asynchronous message broker we develop.
I view the API for resources that do not require interaction with other players and is just retrieving or putting resources onto the server. It is also more fitting for that with the typical http status responses that are built in to handle communicating success or failure and also for easier retry. It also more easily handles rate limiting and is more flexible with authentication since it does not touch live players or the necessarily the game files.
For this case of asking for a replay review since it isn't interacting with live players in my view it belongs on the API. It is completely async and the user doesn't have to be online in order to submit a replay for review. We already have the capability to publish to a rabbitmq broker there just as easily as from the lobby server. Additionally the specs are inherently better documented and interact with the same domain model as in the api already in the form of replays.
With regards to validation implementing this in the lobby server right now removes the ability for any real validation of the message itself. We also had to re-implement a concept of rate limiting. This makes it more impossible for the submitter to get immediate feedback on their submission about whether it might be successful or not. Whereas in the API we could easily check that the replay exists and that the user was a participant in the game. Then provide the proper status code in response to indicate success or failure to the submitter without too much lift.
I am not sure what you see as the difficulty of implementing a endpoints to the API for specific purposes vs adding them to the lobby server. An API endpoint requires adding a method to a controller, defining the dto, performing validation if any, and then publishing to rabbitmq. Where a lobby server message requires adding a handler for the message, defining the dto, performing validation but that is more limited here and publishing to rabbitmq. They have the same lift unless you just blind pass through messages from clients but I don't think that would be such a good idea.
Additionally, there is the fact that making updates and deploying the message broker would break existing connections for a feature that doesn't really benefit them while the API can more easily be deployed since clients don't have a persistent connection.
So it is more to me about separation of domain. And I view requesting a replay review from a trainer very similar to submitting a replay rating and the semantics of http and potential validation the api could do upfront as a better user experience vs just sending in a message with very little feedback. It also allows us to better control things like not being able to request reviews for replays you aren't in.
Sheikah45 commentedon Sep 8, 2026
After discussing with Brutus we came to the conclusion that this feature can be made part of the API in the Elide data model.
There we can take advantage of the existing permission framework and request semantics. We will have to implement a Noop DataStore that can handle ephemeral data models like the replay review request that are not actually stored in the database but have just a create lifecycle hook binding that will be utilized to publish to rabbitmq.
The api is in the repo https://github.com/FAForever/faf-java-api.
Sheikah45 commentedon Sep 8, 2026
And a minimal model has been added in #1180. Example for how to do the lifecycle hooks can be found on the Player.currentAvatar field
[-]Replay review requests from the client, via the lobby server and RabbitMQ[/-][+]Replay review requests from the client, via the api and RabbitMQ[/+]