diff --git a/CLAUDE.md b/CLAUDE.md index 1ef4a60..f4235c2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -283,7 +283,13 @@ persistence) → GUI (Qt widgets that read the manager and connect to its signal - **`lintunes/mpris.py`** — registers `org.mpris.MediaPlayer2.lintunes` over D-Bus so the desktop's media keys / now-playing popup control playback. Spacebar and - arrow keys are handled locally via `MainWindow.eventFilter`. + arrow keys are handled locally via `MainWindow.eventFilter`, which **ignores + auto-repeat**: on Wayland repeats are generated client-side until the + compositor delivers the release, and a busy gnome-shell turned one held + arrow into 1,676 track skips (Round 56). Every list sent over D-Bus must be + a typed `QDBusArgument` (`mpris.string_array`) — a plain Python list goes + out as `av`, which GDBus rejects. That was `xesam:artist`, and the error + flood it caused is what kept gnome-shell busy. - **`lintunes/trash.py`** — freedesktop.org Trash spec 1.0, hand-rolled (no new dep). The trash is **per-filesystem**: music usually lives on a mounted volume, diff --git a/lintunes/__init__.py b/lintunes/__init__.py index 34a78be..4307ba2 100644 --- a/lintunes/__init__.py +++ b/lintunes/__init__.py @@ -1,3 +1,3 @@ """LinTunes — iTunes-style music library manager and player for Linux.""" -__version__ = "0.20.5" +__version__ = "0.20.6" diff --git a/lintunes/gui/main_window.py b/lintunes/gui/main_window.py index 2a2f5d1..3e6d474 100644 --- a/lintunes/gui/main_window.py +++ b/lintunes/gui/main_window.py @@ -1626,6 +1626,15 @@ class MainWindow(QMainWindow): QAbstractSpinBox, QComboBox)): return False key = event.key() + if key in (Qt.Key.Key_Space, Qt.Key.Key_Right, Qt.Key.Key_Left) \ + and event.isAutoRepeat(): + # On Wayland key repeat is generated client-side until the + # compositor delivers the release — and a compositor busy + # with our own MPRIS churn delivers it late, so one held + # arrow became 1,676 track skips (Round 56). A transport key + # acts once per press; the repeats are swallowed so they + # don't fall through and move the table's cursor either. + return True if key == Qt.Key.Key_Space: self._log_key(event, "space") self.play_pause() diff --git a/lintunes/mpris.py b/lintunes/mpris.py index 52602bb..20cf175 100644 --- a/lintunes/mpris.py +++ b/lintunes/mpris.py @@ -259,7 +259,7 @@ class MprisPlayerAdaptor(QDBusAbstractAdaptor): meta = { "mpris:trackid": QDBusObjectPath(f"/org/lintunes/track/{track.track_id}"), "xesam:title": track.name or "", - "xesam:artist": [track.artist or ""], + "xesam:artist": string_array([track.artist or ""]), "xesam:album": track.album or "", } if track.total_time: @@ -275,17 +275,31 @@ class MprisPlayerAdaptor(QDBusAbstractAdaptor): QDBusConnection.sessionBus().send(build_properties_changed(changed)) +def string_array(values) -> QDBusArgument: + """A D-Bus string array ("as"). A plain Python list of str marshals as + "av", which GDBus consumers validate and reject — see + build_properties_changed and xesam:artist in _metadata.""" + string_id = QMetaType(QMetaType.Type.QString.value).id() + arg = QDBusArgument() + arg.beginArray(string_id) + for value in values: + arg.add(value, string_id) + arg.endArray() + return arg + + def build_properties_changed(changed: dict) -> QDBusMessage: """A spec-correct Properties.PropertiesChanged signal for the Player interface. invalidated_properties must be marshalled as a D-Bus string array ("as"); a plain Python [] goes over as "av", and GDBus consumers — including gsd-media-keys, which keys its media-key MRU on seeing PlaybackStatus change — validate the signature and drop the signal. - (Same pitfall as reveal_paths in track_table.)""" + (Same pitfall as reveal_paths in track_table, and as xesam:artist inside + Metadata: sent as "av", gnome-shell logged an error and reloaded the + cover per track change, and during a runaway key repeat that flood is + what froze the desktop — Round 56.)""" msg = QDBusMessage.createSignal( OBJECT_PATH, "org.freedesktop.DBus.Properties", "PropertiesChanged") - invalidated = QDBusArgument() - invalidated.beginArray(QMetaType(QMetaType.Type.QString.value).id()) - invalidated.endArray() - msg.setArguments(["org.mpris.MediaPlayer2.Player", changed, invalidated]) + msg.setArguments(["org.mpris.MediaPlayer2.Player", changed, + string_array([])]) return msg diff --git a/tasks-done.md b/tasks-done.md index 4e4dc37..0d4196f 100644 --- a/tasks-done.md +++ b/tasks-done.md @@ -1,5 +1,25 @@ ## Done +### Round 56 (2026-09-17) — one press, sixteen hundred songs (v0.20.6) + +One Right-arrow press skipped 1,676 tracks in 81 s and hard-froze GNOME badly +enough to need SysRq (incident log `~/claude-diag/logs/issue-log-2026-09-17.md`). +It was a feedback loop, and both halves were ours. + +- [x] **Transport keys act once per press.** On Wayland key repeat is + generated by the *client* until the compositor delivers the release, and + `MainWindow.eventFilter` treated every repeat as a new press. Space, Left + and Right now swallow auto-repeats (swallow, not pass through, so a held + Right doesn't walk the table cursor instead). +- [x] **`xesam:artist` goes out as `as`.** A plain `[str]` inside `Metadata` + marshals as `av`; gnome-shell logged an error for it (32k–50k/min during + the incident) and reloaded the cover per track change. That load is what + kept it from delivering the key release, so the repeats never stopped. + `mpris.string_array` builds the typed array, shared with + `build_properties_changed`'s `invalidated_properties` (Round 17's same + pitfall). The test round-trips the signal into GDBus, which is what + gnome-shell validates with, and fails `'av' == 'as'` on the old code. + ### Round 55 (2026-09-14) — the song that wasn't there (v0.20.4, andTunes 0.2.2) trav couldn't play the first track of his playlist: shuffle off gave him song diff --git a/tests/test_round56.py b/tests/test_round56.py new file mode 100644 index 0000000..33c39ba --- /dev/null +++ b/tests/test_round56.py @@ -0,0 +1,129 @@ +"""Round 56: one held arrow key hard-froze the desktop. + +Incident 2026-09-17: a single Right press became 1,676 track skips in 81 s. +On Wayland key repeat is generated by the *client* until the compositor +delivers the release, and every skip sent gnome-shell an MPRIS Metadata whose +xesam:artist was typed "av" rather than "as" — an error plus a cover reload +per skip, which kept gnome-shell too busy to deliver that release. Both halves +are pinned here: transport keys act once per press, and xesam:artist goes out +as a real string array. +""" +import threading +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest +from PyQt6.QtCore import QCoreApplication, QEvent, Qt +from PyQt6.QtGui import QKeyEvent + +from lintunes.gui import main_window +from lintunes.gui.main_window import MainWindow + + +# ---- transport keys ignore auto-repeat ---- + +def _window_stub(): + return SimpleNamespace( + isActiveWindow=lambda: True, + play_pause=MagicMock(), + player=MagicMock(), + _log_key=MagicMock(), + ) + + +def _press(key, repeat): + return QKeyEvent(QEvent.Type.KeyPress, key, + Qt.KeyboardModifier.NoModifier, "", repeat) + + +@pytest.fixture +def no_focus_widget(monkeypatch): + monkeypatch.setattr(main_window.QApplication, "focusWidget", + staticmethod(lambda: None)) + + +@pytest.mark.parametrize("key, action", [ + (Qt.Key.Key_Right, lambda w: w.player.next), + (Qt.Key.Key_Left, lambda w: w.player.previous), + (Qt.Key.Key_Space, lambda w: w.play_pause), +]) +def test_held_key_acts_once(qapp, no_focus_widget, key, action): + win = _window_stub() + assert MainWindow.eventFilter(win, None, _press(key, repeat=False)) + for _ in range(50): + # Swallowed, not passed through: a held Right mustn't walk the + # table's cursor instead. + assert MainWindow.eventFilter(win, None, _press(key, repeat=True)) + assert action(win).call_count == 1 + assert win._log_key.call_count == 1 + + +def test_separate_presses_each_act(qapp, no_focus_widget): + win = _window_stub() + for _ in range(3): + MainWindow.eventFilter(win, None, _press(Qt.Key.Key_Right, False)) + assert win.player.next.call_count == 3 + + +# ---- xesam:artist is "as" ---- + +def _adaptor_metadata(monkeypatch): + from lintunes import mpris + monkeypatch.setattr(mpris, "export_artwork", lambda track: None) + track = SimpleNamespace(track_id=7, name="I Know", artist="Dionne Farris", + album="Wild Seed — Wild Flower", total_time=229000, + location="/music/i-know.mp3", persistent_id="AB12") + adaptor = SimpleNamespace(_player=SimpleNamespace(current_track=track)) + return mpris.MprisPlayerAdaptor._metadata(adaptor) + + +def test_artist_is_typed_string_array(qapp, monkeypatch): + # A plain [str] is what went out as "av"; only a typed QDBusArgument + # carries "as". (The wire type itself is checked over the bus below.) + from PyQt6.QtDBus import QDBusArgument + meta = _adaptor_metadata(monkeypatch) + assert isinstance(meta["xesam:artist"], QDBusArgument) + + +def test_gdbus_receives_artist_as_string_array(qapp, monkeypatch): + """Round-trip over the real session bus into GDBus — the same library + gnome-shell validates with — and check the type it actually sees.""" + Gio = pytest.importorskip("gi.repository.Gio") + GLib = pytest.importorskip("gi.repository.GLib") + from PyQt6.QtDBus import QDBusConnection + from lintunes.mpris import OBJECT_PATH, build_properties_changed + + bus = QDBusConnection.sessionBus() + if not bus.isConnected(): + pytest.skip("no D-Bus session bus in this environment") + try: + gbus = Gio.bus_get_sync(Gio.BusType.SESSION, None) + except GLib.Error: + pytest.skip("GDBus can't reach the session bus") + + received = [] + sub = gbus.signal_subscribe( + bus.baseService(), "org.freedesktop.DBus.Properties", + "PropertiesChanged", OBJECT_PATH, None, Gio.DBusSignalFlags.NONE, + lambda *args: received.append(args[5])) + try: + meta = _adaptor_metadata(monkeypatch) + assert bus.send(build_properties_changed({"Metadata": meta})) + ctx = GLib.MainContext.default() + for _ in range(200): + QCoreApplication.processEvents() + while ctx.iteration(False): + pass + if received: + break + threading.Event().wait(0.01) + finally: + gbus.signal_unsubscribe(sub) + + assert received, "signal was not delivered over the bus" + params = received[0] + assert params.get_type_string() == "(sa{sv}as)" + metadata = params.get_child_value(1).lookup_value("Metadata", None) + artist = metadata.lookup_value("xesam:artist", None) + assert artist.get_type_string() == "as" + assert artist.unpack() == ["Dionne Farris"]