mirror of
https://github.com/Shik3i/KoalaSync.git
synced 2026-07-26 12:08:15 +00:00
fix(host-control-mode): audit findings — host dedup demotion, episode-lobby freeze, +cleanups
Findings recovered from the multi-agent audit (verification phase was cut off by a
usage limit; verified inline against the code):
HIGH:
- Host was demoted on peerId dedup (fast reconnect / second tab): the dedup path
removes the old socket and the kicked socket's 'disconnect' both ran the
host-leave fallback before the same peerId re-joined → room silently unlocked on
every host network blip. Now skip the fallback while a join for that peerId is in
flight (peerJoinLocks). Regression test added. (A long real disconnect still
falls back — that's the deferred host-grace EC-10.)
- Episode auto-advance froze a gated guest: in host-only the guest's EPISODE_LOBBY
is dropped server-side, but the guest still self-paused (PAUSE_FOR_LOBBY) waiting
for readies that never came → 60s freeze. A host-only guest now skips creating a
lobby (the host drives episode sync).
LOW / cleanup:
- GET_HCM_STRINGS could return a raw key name ("HCM_DIALOG_TITLE") if the locale
dictionary failed to load (getMessage returns the key on miss) → omit it so
content keeps its English fallback.
- hcmReset now also clears the snap-back cooldown + buffering grace, so a stale
cooldown can't swallow the first snap-back after a room/host change.
- Popup play/pause labels reset on lock too (not just unlock), so a button can't
freeze on "Playing…" when host-only activates mid-click.
- Fix a stale comment (heartbeat now carries 'desynced').
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+24
-9
@@ -1236,9 +1236,10 @@ function handleServerEvent(event, data) {
|
||||
peer.mediaTitle = data.mediaTitle !== undefined ? data.mediaTitle : peer.mediaTitle;
|
||||
peer.volume = data.volume !== undefined ? data.volume : peer.volume;
|
||||
peer.muted = data.muted !== undefined ? data.muted : peer.muted;
|
||||
// Only update when present — the background-driven keepAlive
|
||||
// heartbeat omits 'desynced', and clobbering it to false there
|
||||
// would make the host's "Solo" badge flicker between heartbeats.
|
||||
// Only update when present. Our own heartbeats now carry
|
||||
// 'desynced', but other PEER_STATUS variants (server join
|
||||
// broadcast, future/old clients) omit it — and clobbering it
|
||||
// to false there would flicker the host's "Solo" badge.
|
||||
if (data.desynced !== undefined) peer.desynced = data.desynced === true;
|
||||
|
||||
const timeSinceReactive = peer.lastReactiveUpdate ? (Date.now() - peer.lastReactiveUpdate) : Infinity;
|
||||
@@ -1746,13 +1747,17 @@ async function handleAsyncMessage(message, sender, sendResponse) {
|
||||
const settings = await chrome.storage.local.get(['locale']);
|
||||
const lang = settings.locale || getSystemLanguage();
|
||||
await loadLocale(lang);
|
||||
// getMessage returns the key name itself if the dictionary failed to load.
|
||||
// Return undefined in that case so content keeps its English fallback rather
|
||||
// than rendering a raw key like "HCM_DIALOG_TITLE".
|
||||
const m = (k) => { const v = getMessage(k); return v === k ? undefined : v; };
|
||||
sendResponse({
|
||||
title: getMessage('HCM_DIALOG_TITLE'),
|
||||
body: getMessage('HCM_DIALOG_BODY'),
|
||||
stay: getMessage('HCM_DIALOG_STAY'),
|
||||
solo: getMessage('HCM_DIALOG_SOLO'),
|
||||
badge: getMessage('HCM_BADGE_SOLO'),
|
||||
resync: getMessage('HCM_BADGE_RESYNC')
|
||||
title: m('HCM_DIALOG_TITLE'),
|
||||
body: m('HCM_DIALOG_BODY'),
|
||||
stay: m('HCM_DIALOG_STAY'),
|
||||
solo: m('HCM_DIALOG_SOLO'),
|
||||
badge: m('HCM_BADGE_SOLO'),
|
||||
resync: m('HCM_BADGE_RESYNC')
|
||||
});
|
||||
} else if (message.type === 'HCM_DESYNC_STATE') {
|
||||
// content.js tells us whether the local user chose to watch on their own.
|
||||
@@ -2139,6 +2144,16 @@ async function handleAsyncMessage(message, sender, sendResponse) {
|
||||
return;
|
||||
}
|
||||
|
||||
// Host Control Mode: a gated guest must NOT initiate an episode lobby — the
|
||||
// server drops the guest's EPISODE_LOBBY, so the lobby would never complete
|
||||
// and the guest would self-pause (PAUSE_FOR_LOBBY) into a 60s freeze. In
|
||||
// host-only the host drives episode sync; the guest just follows / snaps back.
|
||||
if (controlMode === CONTROL_MODES.HOST_ONLY && !amHost()) {
|
||||
addLog(`Episode change ("${newTitle}") — host-only guest, not creating a lobby (host drives).`, 'info');
|
||||
sendResponse({ status: 'host_only_guest_skip' });
|
||||
return;
|
||||
}
|
||||
|
||||
// Variant A: alone in the room → no one to wait for. Skip the lobby
|
||||
// entirely so the next episode just plays through (no pause, no traffic).
|
||||
// Live peer check, so the moment someone joins the next transition syncs.
|
||||
|
||||
@@ -390,6 +390,8 @@
|
||||
const wasDesynced = hcmDesynced;
|
||||
hcmDesynced = false;
|
||||
hcmDeferredSnapPending = false;
|
||||
hcmSnapBackCooldownUntil = 0; // don't let a stale cooldown swallow the next snap-back
|
||||
hcmBufferingUntil = 0;
|
||||
hcmRemoveDialog();
|
||||
hcmRemoveBadge();
|
||||
// If we were desynced, notify background so it stops reporting us as
|
||||
|
||||
+7
-9
@@ -355,15 +355,13 @@ function setRemoteControlsLocked(locked) {
|
||||
btn.style.cursor = locked ? 'not-allowed' : '';
|
||||
btn.title = locked ? (getMessage('NOTICE_HOST_CONTROLS') || 'The host controls playback for everyone.') : '';
|
||||
});
|
||||
// When unlocking, also restore the default labels. The action handlers leave
|
||||
// the text in a transitional state ("Playing..." / "Pausing...") and the 2.5s
|
||||
// safety reset skips the refresh while we were guest-locked, so without this
|
||||
// the button can be re-enabled with stale text after the host disables
|
||||
// host-only (L-1).
|
||||
if (!locked) {
|
||||
if (elements.playBtn) elements.playBtn.textContent = getMessage('BTN_PLAY') || 'Play';
|
||||
if (elements.pauseBtn) elements.pauseBtn.textContent = getMessage('BTN_PAUSE') || 'Pause';
|
||||
}
|
||||
// Always restore the default labels. The action handlers leave the text in a
|
||||
// transitional state ("Playing..." / "Pausing...") and the 2.5s safety reset
|
||||
// skips the refresh while guest-locked — so reset on BOTH transitions: on lock
|
||||
// (host enabled host-only just as the guest clicked → don't freeze "Playing...")
|
||||
// and on unlock (L-1).
|
||||
if (elements.playBtn) elements.playBtn.textContent = getMessage('BTN_PLAY') || 'Play';
|
||||
if (elements.pauseBtn) elements.pauseBtn.textContent = getMessage('BTN_PAUSE') || 'Pause';
|
||||
}
|
||||
|
||||
if (elements.hostControlToggle) {
|
||||
|
||||
@@ -151,6 +151,25 @@ try {
|
||||
assert.ok(dSenderResync && dSenderResync.controlMode==='host-only',
|
||||
'debounced toggle re-syncs sender to actual state');
|
||||
close();
|
||||
resetConnectionRate();
|
||||
|
||||
// --- Host role survives peerId dedup (reconnect / second tab) ---
|
||||
const hdrid = 'dedup-host-'+Date.now();
|
||||
const hd1 = await c(), hd2 = await c();
|
||||
await j(hd1, hdrid, 'dhost'); await j(hd2, hdrid, 'dguest'); hd1._m.length = hd2._m.length = 0;
|
||||
s(hd1,'set_control_mode',{controlMode:'host-only'});
|
||||
await w(hd1,'control_mode'); await w(hd2,'control_mode');
|
||||
hd1._m.length = hd2._m.length = 0;
|
||||
// The host's peerId re-joins on a fresh socket → server dedupes the old socket.
|
||||
// This must NOT demote the host or reset the mode (a network blip / second tab).
|
||||
const hd3 = await c();
|
||||
s(hd3,'join_room',{roomId:hdrid,peerId:'dhost',protocolVersion:'1.0.0'});
|
||||
const hdrd = await a(hd3);
|
||||
assert.equal(hdrd[0],'room_data');
|
||||
assert.ok(hdrd[1].controlMode === 'host-only' && hdrd[1].hostPeerId === 'dhost',
|
||||
'host role + host-only mode survive peerId dedup (reconnect/second tab)');
|
||||
close();
|
||||
resetConnectionRate();
|
||||
|
||||
// --- Password room ---
|
||||
const prid = 'pw-'+Date.now();
|
||||
|
||||
+7
-1
@@ -228,7 +228,13 @@ function removePeerFromRoom(socketId, roomId, reason) {
|
||||
// socket), fall back to 'everyone' so the room never gets stuck locked, and
|
||||
// reassign host to the earliest remaining peer so the feature stays usable.
|
||||
// (v1: immediate fallback, no grace period — see host-control-mode docs.)
|
||||
if (!isPeerStillConnected && room.hostPeerId === peerId && room.peers.size > 0) {
|
||||
// Skip while a join for this peerId is in flight (peerJoinLocks holds it):
|
||||
// that's a reconnect / second tab where the same peerId is being re-added
|
||||
// right after — covers both the explicit 'dedupe' removal AND the
|
||||
// 'disconnect' the kicked old socket fires. Demoting there would silently
|
||||
// unlock the room on every host network blip.
|
||||
const peerRejoining = peerJoinLocks.has(peerId);
|
||||
if (!peerRejoining && !isPeerStillConnected && room.hostPeerId === peerId && room.peers.size > 0) {
|
||||
const nextPeerData = room.peerData.values().next().value;
|
||||
room.hostPeerId = nextPeerData ? nextPeerData.peerId : null;
|
||||
room.controlMode = CONTROL_MODES.EVERYONE;
|
||||
|
||||
Reference in New Issue
Block a user