Talon/voice-cat
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(protocol): deliver self-initiated state changes to the actor too

Browse Source 
A connected Windows client would randomly snap from its joined channel
back to Lobby. Root cause was a state-sync inconsistency, not a drop:
the server delivered self-initiated state changes (channel join/leave,
stream announce/stop) only as a private *Result to the actor and
broadcast the authoritative UserEvent::UPDATED to everyone else. The
core never applied the result to its SessionModel, so vc_list_users()
kept self in the old channel; the Windows HandleUserUpdated rebuilds
_currentChannelId from vc_list_users() on any user's UPDATED event, so
the next unrelated event surfaced the stale self-channel.

Fix, per the response-vs-broadcast contract now documented in
docs/protocol.md §6: the *Result is pure ack/correlation/actor-private
payload; the resulting state change is broadcast to every client
INCLUDING the actor, and clients apply it to their local model rather
than re-deriving own state from a *Result.

- server: join/leave/stream announce+stop broadcast with exclude=0
- server: text fan-out includes the sender (channel + private echo)
- core: response handlers no longer mutate session_model_
- windows: drop optimistic text echo; render own message via the relay
- docs/protocol.md §6: document the response-vs-broadcast contract

Registry-level admin broadcasts (move/mute/kick/channel CRUD) already
used exclude=0 and were correct. ctest build/m1-dev 18/18 green;
VoiceCat.App builds 0 warnings.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Talon
2026-06-17 20:48:50 +02:00
parent 9b321d0d4f
commit 118ca5129f
6 changed files with 68 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
PROGRESS.md
View File

@@ -24,6 +24,28 @@ up instantly. Newest status at the top.
channel CRUD with full per-channel Opus audio config, user moderation (kick/ban/move/server
mute/server deafen/set permissions), and server account management. `dotnet test` of the
Windows solution passes. Still to do: DRED/audio-quality polish.
- **Done:** **Fixed "randomly bumped to Lobby" in the Windows client — actors were excluded
from their own state-change broadcasts** (2026-06-17, reported live: a connected client would
intermittently snap from its joined channel back to Lobby in the UI). Root cause was a design
inconsistency, not a disconnect: the server delivered self-initiated state changes (channel
join/leave, stream announce/stop) only as a private `*Result` to the actor and broadcast the
authoritative `UserEvent::UPDATED` to *everyone else*. The core never applied the join result
to its `SessionModel`, so `vc_list_users()` kept self in the old channel; the Windows
`HandleUserUpdated` rebuilds `_currentChannelId` from `vc_list_users()` on **any** user's
UPDATED event, so the next unrelated event (someone joining, announcing/stopping a stream,
being muted) surfaced the stale self-channel → "bumped to Lobby." Flaky because it depended on
other users' activity. **Fix (broad, per the actor-sees-own-changes principle):** the server
now broadcasts these `UserEvent::UPDATED`s to **all** clients including the actor
(`server/src/conn_session.cpp`, `broadcast(…, /*exclude*/ 0)`), and text fan-out now includes
the sender (`resolve_text_targets`), so every client converges via one authoritative path.
The `*Result` is now purely ack/correlation/actor-private payload; response handlers no longer
mutate the local model. Windows client drops its optimistic text echo (the relay comes back)
and renders the sender's own message via `HandleTextMessage`. Documented the
response-vs-broadcast contract in `docs/protocol.md` §6. Registry-level admin broadcasts
(move/mute/kick/channel CRUD) already used `exclude=0` and were correct. `ctest --test-dir
build/m1-dev` — **18/18 green** (PowerShell); Windows `VoiceCat.App` builds 0 warnings.
**Latent, not fixed:** the client never sends the `Ping` keepalive that `docs/protocol.md` §7
describes (only the server answers pings) — unrelated to this bug, noted for later.
- **Done:** **Fixed a *second* silent-playback bug — the playout clock free-ran and drifted off
the stream** (2026-06-17, reported live: both `vccli` and the Windows client showed `talking=1/0`
correctly on VAD/PTT, mic + screen-share were recognized by peers, but nothing was audible).

