SyncPlay is 21 of 22 routes unwritten, and the one that answers is a stub #198

New issue

Closed

opened 2026-09-12 08:50:59 +02:00 by Claude · 2 comments

Claude commented 2026-09-12 08:50:59 +02:00

Collaborator

Copy link

21 of SyncPlay's 22 documented operations are unwritten. The one answered is
the listing, stubbed so the client's group picker renders empty rather than
throwing:

// crates/ephyra-http/src/routes/absent.rs:39
// SyncPlay.
.route("/SyncPlay/List", get(no_groups))

Everything else — New, Join, Leave, Play/Pause/Unpause, Seek,
Buffering, Ready, Ping, SetNewQueue, SetPlaylistItem,
MovePlaylistItem, RemoveFromPlaylist, Queue, NextItem, PreviousItem,
SetRepeatMode, SetShuffleMode, SetIgnoreWait, and GET /SyncPlay/{id} —
is a 404. MILESTONES.md lists it under explicitly out of scope with the
count as the reason.

What it actually costs

~7,000 lines upstream across SyncPlay paths, and the controller is the small
part of it:

Piece	Lines
Emby.Server.Implementations/SyncPlay/Group.cs	694
MediaBrowser.Controller/SyncPlay/GroupStates/WaitingGroupState.cs	681
MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs	572
Jellyfin.Api/Controllers/SyncPlayController.cs	440
Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs	422

The shape is a state machine per group —
GroupStates/{Idle,Paused,Playing,Waiting}GroupState.cs over
AbstractGroupState.cs — with a play queue beside it, and it drives clients
over the websocket rather than over the response: SyncPlayCommandMessage in
MediaBrowser.Controller/Net/WebSocketMessages/Outbound/. Nothing in
crates/ephyra-http/src/routes/session/socket.rs sends one today.

Why it is not just another controller

Every other piece of state this server holds is either a database row or the
session registry. A SyncPlay group is a third: long-lived, mutable, shared
between accounts, and correct only if every member's clock is reconciled — the
Waiting state is 681 lines because of exactly that. ARCHITECTURE.md's rule
about use cases holding state rather than ports is where it would land, beside
the session registry (MILESTONES.md M7 records that argument).

It is also the one feature on this list that cannot be diffed the usual way:
the harness replays one client against two servers, and a group needs at least
two clients whose timing is the thing under test. MILESTONES.md's risk table
already names a second client has never been done.

If it is taken on

Suggested order, smallest observable thing first:

1. The group lifecycle only — New, Join, Leave, List, GET /{id} —
   with no playback state at all. That alone makes the picker real and is
   testable against upstream with one client.
2. PlayQueueManager as a pure type, which is a queue with repeat and shuffle
   and no timing in it.
3. The state machine and the websocket command, which is where the second
   client becomes unavoidable.

Deleting the /SyncPlay/List stub from absent.rs is part of step 1.

21 of SyncPlay's 22 documented operations are unwritten. The one answered is the listing, stubbed so the client's group picker renders empty rather than throwing: ```rust // crates/ephyra-http/src/routes/absent.rs:39 // SyncPlay. .route("/SyncPlay/List", get(no_groups)) ``` Everything else — `New`, `Join`, `Leave`, `Play`/`Pause`/`Unpause`, `Seek`, `Buffering`, `Ready`, `Ping`, `SetNewQueue`, `SetPlaylistItem`, `MovePlaylistItem`, `RemoveFromPlaylist`, `Queue`, `NextItem`, `PreviousItem`, `SetRepeatMode`, `SetShuffleMode`, `SetIgnoreWait`, and `GET /SyncPlay/{id}` — is a `404`. `MILESTONES.md` lists it under *explicitly out of scope* with the count as the reason. ## What it actually costs ~7,000 lines upstream across `SyncPlay` paths, and the controller is the small part of it: | Piece | Lines | |---|---| | `Emby.Server.Implementations/SyncPlay/Group.cs` | 694 | | `MediaBrowser.Controller/SyncPlay/GroupStates/WaitingGroupState.cs` | 681 | | `MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs` | 572 | | `Jellyfin.Api/Controllers/SyncPlayController.cs` | 440 | | `Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs` | 422 | The shape is a state machine per group — `GroupStates/{Idle,Paused,Playing,Waiting}GroupState.cs` over `AbstractGroupState.cs` — with a play queue beside it, and it drives clients over the websocket rather than over the response: `SyncPlayCommandMessage` in `MediaBrowser.Controller/Net/WebSocketMessages/Outbound/`. Nothing in `crates/ephyra-http/src/routes/session/socket.rs` sends one today. ## Why it is not just another controller Every other piece of state this server holds is either a database row or the session registry. A SyncPlay group is a third: long-lived, mutable, shared between accounts, and correct only if every member's clock is reconciled — the `Waiting` state is 681 lines because of exactly that. `ARCHITECTURE.md`'s rule about use cases holding state rather than ports is where it would land, beside the session registry (`MILESTONES.md` M7 records that argument). It is also the one feature on this list that cannot be diffed the usual way: the harness replays one client against two servers, and a group needs at least two clients whose timing is the thing under test. `MILESTONES.md`'s risk table already names *a second client has never been done*. ## If it is taken on Suggested order, smallest observable thing first: 1. The group lifecycle only — `New`, `Join`, `Leave`, `List`, `GET /{id}` — with no playback state at all. That alone makes the picker real and is testable against upstream with one client. 2. `PlayQueueManager` as a pure type, which is a queue with repeat and shuffle and no timing in it. 3. The state machine and the websocket command, which is where the second client becomes unavoidable. Deleting the `/SyncPlay/List` stub from `absent.rs` is part of step 1.

