Add comprehensive test coverage and improve error handling

.github/workflows/ci.yml

@@ -4,12 +4,15 @@ on:
   push:
   pull_request:
 
+permissions:
+  contents: read
+
 jobs:
   test-and-build:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/setup-node@v7
       - uses: actions/checkout@v7
+      - uses: actions/setup-node@v7
         with:
           node-version: 22
           cache: npm

src/App.tsx

@@ -8,6 +8,7 @@ import { createHistoryStore } from './core/historyStore';
 import type { PlaylistSummary } from './core/playlists';
 import { createApi } from './spotify/api';
 import { createAuth } from './spotify/auth';
+import { describeError } from './spotify/errors';
 import { createWebPlayer, type WebPlayer } from './spotify/player';
 
 const CLIENT_ID = import.meta.env.VITE_SPOTIFY_CLIENT_ID ?? '';
@@ -23,7 +24,7 @@ const auth = createAuth({
 
 const api = createApi({
   getAccessToken: () => auth.getAccessToken(),
-  forceRefresh: () => auth.forceRefresh(),
+  forceRefresh: (rejectedToken) => auth.forceRefresh(rejectedToken),
   fetch: (input, init) => window.fetch(input, init),
 });
 
@@ -108,7 +109,15 @@ export function App() {
         </main>
       );
     case 'connect':
-      return <ConnectScreen clientIdMissing={!CLIENT_ID} error={screen.error} onConnect={() => void auth.login()} />;
+      return (
+        <ConnectScreen
+          clientIdMissing={!CLIENT_ID}
+          error={screen.error}
+          onConnect={() =>
+            auth.login().catch((e: unknown) => setScreen({ name: 'connect', error: describeError(e) }))
+          }
+        />
+      );
     case 'picker':
       return (
         <PlaylistPicker

src/components/DedupeScreen.tsx

@@ -185,7 +185,7 @@ function DedupeView({
           <div className="empty">
             <h2>No duplicates in {playlist.name}</h2>
             <p className="muted">Every song appears once.</p>
-            <button className="primary" onClick={onExit}>
+            <button className="primary" onClick={onExit} disabled={snap.busy}>
               Back to playlists
             </button>
           </div>

src/components/HistoryPanel.tsx

@@ -1,3 +1,4 @@
+import { useEffect, useRef } from 'react';
 import type { RemovedEntry } from '../core/historyStore';
 
 interface Props {
@@ -10,8 +11,27 @@ interface Props {
 }
 
 export function HistoryPanel({ entries, sessionId, note, onRestore, onClose }: Props) {
+  const panelRef = useRef<HTMLElement>(null);
+  const onCloseRef = useRef(onClose);
+  onCloseRef.current = onClose;
+
+  useEffect(() => {
+    // The panel can cover the card: move focus into it (the panel itself, not a button,
+    // so Space still toggles playback), let Escape close it, and give focus back after.
+    const previous = document.activeElement as HTMLElement | null;
+    panelRef.current?.focus();
+    const onKey = (e: KeyboardEvent) => {
+      if (e.key === 'Escape') onCloseRef.current();
+    };
+    window.addEventListener('keydown', onKey);
+    return () => {
+      window.removeEventListener('keydown', onKey);
+      previous?.focus?.();
+    };
+  }, []);
+
   return (
-    <aside className="history" aria-label="Removed songs">
+    <aside ref={panelRef} className="history" aria-label="Removed songs" tabIndex={-1}>
       <header>
         <h2>Removed songs</h2>
         <button className="link" onClick={onClose} aria-label="Close">
@@ -33,7 +53,9 @@ export function HistoryPanel({ entries, sessionId, note, onRestore, onClose }: P
                   {entry.sessionId !== sessionId && ` · ${new Date(entry.removedAt).toLocaleDateString()}`}
                 </span>
               </div>
-              <button onClick={() => onRestore(entry.uri)}>Restore</button>
+              <button onClick={() => onRestore(entry.uri)} aria-label={`Restore ${entry.name}`}>
+                Restore
+              </button>
             </li>
           ))}
         </ul>

src/components/SwipeCard.tsx

@@ -1,10 +1,8 @@
 import { motion, useMotionValue, useTransform, type PanInfo } from 'motion/react';
 import type { Card } from '../core/deck';
+import { SWIPE_DISTANCE, swipeIntent, type SwipeDirection } from './gestures';
 
-export type SwipeDirection = 'keep' | 'remove';
-
-const SWIPE_DISTANCE = 120;
-const SWIPE_VELOCITY = 600;
+export type { SwipeDirection } from './gestures';
 
 const variants = {
   enter: { scale: 0.95, opacity: 0 },
@@ -29,8 +27,8 @@ export function SwipeCard({ card, direction, onSwipe }: Props) {
   const removeOpacity = useTransform(x, [-SWIPE_DISTANCE, -30], [1, 0]);
 
   const onDragEnd = (_event: unknown, info: PanInfo) => {
-    if (info.offset.x > SWIPE_DISTANCE || info.velocity.x > SWIPE_VELOCITY) onSwipe('keep');
-    else if (info.offset.x < -SWIPE_DISTANCE || info.velocity.x < -SWIPE_VELOCITY) onSwipe('remove');
+    const intent = swipeIntent(info.offset.x, info.velocity.x);
+    if (intent) onSwipe(intent);
   };
 
   return (

src/components/SwipeScreen.tsx

@@ -11,6 +11,7 @@ import type { SpotifyApi } from '../spotify/api';
 import { AuthError, describeError } from '../spotify/errors';
 import type { WebPlayer } from '../spotify/player';
 import { DoneScreen } from './DoneScreen';
+import { shortcutFor } from './gestures';
 import { HistoryPanel } from './HistoryPanel';
 import { Loading } from './Loading';
 import { PlayerControls } from './PlayerControls';
@@ -123,23 +124,15 @@ function SwipeView({
   useEffect(() => {
     if (!started) return;
     const onKey = (e: KeyboardEvent) => {
-      if (e.repeat || e.target instanceof HTMLInputElement) return;
-      // While the History panel is open it can cover the card at narrow widths, so
-      // ←/→ must not remove or keep a song the user can't see. Undo and Space still work.
-      if (e.key === 'ArrowRight') {
-        if (!showHistory) swipe('keep');
-      } else if (e.key === 'ArrowLeft') {
-        if (!showHistory) swipe('remove');
-      } else if (e.key === ' ') {
-        e.preventDefault();
-        // Only the current card's own playback should toggle: on an unplayable card or
-        // the Done screen the SDK still holds the previous track, so togglePlay would
-        // resume that instead.
-        if (card?.isPlayable) void player.togglePlay();
-      } else if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'z') {
-        e.preventDefault();
-        controller.undo();
-      }
+      const shortcut = shortcutFor(e, { historyOpen: showHistory });
+      if (!shortcut) return;
+      e.preventDefault();
+      if (shortcut === 'keep' || shortcut === 'remove') swipe(shortcut);
+      else if (shortcut === 'undo') controller.undo();
+      // Only the current card's own playback should toggle: on an unplayable card or
+      // the Done screen the SDK still holds the previous track, so togglePlay would
+      // resume that instead.
+      else if (card?.isPlayable) void player.togglePlay();
     };
     window.addEventListener('keydown', onKey);
     return () => window.removeEventListener('keydown', onKey);

src/components/gestures.test.ts

@@ -0,0 +1,91 @@
+import { describe, expect, it } from 'vitest';
+import { shortcutFor, swipeIntent, type KeyLike } from './gestures';
+
+describe('swipeIntent', () => {
+  it('swipes once the card is dragged far enough', () => {
+    expect(swipeIntent(130, 0)).toBe('keep');
+    expect(swipeIntent(-130, 0)).toBe('remove');
+  });
+
+  it('snaps back from a short, slow drag', () => {
+    expect(swipeIntent(50, 0)).toBeNull();
+    expect(swipeIntent(-50, 100)).toBeNull();
+  });
+
+  it('swipes on a fast flick in the direction of the drag', () => {
+    expect(swipeIntent(40, 800)).toBe('keep');
+    expect(swipeIntent(-40, -800)).toBe('remove');
+  });
+
+  it('does not remove a song when a keep drag is flicked back to the middle', () => {
+    expect(swipeIntent(50, -700)).toBeNull();
+    expect(swipeIntent(-50, 700)).toBeNull();
+  });
+
+  it('cancels a long drag that is flicked back', () => {
+    expect(swipeIntent(200, -900)).toBeNull();
+  });
+
+  it('ignores a flick that barely moved the card', () => {
+    expect(swipeIntent(5, 900)).toBeNull();
+  });
+});
+
+const key = (k: string, extra: Partial<KeyLike> = {}): KeyLike => ({
+  key: k,
+  repeat: false,
+  altKey: false,
+  ctrlKey: false,
+  metaKey: false,
+  shiftKey: false,
+  target: null,
+  ...extra,
+});
+
+/** A stand-in for a DOM element: `closest` matches when the selector lists the tag. */
+const element = (tag: string) => ({ closest: (selector: string) => (selector.split(', ').includes(tag) ? {} : null) }) as unknown as EventTarget;
+
+describe('shortcutFor', () => {
+  const closed = { historyOpen: false };
+
+  it('maps arrows, Space and Ctrl/Cmd+Z', () => {
+    expect(shortcutFor(key('ArrowRight'), closed)).toBe('keep');
+    expect(shortcutFor(key('ArrowLeft'), closed)).toBe('remove');
+    expect(shortcutFor(key(' '), closed)).toBe('toggle');
+    expect(shortcutFor(key('z', { ctrlKey: true }), closed)).toBe('undo');
+    expect(shortcutFor(key('Z', { metaKey: true }), closed)).toBe('undo');
+  });
+
+  it('ignores held-down keys and typing in text fields', () => {
+    expect(shortcutFor(key('ArrowLeft', { repeat: true }), closed)).toBeNull();
+    expect(shortcutFor(key('ArrowLeft', { target: element('input') }), closed)).toBeNull();
+    expect(shortcutFor(key('z', { ctrlKey: true, target: element('textarea') }), closed)).toBeNull();
+  });
+
+  it('never removes a song on the browser’s Back shortcut or other chords', () => {
+    expect(shortcutFor(key('ArrowLeft', { altKey: true }), closed)).toBeNull();
+    expect(shortcutFor(key('ArrowLeft', { metaKey: true }), closed)).toBeNull();
+    expect(shortcutFor(key('ArrowLeft', { ctrlKey: true }), closed)).toBeNull();
+    expect(shortcutFor(key('ArrowRight', { shiftKey: true }), closed)).toBeNull();
+  });
+
+  it('does not keep or remove while History covers the card, but still undoes and toggles', () => {
+    const open = { historyOpen: true };
+    expect(shortcutFor(key('ArrowRight'), open)).toBeNull();
+    expect(shortcutFor(key('ArrowLeft'), open)).toBeNull();
+    expect(shortcutFor(key('z', { ctrlKey: true }), open)).toBe('undo');
+    expect(shortcutFor(key(' '), open)).toBe('toggle');
+    // The open panel itself has focus, not one of its buttons.
+    expect(shortcutFor(key(' ', { target: element('aside') }), open)).toBe('toggle');
+  });
+
+  it('lets Space press a focused button instead of toggling playback', () => {
+    expect(shortcutFor(key(' ', { target: element('button') }), closed)).toBeNull();
+    expect(shortcutFor(key('ArrowRight', { target: element('button') }), closed)).toBe('keep');
+  });
+
+  it('ignores other keys', () => {
+    expect(shortcutFor(key('a'), closed)).toBeNull();
+    expect(shortcutFor(key('Enter'), closed)).toBeNull();
+  });
+});

src/components/gestures.ts

@@ -0,0 +1,53 @@
+export type SwipeDirection = 'keep' | 'remove';
+
+export const SWIPE_DISTANCE = 120;
+const SWIPE_VELOCITY = 600;
+/** A flick must have moved the card at least this far in the flick's direction. */
+const FLICK_MIN_OFFSET = 30;
+
+const directionOf = (dx: number): SwipeDirection => (dx > 0 ? 'keep' : 'remove');
+
+/**
+ * What a released drag means: past the distance threshold, or a fast flick the same
+ * way the card was dragged. Flicking back toward the middle cancels the swipe.
+ */
+export function swipeIntent(offsetX: number, velocityX: number): SwipeDirection | null {
+  const flicking = Math.abs(velocityX) > SWIPE_VELOCITY;
+  const sameWay = Math.sign(offsetX) === Math.sign(velocityX);
+  if (Math.abs(offsetX) > SWIPE_DISTANCE) return flicking && !sameWay ? null : directionOf(offsetX);
+  if (flicking && sameWay && Math.abs(offsetX) >= FLICK_MIN_OFFSET) return directionOf(offsetX);
+  return null;
+}
+
+export type Shortcut = SwipeDirection | 'toggle' | 'undo';
+
+export interface KeyLike {
+  key: string;
+  repeat: boolean;
+  altKey: boolean;
+  ctrlKey: boolean;
+  metaKey: boolean;
+  shiftKey: boolean;
+  target: EventTarget | null;
+}
+
+const TEXT_FIELDS = 'input, select, textarea, [contenteditable]';
+const CONTROLS = `button, a[href], ${TEXT_FIELDS}`;
+
+const targetMatches = (target: EventTarget | null, selector: string) =>
+  Boolean((target as Element | null)?.closest?.(selector));
+
+/** Maps a keydown on the swipe screen to its shortcut, or null to leave it to the browser. */
+export function shortcutFor(e: KeyLike, { historyOpen }: { historyOpen: boolean }): Shortcut | null {
+  if (e.repeat || targetMatches(e.target, TEXT_FIELDS)) return null;
+  if ((e.ctrlKey || e.metaKey) && !e.altKey && e.key.toLowerCase() === 'z') return 'undo';
+  // Alt/Cmd+← is the browser's Back: never let it (or other chords) remove a song.
+  if (e.altKey || e.ctrlKey || e.metaKey || e.shiftKey) return null;
+  // While the History panel is open it can cover the card at narrow widths, so
+  // ←/→ must not remove or keep a song the user can't see. Undo and Space still work.
+  if (e.key === 'ArrowRight') return historyOpen ? null : 'keep';
+  if (e.key === 'ArrowLeft') return historyOpen ? null : 'remove';
+  // Space on a focused button must press that button, not toggle playback.
+  if (e.key === ' ') return targetMatches(e.target, CONTROLS) ? null : 'toggle';
+  return null;
+}

src/core/historyStore.test.ts

@@ -49,6 +49,21 @@ describe('createHistoryStore', () => {
     expect(createHistoryStore(storage).load('p1')).toEqual([]);
   });
 
+  it('drops malformed stored entries instead of failing', () => {
+    const storage = memoryStorage();
+    storage.setItem('spotify-swipe:history:pl', JSON.stringify([null, { uri: 'x' }, 'junk', entry('a')]));
+    const store = createHistoryStore(storage);
+    expect(store.load('pl')).toEqual([entry('a')]);
+    store.add('pl', entry('b', 2));
+    expect(store.load('pl').map((e) => e.uri)).toEqual(['b', 'a']);
+  });
+
+  it('treats stored JSON that is not a list as empty', () => {
+    const storage = memoryStorage();
+    storage.setItem('spotify-swipe:history:pl', '{"uri":"a"}');
+    expect(createHistoryStore(storage).load('pl')).toEqual([]);
+  });
+
   it('keeps working in memory when storage writes fail', () => {
     const storage = memoryStorage();
     storage.setItem = () => {

src/core/historyStore.ts

@@ -20,6 +20,21 @@ export interface HistoryStore {
 // Keys keep the app's old name so existing logins and History survive the rename to Swipify.
 const storageKey = (playlistId: string) => `spotify-swipe:history:${playlistId}`;
 
+function isEntry(value: unknown): value is RemovedEntry {
+  const e = value as Partial<RemovedEntry> | null;
+  return (
+    typeof e === 'object' &&
+    e !== null &&
+    typeof e.uri === 'string' &&
+    typeof e.name === 'string' &&
+    Array.isArray(e.artists) &&
+    Array.isArray(e.positions) &&
+    e.positions.every((p) => Number.isInteger(p)) &&
+    typeof e.sessionId === 'string' &&
+    typeof e.removedAt === 'number'
+  );
+}
+
 /**
  * Removal history kept in memory and mirrored to `storage` so it survives reloads.
  * Storage failures are swallowed: the in-memory copy keeps undo/restore working.
@@ -34,7 +49,7 @@ export function createHistoryStore(storage: Storage | null): HistoryStore {
     try {
       const raw = storage?.getItem(storageKey(playlistId));
       const parsed: unknown = raw ? JSON.parse(raw) : [];
-      if (Array.isArray(parsed)) entries = parsed as RemovedEntry[];
+      if (Array.isArray(parsed)) entries = parsed.filter(isEntry);
     } catch {
       // Corrupt or unreadable storage: start with an empty history.
     }