23
clients/windows/VoiceCat.App/Forms/MainForm.cs
View File

@@ -320,8 +320,9 @@ public partial class MainForm : Form
if (ev.Result == VcResult.Ok)
{
_currentChannelId = ev.ChannelId;
// Server doesn't echo UserJoined/UserUpdated back to the mover — patch our
// own entry in _users so RefreshUserList shows us in the new channel.
// The authoritative UserEvent::UPDATED broadcast also reflects this move, but it
// may arrive after this result — patch our own entry now for instant, flicker-free
// feedback. The later UPDATED is idempotent (sets the same channel).
if (_users.TryGetValue(_selfUserId, out var self))
_users[_selfUserId] = self with { ChannelId = ev.ChannelId };
RefreshChannelTree();
@@ -351,7 +352,12 @@ public partial class MainForm : Form
.LocalDateTime.ToString("HH:mm")
: DateTime.Now.ToString("HH:mm");
string sender = GetNickname(ev.UserId);
string prefix = ev.TextScope == VcTextScope.Private ? "(private) " : "";
// For a private message, ev.ChannelId carries target_id (the recipient user_id).
// When the server relays our own private message back to us, label it with the
// recipient; an incoming private message just shows "(private)".
string prefix = ev.TextScope == VcTextScope.Private
? (ev.UserId == _selfUserId ? $"(private to {GetNickname(ev.ChannelId)}) " : "(private) ")
: "";
rtbChat.AppendText($"[{time}] {prefix}{sender}: {ev.Text ?? ""}\n");
rtbChat.ScrollToCaret();
if (ev.TextScope == VcTextScope.Private && ev.UserId != _selfUserId)
@@ -882,14 +888,9 @@ public partial class MainForm : Form
_client.SendText(scope, targetId, msg);
txtCompose.Clear();
// Server excludes sender from channel fan-out — echo our own message locally.
string time = DateTime.Now.ToString("HH:mm");
string prefix = scope == VcTextScope.Private
? $"(private to {GetNickname(targetId)}) "
: "";
rtbChat.AppendText($"[{time}] {prefix}{_nickname}: {msg}\n");
rtbChat.ScrollToCaret();
// No optimistic echo: the server relays our own message back to us (it no longer
// excludes the sender), so HandleTextMessage renders it through the same path as
// every other message. Echoing here too would double it.
}
// ── Utility ───────────────────────────────────────────────────────────────

3
core/src/core/client.cpp
View File

@@ -532,6 +532,9 @@ void vc_client::handle_channel_event(const voicecat::v1::ChannelEvent& ce) {
}
void vc_client::handle_join_channel_result(const voicecat::v1::JoinChannelResult& msg) {
// This response only acks the request + carries our private audio config. The
// authoritative channel change reaches us as a UserEvent::UPDATED broadcast (the server
// no longer excludes the mover), which keeps session_model_ in sync via apply_user_event.
vc_event ev{};
ev.type = VC_EVENT_JOIN_RESULT;
ev.result = msg.ok() ? VC_OK : VC_ERR_PROTOCOL;

8
docs/protocol.md
View File

@@ -274,6 +274,14 @@ message TextMessage {
user, kick, ban, server-mute, set-permission, create/reset/delete account). Error `code`s
are an enumerated, stable list.
- Fatal conditions send **`Disconnect { code; reason }`** then close the TLS connection.
- **The response is for the request; the broadcast is for the state.** A `*Result` only
acknowledges the actor's request (correlation via `request_id`, error text, and any
actor-private payload — e.g. the channel `AudioConfig` in `JoinChannelResult`). The
resulting *state change* is delivered to **every** connected client **including the actor**
via the normal `UserEvent` / `ChannelEvent` / relayed `TextMessage` path. Clients apply
those events to their local model and never re-derive their own state from a `*Result`
(doing so drifts: the actor would miss its own change and a later event for another user
would surface the stale value).
## 7. Keepalive & timeouts

