From 707037a0e955984cfbe5f39f15491468a1d44db7 Mon Sep 17 00:00:00 2001 From: Jono Date: Tue, 29 Sep 2026 23:02:10 -0700 Subject: [PATCH 1/3] feat: explain and recover from reconnect, lock, and error states (#64) Surface four previously opaque failure states in plain language, each with a path back to a working surface: - reconnect/disconnected overlay showing the attempt count and last error with a Cancel/Retry affordance. The socket hook now tracks attempt, lastError, retrying and cancelled, and exposes cancelReconnect/retryNow. A drop parks in `closed` for the whole backoff, so `retrying` keeps the overlay honest as "Reconnecting" rather than "Disconnected". - lock takeover names the cause (screensaver or lock screen) and states that input is held until unlock; the blank-without-lock banner names suspend / display blank. - deck load errors carry structured `error_deck` / `error_widget` fields so the client names the deck and offending widget, plus a Return to focused app button. YAML syntax errors now set `current_error` too (previously they silently kept the last-good deck live). - failed media commands push a new `media_error` frame naming the player, rendered as a Retry/Dismiss toast. MPRIS `send_command` raises a typed `MprisCommandError` for a vanished player or a rejected method; the HTTP command endpoint maps those to 404/502 instead of a silent 200. Protocol: adds MediaErrorMessage and the two DeckMessage fields to `protocol.py`, regenerated into the TS mirror. All four states announce via the aria-live region and honour prefers-reduced-motion. --- client/src/App.test.tsx | 215 +++++++++++++++++- client/src/App.tsx | 135 ++++++++++- client/src/ConnectionOverlay.tsx | 60 +++++ client/src/MediaErrorToast.tsx | 40 ++++ client/src/playground/usePlaygroundDaemon.ts | 9 +- client/src/protocol.generated.ts | 51 ++++- client/src/protocol.ts | 8 + client/src/socket.test.ts | 118 ++++++++++ client/src/socket.ts | 115 +++++++++- client/src/style.css | 189 ++++++++++++++++ daemon/deckd/decks.py | 106 ++++++++- daemon/deckd/mpris.py | 64 +++++- daemon/deckd/protocol.py | 42 +++- daemon/deckd/server.py | 110 ++++++++- docs/GUIDE.md | 13 +- tests/test_decks.py | 82 ++++++- tests/test_feedback.py | 223 +++++++++++++++++++ tests/test_mpris_dbus.py | 34 ++- 18 files changed, 1566 insertions(+), 48 deletions(-) create mode 100644 client/src/ConnectionOverlay.tsx create mode 100644 client/src/MediaErrorToast.tsx create mode 100644 client/src/socket.test.ts create mode 100644 tests/test_feedback.py diff --git a/client/src/App.test.tsx b/client/src/App.test.tsx index a1f8830..0b8d12a 100644 --- a/client/src/App.test.tsx +++ b/client/src/App.test.tsx @@ -16,7 +16,7 @@ */ import { act, cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import type { ClientMessage, ServerChromeMedia, ServerDeck, ServerState } from "./protocol"; +import type { ClientMessage, ServerChromeMedia, ServerDeck, ServerMediaError, ServerState } from "./protocol"; /** Replace the real socket hook with a controllable fake. The chrome * icon's job is to call ``send`` with the right message; the test @@ -32,9 +32,16 @@ const onDeck = vi.fn<(m: ServerDeck) => void>(); * gate / focus restoration flow can set ``mockStatus`` to drive * ``status`` through ``"unauthorized"`` → ``"open"`` transitions. */ let mockStatus: "connecting" | "open" | "closed" | "unauthorized" = "open"; +let mockAttempt = 0; +let mockLastError = ""; +let mockCancelled = false; +let mockRetrying = false; const authenticate = vi.fn(); const deauthenticate = vi.fn(); +const cancelReconnect = vi.fn(); +const retryNow = vi.fn(); let sessionStateHandler: ((m: ServerState) => void) | null = null; +let mediaErrorHandler: ((m: ServerMediaError) => void) | null = null; vi.mock("./socket", () => ({ useDeckdSocket: ( deckCb: (m: ServerDeck) => void, @@ -44,11 +51,13 @@ vi.mock("./socket", () => ({ _confirmRequestCb: unknown, _runningWindowsCb: unknown, sessionStateCb: ((m: ServerState) => void) | undefined, + mediaErrorCb: ((m: ServerMediaError) => void) | undefined, _options: unknown, ) => { onDeck.mockImplementation(deckCb); chromeMediaHandler = chromeMediaCb ?? null; sessionStateHandler = sessionStateCb ?? null; + mediaErrorHandler = mediaErrorCb ?? null; return { get status() { return mockStatus; @@ -57,12 +66,37 @@ vi.mock("./socket", () => ({ authenticate, deauthenticate, hasPassword: false, + get attempt() { + return mockAttempt; + }, + get lastError() { + return mockLastError; + }, + get cancelled() { + return mockCancelled; + }, + get retrying() { + return mockRetrying; + }, + cancelReconnect, + retryNow, }; }, })); import { App } from "./App"; +// Reconnect telemetry is module-level state shared by every describe block; +// reset it before each test so a drop simulated in one test can't leak a +// connection overlay into the next. Inner blocks still override as needed. +beforeEach(() => { + mockStatus = "open"; + mockAttempt = 0; + mockLastError = ""; + mockCancelled = false; + mockRetrying = false; +}); + describe("App — chrome media icon", () => { afterEach(cleanup); beforeEach(() => { @@ -1018,13 +1052,15 @@ describe("App — session lock takeover", () => { expect(screen.getByRole("button", { name: /open/i })).toBeTruthy(); act(() => sessionStateHandler?.({ type: "state", locked: true, blanked: false })); expect(screen.getAllByText("Screen locked").length).toBeGreaterThan(0); - expect(screen.getByText(/Controls resume when you unlock/)).toBeTruthy(); + // Issue #64: the takeover names the cause and that input is held. + expect(screen.getByText(/Screensaver or lock screen/)).toBeTruthy(); + expect(screen.getByText(/Input is held until you unlock/)).toBeTruthy(); }); it("names the host in the lock copy", () => { render(); act(() => sessionStateHandler?.({ type: "state", locked: true, blanked: false })); - expect(screen.getByText(/localhost/)).toBeTruthy(); + expect(screen.getAllByText(/localhost/).length).toBeGreaterThan(0); }); it("keeps the connection dot's 'live' status while locked", () => { @@ -1144,3 +1180,176 @@ describe("App — fullscreen toggle", () => { expect(screen.getByRole("button", { name: "fullscreen" }).getAttribute("aria-pressed")).toBe("false"); }); }); + +/* ------------------------------------------------------------------------- + * Reconnect / disconnected overlay (issue #64): attempt count, last error, + * and a single Cancel/Retry affordance. + * --------------------------------------------------------------------- */ + +describe("App — reconnect overlay", () => { + afterEach(cleanup); + beforeEach(() => { + send.mockReset(); + cancelReconnect.mockReset(); + retryNow.mockReset(); + mockStatus = "open"; + mockAttempt = 0; + mockLastError = ""; + mockCancelled = false; + mockRetrying = false; + window.history.replaceState(null, "", "/?demo=default"); + }); + + it("hides itself when the socket is open", () => { + render(); + expect(screen.queryByText(/Reconnecting/)).toBeNull(); + expect(screen.queryByText("Disconnected")).toBeNull(); + }); + + it("shows attempt count and last error while reconnecting", () => { + mockStatus = "connecting"; + mockAttempt = 2; + mockLastError = "daemon not reachable"; + render(); + expect(screen.getByText("Reconnecting…")).toBeTruthy(); + expect(screen.getByText(/Attempt 2 — daemon not reachable/)).toBeTruthy(); + expect(screen.getByRole("button", { name: "Cancel reconnecting" })).toBeTruthy(); + }); + + it("cancels the retry loop", () => { + mockStatus = "connecting"; + mockAttempt = 1; + mockRetrying = true; + render(); + fireEvent.click(screen.getByRole("button", { name: "Cancel reconnecting" })); + expect(cancelReconnect).toHaveBeenCalledTimes(1); + }); + + it("offers Retry once cancelled and re-arms the connection", () => { + mockStatus = "closed"; + mockCancelled = true; + mockLastError = "connection lost (code 1006)"; + render(); + expect(screen.getByText("Disconnected")).toBeTruthy(); + expect(screen.getByText("connection lost (code 1006)")).toBeTruthy(); + fireEvent.click(screen.getByRole("button", { name: "Retry connection" })); + expect(retryNow).toHaveBeenCalledTimes(1); + }); +}); + +/* ------------------------------------------------------------------------- + * Deck load-error surface naming the deck and offender (issue #64). + * --------------------------------------------------------------------- */ + +describe("App — deck error feedback", () => { + afterEach(cleanup); + beforeEach(() => { + send.mockReset(); + onDeck.mockReset(); + mockStatus = "open"; + window.history.replaceState(null, "", "/?demo=default"); + }); + + it("names the deck and offending widget, and returns to the focused app", () => { + render(); + act(() => { + onDeck({ + type: "deck", + app: "default", + jogstrip_enabled: true, + widgets: [], + error: "1 validation error for Deck", + error_deck: "firefox", + error_widget: "back", + }); + }); + expect(screen.getByText(/Couldn't load the "firefox" deck/)).toBeTruthy(); + expect(screen.getByText(/Widget "back" is invalid/)).toBeTruthy(); + fireEvent.click(screen.getByRole("button", { name: /return to focused app/i })); + expect(send).toHaveBeenCalledWith({ type: "clear_view" }); + }); + + it("announces the deck error via the live region", () => { + render(); + act(() => { + onDeck({ + type: "deck", + app: "default", + jogstrip_enabled: true, + widgets: [], + error: "1 validation error for Deck", + error_deck: "firefox", + }); + }); + expect(screen.getByRole("status").textContent).toBe("Deck error: firefox"); + }); +}); + +/* ------------------------------------------------------------------------- + * Media-command failure toast (issue #64): names the player and offers + * Retry (re-sends the last command) / Dismiss. + * --------------------------------------------------------------------- */ + +describe("App — media error toast", () => { + afterEach(cleanup); + beforeEach(() => { + send.mockReset(); + mediaErrorHandler = null; + mockStatus = "open"; + window.history.replaceState(null, "", "/?demo=mpris"); + }); + + it("surfaces a media_error frame naming the player", () => { + render(); + act(() => + mediaErrorHandler?.({ + type: "media_error", + id: "mpris.vlc", + player: "VLC media player", + message: "player disappeared", + retryable: true, + }), + ); + // The now-playing rows also name the player; the toast is the only + // element carrying the failure message. + expect(screen.getByText(/player disappeared/)).toBeTruthy(); + expect(screen.getAllByText("VLC media player").length).toBeGreaterThan(0); + }); + + it("retries by re-sending the last command for that id", () => { + render(); + // Issue a command through the real cell, so the App records it. + fireEvent.click(screen.getAllByLabelText("Next")[0]); + const sent = send.mock.calls[send.mock.calls.length - 1][0]; + if (sent.type !== "media_command") throw new Error("expected media_command"); + + act(() => + mediaErrorHandler?.({ + type: "media_error", + id: sent.id, + player: "VLC media player", + message: "player rejected the command", + retryable: true, + }), + ); + fireEvent.click(screen.getByRole("button", { name: "Retry" })); + expect(send.mock.calls[send.mock.calls.length - 1][0]).toEqual(sent); + }); + + it("dismisses without re-sending", () => { + render(); + const before = send.mock.calls.length; + act(() => + mediaErrorHandler?.({ + type: "media_error", + id: "mpris.vlc", + player: "VLC media player", + message: "player disappeared", + retryable: true, + }), + ); + fireEvent.click(screen.getByRole("button", { name: "Dismiss" })); + expect(screen.queryByText(/player disappeared/)).toBeNull(); + expect(send.mock.calls.length).toBe(before); + }); +}); diff --git a/client/src/App.tsx b/client/src/App.tsx index 838bff3..82d7f09 100644 --- a/client/src/App.tsx +++ b/client/src/App.tsx @@ -39,6 +39,7 @@ import type { Icon as IconRef, ServerChromeMedia, ServerDeck, + ServerMediaError, ServerRunningWindows, ServerState, WindowListEntry, @@ -48,7 +49,9 @@ import { isTypingTarget, onActivate } from "./a11y"; import { Editor } from "./Editor"; import { ReflowHelp } from "./ReflowHelp"; import { ConfirmModal } from "./ConfirmModal"; -import type { Widget, ConfirmRequestMessage } from "./protocol"; +import { ConnectionOverlay } from "./ConnectionOverlay"; +import { MediaErrorToast } from "./MediaErrorToast"; +import type { Widget, ConfirmRequestMessage, MediaCommandMessage } from "./protocol"; import { wireWindowsToServer } from "./protocol"; import { pathForView, viewFromPath, type View } from "./view-routing"; @@ -203,6 +206,16 @@ export function App() { // presses enabled so the first tap wakes the machine. const [sessionState, setSessionState] = useState(null); const onSessionState = useCallback((m: ServerState) => setSessionState(m), []); + // Media-command failure feedback (issue #64). The daemon answers a + // failed media command with a ``media_error`` frame naming the player; + // we hold the most recent one and render a toast. The *command* itself + // isn't echoed back, so the client remembers the last command it sent + // per id (``lastMediaCommandRef``) to make Retry possible. + const [mediaError, setMediaError] = useState(null); + const lastMediaCommandRef = useRef< + Record + >({}); + const onMediaError = useCallback((m: ServerMediaError) => setMediaError(m), []); // Demo mode has no socket, so seed the media store once on mount with the // fixture readings — otherwise a media widget renders as "unavailable". const isDemo = demoDeck !== null; @@ -240,6 +253,7 @@ export function App() { onConfirmRequest, onRunningWindows, onSessionState, + onMediaError, { enabled: !demoDeck && !isPlayground }, ); const playgroundSocket = usePlaygroundDaemon( @@ -251,8 +265,19 @@ export function App() { onRunningWindows, { enabled: isPlayground }, ); - const { status, send, authenticate, deauthenticate, hasPassword } = - isPlayground ? playgroundSocket : realSocket; + const { + status, + send, + authenticate, + deauthenticate, + hasPassword, + attempt, + lastError, + cancelled, + retrying, + cancelReconnect, + retryNow, + } = isPlayground ? playgroundSocket : realSocket; // Derived two-state (issue #160). ``locked`` requires credentials — // full takeover of focus-targeting surfaces. ``blanked`` without // ``locked`` is the soft "asleep, press anything to wake" state. @@ -336,7 +361,10 @@ export function App() { const padDrag = (state: "start" | "end") => send({ type: "pad_drag", id: TRACKPAD_ID, state }); const typeText = (text: string) => send({ type: "type", text }); const keyCombo = (combo: string) => send({ type: "key", combo }); - const mediaCommand = (id: string, command: "volume" | "seek" | "rate", value: number) => send({ type: "media_command", id, command, value }); + const mediaCommand = (id: string, command: "volume" | "seek" | "rate", value: number) => { + lastMediaCommandRef.current[id] = { command, value }; + send({ type: "media_command", id, command, value }); + }; // Now-playing per-row transport (issue #54): the cell sends // three value-less commands — ``play-pause`` / ``next`` / ``previous`` // — keyed by the row's ``mpris.`` id. The server routes the @@ -344,8 +372,25 @@ export function App() { // ``media_command`` family keeps going to the VLC path. This // callback is the only thing the cell needs to know about the // wire surface. - const nowPlayingCommand = (id: string, command: "play-pause" | "next" | "previous") => + const nowPlayingCommand = (id: string, command: "play-pause" | "next" | "previous") => { + lastMediaCommandRef.current[id] = { command }; send({ type: "media_command", id, command }); + }; + // Retry re-sends the last command issued for the failed id (issue #64). + // The toast is cleared first so the retry is visibly a fresh attempt; a + // second failure re-opens it with the daemon's latest message. + const retryMedia = useCallback(() => { + const failed = mediaError; + setMediaError(null); + if (!failed) return; + const last = lastMediaCommandRef.current[failed.id]; + if (!last) return; + send( + last.value === undefined + ? { type: "media_command", id: failed.id, command: last.command } + : { type: "media_command", id: failed.id, command: last.command, value: last.value }, + ); + }, [mediaError, send]); // Chrome view toggle (issue #51): the media icon mirrors the existing // trackpad / settings buttons. When opened it sends ``select_view`` // so the daemon pushes the mpris deck; when closed it sends @@ -608,10 +653,21 @@ export function App() { if (prevDeck.current === deck) return; prevDeck.current = deck; if (!deck) return; + if (deck.error) { + // eslint-disable-next-line react-hooks/set-state-in-effect + setLiveText(`Deck error: ${deck.error_deck || "unknown deck"}`); + return; + } const app = deck.display_name?.trim() || deck.app || "deckd"; - // eslint-disable-next-line react-hooks/set-state-in-effect setLiveText(`Deck: ${app}${programSuffix}`); }, [deck, programSuffix]); + // Announce a failed media command (issue #64) — same live region, so a + // screen reader hears the toast without a separate focus move. + useEffect(() => { + if (!mediaError) return; + // eslint-disable-next-line react-hooks/set-state-in-effect + setLiveText(`Media error: ${mediaError.player || mediaError.id}`); + }, [mediaError]); // Chrome app-identity badge (ADR-0007): the daemon relays an // optional ``display_name`` / ``theme`` / ``icon`` per deck; the @@ -675,6 +731,14 @@ export function App() { const lockBlockedView = view === "deck" || view === "trackpad"; const renderLockTakeover = screenLocked && lockBlockedView; const renderBlankBanner = screenBlanked && !screenLocked && view === "deck"; + // Reconnect / disconnected overlay (issue #64). Shown only once there's + // actual trouble — a drop, a recorded error, or an active retry loop — so + // the initial dial doesn't flash an overlay over the first paint. Demo and + // playground report an ``open`` socket with no retry, so they never show + // it; the lock takeover outranks it. + const reconnecting = !cancelled && (status === "connecting" || retrying); + const showConnectionOverlay = + !screenLocked && (retrying || status === "closed" || !!lastError); return ( <> @@ -697,12 +761,28 @@ export function App() { } >

{headingText}

+ {/* Reconnect / disconnected overlay (issue #64). Sits over the + last-rendered surface so a stale grid stays visible behind it + while naming the attempt count and last error. Never shown + while locked (the lock takeover owns the surface then). */} + {showConnectionOverlay && ( + + )} {renderLockTakeover ? (
Screen locked + + {`Screensaver or lock screen active on ${HOSTNAME}`} + - {`Controls resume when you unlock ${HOSTNAME}`} + {`Input is held until you unlock ${HOSTNAME}`}
) : ( @@ -710,7 +790,9 @@ export function App() { {renderBlankBanner && (
- {`Screen asleep — press anything to wake ${HOSTNAME}`} + + {`Screen asleep — press anything to wake ${HOSTNAME} (suspend or display blank)`} +
)} {view === "trackpad" ? ( @@ -848,7 +930,33 @@ export function App() { ) : deck?.error ? (
Deck error + {(deck.error_deck || deck.error_widget) && ( + + {deck.error_deck + ? `Couldn't load the "${deck.error_deck}" deck.` + : "Couldn't load a deck."} + {deck.error_widget + ? ` Widget "${deck.error_widget}" is invalid.` + : ""} + + )} + {deck.error === "view not found" && ( + + That view isn't available on this daemon. + + )} 
{deck.error}
+
) : deck ? ( ) : null} + {/* Failed-media-command toast (issue #64). Non-blocking: the deck + stays usable and the toast names the player with Retry / Dismiss. */} + {mediaError ? ( + setMediaError(null)} + /> + ) : null} ); } diff --git a/client/src/ConnectionOverlay.tsx b/client/src/ConnectionOverlay.tsx new file mode 100644 index 0000000..385d678 --- /dev/null +++ b/client/src/ConnectionOverlay.tsx @@ -0,0 +1,60 @@ +/** Full-surface reconnect / disconnected overlay (issue #64). + * + * The socket hook owns the facts — whether the retry loop is active, the + * attempt count, and the last error — and this component turns them into + * copy plus a single Cancel/Retry affordance. It renders *over* whatever + * the surface was last showing, so a stale grid stays readable behind it. + * + * Deliberately ``aria-live="polite"`` without ``role="status"``: the app + * already keeps one ``role="status"`` live region for announcements, and a + * second implicit status role would make ``getByRole("status")`` ambiguous + * for both users and tests. The spinner stops under the app's + * ``a11y-reduce-motion`` class and the OS ``prefers-reduced-motion`` query. + */ +export function ConnectionOverlay({ + reconnecting, + attempt, + lastError, + onCancel, + onRetry, +}: { + /** The auto-retry loop is active (dialing or waiting out the backoff). + * False means the loop is paused/stopped, so offer Retry. */ + reconnecting: boolean; + attempt: number; + lastError: string; + onCancel: () => void; + onRetry: () => void; +}) { + const detail = lastError || "contacting the daemon…"; + return ( +
+
+ {reconnecting ? : null} + + {reconnecting ? "Reconnecting…" : "Disconnected"} + + + {reconnecting ? `Attempt ${attempt} — ${detail}` : detail} + + {reconnecting ? ( + + ) : ( + + )} +
+
+ ); +} diff --git a/client/src/MediaErrorToast.tsx b/client/src/MediaErrorToast.tsx new file mode 100644 index 0000000..3c96613 --- /dev/null +++ b/client/src/MediaErrorToast.tsx @@ -0,0 +1,40 @@ +/** Non-blocking toast for a failed media command (issue #64). + * + * The daemon sends a ``media_error`` frame naming the player and whether a + * retry is worth attempting; this renders it above the bottom chrome with + * Retry / Dismiss. It never blocks the grid — a rejected volume tap should + * not cost you the rest of the deck. ``aria-live="polite"`` announces it. + */ +export function MediaErrorToast({ + player, + message, + retryable, + onRetry, + onDismiss, +}: { + player: string; + message: string; + retryable: boolean; + onRetry: () => void; + onDismiss: () => void; +}) { + return ( +
+ + {player} + {" — "} + {message} + + + {retryable ? ( + + ) : null} + + +
+ ); +} diff --git a/client/src/playground/usePlaygroundDaemon.ts b/client/src/playground/usePlaygroundDaemon.ts index 3f7bb02..cc8f007 100644 --- a/client/src/playground/usePlaygroundDaemon.ts +++ b/client/src/playground/usePlaygroundDaemon.ts @@ -67,12 +67,19 @@ export function usePlaygroundDaemon( }, []); // Report "open" so the chrome connection indicator reads "live", matching - // how demo mode presents a fixture as a live surface. + // how demo mode presents a fixture as a live surface. The reconnect + // telemetry (issue #64) is inert — the MockDaemon can't disconnect. return { status: "open" as const, send, authenticate: noop, deauthenticate: noop, hasPassword: false, + attempt: 0, + lastError: "", + cancelled: false, + retrying: false, + cancelReconnect: noop, + retryNow: noop, }; } diff --git a/client/src/protocol.generated.ts b/client/src/protocol.generated.ts index c922747..c336ada 100644 --- a/client/src/protocol.generated.ts +++ b/client/src/protocol.generated.ts @@ -77,6 +77,17 @@ export type DeckMessage = { * decks live so a fix on disk restores service without a restart. */ web_app?: boolean; + /** + * Structured companions to ``error`` (issue #64): the human-readable + * deck name and the offending widget, when the daemon could identify + * them. The raw ``error`` text (a pydantic dump) is precise but + * machine-shaped; these let the client say *which deck* and *which + * widget* in plain language. Both are ``None`` for non-deck errors + * (e.g. a chrome-view not found) and when the location is unknown. + */ + error?: string | null; + + error_deck?: string | null; /** * The currently focused app's identity, populated when the daemon has a * focus backend (never in headless mode). ``None`` before the first focus @@ -84,7 +95,7 @@ export type DeckMessage = { * prefill ``match`` tokens for the detect-and-offer prompt and the * browser-vs-site branch. */ - error?: string | null; + error_widget?: string | null; /** * True only on a genuine focus-driven fallback to the default deck — * i.e. the resolution missed every loaded deck and ``store.default()`` @@ -214,6 +225,42 @@ export type ErrorMessage = { reason: string; }; +/** + * Daemon -> client push: a media command the client issued failed + * (issue #64). + * + * The client's media surfaces (the VLC ``media`` widget and the + * ``nowplaying`` rows) are fire-and-forget: a command to a player that + * disappeared between the last snapshot and the tap, or one the player + * rejected, previously vanished into a daemon log. This frame tells the + * *requesting* session what happened so it can explain which player + * was affected and offer a retry — it is not broadcast (the failure is + * a consequence of one session's action). + * + * ``id`` echoes the widget / row id the command targeted (the same id + * the client sent), so the client can correlate and re-send. ``player`` + * is the human-readable player name (MPRIS ``Identity``, or the VLC + * widget's label); the daemon falls back to the raw id when it can't + * name the player, but the field stays nullable so an older server + * leaves the client's own fallback reachable. ``retryable`` is ``True`` + * for every failure the daemon reports today (a vanished player or a + * rejected call can both succeed on a retry) and defaults to ``True``; + * it is reserved for a future failure a retry can't help. + * + */ +export type MediaErrorMessage = { + + type: "media_error"; + + id: string; + + player?: string | null; + + message: string; + + retryable?: boolean; +}; + /** * Daemon -> client push: the chrome media icon's passive * playback-state snapshot (issue #47). @@ -414,7 +461,7 @@ export type RunningWindowsMessage = { windows: WindowListEntry[]; }; -export type ServerMessage = DeckMessage | StateMessage | BrightnessMessage | WidgetUpdateMessage | MediaStateMessage | ChromeMediaMessage | EventMessage | MacroResultMessage | ConfirmRequestMessage | RunningWindowsMessage | ErrorMessage; +export type ServerMessage = DeckMessage | StateMessage | BrightnessMessage | WidgetUpdateMessage | MediaStateMessage | MediaErrorMessage | ChromeMediaMessage | EventMessage | MacroResultMessage | ConfirmRequestMessage | RunningWindowsMessage | ErrorMessage; export type HelloMessage = { diff --git a/client/src/protocol.ts b/client/src/protocol.ts index 5516c47..d8ea28f 100644 --- a/client/src/protocol.ts +++ b/client/src/protocol.ts @@ -27,6 +27,7 @@ import type { MediaStateMessage, ChromeMediaMessage, ErrorMessage, + MediaErrorMessage, MacroResultMessage, ConfirmRequestMessage, RunningWindowsMessage, @@ -58,6 +59,7 @@ export type { BrightnessMessage, WidgetUpdateMessage, MediaStateMessage, + MediaErrorMessage, ChromeMediaMessage, EventMessage, MacroResultMessage, @@ -181,6 +183,11 @@ export interface ServerDeck { icon?: Icon | null; web_app?: boolean; error?: string | null; + /** Structured companions to ``error`` (issue #64): the human deck name + * and offending widget, when the daemon could identify them. Both are + * ``null``/absent for non-deck errors (e.g. a chrome view not found). */ + error_deck?: string | null; + error_widget?: string | null; focused_app?: FocusedAppInfo | null; is_default?: boolean; } @@ -189,6 +196,7 @@ export type ServerBrightness = BrightnessMessage; export type ServerWidgetUpdate = WidgetUpdateMessage; export type ServerChromeMedia = ChromeMediaMessage; export type ServerError = ErrorMessage; +export type ServerMediaError = MediaErrorMessage; export type ServerMacroResult = MacroResultMessage; export type ServerConfirmRequest = ConfirmRequestMessage; // WindowListEntry carries an opaque ``icon`` field on the wire but the diff --git a/client/src/socket.test.ts b/client/src/socket.test.ts new file mode 100644 index 0000000..4ed2932 --- /dev/null +++ b/client/src/socket.test.ts @@ -0,0 +1,118 @@ +/** Reconnect telemetry on the socket hook (issue #64). + * + * The App renders the reconnecting overlay from facts the hook owns: + * the attempt counter, the last error, and whether the user cancelled the + * auto-retry loop. These tests drive a stub WebSocket so those facts can + * be asserted without a daemon. + */ +import { act, cleanup, renderHook, waitFor } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { useDeckdSocket } from "./socket"; + +class FakeWebSocket { + static instances: FakeWebSocket[] = []; + static OPEN = 1; + url: string; + readyState = 0; + sent: string[] = []; + onopen: (() => void) | null = null; + onclose: ((ev: { code: number; reason: string }) => void) | null = null; + onmessage: ((ev: { data: string }) => void) | null = null; + onerror: (() => void) | null = null; + + constructor(url: string) { + this.url = url; + FakeWebSocket.instances.push(this); + } + send(data: string) { + this.sent.push(data); + } + close() { + this.readyState = 3; + } + open() { + this.readyState = FakeWebSocket.OPEN; + this.onopen?.(); + } + fail(code: number, reason = "") { + this.readyState = 3; + this.onclose?.({ code, reason }); + } +} + +const noop = () => {}; + +function hookArgs() { + // Stable identities: a fresh callback per render would re-run the hook's + // connect effect and spin up a feedback loop of sockets. + return [noop, noop, noop] as const; +} + +describe("useDeckdSocket — reconnect feedback (issue #64)", () => { + beforeEach(() => { + FakeWebSocket.instances = []; + vi.stubGlobal("WebSocket", FakeWebSocket); + vi.stubGlobal( + "fetch", + vi.fn(() => Promise.resolve(new Response(null, { status: 200 }))), + ); + window.history.replaceState(null, "", "/"); + }); + afterEach(() => { + // Vitest globals are off, so RTL's auto-cleanup can't register itself; + // unmount explicitly or a pending retry timer leaks into the next test. + cleanup(); + vi.unstubAllGlobals(); + }); + + it("resets attempt on open and records the last error on close", async () => { + const { result } = renderHook(() => useDeckdSocket(...hookArgs())); + await waitFor(() => expect(FakeWebSocket.instances.length).toBe(1)); + act(() => FakeWebSocket.instances[0].open()); + expect(result.current.status).toBe("open"); + expect(result.current.attempt).toBe(0); + expect(result.current.lastError).toBe(""); + + act(() => FakeWebSocket.instances[0].fail(1006)); + expect(result.current.status).toBe("closed"); + expect(result.current.lastError).toBe("daemon not reachable"); + }); + + it("counts attempts as the retry loop runs", async () => { + const { result } = renderHook(() => useDeckdSocket(...hookArgs())); + await waitFor(() => expect(FakeWebSocket.instances.length).toBe(1)); + act(() => FakeWebSocket.instances[0].open()); + act(() => FakeWebSocket.instances[0].fail(1006)); + + // The 500 ms backoff fires and opens a second socket. + await waitFor( + () => expect(FakeWebSocket.instances.length).toBe(2), + { timeout: 2000 }, + ); + await waitFor( + () => expect(result.current.attempt).toBe(1), + { timeout: 2000 }, + ); + }); + + it("cancel stops the retry loop; retry resumes it", async () => { + const { result } = renderHook(() => useDeckdSocket(...hookArgs())); + await waitFor(() => expect(FakeWebSocket.instances.length).toBe(1)); + act(() => FakeWebSocket.instances[0].open()); + act(() => FakeWebSocket.instances[0].fail(1006)); + + act(() => result.current.cancelReconnect()); + expect(result.current.cancelled).toBe(true); + + // No new socket while cancelled, even after the backoff window. + await new Promise((resolve) => setTimeout(resolve, 700)); + expect(FakeWebSocket.instances.length).toBe(1); + + act(() => result.current.retryNow()); + await waitFor( + () => expect(FakeWebSocket.instances.length).toBe(2), + { timeout: 2000 }, + ); + expect(result.current.cancelled).toBe(false); + }); +}); diff --git a/client/src/socket.ts b/client/src/socket.ts index d003c2f..3b7aa63 100644 --- a/client/src/socket.ts +++ b/client/src/socket.ts @@ -1,5 +1,5 @@ import { useCallback, useEffect, useRef, useState } from "react"; -import type { ClientMessage, ServerChromeMedia, ServerConfirmRequest, ServerDeck, ServerMessage, ServerRunningWindows, ServerState, ServerWidgetUpdate, MediaState } from "./protocol"; +import type { ClientMessage, ServerChromeMedia, ServerConfirmRequest, ServerDeck, ServerMediaError, ServerMessage, ServerRunningWindows, ServerState, ServerWidgetUpdate, MediaState } from "./protocol"; import { wireDeckToServer, wireWindowsToServer } from "./protocol"; type Status = "connecting" | "open" | "closed" | "unauthorized"; @@ -41,6 +41,16 @@ function readPinnedDeck(): string { } } +/** Human-readable one-liner for a socket close, shown in the reconnect + * overlay (issue #64). Keyed off the WebSocket close code so a dropped + * daemon reads differently from a deliberate close. */ +function describeClose(code: number, reason: string): string { + if (code === 1006) return "daemon not reachable"; + if (code === 1000) return "connection closed"; + const suffix = reason ? ` — ${reason}` : ""; + return `connection lost (code ${code})${suffix}`; +} + export function useDeckdSocket( onDeck: (m: ServerDeck) => void, onWidgetUpdate: (m: ServerWidgetUpdate) => void, @@ -49,6 +59,7 @@ export function useDeckdSocket( onConfirmRequest?: (m: ServerConfirmRequest) => void, onRunningWindows?: (m: ServerRunningWindows) => void, onSessionState?: (m: ServerState) => void, + onMediaError?: (m: ServerMediaError) => void, options: { enabled?: boolean } = {}, ) { const { enabled = true } = options; @@ -68,14 +79,38 @@ export function useDeckdSocket( const unauthorizedRef = useRef(false); // Bumped by ``authenticate`` to force the connect effect to re-run. const [gen, setGen] = useState(0); + // Reconnect feedback (issue #64). ``attempt`` counts connect attempts + // since the last successful open; ``lastError`` is the human-readable + // reason for the latest failure; ``cancelled`` is true after the user + // presses Cancel and pauses the auto-retry loop. All three drive the + // reconnecting overlay. + const [attempt, setAttempt] = useState(0); + const [lastError, setLastError] = useState(""); + const [cancelled, setCancelled] = useState(false); + // True while the auto-retry loop is in flight (dialing or waiting out the + // backoff) and not cancelled. Distinguishes "actively reconnecting" from + // "given up" for the overlay: a drop parks in ``closed`` for the whole + // backoff, so ``status`` alone would mislabel the wait as Disconnected. + const [retrying, setRetrying] = useState(false); + const attemptRef = useRef(0); + // Read inside the effect's scheduler; a plain ref so a Cancel can stop + // a pending timer without re-running the effect (which would tear down + // the socket that's already gone). + const cancelledRef = useRef(false); useEffect(() => { if (!enabled) return; + if (cancelledRef.current) { + setStatus("closed"); + return; + } let stopped = false; let timer: number | undefined; const connect = () => { - if (stopped) return; + if (stopped || cancelledRef.current) return; + attemptRef.current += 1; + setAttempt(attemptRef.current); const ws_url = resolve_ws_url(); let ws: WebSocket; try { @@ -83,6 +118,8 @@ export function useDeckdSocket( } catch (err) { console.error("invalid deckd WebSocket URL", ws_url, err); setStatus("closed"); + setLastError("could not open a connection"); + setRetrying(true); timer = window.setTimeout(connect, Math.min(backoffRef.current, 8000)); backoffRef.current *= 2; return; @@ -92,7 +129,11 @@ export function useDeckdSocket( ws.onopen = () => { setStatus("open"); + setRetrying(false); backoffRef.current = 500; + attemptRef.current = 0; + setAttempt(0); + setLastError(""); const password = passwordRef.current; const pinnedDeck = readPinnedDeck(); const hello: ClientMessage = { @@ -138,6 +179,11 @@ export function useDeckdSocket( else if (msg.type === "state" && onSessionState) { onSessionState(msg); } + // Issue #64: a media command this session issued failed. Surface + // it so the client can name the player and offer a retry. + else if (msg.type === "media_error" && onMediaError) { + onMediaError(msg); + } else if (msg.type === "error" && msg.reason === "unauthorized") { // Wrong/absent password: stop reconnecting and prompt the user. unauthorizedRef.current = true; @@ -166,11 +212,13 @@ export function useDeckdSocket( setStatus("unauthorized"); return; } - if (stopped || unauthorizedRef.current) { + if (stopped || unauthorizedRef.current || cancelledRef.current) { if (!unauthorizedRef.current) setStatus("closed"); return; } setStatus("closed"); + setLastError(describeClose(ev.code, ev.reason)); + setRetrying(true); const wait = Math.min(backoffRef.current, 8000); backoffRef.current = wait * 2; timer = window.setTimeout(connect, wait); @@ -185,12 +233,14 @@ export function useDeckdSocket( // doesn't log a connection error when the daemon is still starting up // (the `just dev` recipe starts daemon and Vite simultaneously). const healthUrl = resolve_health_url(); - // eslint-disable-next-line react-hooks/set-state-in-effect setStatus("connecting"); fetch(healthUrl) .then(() => connect()) .catch(() => { - if (!stopped) { + if (!stopped && !cancelledRef.current) { + setStatus("closed"); + setLastError("daemon not reachable"); + setRetrying(true); timer = window.setTimeout(() => connect(), 1000); } }); @@ -199,22 +249,31 @@ export function useDeckdSocket( if (timer) window.clearTimeout(timer); wsRef.current?.close(); }; - }, [onDeck, onWidgetUpdate, onMediaState, onChromeMedia, onConfirmRequest, onRunningWindows, onSessionState, enabled, gen]); + }, [onDeck, onWidgetUpdate, onMediaState, onChromeMedia, onConfirmRequest, onRunningWindows, onSessionState, onMediaError, enabled, gen]); const send = (msg: ClientMessage) => { const ws = wsRef.current; if (ws && ws.readyState === WebSocket.OPEN) ws.send(JSON.stringify(msg)); }; + // Clear the reconnect latches and kick off a fresh connect. Shared by the + // password actions and by the overlay's Retry so the flag/backoff/gen reset + // lives in exactly one place. + const restartConnection = useCallback(() => { + cancelledRef.current = false; + setCancelled(false); + backoffRef.current = 500; + setGen((g) => g + 1); + }, []); + // Store a new password and force an immediate reconnect that presents it. const authenticate = useCallback((password: string) => { passwordRef.current = password; storePassword(password); setHasPassword(!!password); unauthorizedRef.current = false; - backoffRef.current = 500; - setGen((g) => g + 1); - }, []); + restartConnection(); + }, [restartConnection]); // Forget the stored password and reconnect with none — the daemon (auth on) // then rejects and the gate reappears. A no-op-looking reconnect if the @@ -224,11 +283,43 @@ export function useDeckdSocket( storePassword(""); setHasPassword(false); unauthorizedRef.current = false; - backoffRef.current = 500; - setGen((g) => g + 1); + restartConnection(); + }, [restartConnection]); + + // Pause the automatic reconnect loop (issue #64). The pending retry + // timer bails on ``cancelledRef``, so the loop stops without needing to + // reach into the effect's local timer handle. + const cancelReconnect = useCallback(() => { + cancelledRef.current = true; + setCancelled(true); + setRetrying(false); + setStatus("closed"); + wsRef.current?.close(); }, []); - return { status, send, authenticate, deauthenticate, hasPassword }; + // Resume immediately from a cancelled or closed state (issue #64). The + // generation bump tears down the old effect and starts a fresh connect + // (including the /health gate) right away. + const retryNow = useCallback(() => { + attemptRef.current = 0; + setAttempt(0); + setLastError(""); + restartConnection(); + }, [restartConnection]); + + return { + status, + send, + authenticate, + deauthenticate, + hasPassword, + attempt, + lastError, + cancelled, + retrying, + cancelReconnect, + retryNow, + }; } function resolve_health_url(): string { diff --git a/client/src/style.css b/client/src/style.css index 034e484..461e524 100644 --- a/client/src/style.css +++ b/client/src/style.css @@ -97,6 +97,8 @@ button:focus-visible { padding: 10px; min-height: 0; min-width: 0; + /* Anchors the reconnect overlay (issue #64). */ + position: relative; } .chrome-jogstrip { @@ -754,6 +756,166 @@ button:focus-visible { max-width: 30ch; } +.lock-takeover-cause { + color: #f0b429; + font-size: calc(13px * var(--label-scale, 1)); + max-width: 34ch; +} + +/* --------------------------------------------------------------------------- + * Reconnect / disconnected overlay (issue #64). Sits over the last-rendered + * surface, naming the attempt count and the last error, with a single + * Cancel/Retry control. The spinner is animation-only and stops under + * prefers-reduced-motion (both the OS query and the per-device class). + * ------------------------------------------------------------------------- */ +.conn-overlay { + position: absolute; + inset: 10px; + z-index: 5; + display: flex; + align-items: center; + justify-content: center; + background: color-mix(in srgb, #0b0f14 72%, transparent); + backdrop-filter: blur(3px); + border-radius: 16px; +} + +.conn-overlay-card { + display: flex; + flex-direction: column; + align-items: center; + gap: 10px; + padding: 22px 26px; + border-radius: 14px; + background: #11161b; + border: 1px solid #2a333d; + text-align: center; + max-width: 34ch; +} + +.conn-overlay-spinner { + width: 22px; + height: 22px; + border-radius: 50%; + border: 3px solid #2a333d; + border-top-color: #7dd3fc; + animation: conn-spin 900ms linear infinite; +} + +@keyframes conn-spin { + to { + transform: rotate(360deg); + } +} + +.conn-overlay-title { + font-size: calc(17px * var(--label-scale, 1)); + font-weight: 700; + color: #e6ebf0; +} + +.conn-overlay-sub { + font-size: calc(13px * var(--label-scale, 1)); + color: #8a96a3; +} + +.conn-overlay-btn { + margin-top: 2px; + padding: 8px 18px; + border-radius: 10px; + border: 1px solid #2a333d; + background: #1b222a; + color: #e6ebf0; + font-weight: 600; +} + +.conn-overlay-btn:active { + transform: scale(0.97); +} + +@media (prefers-reduced-motion: reduce) { + .conn-overlay-spinner { + animation: none; + } + .conn-overlay-btn:active { + transform: none; + } + .media-error-toast { + animation: none; + } + .deck-error-return:active { + transform: none; + } +} + +.a11y-reduce-motion .conn-overlay-spinner { + animation: none; +} + +.a11y-reduce-motion .conn-overlay-btn:active { + transform: none; +} + +/* --------------------------------------------------------------------------- + * Media-command failure toast (issue #64). Non-blocking, above the bottom + * chrome, with Retry / Dismiss. Slide-in is disabled under reduced motion. + * ------------------------------------------------------------------------- */ +.media-error-toast { + position: fixed; + left: 50%; + bottom: calc(64px + env(safe-area-inset-bottom)); + transform: translateX(-50%); + z-index: 30; + display: flex; + align-items: center; + gap: 12px; + max-width: min(92vw, 520px); + padding: 10px 14px; + border-radius: 12px; + background: #2a1b1b; + border: 1px solid #7f1d1d; + color: #fecaca; + box-shadow: 0 8px 24px rgba(0, 0, 0, 0.45); + animation: media-toast-in 160ms ease-out; +} + +@keyframes media-toast-in { + from { + opacity: 0; + transform: translate(-50%, 8px); + } +} + +.media-error-text { + font-size: calc(13px * var(--label-scale, 1)); + line-height: 1.35; +} + +.media-error-player { + font-weight: 700; + color: #fecaca; +} + +.media-error-actions { + display: flex; + gap: 6px; + flex-shrink: 0; +} + +.media-error-btn { + padding: 6px 12px; + border-radius: 8px; + border: 1px solid #7f1d1d; + background: #3a2222; + color: #fee2e2; + font-size: 12px; + font-weight: 600; +} + +.a11y-reduce-motion .media-error-toast { + animation: none; +} + .lock-banner { display: flex; align-items: center; @@ -1023,6 +1185,33 @@ button:focus-visible { color: #fecaca; } +.deck-error-summary { + font-size: clamp(13px, 2.2vw, 16px); + color: #fecaca; +} + +.deck-error-summary strong { + color: #fff; +} + +.deck-error-return { + align-self: flex-start; + padding: 8px 14px; + border-radius: 10px; + border: 1px solid #7f1d1d; + background: #2a1b1b; + color: #fee2e2; + font-weight: 600; +} + +.deck-error-return:active { + transform: scale(0.97); +} + +.a11y-reduce-motion .deck-error-return:active { + transform: none; +} + /* --------------------------------------------------------------------------- * Password gate (issue #16): full-surface landing shown when the daemon * rejects a remote connection as unauthorized. A single password field. diff --git a/daemon/deckd/decks.py b/daemon/deckd/decks.py index 4c7b430..d6a9dc0 100644 --- a/daemon/deckd/decks.py +++ b/daemon/deckd/decks.py @@ -464,12 +464,101 @@ def matches_identity(self, app: AppInfo) -> bool: return False +class DeckLoadError(SystemExit): + """A deck file failed to load — YAML syntax or schema violation + (issue #64). + + ``str(exc)`` stays the precise, machine-shaped message (the pydantic + dump, kept for logs and the wire's ``error`` field). ``deck`` and + ``widget`` are best-effort *human* identifiers so the client can say + which deck and which widget are affected without parsing the dump. + """ + + def __init__( + self, + message: str, + *, + deck: str | None = None, + widget: str | None = None, + ) -> None: + super().__init__(message) + self.message = message + self.deck = deck + self.widget = widget + + +def _identify_deck(data: Any, path: Path) -> str: + """Best-effort human deck name from the raw (unvalidated) YAML. + + The file has already failed validation, so there's no ``Deck`` object + to read an id from. Prefer the authored ``id`` / ``display_name`` / + first ``match`` token, falling back to the filename stem (which is + how a broken deck is easiest to find on disk). + """ + if isinstance(data, dict): + for key in ("id", "display_name"): + value = data.get(key) + if isinstance(value, str) and value.strip(): + return value.strip() + match = data.get("match") + if ( + isinstance(match, list) + and match + and isinstance(match[0], str) + and match[0].strip() + ): + return match[0].strip() + return path.stem + + +def _identify_widget(data: Any, errors: list[Any]) -> str | None: + """Best-effort offending-widget name from a pydantic error list. + + A field error under ``widgets.`` names the widget by its authored + ``id`` (or ``#`` when even that is missing). A model-level error + (the duplicate-id validator) has no ``loc`` but embeds the id in its + message, so scan for that too. + """ + for err in errors: + loc = err.get("loc") or () + if len(loc) >= 2 and loc[0] == "widgets" and isinstance(loc[1], int): + widgets = data.get("widgets") if isinstance(data, dict) else None + if isinstance(widgets, list) and 0 <= loc[1] < len(widgets): + widget = widgets[loc[1]] + if ( + isinstance(widget, dict) + and isinstance(widget.get("id"), str) + and widget["id"] + ): + return widget["id"] + return f"widget #{loc[1]}" + for err in errors: + msg = err.get("msg") or "" + found = re.search(r"duplicate widget id: ['\"]([^'\"]+)['\"]", msg) + if found: + return found.group(1) + return None + + def load_deck(path: Path) -> Deck: - data = yaml.safe_load(path.read_text()) + try: + data = yaml.safe_load(path.read_text()) + except yaml.YAMLError as exc: + # A syntax error never reaches pydantic and — before issue #64 — + # never set ``current_error`` either, so a malformed file left the + # last-good deck on screen with no indication anything was wrong. + raise DeckLoadError( + f"invalid deck YAML at {path}:\n{exc}", + deck=path.stem, + ) from exc try: deck = Deck.model_validate(data) except ValidationError as exc: - raise SystemExit(f"invalid deck YAML at {path}:\n{exc}") from exc + raise DeckLoadError( + f"invalid deck YAML at {path}:\n{exc}", + deck=_identify_deck(data, path), + widget=_identify_widget(data, exc.errors()), + ) from exc if deck.match: deck.id = deck.match[0] return deck @@ -695,20 +784,17 @@ def _record(deck: Deck, path: Path) -> None: for path in sorted(overlay_dir.glob("*.y*ml")): if path.suffix not in {".yaml", ".yml"}: continue - try: - deck = load_deck(path) - except SystemExit as exc: - raise SystemExit(f"{exc}") from None + # ``load_deck`` raises a ``DeckLoadError`` (a ``SystemExit`` + # subclass) that already carries the structured deck/widget + # identity — let it propagate untouched. + deck = load_deck(path) _record(deck, path) overlay_ids = {deck.id for deck in decks if deck.id} for path in sorted(decks_dir.glob("*.y*ml")): if path.suffix not in {".yaml", ".yml"}: continue - try: - deck = load_deck(path) - except SystemExit as exc: - raise SystemExit(f"{exc}") from None + deck = load_deck(path) if deck.id and deck.id in overlay_ids: log.info("deck %r overridden by overlay %s", deck.id, path) continue diff --git a/daemon/deckd/mpris.py b/daemon/deckd/mpris.py index 6e0ccdb..542f20d 100644 --- a/daemon/deckd/mpris.py +++ b/daemon/deckd/mpris.py @@ -184,12 +184,36 @@ class NowPlaying(BaseModel): empty_state: NowPlayingEmptyState = "show" +class MprisCommandError(Exception): + """A media command couldn't be delivered (issue #64). + + Raised by :meth:`DbusMprisBackend.send_command` when the target row + vanished (the player quit between the last snapshot and the tap) or + the player's D-Bus method rejected the call. The server turns it into + a ``media_error`` frame for the requesting session. Unknown *commands* + stay silent no-ops — a client never legitimately sends one, and there + is nothing useful to retry. + """ + + def __init__(self, row_id: str, message: str, *, retryable: bool = True) -> None: + super().__init__(message) + self.row_id = row_id + self.message = message + self.retryable = retryable + + class MprisBackend(Protocol): """Backend seam for enumerating MPRIS rows and controlling them.""" def row_ids(self) -> list[str]: ... + def identity(self, row_id: str) -> str | None: + """The row's cached human-readable player name (MPRIS + ``Identity``), or ``None`` when unknown. Used to name the player + in a ``media_error`` frame (issue #64).""" + ... + async def read_state(self, row_id: str) -> MediaState | None: ... @@ -346,6 +370,12 @@ def __init__(self, states: dict[str, MediaState] | None = None) -> None: def row_ids(self) -> list[str]: return list(self.states) + def identity(self, row_id: str) -> str | None: + """No live bus, so fall back to a seeded state's ``app_name`` + (issue #64).""" + state = self.states.get(row_id) + return getattr(state, "app_name", None) if state is not None else None + async def read_state(self, row_id: str) -> MediaState | None: state = self.states.get(row_id) if state is None: @@ -1017,22 +1047,32 @@ async def _read_identity(self, row_id: str) -> str | None: self._identities[row_id] = identity return identity + def identity(self, row_id: str) -> str | None: + """The row's cached MPRIS ``Identity`` (issue #64).""" + return self._identities.get(row_id) + async def send_command(self, row_id: str, command: str) -> None: """Dispatch a browser command to the corresponding MPRIS method. - Unknown rows, unknown commands, and bus errors are all no-ops - the server's pump catches and logs — the wire side never sees - a failure, only a log line. + Unknown *commands* are no-ops (the wire literal makes one + impossible from a real client). A row that vanished before the + command arrived, or a player that rejected the method, raises + :class:`MprisCommandError` so the server can explain the failure + to the requesting session (issue #64). + + Diagnostic event emission is unchanged: successful dispatches + still publish ``command``; failures publish ``dbus_error`` first. """ if self._bus is None or row_id not in self._owned_names: - return + raise MprisCommandError(row_id, "player disappeared") method = _COMMANDS.get(command) if method is None: return + from dbus_fast import MessageType from dbus_fast.message import Message try: - await self._bus.call( + reply = await self._bus.call( Message( destination=f"{MPRIS_BUS_PREFIX}.{row_id}", path=MPRIS_OBJECT_PATH, @@ -1040,7 +1080,6 @@ async def send_command(self, row_id: str, command: str) -> None: member=method, ) ) - self._emit_diagnostic("command", row_id, {"command": command}) except Exception as exc: log.warning( "MPRIS %s.%s on %s failed: %s", @@ -1054,6 +1093,19 @@ async def send_command(self, row_id: str, command: str) -> None: row_id, {"command": command, "error": repr(exc)}, ) + raise MprisCommandError(row_id, "player rejected the command") from exc + if getattr(reply, "message_type", None) == MessageType.ERROR: + # A rejected method comes back as an ERROR *reply* (dbus-fast + # returns it rather than raising), so checking the reply type + # is the only way to notice it. + log.warning("MPRIS %s.%s on %s was rejected", PLAYER_INTERFACE, method, row_id) + self._emit_diagnostic( + "dbus_error", + row_id, + {"command": command, "error": "method rejected"}, + ) + raise MprisCommandError(row_id, "player rejected the command") + self._emit_diagnostic("command", row_id, {"command": command}) def _playback_to_playing( diff --git a/daemon/deckd/protocol.py b/daemon/deckd/protocol.py index 962378b..47be106 100644 --- a/daemon/deckd/protocol.py +++ b/daemon/deckd/protocol.py @@ -75,6 +75,14 @@ class DeckMessage(BaseModel): # message in place of the widget grid; the daemon keeps the last-good # decks live so a fix on disk restores service without a restart. error: str | None = None + # Structured companions to ``error`` (issue #64): the human-readable + # deck name and the offending widget, when the daemon could identify + # them. The raw ``error`` text (a pydantic dump) is precise but + # machine-shaped; these let the client say *which deck* and *which + # widget* in plain language. Both are ``None`` for non-deck errors + # (e.g. a chrome-view not found) and when the location is unknown. + error_deck: str | None = None + error_widget: str | None = None # The currently focused app's identity, populated when the daemon has a # focus backend (never in headless mode). ``None`` before the first focus # event arrives. The editor's new-deck creation flow (#104) uses this to @@ -176,6 +184,38 @@ class ErrorMessage(BaseModel): reason: str +class MediaErrorMessage(BaseModel): + """Daemon -> client push: a media command the client issued failed + (issue #64). + + The client's media surfaces (the VLC ``media`` widget and the + ``nowplaying`` rows) are fire-and-forget: a command to a player that + disappeared between the last snapshot and the tap, or one the player + rejected, previously vanished into a daemon log. This frame tells the + *requesting* session what happened so it can explain which player + was affected and offer a retry — it is not broadcast (the failure is + a consequence of one session's action). + + ``id`` echoes the widget / row id the command targeted (the same id + the client sent), so the client can correlate and re-send. ``player`` + is the human-readable player name (MPRIS ``Identity``, or the VLC + widget's label); the daemon falls back to the raw id when it can't + name the player, but the field stays nullable so an older server + leaves the client's own fallback reachable. ``retryable`` is ``True`` + for every failure the daemon reports today (a vanished player or a + rejected call can both succeed on a retry) and defaults to ``True``; + it is reserved for a future failure a retry can't help. + """ + + model_config = ConfigDict(extra="forbid") + + type: Literal["media_error"] + id: str + player: str | None = None + message: str + retryable: bool = True + + class ChromeMediaMessage(BaseModel): """Daemon -> client push: the chrome media icon's passive playback-state snapshot (issue #47). @@ -341,7 +381,7 @@ class RunningWindowsMessage(BaseModel): ServerMessage = Annotated[ - Union[DeckMessage, StateMessage, BrightnessMessage, WidgetUpdateMessage, MediaStateMessage, ChromeMediaMessage, EventMessage, MacroResultMessage, ConfirmRequestMessage, RunningWindowsMessage, ErrorMessage], + Union[DeckMessage, StateMessage, BrightnessMessage, WidgetUpdateMessage, MediaStateMessage, MediaErrorMessage, ChromeMediaMessage, EventMessage, MacroResultMessage, ConfirmRequestMessage, RunningWindowsMessage, ErrorMessage], Field(discriminator="type"), ] diff --git a/daemon/deckd/server.py b/daemon/deckd/server.py index 4990a1e..62f8080 100644 --- a/daemon/deckd/server.py +++ b/daemon/deckd/server.py @@ -23,6 +23,7 @@ from .input import ScrollController, parse_key_combo, text_to_combos from .decks import ( Deck, + DeckLoadError, DeckStore, icon_for_window, label_for_window, @@ -33,7 +34,13 @@ slugify_deck_id, ) from .media import MediaManager, MediaState, effective_art_token -from .mpris import ChromeMediaState, DbusMprisBackend, MprisBackend, connect_mpris_backend +from .mpris import ( + ChromeMediaState, + DbusMprisBackend, + MprisBackend, + MprisCommandError, + connect_mpris_backend, +) from .mpris_art import resolve_mpris_art_async from .diagnostics import ( ActionRecord, @@ -446,6 +453,12 @@ async def push_current(self) -> None: deck = self.server.current_deck app_id = self.server.current_app_id error = self.server.current_error + # Structured companions to the global deck error (issue #64). They + # stay ``None`` unless the *global* on-disk deck error is what we + # end up sending — a pin, a resolved chrome view, or a view-not-found + # error carries no deck/widget identity of its own. + error_deck = self.server.current_error_deck + error_widget = self.server.current_error_widget # Pinned demo session: render the named deck regardless of host focus. # Re-resolved from the store each push, so a deck-file edit + reload # refreshes the pinned view; if the deck was removed on reload, fall @@ -455,6 +468,8 @@ async def push_current(self) -> None: deck = self.server.decks[pin] app_id = pin error = None + error_deck = None + error_widget = None # Chrome view pin (issue #50): render the selected view's deck # regardless of host focus, with ``view`` set so the client knows to # stay on this chrome view. The view id is the synthetic token the @@ -472,10 +487,14 @@ async def push_current(self) -> None: deck = self.server.decks[self.view] app_id = self.view error = None + error_deck = None + error_widget = None view_id = self.view else: view_id = self.view view_error = "view not found" + error_deck = None + error_widget = None # Chrome app badge fields are relayed from the active deck even in # the error path: the bottom chrome remains the chrome, and a branded # badge is more useful than a bare match token while the user fixes @@ -530,6 +549,8 @@ async def push_current(self) -> None: is_default=is_default, widgets=[], error=error, + error_deck=error_deck, + error_widget=error_widget, ) else: widgets = [w.model_dump() for w in deck.widgets] @@ -728,6 +749,12 @@ async def _timed_call(message: DbusMessage) -> DbusMessage: # the shield (e.g. fully scripted kiosks). self._allow_while_locked = allow_while_locked self._current_error: str | None = None + # Structured companions to ``_current_error`` (issue #64): the + # best-effort human deck name and offending widget the error + # refers to, so the client can explain which deck/widget is + # broken without parsing the pydantic dump. + self._current_error_deck: str | None = None + self._current_error_widget: str | None = None self._deckd_window_focused = False # Stage 2 (#120 / #126): last pushed running-windows snapshot, # used to dedupe identical pushes (the daemon-side debounce that @@ -773,6 +800,14 @@ def current_app(self) -> "AppInfo | None": def current_error(self) -> str | None: return self._current_error + @property + def current_error_deck(self) -> str | None: + return self._current_error_deck + + @property + def current_error_widget(self) -> str | None: + return self._current_error_widget + def reload_decks(self) -> None: """Re-read every deck YAML in ``decks_dir`` (and overlay_dir). @@ -787,8 +822,22 @@ def reload_decks(self) -> None: self.metrics.deck_reload_total += 1 try: new_store = load_decks(self.decks_dir, self.overlay_dir) + except DeckLoadError as exc: + # Structured deck/widget identifiers (issue #64) ride the + # DeckMessage alongside the raw text so the client can name + # the broken deck and widget in plain language. + self._current_error = exc.message + self._current_error_deck = exc.deck + self._current_error_widget = exc.widget + self.metrics.deck_error_total += 1 + log.error("deck reload failed (keeping last-good): %s", exc) + return except SystemExit as exc: + # A non-file failure (e.g. the decks directory vanished): + # no deck/widget to name. self._current_error = str(exc) + self._current_error_deck = None + self._current_error_widget = None self.metrics.deck_error_total += 1 log.error("deck reload failed (keeping last-good): %s", exc) return @@ -804,6 +853,8 @@ def reload_decks(self) -> None: # the resolved object identity so the wire stays truthful. self._current_is_default = new_deck is self.decks.default() self._current_error = None + self._current_error_deck = None + self._current_error_widget = None self.metrics.deck_reload_ok_total += 1 # A deck reload can change which meter sources the active # deck uses (a meter added in the new YAML, an old one @@ -1208,6 +1259,34 @@ def _lock_gate_active(self) -> bool: """ return self._session_locked and not self._allow_while_locked + async def _send_media_error( + self, + session: Session, + *, + widget_id: str, + player: str, + message: str, + retryable: bool = True, + ) -> None: + """Report a failed media command to the requesting session (#64). + + One place owns the wire shape so the MPRIS and VLC paths can't + drift; a send failure is swallowed like every other per-session + push (the connection is already going away). + """ + try: + await session.send( + p.MediaErrorMessage( + type="media_error", + id=widget_id, + player=player, + message=message, + retryable=retryable, + ) + ) + except (ConnectionResetError, RuntimeError, ConnectionError): + pass + async def _refuse_locked(self, session: Session, what: str, primitive: str) -> None: """Record a lock-refused attempt and reply with an error frame. @@ -2099,6 +2178,13 @@ async def _mpris_command(self, req: web.Request) -> web.Response: return web.json_response( {"ok": True, "row_id": row_id, "command": payload.command} ) + except MprisCommandError as exc: + # Issue #64: a vanished row is a 404, a rejected method a + # 502 — both structured, rather than the old silent 200. + self.metrics.record_mpris_command(payload.command, ok=False) + log.warning("MPRIS command %s on %s failed: %s", payload.command, row_id, exc) + status = 404 if exc.message == "player disappeared" else 502 + return web.json_response({"ok": False, "error": exc.message}, status=status) except Exception as exc: self.metrics.record_mpris_command(payload.command, ok=False) log.warning("MPRIS command %s on %s failed: %s", payload.command, row_id, exc) @@ -2527,6 +2613,20 @@ async def _dispatch(self, session: Session, data: dict) -> None: media_command.id.removeprefix("mpris."), media_command.command ) self.metrics.record_mpris_command(media_command.command, ok=True) + except MprisCommandError as exc: + # Issue #64: tell the *requesting* session which player + # failed and whether a retry is worth offering. The row + # may already be gone, so name it from the cached + # Identity when we can, else fall back to the row id. + self.metrics.record_mpris_command(media_command.command, ok=False) + log.warning("MPRIS command %s failed: %s", media_command.command, exc) + await self._send_media_error( + session, + widget_id=media_command.id, + player=self.mpris.identity(exc.row_id) or exc.row_id, + message=exc.message, + retryable=exc.retryable, + ) except Exception as exc: self.metrics.record_mpris_command(media_command.command, ok=False) log.warning("MPRIS command %s failed: %s", media_command.command, exc) @@ -2547,7 +2647,15 @@ async def _dispatch(self, session: Session, data: dict) -> None: except Exception as exc: # A failed/unsupported media command must not tear down the # websocket session (which would reset the client UI). + # Issue #64: report it to the requesting session instead of + # leaving the tap silently dead. log.warning("media command %s failed: %s", media_command.command, exc) + await self._send_media_error( + session, + widget_id=media_command.id, + player=widget.label or widget.id, + message="player rejected the command", + ) return if msg_type == "select_view": select_view = p.SelectViewMessage.model_validate(data) diff --git a/docs/GUIDE.md b/docs/GUIDE.md index 306f631..36bc278 100644 --- a/docs/GUIDE.md +++ b/docs/GUIDE.md @@ -272,6 +272,17 @@ Every deck renders inside a persistent **chrome** shell that the daemon does not Widgets in a deck's `widgets:` list are an **ordered list** that reflows against the viewport width (ADR-0010). There are no grid coordinates. The client packs widgets left-to-right and wraps down, computing the column count from the available width against a client-side cell-size band. A widget may carry a `size: [w, h]` span (default `[1, 1]`) for non-uniform cells; the list order is the only positional input. Portrait just fits fewer columns — no transpose, no orientation conventions. +### Feedback and recovery + +Every non-happy state explains itself in plain language rather than leaving a stale or dead surface: + +- **Reconnecting / disconnected** — when the socket drops, an overlay sits over the last-rendered deck naming the attempt count and the last error (e.g. *Attempt 3 — daemon not reachable*). While the client is retrying it offers **Cancel** to pause the auto-retry loop; once paused (or when the socket is closed) it offers **Retry** to reconnect immediately. The bottom-strip connection dot keeps showing live / reconnecting / disconnected alongside it. +- **Screen locked / asleep** — the full-surface lock takeover names the cause (*Screensaver or lock screen active on ‹host›*) and states that input is held until you unlock. The soft "asleep" banner shown for a blank that hasn't locked says *suspend or display blank*, and a press wakes the machine. +- **Deck errors** — a deck that fails to load (bad YAML or a schema violation) replaces the grid with the pydantic detail plus a plain-language line naming the deck and the offending widget (`Couldn't load the "firefox" deck. Widget "back" is invalid.`) and a **Return to focused app** button that clears a pinned chrome view. A chrome view that no longer resolves shows *That view isn't available on this daemon.* with the same button. +- **Media errors** — a media command the daemon couldn't deliver (player gone, method rejected) surfaces a non-blocking toast naming the player (`VLC media player — player disappeared`) with **Retry** (re-sends the last command for that player) and **Dismiss**. + +Each of these is announced through the client's `aria-live` region, and every animation is suppressed under `prefers-reduced-motion: reduce` or the in-app *Reduce motion* setting. + ### Manual control mode Tap the `manual control` button in the bottom chrome and the deck area is replaced by a single combined surface: a **trackpad** for cursor movement and a **keyboard passthrough** for typing into the currently-focused desktop app, both live at the same time. No mode switching. The trackpad handles pointing and clicking, and a small **strip at the top** of the surface hosts the few keys mobile IMEs can't produce (Esc, Tab, arrows) plus a keyboard-icon toggle that raises the phone's soft keyboard when you want to type. When the IME is open you can still drag on the trackpad area to move the cursor — the two coexist. @@ -409,7 +420,7 @@ The client is usable end-to-end without a mouse (issues [#60](https://github.com **Focus restoration** — opening a chrome view (settings, trackpad, now playing) pushes focus into the first interactive element of that view; closing it (via `Escape` or the same button) hands focus back to the chrome button that opened it. The password gate also restores focus to the surface after a successful submit, so a keyboard user can Tab into the deck without clicking anywhere. -**OS-level preferences** — the theme respects `prefers-contrast: more` (thicker focus ring, higher-contrast cell borders, white halo on the connection dot) and `prefers-reduced-motion: reduce` (the connection-state pulse and the media-icon playback dot stop animating; press feedback loses its scale-down but keeps the static brightness shift). Status (connection state, playback state) is conveyed by **icon + text + colour** so it doesn't depend on colour alone: the connection indicator has a visible "live" / "reconnecting" / "disconnected" / "locked" label, and the media icon carries a screen-reader-only "now playing" / "idle" string alongside the pulsing green dot. +**OS-level preferences** — the theme respects `prefers-contrast: more` (thicker focus ring, higher-contrast cell borders, white halo on the connection dot) and `prefers-reduced-motion: reduce` (the connection-state pulse and the media-icon playback dot stop animating; press feedback loses its scale-down but keeps the static brightness shift). Status (connection state, playback state) is conveyed by **icon + text + colour** so it doesn't depend on colour alone: the connection indicator has a visible "live" / "reconnecting" / "disconnected" / "sign-in needed" label, and the media icon carries a screen-reader-only "now playing" / "idle" string alongside the pulsing green dot. **Typography** — labels scale with the browser zoom *and* the existing Text-size slider; cell sizes use `clamp(min, vw, max)` so they grow with the viewport. The Content-size and Bottom-bar sliders affect icon + chrome sizes without clipping adjacent content. diff --git a/tests/test_decks.py b/tests/test_decks.py index 1e43333..3af1275 100644 --- a/tests/test_decks.py +++ b/tests/test_decks.py @@ -12,7 +12,7 @@ import pytest -from deckd.decks import Deck, Widget, load_decks, resolve_deck +from deckd.decks import Deck, DeckLoadError, Widget, load_decks, resolve_deck from deckd.platform import AppInfo @@ -654,6 +654,86 @@ def test_action_terminal_string_rejected_with_guidance(tmp_path: Path) -> None: assert 'shell: "tilix"' in msg +# --------------------------------------------------------------------------- +# Structured load errors (issue #64) +# +# The raw pydantic dump is precise but machine-shaped. The client needs the +# deck name and the offending widget in plain language so its error surface +# can explain what broke instead of pasting a traceback. +# --------------------------------------------------------------------------- + + +def test_deck_load_error_names_deck_and_widget(tmp_path: Path) -> None: + """A widget field error surfaces the deck name and the widget id.""" + body = """ +match: + - firefox +widgets: + - id: back + kind: button + action: + key: "alt+Left" + - id: broken + kind: media + controls: [bogus-control] +""" + _write(tmp_path, "firefox.yaml", body) + with pytest.raises(DeckLoadError) as exc: + load_decks(tmp_path) + err = exc.value + assert err.deck == "firefox" + assert err.widget == "broken" + assert "controls" in str(err) + + +def test_deck_load_error_names_deck_from_display_name(tmp_path: Path) -> None: + """When the authored ``id`` is absent, the display name is used.""" + body = """ +match: + - firefox +display_name: Firefox +widgets: + - id: broken + kind: button + action: + nope: true +""" + _write(tmp_path, "firefox.yaml", body) + with pytest.raises(DeckLoadError) as exc: + load_decks(tmp_path) + assert exc.value.deck == "Firefox" + assert exc.value.widget == "broken" + + +def test_deck_load_error_finds_duplicate_widget_id(tmp_path: Path) -> None: + """The model-level duplicate-id error has no ``loc``; the id still + comes out so the UI can name the culprit.""" + body = """ +match: + - default +widgets: + - id: back + kind: button + - id: back + kind: button +""" + _write(tmp_path, "default.yaml", body) + with pytest.raises(DeckLoadError) as exc: + load_decks(tmp_path) + assert exc.value.widget == "back" + + +def test_yaml_syntax_error_is_a_structured_load_error(tmp_path: Path) -> None: + """A malformed YAML document used to escape as a bare yaml error and + silently keep the last-good deck live (issue #64). It now surfaces as + a ``DeckLoadError`` so the client shows something.""" + _write(tmp_path, "default.yaml", "match: [default\nwidgets: []\n") + with pytest.raises(DeckLoadError) as exc: + load_decks(tmp_path) + assert exc.value.deck == "default" + assert exc.value.widget is None + + # --------------------------------------------------------------------------- # Platform overlay # diff --git a/tests/test_feedback.py b/tests/test_feedback.py new file mode 100644 index 0000000..489a48f --- /dev/null +++ b/tests/test_feedback.py @@ -0,0 +1,223 @@ +"""Issue #64: reconnect, locked, and error feedback that explains and recovers. + +The client-facing half of these states lives in the React app; these tests +cover the daemon seam that has to *supply the facts* the client explains: + +* a failed deck reload now carries the deck name and offending widget so + the error surface can name them (instead of pasting a pydantic dump); +* a failed media command — MPRIS player gone / method rejected, or the VLC + HTTP call failing — is reported to the requesting session as a + ``media_error`` frame naming the player, rather than vanishing into a log. +""" +from __future__ import annotations + +import asyncio +import json +from pathlib import Path + +import aiohttp +import websockets +from aiohttp.test_utils import TestServer + +from conftest import make_test_server +from deckd.media import MediaState +from deckd.mpris import FakeMprisBackend, MprisCommandError + + +async def _recv_type(ws, wanted: str, timeout: float = 2.0) -> dict: + """Read frames until one of ``wanted`` type arrives (or time out).""" + while True: + frame = json.loads(await asyncio.wait_for(ws.recv(), timeout)) + if frame.get("type") == wanted: + return frame + + +# --------------------------------------------------------------------------- +# Deck reload error: structured deck / widget names on the wire +# --------------------------------------------------------------------------- + + +_GOOD = """ +match: [default] +widgets: + - id: home + kind: button +""" + +_BAD = """ +match: [default] +widgets: + - id: home + kind: button + action: + nope: true +""" + + +async def test_deck_reload_error_carries_deck_and_widget(tmp_path: Path) -> None: + (tmp_path / "default.yaml").write_text(_GOOD) + server, *_ = make_test_server(decks_dir=tmp_path) + test_server = TestServer(server.app, host="127.0.0.1") + await test_server.start_server() + try: + async with websockets.connect(f"ws://127.0.0.1:{test_server.port}/ws") as ws: + first = json.loads(await asyncio.wait_for(ws.recv(), 2)) + assert first["type"] == "deck" + assert first["error"] is None + + # Break the file on disk and reload; the daemon keeps the + # last-good deck but flags the error on the next push. + (tmp_path / "default.yaml").write_text(_BAD) + await server.reload_and_push() + + err = await _recv_type(ws, "deck") + assert err["error"], "raw error text still rides for diagnostics" + assert err["error_deck"] == "default" + assert err["error_widget"] == "home" + assert err["widgets"] == [] + finally: + await server.stop() + await test_server.close() + + +# --------------------------------------------------------------------------- +# Media errors: MPRIS player gone / command rejected +# --------------------------------------------------------------------------- + + +class _RaisingMpris(FakeMprisBackend): + """FakeMprisBackend whose commands always fail with a given error.""" + + def __init__(self, states: dict[str, MediaState], error: MprisCommandError) -> None: + super().__init__(states) + self._error = error + + async def send_command(self, row_id: str, command: str) -> None: + raise self._error + + def identity(self, row_id: str) -> str | None: + return "VLC media player" + + +async def test_mpris_media_error_is_reported_to_requester(tmp_path: Path) -> None: + (tmp_path / "default.yaml").write_text( + """ +match: [default] +widgets: + - id: browser + kind: nowplaying + size: [4, 2] +""" + ) + backend = _RaisingMpris( + {"vlc": MediaState(available=True, stale=False, playing=True, title="VLC")}, + MprisCommandError("vlc", "player disappeared"), + ) + server, *_ = make_test_server(decks_dir=tmp_path, mpris_backend=backend) + test_server = TestServer(server.app, host="127.0.0.1") + await test_server.start_server() + try: + async with websockets.connect(f"ws://127.0.0.1:{test_server.port}/ws") as ws: + await _recv_type(ws, "deck") + await ws.send( + json.dumps( + {"type": "media_command", "id": "mpris.vlc", "command": "next"} + ) + ) + err = await _recv_type(ws, "media_error") + assert err["id"] == "mpris.vlc" + assert err["player"] == "VLC media player" + assert err["retryable"] is True + assert "player disappeared" in err["message"] + finally: + await server.stop() + await test_server.close() + + +async def test_mpris_http_command_reports_missing_player(tmp_path: Path) -> None: + """The authenticated HTTP command endpoint mirrors the WS feedback: + a vanished row is a structured 404, not a silent 200 (issue #64).""" + (tmp_path / "default.yaml").write_text( + """ +match: [default] +widgets: + - id: browser + kind: nowplaying + size: [4, 2] +""" + ) + backend = _RaisingMpris( + {"vlc": MediaState(available=True, stale=False, playing=True, title="VLC")}, + MprisCommandError("vlc", "player disappeared"), + ) + server, *_ = make_test_server(decks_dir=tmp_path, mpris_backend=backend) + test_server = TestServer(server.app, host="127.0.0.1") + await test_server.start_server() + try: + async with aiohttp.ClientSession() as client: + resp = await client.post( + f"http://127.0.0.1:{test_server.port}/mpris/vlc/command", + json={"command": "next"}, + ) + assert resp.status == 404 + body = await resp.json() + assert body == {"ok": False, "error": "player disappeared"} + finally: + await server.stop() + await test_server.close() + + +class _FailingMediaManager: + def stop(self) -> None: + pass + + async def read(self, key: str, *, host: str, port: int, password_ref: str | None) -> MediaState: + return MediaState(available=True, stale=False, playing=True) + + async def command( + self, + key: str, + command: str, + value: float, + *, + host: str, + port: int, + password_ref: str | None, + ) -> None: + raise RuntimeError("connection refused") + + +async def test_vlc_media_error_is_reported_to_requester(tmp_path: Path) -> None: + (tmp_path / "default.yaml").write_text( + """ +match: [default] +widgets: + - id: media + kind: media + label: Living Room VLC + size: [4, 2] + media_http: + host: media.local + port: 9090 +""" + ) + server, *_ = make_test_server( + decks_dir=tmp_path, media_manager=_FailingMediaManager() + ) + test_server = TestServer(server.app, host="127.0.0.1") + await test_server.start_server() + try: + async with websockets.connect(f"ws://127.0.0.1:{test_server.port}/ws") as ws: + await _recv_type(ws, "deck") + await ws.send( + json.dumps( + {"type": "media_command", "id": "media", "command": "volume", "value": 55} + ) + ) + err = await _recv_type(ws, "media_error") + assert err["id"] == "media" + assert err["player"] == "Living Room VLC" + assert err["retryable"] is True + finally: + await server.stop() + await test_server.close() diff --git a/tests/test_mpris_dbus.py b/tests/test_mpris_dbus.py index ef09a6b..b558a6e 100644 --- a/tests/test_mpris_dbus.py +++ b/tests/test_mpris_dbus.py @@ -37,6 +37,7 @@ FakeMprisBackend, MPRIS_BUS_PREFIX, MPRIS_OBJECT_PATH, + MprisCommandError, PLAYER_INTERFACE, PROPERTIES_INTERFACE, ROOT_INTERFACE, @@ -418,16 +419,45 @@ async def test_send_command_unknown_command_is_a_noop() -> None: @pytest.mark.asyncio -async def test_send_command_unknown_row_is_a_noop() -> None: +async def test_send_command_unknown_row_raises_for_feedback() -> None: + """A row that vanished is reported, not silently dropped (issue #64).""" bus = FakeDbusBus() backend = DbusMprisBackend() backend._bus = bus backend._owned_names = {"vlc"} - await backend.send_command("nonexistent", "next") + with pytest.raises(MprisCommandError) as excinfo: + await backend.send_command("nonexistent", "next") + assert excinfo.value.row_id == "nonexistent" + assert excinfo.value.retryable is True assert bus.calls == [] +@pytest.mark.asyncio +async def test_send_command_bus_error_raises_and_still_reports_diagnostic() -> None: + """A rejected D-Bus call raises for feedback (issue #64) while keeping + the diagnostic event the ring buffer already relied on.""" + bus = FakeDbusBus() + bus.player_methods = lambda _dest, _member: False # ERROR reply + backend = DbusMprisBackend() + backend._bus = bus + backend._owned_names = {"vlc"} + seen: list[tuple[str, str | None, dict[str, Any]]] = [] + backend.set_diagnostic_listener(lambda kind, row, data: seen.append((kind, row, data))) + + with pytest.raises(MprisCommandError): + await backend.send_command("vlc", "next") + assert [kind for kind, _, _ in seen] == ["dbus_error"] + + +@pytest.mark.asyncio +async def test_identity_returns_cached_mpris_identity() -> None: + backend = DbusMprisBackend() + backend._identities["vlc"] = "VLC media player" + assert backend.identity("vlc") == "VLC media player" + assert backend.identity("gone") is None + + # --------------------------------------------------------------------------- # Slice 4 — NameOwnerChanged subscribes and reconciles row_ids # --------------------------------------------------------------------------- From 52f30c2639347c9ac308b8435b573b89d5758792 Mon Sep 17 00:00:00 2001 From: Jono Date: Tue, 29 Sep 2026 23:06:55 -0700 Subject: [PATCH 2/3] fix(client): count the failed health gate as reconnect attempt 1 The initial /health failure skipped connect(), so the overlay read "Attempt 0" before the first retry. Count the health check as an attempt and assert it in the socket hook test. --- client/src/socket.test.ts | 11 +++++++++++ client/src/socket.ts | 4 ++++ 2 files changed, 15 insertions(+) diff --git a/client/src/socket.test.ts b/client/src/socket.test.ts index 4ed2932..0d16ac8 100644 --- a/client/src/socket.test.ts +++ b/client/src/socket.test.ts @@ -65,6 +65,17 @@ describe("useDeckdSocket — reconnect feedback (issue #64)", () => { vi.unstubAllGlobals(); }); + it("counts a failed health gate as attempt 1", async () => { + vi.stubGlobal("fetch", vi.fn(() => Promise.reject(new Error("down")))); + const { result } = renderHook(() => useDeckdSocket(...hookArgs())); + await waitFor(() => + expect(result.current.lastError).toBe("daemon not reachable"), + ); + expect(result.current.attempt).toBe(1); + expect(result.current.retrying).toBe(true); + expect(FakeWebSocket.instances.length).toBe(0); + }); + it("resets attempt on open and records the last error on close", async () => { const { result } = renderHook(() => useDeckdSocket(...hookArgs())); await waitFor(() => expect(FakeWebSocket.instances.length).toBe(1)); diff --git a/client/src/socket.ts b/client/src/socket.ts index 3b7aa63..6679e0c 100644 --- a/client/src/socket.ts +++ b/client/src/socket.ts @@ -240,6 +240,10 @@ export function useDeckdSocket( if (!stopped && !cancelledRef.current) { setStatus("closed"); setLastError("daemon not reachable"); + // The health gate failing is itself a connection attempt, so the + // overlay reads "Attempt 1" rather than "Attempt 0". + attemptRef.current += 1; + setAttempt(attemptRef.current); setRetrying(true); timer = window.setTimeout(() => connect(), 1000); } From d5b406ad872649ead2f2a7eb9cc1efedd0e57aef Mon Sep 17 00:00:00 2001 From: Jono Date: Wed, 30 Sep 2026 09:23:18 -0700 Subject: [PATCH 3/3] fix(platform): parse login1 LockedHint's GVariant tuple gdbus prints a GVariant of type v wrapped in angle brackets ((,)), so ast.literal_eval raised a SyntaxError on every watch_session_state tick and no lock/blank state was ever pushed. Strip the brackets and add a test for the real form. Found while reproducing issue #64; the detection itself belongs to #160. --- daemon/deckd/platform.py | 13 ++++++++++++- tests/test_session_lock.py | 11 +++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/daemon/deckd/platform.py b/daemon/deckd/platform.py index 8055229..75ddd41 100644 --- a/daemon/deckd/platform.py +++ b/daemon/deckd/platform.py @@ -1366,8 +1366,19 @@ def _parse_single_bool_tuple(value: str) -> bool: ``ast.literal_eval`` can't parse, so normalise to Python's capitalised literals first — same one-arg-tuple discipline as :func:`_parse_single_string_tuple`. + + A *variant*-typed property comes wrapped in angle brackets: + ``login1``'s ``LockedHint`` is type ``v``, so gdbus prints + ``(,)``. Strip those too, or the whole lock tick aborts on a + ``SyntaxError`` and no lock state is ever pushed. """ - normalised = value.replace("true", "True").replace("false", "False") + normalised = ( + value.strip() + .replace("true", "True") + .replace("false", "False") + .replace("<", "") + .replace(">", "") + ) parsed = ast.literal_eval(normalised) if not isinstance(parsed, tuple) or len(parsed) != 1 or not isinstance(parsed[0], bool): raise RuntimeError(f"unexpected gdbus response: {value}") diff --git a/tests/test_session_lock.py b/tests/test_session_lock.py index 90a6038..340b190 100644 --- a/tests/test_session_lock.py +++ b/tests/test_session_lock.py @@ -86,6 +86,17 @@ async def test_gnome_locked(fake_gdbus) -> None: assert state.blanked is True +async def test_login_locked_hint_parses_real_variant_form(fake_gdbus) -> None: + """``login1``'s ``LockedHint`` is a GVariant of type ``v``, and gdbus + prints it as ``(,)`` — angle brackets and all. The parser must + strip them or every lock tick raises SyntaxError and nothing is ever + pushed (regression: issue #160, found reproducing #64).""" + fake_gdbus.scripted = {"GetActive": "(,)", "LockedHint": "(,)"} + state = await fake_gdbus._session_state_once() + assert state.locked is True + assert state.blanked is True + + async def test_gnome_login1_failure_degrades_to_unlocked(fake_gdbus) -> None: """login1 unreachable → the lock half stays false, never an error.""" fake_gdbus.scripted = {"GetActive": "(false,)"}