From 7d8bb30fc54414244389f023e2884518673782f5 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 25 Sep 2026 08:53:18 +0000 Subject: [PATCH 1/2] Fix review findings and extend the test suite CI - Run checkout before setup-node and pass node-version/cache to setup-node (a merge had swapped them, so CI never pinned Node 22); read-only token. Spotify auth/API/player - A refresh rejected because another tab already rotated the refresh token now uses that tab's tokens instead of logging every tab out. - Logout during an in-flight refresh stays logged out. - forceRefresh(rejectedToken) skips the refresh when a concurrent request already replaced the token. - The OAuth callback requires a stored state and always clears the verifier/state; logout clears them too. - Cap Retry-After at 30 s and fail with a readable message beyond that. - A failed SDK load is retried by the next player; disconnect() before the SDK loads no longer leaves a stray connected player. Swipe session / dedupe / history - Undo followed immediately by removing the same song no longer loses its History entry (the restore leaves a newer entry alone). - Restores ignore removals that are still queued, so they land at the right index. - A restore that fails partway trims the entry, so retrying or Undo does not add a copy twice (session and dedupe). - A second Restore click while one is queued is ignored. - Undo on Liked Songs re-likes in reverse so the original order is kept. - Malformed stored History entries are dropped instead of crashing. UI - Swipe gestures and keyboard shortcuts are pure, tested functions: flicking a keep-drag back no longer removes the song; Alt/Cmd+Arrow (browser Back) and other chords no longer swipe; Space on a focused button presses the button. - A stale 404 play-retry no longer starts the previous card's song. - Login errors are shown instead of being swallowed; reduced motion is respected; History panel takes focus, closes on Escape and labels each Restore button; dedupe empty-state Back is disabled while busy. Tests: 129 -> 184, including a new player.ts suite, gestures and playWithRetry tests; each regression test fails on the previous code. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RofS2KTGcgYYorEqtuB5Uo --- .github/workflows/ci.yml | 5 +- src/App.tsx | 13 ++- src/components/DedupeScreen.tsx | 2 +- src/components/HistoryPanel.tsx | 21 +++- src/components/SwipeCard.tsx | 10 +- src/components/SwipeScreen.tsx | 27 ++--- src/components/gestures.test.ts | 89 +++++++++++++++ src/components/gestures.ts | 53 +++++++++ src/core/historyStore.test.ts | 15 +++ src/core/historyStore.ts | 17 ++- src/dedupe/controller.test.ts | 42 +++++++ src/dedupe/controller.ts | 23 +++- src/hooks/usePlayer.test.ts | 60 ++++++++++ src/hooks/usePlayer.ts | 22 +++- src/main.tsx | 7 +- src/session/controller.test.ts | 87 +++++++++++++++ src/session/controller.ts | 41 ++++--- src/session/restore.test.ts | 42 +++++++ src/session/restore.ts | 28 ++++- src/spotify/api.test.ts | 15 +++ src/spotify/api.ts | 18 ++- src/spotify/auth.test.ts | 94 ++++++++++++++++ src/spotify/auth.ts | 33 +++++- src/spotify/player.test.ts | 189 ++++++++++++++++++++++++++++++++ src/spotify/player.ts | 19 +++- 25 files changed, 902 insertions(+), 70 deletions(-) create mode 100644 src/components/gestures.test.ts create mode 100644 src/components/gestures.ts create mode 100644 src/hooks/usePlayer.test.ts create mode 100644 src/spotify/player.test.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b521d83..152ef2f 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/src/App.tsx b/src/App.tsx index 483b1df..2f9f355 100644 --- a/src/App.tsx +++ b/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() { ); case 'connect': - return void auth.login()} />; + return ( + + auth.login().catch((e: unknown) => setScreen({ name: 'connect', error: describeError(e) })) + } + /> + ); case 'picker': return (

No duplicates in {playlist.name}

Every song appears once.

- diff --git a/src/components/HistoryPanel.tsx b/src/components/HistoryPanel.tsx index cabdda6..646ceda 100644 --- a/src/components/HistoryPanel.tsx +++ b/src/components/HistoryPanel.tsx @@ -1,3 +1,4 @@ +import { useEffect, useRef } from 'react'; import type { RemovedEntry } from '../core/historyStore'; interface Props { @@ -10,11 +11,25 @@ interface Props { } export function HistoryPanel({ entries, sessionId, note, onRestore, onClose }: Props) { + const closeRef = useRef(null); + const onCloseRef = useRef(onClose); + onCloseRef.current = onClose; + + useEffect(() => { + // The panel can cover the card: move focus into it, and let Escape close it. + closeRef.current?.focus(); + const onKey = (e: KeyboardEvent) => { + if (e.key === 'Escape') onCloseRef.current(); + }; + window.addEventListener('keydown', onKey); + return () => window.removeEventListener('keydown', onKey); + }, []); + return (