Claude added the

http

p3

labels 2026-09-12 08:50:59 +02:00

Claude commented 2026-09-12 12:27:32 +02:00

Author

Collaborator

Copy link

Steps 1 and 2 are done in d5019e7c. Step 3 is not, so this stays open.

What answers now

New, Join, Leave, List and GET /SyncPlay/{id}. SyncPlay is a use
case holding state beside the session registry — the third kind of state here
after the database and the sessions — and it composes no messages: it answers
Told { sessions, message } and the adapter turns each into a
SyncPlayGroupUpdate or a SyncPlayCommand. GroupJoined, UserJoined,
GroupLeft, UserLeft, NotInGroup and GroupDoesNotExist go out over the
socket in wire shapes captured from a live upstream socket. Upstream's three
access policies are types beside the others in auth.rs, and a session whose
last socket closes leaves its group — all five sessions_ended call sites go
through one function now, so a route cannot file the audit line and forget the
group.

PlayQueueManager is ported whole with 33 unit tests and wired to nothing,
which is where step 3 picks it up.

Measured

Two live servers side by side — upstream on 8140 over a cp -a snapshot copy,
ours on 8141 over a copy of the same state — 44 requests across three accounts
holding the three values of SyncPlayAccess. Statuses, bodies, participants,
access refusals, the 400 for a caller naming no account, the 404 for a
non-guid segment and the case-insensitive spellings all agree.

Nine mutations were applied and caught. A tenth was not: dropping the
api_key || !account guard in auth.rs changed nothing, because the row
lookup below it already covered every case. The guard was dead and went rather
than earning a test for a branch that cannot differ.

What a user gets, stated plainly

A group can be entered and playback in it does not sync. The seventeen
playback routes are unwritten, so Ping, Ready, Buffering, SetNewQueue,
Pause, Unpause, Seek and Stop all answer 405 — the web client calls
every one of them once it is in a group. That is a knowing trade: the picker
is real and diffed, and the timing half is the part that needs a second client
to test honestly.

Divergences, each with its reason

• A group is always Idle. No playback state exists, so it never leaves
  the state a new one starts in; upstream moves a group opened by a session
  that is already playing to Waiting.
• {"GroupName":null} and {"GroupId":"zz"} answer 400 as plain text where
  upstream writes a validation ProblemDetails — the body class json.rs
  already records as accepted.
• GET on New/Join/Leave answers 405 with Allow: GET, POST against
  upstream's Allow: POST: upstream constrains /SyncPlay/{id} to a guid and
  the route table cannot, so the id route's GET joins the union.
• The idle stop command is implemented — IdleGroupState.SessionJoined in
  full, one command to the joining session against the group's own last
  activity, with no timing reconciliation in it. Without it a client that joins
  while playing something keeps playing.

Not done

No click-through: the two-server differential was judged the stronger evidence.
The claim that the picker is real rests on the routes and socket messages
agreeing with upstream, not on having watched it in a browser.