13
server/src/conn_session.cpp
View File

@@ -418,13 +418,16 @@ void ConnSession::handle_join_channel(uint64_t req_id,
res->set_channel_id(msg.channel_id());
*res->mutable_audio() = ch->audio();
// Broadcast that this user changed channel.
// Broadcast that this user changed channel — to everyone INCLUDING the mover.
// The JoinChannelResult only acks the request; this UserEvent is the authoritative
// state change every client (mover included) applies to its local model. Excluding
// the mover here is what made it read a stale self-channel. See docs/protocol.md §6.
if (auto updated_user = registry_->user_snapshot_user(uid)) {
auto bcast = make_env();
auto* ue = bcast.mutable_user_event();
ue->set_kind(voicecat::v1::UserEvent::UPDATED);
*ue->mutable_user() = *updated_user;
registry_->broadcast(bcast, session_id_);
registry_->broadcast(bcast, /*exclude*/ 0);
}
}
send_envelope(env);
@@ -439,7 +442,7 @@ void ConnSession::handle_leave_channel() {
auto* ue = bcast.mutable_user_event();
ue->set_kind(voicecat::v1::UserEvent::UPDATED);
*ue->mutable_user() = *updated_user;
registry_->broadcast(bcast, session_id_);
registry_->broadcast(bcast, /*exclude*/ 0); // include the leaver — same as join
}
}
@@ -544,7 +547,7 @@ void ConnSession::handle_stream_announce(uint64_t req_id,
auto* ue = bcast.mutable_user_event();
ue->set_kind(voicecat::v1::UserEvent::UPDATED);
*ue->mutable_user() = *updated;
registry_->broadcast(bcast, session_id_);
registry_->broadcast(bcast, /*exclude*/ 0); // include announcer; result only acks
}
}
@@ -560,7 +563,7 @@ void ConnSession::handle_stream_stop(const voicecat::v1::StreamStop& msg) {
auto* ue = bcast.mutable_user_event();
ue->set_kind(voicecat::v1::UserEvent::UPDATED);
*ue->mutable_user() = *updated;
registry_->broadcast(bcast, session_id_);
registry_->broadcast(bcast, /*exclude*/ 0); // include the stopper
}
}

27
server/src/session_registry.cpp
View File

@@ -163,24 +163,27 @@ std::vector<std::shared_ptr<ConnSession>> SessionRegistry::resolve_text_targets(
std::shared_lock lk(mu_);
std::vector<std::shared_ptr<ConnSession>> targets;
auto add_session = [&](uint64_t sid) {
auto sit = sessions_.find(sid);
if (sit == sessions_.end()) return;
if (auto sess = sit->second.lock()) targets.push_back(sess);
};
if (scope == voicecat::v1::TEXT_CHANNEL) {
// Find channel_id of the target, then all users in that channel
// Fan out to everyone in the target channel, INCLUDING the sender, so the sender's
// own client renders the message through the same authoritative relay everyone else
// gets (no optimistic local echo). See docs/protocol.md §6.
for (auto& [uid, entry] : users_) {
if (entry.proto.channel_id() != target_id) continue;
if (entry.session_id == sender_session_id) continue;
auto sit = sessions_.find(entry.session_id);
if (sit == sessions_.end()) continue;
if (auto sess = sit->second.lock()) targets.push_back(sess);
add_session(entry.session_id);
}
} else if (scope == voicecat::v1::TEXT_PRIVATE) {
// target_id is user_id
// target_id is the recipient user_id: deliver to the recipient and echo to the
// sender (skip the echo if they messaged themselves, to avoid a duplicate).
auto user_it = users_.find(target_id);
if (user_it != users_.end()) {
auto sit = sessions_.find(user_it->second.session_id);
if (sit != sessions_.end()) {
if (auto sess = sit->second.lock()) targets.push_back(sess);
}
}
if (user_it != users_.end()) add_session(user_it->second.session_id);
if (user_it == users_.end() || user_it->second.session_id != sender_session_id)
add_session(sender_session_id);
}
return targets;
}