diff options
Diffstat (limited to 'packages')
17 files changed, 633 insertions, 15 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js index 38314e7..92dcb28 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js @@ -20,7 +20,9 @@ import { GroupPage } from './group-page.js'; import { SearchPage, ConnectionPool } from './search-page.js'; import { MusicPlayerBar } from './music-player.js'; import { NameModal } from './playlist-menu.js'; -import { saveQueueAsPlaylist } from './playlists.js'; +import { + saveQueueAsPlaylist, setPlaylistTransport, pullOnce, flushPush, +} from './playlists.js'; import { IndexingDock } from './index-dock.js'; import { SettingsPage } from './settings-page.js'; import { ProfilePage } from './profile-page.js'; @@ -752,6 +754,63 @@ function App() { const handleStopMusic = useCallback(() => setMusicQueue(null), []); + // ── Playlists reach a node through here ──────────────────────────────── + // + // `playlists.js` pushes on every write and needs a transport to push + // through; the shell is what owns connections. Preferring the group already + // open costs nothing, and dialing when there is none is a change of position + // that is worth stating: §7 refused dialing *at sign-in for a feature nobody + // had asked to use*. Somebody who has just made a playlist has asked, and one + // dial to save their work is cheaper than losing it. + useEffect(() => { + if (!user) { setPlaylistTransport(null); return undefined; } + setPlaylistTransport(async () => { + const gt = groupTransportRef.current; + const live = gt && gt.transportRef && gt.transportRef.current; + if (live && live.connected) return live; + // Bounded: an account with twenty groups, all offline, must not spend + // twenty ten-second timeouts on a background push. + for (const g of groups.slice(0, 3)) { + try { + const conn = await getMusicConnection(g.id); + if (conn && conn.transport && conn.transport.connected) return conn.transport; + } catch { /* try the next one */ } + } + return null; + }); + // A pending push has a second and a half to wait, and closing a laptop + // inside that window is not rare. `pagehide` covers a tab closing and a + // navigation; `visibilitychange` covers a phone being backgrounded, which + // on iOS is the only one of the two that reliably fires at all. + const flush = () => { flushPush().catch(() => {}); }; + const onHidden = () => { if (document.visibilityState === 'hidden') flush(); }; + window.addEventListener('pagehide', flush); + document.addEventListener('visibilitychange', onHidden); + + return () => { + window.removeEventListener('pagehide', flush); + document.removeEventListener('visibilitychange', onHidden); + setPlaylistTransport(null); + }; + }, [user, groups, getMusicConnection]); + + // Reconcile once per sign-in, whatever this browser already holds. + // + // Nothing went looking until a group's Music tab happened to be opened, which + // is not where anybody looks for a playlist — and bounding this to an empty + // device, as the first version did, left out the ordinary case: a phone that + // has nine playlists and is missing the tenth. + // + // `groups.length` moves as the sidebar fills, so the ref is what makes "once" + // mean once rather than once per group that arrives. + const pulledForRef = useRef(null); + useEffect(() => { + if (!user || !groups.length) return; + if (pulledForRef.current === user.userId) return; + pulledForRef.current = user.userId; + pullOnce(user.userId).catch(() => {}); + }, [user, groups.length]); + // Saving the queue is a shell-level action because the queue is: the player // bar outlives every page, and the account it belongs to is here. const [saveQueue, setSaveQueue] = useState(null); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js index 9dec910..4cfa0a7 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js @@ -335,6 +335,7 @@ export default { 'playlists.deleted': '„{name}“ gelöscht', 'playlists.synced': 'Playlists synchronisiert', 'playlists.sync_failed': 'Synchronisierung fehlgeschlagen', + 'playlists.err_no_key': 'Melde dich einmal ab und mit deinem Passwort wieder an, damit Playlists zwischen Geräten synchronisiert werden.', 'playlists.track_removed': 'Titel entfernt', 'playlists.save_queue': 'Warteschlange als Playlist speichern…', 'playlists.cancel': 'Abbrechen', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js index 37c7c1b..1616eb5 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js @@ -333,6 +333,7 @@ export default { 'playlists.deleted': '"{name}" deleted', 'playlists.synced': 'Playlists synced', 'playlists.sync_failed': 'Could not sync playlists', + 'playlists.err_no_key': 'Sign out and sign in with your password once to sync playlists across devices.', 'playlists.track_removed': 'Track removed', 'playlists.save_queue': 'Save the queue as a playlist…', 'playlists.cancel': 'Cancel', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js index 3edc78a..ff74857 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js @@ -333,6 +333,7 @@ export default { 'playlists.deleted': '«{name}» eliminada', 'playlists.synced': 'Listas sincronizadas', 'playlists.sync_failed': 'No se pudieron sincronizar', + 'playlists.err_no_key': 'Cierra sesión y vuelve a entrar con tu contraseña una vez para sincronizar las listas entre dispositivos.', 'playlists.track_removed': 'Pista quitada', 'playlists.save_queue': 'Guardar la cola como lista…', 'playlists.cancel': 'Cancelar', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js index c7e1ed3..52af974 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js @@ -334,6 +334,7 @@ export default { 'playlists.deleted': '« {name} » supprimée', 'playlists.synced': 'Playlists synchronisées', 'playlists.sync_failed': 'Synchronisation impossible', + 'playlists.err_no_key': 'Déconnecte-toi puis reconnecte-toi avec ton mot de passe une fois, pour synchroniser les playlists entre appareils.', 'playlists.track_removed': 'Morceau retiré', 'playlists.save_queue': 'Enregistrer la file comme playlist…', 'playlists.cancel': 'Annuler', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js index bfeb20d..75b420d 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js @@ -334,6 +334,7 @@ export default { 'playlists.deleted': '«{name}» eliminata', 'playlists.synced': 'Playlist sincronizzate', 'playlists.sync_failed': 'Sincronizzazione non riuscita', + 'playlists.err_no_key': 'Esci e rientra con la tua password una volta per sincronizzare le playlist tra dispositivi.', 'playlists.track_removed': 'Brano rimosso', 'playlists.save_queue': 'Salva la coda come playlist…', 'playlists.cancel': 'Annulla', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js index caf729a..c87d795 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js @@ -331,6 +331,7 @@ export default { 'playlists.deleted': '「{name}」を削除しました', 'playlists.synced': 'プレイリストを同期しました', 'playlists.sync_failed': '同期できませんでした', + 'playlists.err_no_key': 'プレイリストを端末間で同期するには、一度サインアウトしてパスワードでサインインしてください。', 'playlists.track_removed': '曲を削除しました', 'playlists.save_queue': 'キューをプレイリストとして保存…', 'playlists.cancel': 'キャンセル', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js index b9e719e..4fa3d01 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js @@ -335,6 +335,7 @@ export default { 'playlists.deleted': '“{name}” verwijderd', 'playlists.synced': 'Afspeellijsten gesynchroniseerd', 'playlists.sync_failed': 'Synchroniseren mislukt', + 'playlists.err_no_key': 'Meld je één keer af en met je wachtwoord weer aan om afspeellijsten tussen apparaten te synchroniseren.', 'playlists.track_removed': 'Nummer verwijderd', 'playlists.save_queue': 'Wachtrij opslaan als afspeellijst…', 'playlists.cancel': 'Annuleren', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js index 7f87537..d0fd387 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js @@ -344,6 +344,7 @@ export default { 'playlists.deleted': 'Usunięto „{name}”', 'playlists.synced': 'Playlisty zsynchronizowane', 'playlists.sync_failed': 'Nie udało się zsynchronizować', + 'playlists.err_no_key': 'Wyloguj się i zaloguj raz hasłem, aby synchronizować playlisty między urządzeniami.', 'playlists.track_removed': 'Utwór usunięty', 'playlists.save_queue': 'Zapisz kolejkę jako playlistę…', 'playlists.cancel': 'Anuluj', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js index 961b353..94c56a6 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js @@ -335,6 +335,7 @@ export default { 'playlists.deleted': '"{name}" excluída', 'playlists.synced': 'Playlists sincronizadas', 'playlists.sync_failed': 'Não foi possível sincronizar', + 'playlists.err_no_key': 'Saia e entre com sua senha uma vez para sincronizar as playlists entre dispositivos.', 'playlists.track_removed': 'Faixa removida', 'playlists.save_queue': 'Salvar a fila como playlist…', 'playlists.cancel': 'Cancelar', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js index 893b2eb..bc77d5f 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js @@ -328,6 +328,7 @@ export default { 'playlists.deleted': '已删除“{name}”', 'playlists.synced': '播放列表已同步', 'playlists.sync_failed': '无法同步', + 'playlists.err_no_key': '请退出并用密码重新登录一次，以便在设备间同步播放列表。', 'playlists.track_removed': '已移除曲目', 'playlists.save_queue': '将队列保存为播放列表…', 'playlists.cancel': '取消', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/playlist-menu.js b/packages/meshbay-hub/src/meshbay_hub/static/playlist-menu.js index 8d85b68..e2fb3f9 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/playlist-menu.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/playlist-menu.js @@ -183,7 +183,25 @@ function PlaylistMenuButton({ userId, lists, reload, onPlayQueue, onSync, cached onSelect: async () => { const r = await onSync(); await reload(); - say(r && r.ok ? t('playlists.synced') : t('playlists.sync_failed')); + // The reason, not just "it failed": a sync that quietly does nothing + // is the one failure mode nobody can act on, and the reason is what + // separates an offline node from a wedged one. + // On success, say how much moved; on failure, say why. Either way + // the reader learns something they can act on — the one thing this + // never did. + const last = P.lastSync(); + const why = (r && r.reason) || last.reason || '?'; + // `no_key` is not a fault the reader can do anything about unless it + // is spelled out. This browser signed in with its remembered device + // key, so the only bundle key it has is one persisted before the + // playlist subkey existed — and an AES handle is non-extractable, so + // there is nothing to derive it from. One sign-in with the passphrase + // fixes it for good; a code on screen does not say that. + say(r && r.ok + ? `${t('playlists.synced')} (${r.pushed || 0}↑ ${r.pulled || 0}↓)` + : (why === 'no_key' + ? t('playlists.err_no_key') + : `${t('playlists.sync_failed')}: ${why}`)); }, }, ]); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/playlists.js b/packages/meshbay-hub/src/meshbay_hub/static/playlists.js index 11525eb..5aa501a 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/playlists.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/playlists.js @@ -94,6 +94,19 @@ function forgetPlaylistKey() { _state = null; } +/** + * Drop this account's local copy — what a browser that has never seen it looks + * like. Exported for the tests, which have no other way to be a fresh device. + */ +async function forgetLocal(userId) { + const st = await ensureLoaded(userId); + for (const id of Object.keys(st.manifest.playlists || {})) { + await _idbPut(userId, bodyKind(id), null); + } + await _idbPut(userId, MANIFEST_KIND, null); + _state = null; +} + // ── local storage ──────────────────────────────────────────────────────────── // // Plaintext in IndexedDB, exactly as the cached group indexes already are @@ -135,6 +148,166 @@ async function _idbPut(userId, kind, value) { } catch { /* a private window, or storage refused — the session still works */ } } +// ── pushing, which is this module's job and not the caller's ───────────────── +// +// The first version left it to the call sites: `music-app.js` synced after +// adding tracks, and nothing else did. Creating a playlist, deleting one, +// removing a track and saving the queue all wrote to IndexedDB and stopped +// there — so a second device saw nothing, which is the one thing this feature +// exists for. Found by reading the node's own audit log: two events, ever. +// +// Scattering `syncNow()` across six call sites would be the same mistake with +// five more places to forget it. The store knows when it changed; it pushes. + +let _transport = null; // () => Promise<transport|null>, registered by the shell +let _pushTimer = null; +let _retryTimer = null; +let _pushFor = null; + +// Coalescing window, and the single retry after a push that did not land. +// `let` rather than `const` so the tests can shorten them: a retry nobody can +// wait for is a retry nobody has checked, and this feature has already shipped +// one path that no test ever called. +let PUSH_DEBOUNCE_MS = 1500; +let PUSH_RETRY_MS = 20000; + +function setPushTimings({ debounceMs, retryMs }) { + if (debounceMs != null) PUSH_DEBOUNCE_MS = debounceMs; + if (retryMs != null) PUSH_RETRY_MS = retryMs; +} +// What the last push actually did. A background push that fails reports to +// nobody by construction — which is how playlists stopped leaving a browser for +// two hours while the interface showed them perfectly. `lastSync()` is what +// "Sync now" and a support question can both read. +let _lastSync = { ok: null, reason: 'never run', at: null }; + +function lastSync() { + return { ..._lastSync }; +} + +/** + * How the store reaches a node. The shell registers this because the shell owns + * connections; `null` unregisters it at sign-out. + */ +function setPlaylistTransport(fn) { + _transport = fn; +} + +/** + * Push after a write, coalesced. + * + * Adding an album is one write and should be one push, and so should a burst of + * stars. A second and a half is long enough to swallow a burst and short enough + * that closing the laptop afterwards is not how the edit is lost. + * + * Failure is silent by design: the local copy is the authority (§6.4), and the + * next sync — the next group opened, or "Sync now" — carries it. What must not + * happen is the write failing because the push did. + */ +function _schedulePush(userId, isRetry) { + if (!_transport) return; + _pushFor = userId; + clearTimeout(_pushTimer); + _pushTimer = setTimeout(async () => { + _pushTimer = null; + let result; + try { + const tr = await _transport(); + result = tr ? await syncWith(tr, _pushFor) + : { ok: false, reason: 'no reachable node' }; + } catch (err) { + // The write itself already succeeded locally; this is only the push. + result = { ok: false, reason: err.message || String(err) }; + } + _note(result); + + // One retry, and exactly one. A node that is off tends to stay off, and a + // push that keeps looping spends a phone's battery on a node that is not + // coming back — while the local copy is already correct and the next + // mutation, or the next Music tab opened, carries it anyway. `no_key` is + // never retried: it cannot fix itself without the reader doing something. + if (!result.ok && !isRetry && result.reason !== 'no_key') { + clearTimeout(_retryTimer); + _retryTimer = setTimeout(() => _schedulePush(_pushFor, true), PUSH_RETRY_MS); + } + }, isRetry ? 0 : PUSH_DEBOUNCE_MS); +} + +/** + * A device that has nothing of its own fetches, once. + * + * §7 said "on sign-in, nothing", on the grounds that the local copy is + * authoritative and complete. That is true of a device that has been used + * before and **false of a new one** — which is the case this whole feature + * exists for. Signing in on a phone and finding the playlists absent is exactly + * what the design promised would not happen, and exactly what it did. + * + * It runs **whatever this browser already holds**, which is the correction to + * the correction: bounding it to an empty device left the ordinary case out. + * A phone that already has nine playlists and is missing the tenth would not + * have gone looking either, and would have waited for a group's Music tab to + * be opened — which is not where anybody looks for a playlist. + * + * Once per sign-in, and it is one `list` plus one `fetch`: the manifest is a + * few kilobytes, and bodies are still pulled only when they are behind. + */ +async function pullOnce(userId) { + if (!_transport) return { ok: false, reason: 'no_transport' }; + try { + const tr = await _transport(); + if (!tr) return { ok: false, reason: 'offline' }; + return await syncWith(tr, userId); + } catch (err) { + return { ok: false, reason: err.message || 'failed' }; + } +} + +/** + * Record what a push did, and say so once in the console when it did not work. + * + * Silence was the actual defect here, not any single bug: every path that could + * fail swallowed its reason, so the interface, the console and the node's log + * all agreed that nothing was wrong. + */ +function _note(result) { + _lastSync = { + ok: !!(result && result.ok), + reason: (result && result.reason) || null, + unreadable: !!(result && result.unreadable), + pushed: (result && result.pushed) || 0, + at: new Date().toISOString(), + }; + if (!_lastSync.ok) { + console.warn('[MeshBay] playlists not synced:', _lastSync.reason); + } + return result; +} + +/** + * Send a pending push now, because the page is about to go away. + * + * The shell calls this on `pagehide` and when the tab goes hidden — closing a + * laptop within the coalescing window is not a rare thing to do, and it is the + * one moment when waiting another second and a half means waiting until the + * next time Music is opened. + * + * Best-effort by nature: a browser tearing a page down owes an `await` + * nothing. It costs a message when it works and nothing when it does not. + */ +async function flushPush() { + if (!_pushTimer && !_retryTimer) return; + clearTimeout(_pushTimer); + clearTimeout(_retryTimer); + _pushTimer = null; + _retryTimer = null; + try { + const tr = await _transport(); + if (tr) _note(await syncWith(tr, _pushFor)); + } catch (err) { + _note({ ok: false, reason: err.message || String(err) }); + } +} + // ── in-memory state ────────────────────────────────────────────────────────── let _state = null; // { userId, manifest, bodies: Map(id -> body) } @@ -246,6 +419,7 @@ async function createPlaylist(userId, name) { _entry(st, id, wanted); await _saveManifest(st); await _saveBody(st, emptyBody(id)); + _schedulePush(userId); return id; } @@ -257,6 +431,7 @@ async function renamePlaylist(userId, id, name) { e.device = deviceId(); e.updated_at = Math.floor(Date.now() / 1000); await _saveManifest(st); + _schedulePush(userId); } /** @@ -278,6 +453,7 @@ async function deletePlaylist(userId, id) { await _saveManifest(st); st.bodies.delete(id); await _idbPut(st.userId, bodyKind(id), null); + _schedulePush(userId); } /** @@ -308,6 +484,7 @@ async function addTracks(userId, id, entries, groupId, name) { e.count = next.tracks.length; e.updated_at = Math.floor(Date.now() / 1000); await _saveManifest(st); + _schedulePush(userId); return incoming.length; } @@ -327,6 +504,7 @@ async function removeTrackAt(userId, id, at) { e.count = next.tracks.length; e.updated_at = Math.floor(Date.now() / 1000); await _saveManifest(st); + _schedulePush(userId); return true; } @@ -388,15 +566,31 @@ async function syncWith(transport, userId) { let theirs = null; if (have.has(MANIFEST_KIND)) { + let row = null; try { - const row = await transport.fetchUserBlob(MANIFEST_KIND); - if (row && row.blob_enc) { - theirs = await open(row.blob_enc, MANIFEST_KIND, userId, key); - } + row = await transport.fetchUserBlob(MANIFEST_KIND); } catch (err) { + // The transport failed. Nothing to reconcile against and nothing to + // overwrite, so leave this node alone. result.reason = err.message || 'fetch failed'; return result; } + if (row && row.blob_enc) { + try { + theirs = await open(row.blob_enc, MANIFEST_KIND, userId, key); + } catch (err) { + // A blob this account cannot open with this passphrase is not an older + // copy of anything — it is unreadable, and treating it as a *fetch* + // failure wedged the sync permanently: the first write landed because + // the node was empty, and every sync after it returned here without + // ever pushing again. The client is the authority (§6.4), so an + // unreadable remote copy is an absence, and gets overwritten. + console.warn('[MeshBay] playlist manifest on this node will not open:', + err.message, '— overwriting it with the local copy'); + result.unreadable = true; + theirs = null; + } + } } const merged = mergeManifests(st.manifest, theirs || emptyManifest()); @@ -433,7 +627,12 @@ async function syncWith(transport, userId) { result.pulled += 1; } } - } catch { /* one body failing must not stop the rest */ } + } catch { + // Same reasoning as the manifest above, and already the right shape: + // one body that will not open must not stop the rest, and the local + // copy is pushed over it on the next write to that playlist. + result.unreadable = true; + } } else if (localRev > nodeRev && localRev > 0) { try { await transport.storeUserBlob( @@ -476,6 +675,8 @@ async function syncWith(transport, userId) { export { FAVORITES_ID, + setPlaylistTransport, flushPush, pullOnce, forgetLocal, lastSync, + setPushTimings, deviceId, playlistKey, forgetPlaylistKey, ensureLoaded, listPlaylists, getPlaylistTracks, createPlaylist, renamePlaylist, deletePlaylist, diff --git a/packages/meshbay-hub/tests/harness/playlist_store_probe.py b/packages/meshbay-hub/tests/harness/playlist_store_probe.py index 8dfa4b1..f7573b7 100755 --- a/packages/meshbay-hub/tests/harness/playlist_store_probe.py +++ b/packages/meshbay-hub/tests/harness/playlist_store_probe.py @@ -203,7 +203,167 @@ function fakeNode() { stillThere: holder.rows.has(bodyKind(eveningId)), }); + // ── every mutation reaches a node, not just the one that used to ────── + // + // The defect this exists for: `syncWith` was called from exactly one place + // in the interface, so creating a playlist, deleting one, removing a track + // and saving the queue all wrote to IndexedDB and stopped there. A second + // device saw nothing — which is the one thing playlists are for. Found in + // the node's own audit log: two events, ever. + const auto = fakeNode(); + P.setPlaylistTransport(async () => auto); + const seen = () => auto.stored.map((s) => s.kind).sort(); + const wait = () => new Promise((r) => setTimeout(r, 2300)); // > the debounce + + const p1 = await P.createPlaylist(USER, 'Depuis le menu'); + await wait(); + steps.push({ step: 'create pushes', kinds: seen() }); + + auto.stored.length = 0; + await P.addTracks(USER, p1, [track(21)], 'g1'); + await wait(); + steps.push({ step: 'add pushes', kinds: seen() }); + + auto.stored.length = 0; + await P.removeTrackAt(USER, p1, 0); + await wait(); + steps.push({ step: 'remove pushes', kinds: seen() }); + + auto.stored.length = 0; + const p2 = await P.saveQueueAsPlaylist(USER, 'Depuis la file', [track(31), track(32)]); + await wait(); + steps.push({ step: 'save-queue pushes', kinds: seen() }); + + auto.stored.length = 0; + await P.deletePlaylist(USER, p2); + await wait(); + steps.push({ step: 'delete pushes', kinds: seen(), + bodyGone: !auto.rows.has(bodyKind(p2)) }); + + // A burst is one push, not one per write. + auto.stored.length = 0; + for (let i = 0; i < 5; i++) await P.addTracks(USER, p1, [track(40 + i)], 'g1'); + await wait(); + steps.push({ step: 'a burst is coalesced', writes: auto.stored.length }); + + // ── a push that does not land is retried, once ──────────────────────── + // + // Shortened from twenty seconds: a retry nobody can wait for is a retry + // nobody has checked, and this feature has already shipped one whole path + // that no test ever called. + P.setPushTimings({ debounceMs: 100, retryMs: 400 }); + let refuse = true; + const flaky = fakeNode(); + const flakyStore = flaky.storeUserBlob.bind(flaky); + flaky.storeUserBlob = async (...a) => { + if (refuse) throw new Error('Server busy, retry shortly'); + return flakyStore(...a); + }; + // Sync *passes*, not store calls: one pass writes a body per playlist plus + // the manifest, so counting writes says nothing about how many times the + // push was attempted — which is the whole question for "does it loop". + // Every pass begins with exactly one listing. + let passes = 0; + const flakyList = flaky.listUserBlobs.bind(flaky); + flaky.listUserBlobs = async (...a) => { passes++; return flakyList(...a); }; + P.setPlaylistTransport(async () => flaky); + + await P.addTracks(USER, p1, [track(50)], 'g1'); + await new Promise((r) => setTimeout(r, 250)); + const afterFirst = { stored: flaky.stored.length, last: P.lastSync() }; + refuse = false; // the node comes back + await new Promise((r) => setTimeout(r, 900)); + steps.push({ step: 'a failed push is retried once', + afterFirstTry: afterFirst.stored, + firstReason: afterFirst.last.reason, + afterRetry: flaky.stored.length, + ok: P.lastSync().ok }); + + // ── and only once ───────────────────────────────────────────────────── + refuse = true; + flaky.stored.length = 0; + passes = 0; + await P.addTracks(USER, p1, [track(51)], 'g1'); + await new Promise((r) => setTimeout(r, 2000)); // room for four retries + steps.push({ step: 'a retry does not loop', + stored: flaky.stored.length, passes }); + + // ── closing the page sends what is pending ──────────────────────────── + refuse = false; + P.setPushTimings({ debounceMs: 60000, retryMs: 60000 }); // never on its own + flaky.stored.length = 0; + await P.addTracks(USER, p1, [track(52)], 'g1'); + const beforeFlush = flaky.stored.length; + await P.flushPush(); + steps.push({ step: 'flush sends a pending push', + beforeFlush, afterFlush: flaky.stored.length }); + P.setPushTimings({ debounceMs: 100, retryMs: 400 }); + + // ── a fresh device finds them ───────────────────────────────────────── + // + // What the phone did not do. `auto` now holds this account's playlists; a + // browser that has never seen them must end up with them. + P.setPlaylistTransport(async () => auto); + const before = (await P.listPlaylists(USER)).length; + await P.forgetLocal(USER); + const emptied = (await P.listPlaylists(USER)).length; + const pulled = await P.pullOnce(USER); + steps.push({ step: 'a fresh device pulls once', + before, emptied, result: pulled, + lists: (await P.listPlaylists(USER)).map((p) => p.name).sort() }); + + // A device that already holds playlists must pull too. Bounding this to an + // empty device left out the ordinary case — nine playlists here, a tenth + // made somewhere else — which would then have waited for a Music tab. + const elsewhere = fakeNode(); + const mani = { v: 1, rev: 50, playlists: { + 'made-elsewhere': { name: 'Faite ailleurs', rev: 9, body_rev: 1, count: 1, + device: 'aaa', updated_at: 1, deleted: false } } }; + elsewhere.rows.set(MANIFEST_KIND, { + rev: 50, blob: await seal(mani, MANIFEST_KIND, USER, key) }); + elsewhere.rows.set(bodyKind('made-elsewhere'), { rev: 1, blob: await seal( + { v: 1, id: 'made-elsewhere', rev: 1, device: 'aaa', + tracks: [{ id: 'z1', g: 'g9', hv: 1, n: 'z.flac', s: 1, p: 'P', + t: 'Ailleurs', a: 'A', b: 'B', d: 1, tn: 1 }] }, + bodyKind('made-elsewhere'), USER, key) }); + P.setPlaylistTransport(async () => elsewhere); + const held = (await P.listPlaylists(USER)).length; + const pulled2 = await P.pullOnce(USER); + steps.push({ step: 'a device that already has playlists pulls too', + held, result: pulled2, + found: (await P.listPlaylists(USER)).map((p) => p.name).sort() }); + P.setPlaylistTransport(async () => auto); + + // ── a node holding something this browser cannot read ───────────────── + // + // The defect that made the phone report land: `open()` throwing on the + // node's manifest was treated as a fetch failure and returned before the + // push. The first write landed because the node was empty; every sync + // after it took that path and pushed nothing, for ever. Unreadable is an + // absence, not an error — the client is the authority. + const junkKey = await crypto.subtle.importKey( + 'raw', new Uint8Array(32).fill(200), { name: 'AES-GCM' }, false, + ['encrypt', 'decrypt']); + const wedged = fakeNode(); + wedged.rows.set(MANIFEST_KIND, { + rev: 1, + blob: await seal({ v: 1, rev: 1, playlists: {} }, + MANIFEST_KIND, USER, junkKey) }); + const wedgedResult = await P.syncWith(wedged, USER); + let readBack = null; + try { + readBack = await open(wedged.rows.get(MANIFEST_KIND).blob, + MANIFEST_KIND, USER, key); + } catch { /* still unreadable means it was not overwritten */ } + steps.push({ + step: 'an unreadable manifest does not wedge the sync', + result: wedgedResult, + overwritten: !!readBack, + names: readBack ? Object.values(readBack.playlists).map((p) => p.name).sort() : [], + }); + // ── a session with no HKDF handle degrades rather than failing ───────── + P.setPlaylistTransport(null); P.forgetPlaylistKey(); session.bundleKey = { v2: session.bundleKey.v2 }; // pre-change session const r3 = await P.syncWith(fakeNode(), USER); diff --git a/packages/meshbay-hub/tests/test_playlist_store.py b/packages/meshbay-hub/tests/test_playlist_store.py index 203033c..59afaee 100644 --- a/packages/meshbay-hub/tests/test_playlist_store.py +++ b/packages/meshbay-hub/tests/test_playlist_store.py @@ -145,6 +145,133 @@ def test_a_deleted_playlists_body_is_reclaimed_from_the_node(steps): assert steps["a deleted body is reclaimed"]["stillThere"] is False +# ── every write leaves the browser ─────────────────────────────────────────── +# +# The defect these exist for, reported from a phone: signing in with the same +# account showed no playlists at all. `syncWith` was called from exactly one +# place in the interface — after adding tracks from a cover — so creating a +# playlist, deleting one, removing a track and saving the queue all wrote to +# IndexedDB and stopped there. +# +# Nothing caught it because every test drove `syncWith` directly. The node's own +# audit log did: two events, ever. The store pushes itself now, so a new +# mutation cannot forget to. + +@pytest.mark.parametrize("step, expect_body", [ + ("create pushes", True), + ("add pushes", True), + ("remove pushes", True), + ("save-queue pushes", True), + ("delete pushes", False), +]) +def test_every_mutation_reaches_the_node(steps, step, expect_body): + kinds = steps[step]["kinds"] + assert "playlists" in kinds, f"{step}: the manifest never left the browser" + bodies = [k for k in kinds if k.startswith("playlist:")] + if expect_body: + assert bodies, f"{step}: no playlist body was pushed" + else: + assert not bodies, f"{step}: a deletion pushed a body" + + +def test_deleting_a_playlist_takes_its_body_off_the_node(steps): + assert steps["delete pushes"]["bodyGone"] is True + + +def test_a_burst_of_writes_is_one_push(steps): + """Five stars in a row is one manifest and one body, not five of each. The + push rides a connection borrowed from something else and must not spend it + once per keystroke.""" + assert steps["a burst is coalesced"]["writes"] == 2 + + +def test_a_device_that_has_nothing_fetches_once(steps): + """The report, exactly: a phone signing in with the same account. + + §7 said "on sign-in, nothing", on the grounds that the local copy is + authoritative and complete — true of a device that has been used before and + false of a new one, which is the case the whole feature exists for. Nothing + went looking until a group's Music tab happened to be opened. + """ + s = steps["a fresh device pulls once"] + assert s["before"] > 0 and s["emptied"] == 0, ( + "the fixture did not actually become a fresh device") + assert s["result"]["ok"] is True + assert s["result"]["pulled"] > 0 + assert s["lists"], "a fresh device found no playlists — this is the bug" + assert "Depuis le menu" in s["lists"] + + +# ── a push that does not land the first time ───────────────────────────────── + +def test_a_failed_push_is_retried(steps): + """A node that is busy, or a connection that drops between the write and + the push, used to mean waiting for the next Music tab to be opened. One + retry catches the common case: the node came back.""" + s = steps["a failed push is retried once"] + assert s["afterFirstTry"] == 0, "the fixture's node did not actually refuse" + assert s["firstReason"], "the first failure recorded no reason" + assert s["afterRetry"] > 0, "the push was never retried" + assert s["ok"] is True + + +def test_the_retry_does_not_loop(steps): + """Exactly one. A node that is off tends to stay off, and a push that keeps + looping spends a phone's battery on a node that is not coming back — while + the local copy is already correct and the next mutation carries it. + + Counted as sync *passes*, not writes: one pass writes a body per playlist + plus the manifest, so counting writes would say nothing about how many + times the push was attempted. + """ + assert steps["a retry does not loop"]["passes"] == 2, ( + "one attempt plus one retry, and no more") + + +def test_closing_the_page_sends_what_is_pending(steps): + """A push waits a second and a half to coalesce, and closing a laptop + inside that window is not rare. `pagehide` and a hidden tab flush it.""" + s = steps["flush sends a pending push"] + assert s["beforeFlush"] == 0 + assert s["afterFlush"] > 0 + + +def test_a_device_that_already_has_playlists_still_pulls_at_sign_in(steps): + """The correction to the correction. + + Bounding the sign-in pull to an empty device left out the ordinary case: a + phone holding nine playlists and missing the tenth would not have gone + looking either, and would have waited for a group's Music tab to be opened + — which is not where anybody looks for a playlist. + """ + s = steps["a device that already has playlists pulls too"] + assert s["held"] > 0, "the fixture device was empty, so this checks nothing" + assert s["result"]["ok"] is True + assert s["result"]["pulled"] > 0 + assert "Faite ailleurs" in s["found"] + + +def test_a_node_holding_something_unreadable_does_not_wedge_the_sync(steps): + """The defect behind the report from a phone. + + `open()` throwing on the node's manifest was treated as a *fetch* failure + and returned before the push. The first write landed because the node was + empty, so nothing was fetched; every sync after it took that path and + pushed nothing, for ever, while the interface showed the playlists happily + from IndexedDB. The node's audit log was the only thing that said so. + + A blob this account cannot open with this passphrase is not an older copy + of anything. It is an absence — the client is the authority (§6.4) — and it + gets overwritten. + """ + s = steps["an unreadable manifest does not wedge the sync"] + assert s["result"]["ok"] is True + assert s["result"]["unreadable"] is True, "the failure was not even noticed" + assert s["result"]["pushed"] > 0, "the sync returned without pushing again" + assert s["overwritten"] is True, "the unreadable blob is still there" + assert "Depuis le menu" in s["names"] + + def test_a_session_from_before_the_hkdf_handle_degrades_rather_than_failing(steps): """A bundle key loaded out of IndexedDB from before `deriveBundleKeys` existed has no HKDF handle, and the passphrase is not in memory to diff --git a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py index 298b3cd..99ba3c9 100644 --- a/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py +++ b/packages/meshbay-node/src/meshbay_node/transport/webrtc_server.py @@ -1178,11 +1178,23 @@ class WebRTCPeerSession: # `user_id` in the body would let any member of this group read or # overwrite any other member's blob, which is finding C5 one size down. + def _user_blob_refuse(self, detail: str, kind: str = "") -> None: + """ + Refuse, and say so in the audit log. + + A refusal used to be invisible here: the audit line was written only + after a store *succeeded*, so a client whose writes were all being + turned away looked exactly like a client that never wrote — which is + how a wedged sync went unnoticed for two hours. + """ + self._audit("user_blob_refused", f"{kind} {detail}".strip()) + self._send({"type": "error", "detail": detail}) + def _user_blob_kind(self, msg: dict) -> str | None: """The validated `kind`, or None having already refused.""" kind = msg.get("kind") if not isinstance(kind, str) or not _USER_BLOB_KIND_RE.match(kind): - self._send({"type": "error", "detail": "Unknown blob kind"}) + self._user_blob_refuse("Unknown blob kind", str(kind)[:40]) return None return kind @@ -1206,13 +1218,13 @@ class WebRTCPeerSession: blob = msg.get("blob_enc") if not isinstance(blob, (bytes, bytearray)) or not blob: - self._send({"type": "error", "detail": "Missing blob_enc"}) + self._user_blob_refuse("Missing blob_enc", kind) return blob = bytes(blob) rev = msg.get("rev") if not isinstance(rev, int) or rev < 0: - self._send({"type": "error", "detail": "Missing rev"}) + self._user_blob_refuse("Missing rev", kind) return limit = (USER_BLOB_MANIFEST_MAX if kind == "playlists" @@ -1221,8 +1233,7 @@ class WebRTCPeerSession: # A stated reason, not a bare error: the client turns this into a # sentence the reader can act on ("this playlist is too large"), # and a refusal nobody can read is a support case. - self._send({"type": "error", - "detail": f"Blob too large ({len(blob)} > {limit})"}) + self._user_blob_refuse(f"Blob too large ({len(blob)} > {limit})", kind) return # What the account already uses, minus whatever this call replaces. @@ -1231,9 +1242,9 @@ class WebRTCPeerSession: if existing: used -= len(existing["blob_enc"]) if used + len(blob) > USER_BLOB_ACCOUNT_MAX: - self._send({"type": "error", - "detail": f"Account blob quota exceeded " - f"({used + len(blob)} > {USER_BLOB_ACCOUNT_MAX})"}) + self._user_blob_refuse( + f"Account blob quota exceeded " + f"({used + len(blob)} > {USER_BLOB_ACCOUNT_MAX})", kind) return await store.store_user_blob(self._user_id, kind, rev, blob) @@ -1263,6 +1274,7 @@ class WebRTCPeerSession: if not store: return blobs = await store.list_user_blobs(self._user_id) + self._audit("user_blob_list", f"{len(blobs)} blobs") self._send({"type": MNP.USER_BLOB_LIST_RESP, "v": MNP_VERSION, "blobs": blobs}) diff --git a/packages/meshbay-node/tests/test_user_blob_mnp.py b/packages/meshbay-node/tests/test_user_blob_mnp.py index 296239f..85893af 100644 --- a/packages/meshbay-node/tests/test_user_blob_mnp.py +++ b/packages/meshbay-node/tests/test_user_blob_mnp.py @@ -242,6 +242,37 @@ async def test_an_unauthenticated_session_reaches_nothing(store): @pytest.mark.asyncio +@pytest.mark.parametrize("msg, why", [ + ({"kind": "nonsense", "rev": 1, "blob_enc": b"x"}, "Unknown blob kind"), + ({"kind": "playlists", "rev": 1}, "Missing blob_enc"), + ({"kind": "playlists", "blob_enc": b"x"}, "Missing rev"), + ({"kind": "playlists", "rev": 1, + "blob_enc": b"x" * (USER_BLOB_MANIFEST_MAX + 1)}, "too large"), +]) +async def test_a_refusal_is_audited_and_not_merely_sent(store, msg, why): + """A refusal used to leave no trace at all: the audit line was written only + after a store *succeeded*. So a client whose every write was being turned + away looked exactly like a client that never wrote — which is how a wedged + sync went unnoticed, with the operator's own log saying nothing.""" + s = _session(store) + await s._do_user_blob_store(msg) + assert _last(s)["type"] == "error" + events = [(e, d) for e, d in s.audited if e == "user_blob_refused"] + assert events, "the refusal is invisible in the audit log" + assert why in events[0][1] + + +@pytest.mark.asyncio +async def test_a_listing_is_audited(store): + """Half the traffic was invisible: `user_blob_list` is what every sync does + first, and it wrote no audit line, so the log could not distinguish a client + that was syncing from one that was not.""" + s = _session(store) + await s._do_user_blob_list() + assert ("user_blob_list", "0 blobs") in s.audited + + +@pytest.mark.asyncio async def test_reads_and_writes_are_audited(store): """The node logs that a blob moved, never what was in it — the same line `keypair_bundle_store` already writes.""" |