Steps 1 and 2 are done in `d5019e7c`. Step 3 is not, so this stays open. ### What answers now `New`, `Join`, `Leave`, `List` and `GET /SyncPlay/{id}`. `SyncPlay` is a use case holding state beside the session registry — the third kind of state here after the database and the sessions — and it composes no messages: it answers `Told { sessions, message }` and the adapter turns each into a `SyncPlayGroupUpdate` or a `SyncPlayCommand`. `GroupJoined`, `UserJoined`, `GroupLeft`, `UserLeft`, `NotInGroup` and `GroupDoesNotExist` go out over the socket in wire shapes captured from a live upstream socket. Upstream's three access policies are types beside the others in `auth.rs`, and a session whose last socket closes leaves its group — all five `sessions_ended` call sites go through one function now, so a route cannot file the audit line and forget the group. `PlayQueueManager` is ported whole with 33 unit tests and **wired to nothing**, which is where step 3 picks it up. ### Measured Two live servers side by side — upstream on 8140 over a `cp -a` snapshot copy, ours on 8141 over a copy of the same state — 44 requests across three accounts holding the three values of `SyncPlayAccess`. Statuses, bodies, participants, access refusals, the `400` for a caller naming no account, the `404` for a non-guid segment and the case-insensitive spellings all agree. Nine mutations were applied and caught. A tenth was not: dropping the `api_key || !account` guard in `auth.rs` changed nothing, because the row lookup below it already covered every case. The guard was dead and went rather than earning a test for a branch that cannot differ. ### What a user gets, stated plainly **A group can be entered and playback in it does not sync.** The seventeen playback routes are unwritten, so `Ping`, `Ready`, `Buffering`, `SetNewQueue`, `Pause`, `Unpause`, `Seek` and `Stop` all answer `405` — the web client calls every one of them once it is in a group. That is a knowing trade: the picker is real and diffed, and the timing half is the part that needs a second client to test honestly. ### Divergences, each with its reason - **A group is always `Idle`.** No playback state exists, so it never leaves the state a new one starts in; upstream moves a group opened by a session that is already playing to `Waiting`. - `{"GroupName":null}` and `{"GroupId":"zz"}` answer `400` as plain text where upstream writes a validation `ProblemDetails` — the body class `json.rs` already records as accepted. - `GET` on `New`/`Join`/`Leave` answers `405` with `Allow: GET, POST` against upstream's `Allow: POST`: upstream constrains `/SyncPlay/{id}` to a guid and the route table cannot, so the id route's `GET` joins the union. - The idle stop command *is* implemented — `IdleGroupState.SessionJoined` in full, one command to the joining session against the group's own last activity, with no timing reconciliation in it. Without it a client that joins while playing something keeps playing. ### Not done No click-through: the two-server differential was judged the stronger evidence. The claim that the picker is real rests on the routes and socket messages agreeing with upstream, not on having watched it in a browser.

Claude commented 2026-09-12 15:06:58 +02:00

Author

Collaborator

Copy link

Step 3 is done in d689b605, so all 22 operations answer.

crates/ephyra-app/src/session/sync_play/group.rs is the state machine:
Idle/Waiting/Paused/Playing as one enum carrying the fields upstream
puts on its state objects, with prev threaded through exactly as upstream's
prevState is. PlayQueueManager — ported in step 2 and wired to nothing — is
wired now.

The group adopts what the session opening it is playing, which replaces the
stopgap landed earlier: the queue, the item, its runtime and the position come
over and the group enters Waiting with resume_playing set from whether that
session was paused. No stop command is sent. The playing: bool field and the
comment about leaving a playing session alone are gone.

The lock still holds nothing across an .await and the registry still composes
no messages: the two library facts the machine needs — item runtimes, and
whether every member may see a queue — are resolved by the adapter and handed
in. ARCHITECTURE.md records that edge.

Measured

Pinned upstream and ours side by side, two clients with two websockets each,
53 requests reaching all four states plus a separate adoption scenario. Dates
compared with a tolerance, and a command scheduled against an earlier group
activity compared by which step it points at rather than by its absolute value.

52 of 53 agree. The one that does not is SetShuffleMode: Shuffle: both
put the playing entry at index 0 and report the same multiset, the same mode
and the same position, and the rest of the order differs — which is what a
shuffle is. Adoption was diffed separately, playing and paused, and is
identical in both.

Fourteen mutations were applied and all fourteen caught. The thirteenth
exposed a gap in where a test lived rather than in the code: the access test
sat in ephyra-app, where the permission is a parameter, so breaking the
adapter's wiring changed nothing. It moved to the HTTP layer with a joiner the
parental rating blocks, and both adapter mutations are caught now.

What was not tested, stated rather than implied

Three or more clients, and genuinely divergent real clocks — both clients ran
on one host, so the clock offset was exercised by lying about When in a
request body rather than by two machines disagreeing. The arithmetic and the
branch selection are verified; that real drifting players converge is not.
Nothing played media either: every Ready and Buffering report was
synthetic, so whether the web client is happy with these commands is untested.

Divergences, each with its reason

• One clock reading per request, where upstream reads UtcNow several
  times while handling one. Measured effect: upstream reported a start
  position of 13 ticks — 1.3 µs between two reads — where ours reports 0. It
  is what makes the machine testable without a fake clock inside it.
• Upstream's !IsPlaying → AllGroup filter in the recovery branch is nearly
  dead: it needs a delay beyond twice the highest ping and within 500 ms at
  once, which happens only when a client reports a ping of zero. Reproduced
  faithfully, and a test reaches it.
• /SyncPlay/{id}'s Allow header stays wider than upstream's, which is step
  1's difference and already recorded there.

One thing this found and did not fix

A playing session's position stands still here between reports where upstream's
advances on a timer, which is what made the first adoption diff disagree.
That belongs to the session registry rather than to SyncPlay and is filed as
#201.

Step 3 is done in `d689b605`, so all 22 operations answer. `crates/ephyra-app/src/session/sync_play/group.rs` is the state machine: `Idle`/`Waiting`/`Paused`/`Playing` as one enum carrying the fields upstream puts on its state objects, with `prev` threaded through exactly as upstream's `prevState` is. `PlayQueueManager` — ported in step 2 and wired to nothing — is wired now. **The group adopts what the session opening it is playing**, which replaces the stopgap landed earlier: the queue, the item, its runtime and the position come over and the group enters `Waiting` with `resume_playing` set from whether that session was paused. No stop command is sent. The `playing: bool` field and the comment about leaving a playing session alone are gone. The lock still holds nothing across an `.await` and the registry still composes no messages: the two library facts the machine needs — item runtimes, and whether every member may see a queue — are resolved by the adapter and handed in. `ARCHITECTURE.md` records that edge. ### Measured Pinned upstream and ours side by side, **two clients with two websockets each**, 53 requests reaching all four states plus a separate adoption scenario. Dates compared with a tolerance, and a command scheduled against an earlier group activity compared by which step it points at rather than by its absolute value. **52 of 53 agree.** The one that does not is `SetShuffleMode: Shuffle`: both put the playing entry at index 0 and report the same multiset, the same mode and the same position, and the rest of the order differs — which is what a shuffle is. Adoption was diffed separately, playing and paused, and is identical in both. Fourteen mutations were applied and all fourteen caught. The thirteenth exposed a gap in where a test lived rather than in the code: the access test sat in `ephyra-app`, where the permission is a parameter, so breaking the adapter's wiring changed nothing. It moved to the HTTP layer with a joiner the parental rating blocks, and both adapter mutations are caught now. ### What was not tested, stated rather than implied Three or more clients, and genuinely divergent real clocks — both clients ran on one host, so the clock offset was exercised by lying about `When` in a request body rather than by two machines disagreeing. The arithmetic and the branch selection are verified; that real drifting players converge is not. Nothing played media either: every `Ready` and `Buffering` report was synthetic, so whether the web client is happy with these commands is untested. ### Divergences, each with its reason - **One clock reading per request**, where upstream reads `UtcNow` several times while handling one. Measured effect: upstream reported a start position of 13 ticks — 1.3 µs between two reads — where ours reports 0. It is what makes the machine testable without a fake clock inside it. - Upstream's `!IsPlaying → AllGroup` filter in the recovery branch is nearly dead: it needs a delay beyond twice the highest ping *and* within 500 ms at once, which happens only when a client reports a ping of zero. Reproduced faithfully, and a test reaches it. - `/SyncPlay/{id}`'s `Allow` header stays wider than upstream's, which is step 1's difference and already recorded there. ### One thing this found and did not fix A playing session's position stands still here between reports where upstream's advances on a timer, which is what made the first adoption diff disagree. That belongs to the session registry rather than to SyncPlay and is filed as #201.

Claude closed this issue 2026-09-12 15:06:59 +02:00

alex referenced this issue from a commit 2026-09-15 08:26:29 +02:00

syncplay: a group is opened, joined, listed and left

alex referenced this issue from a commit 2026-09-15 08:26:29 +02:00

syncplay: the state machine, and the seventeen routes that drive it

alex referenced this issue from a commit 2026-10-11 12:48:15 +02:00

syncplay: a group is opened, joined, listed and left

alex referenced this issue from a commit 2026-10-11 12:48:15 +02:00

syncplay: the state machine, and the seventeen routes that drive it

Sign in to join this conversation.

No Branch/Tag specified

Branches Tags

main

No results found.

Labels

Clear labels   

correctness

Diverges from upstream

  

encoding

Transcoding, ffmpeg, and the arg strings

  

http

Route behaviour

  

jobs

Scheduled tasks

  

operations

Installing a server and keeping one running

  

p1

Do this first

  

p2

After the p1s

  

p3

Real, and not urgent

  

performance

Speed or resource use

  

providers

Metadata from somewhere other than the file

  

scanner

The library scan

  

security

Who is allowed to see what

  

testing

Test coverage and harness

No labels

correctness

encoding

http

jobs

operations

p1

p2

p3

performance

providers

scanner

security

testing

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

alex/ephyra#198

No description provided.