From ec1b3a3397e30acc4c855fe2116b1441aa9de287 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 3 May 2026 15:32:43 -0400 Subject: [PATCH] chore: untrack docs/superpowers, ignore going forward Agent-authored design specs and implementation plans are kept local on the working machine; the canonical record lives in commit messages and the running code. The 48 historical files remain in git history and can be retrieved via git checkout -- docs/superpowers/ if ever needed; they just stop accruing on the dev tip. --- .gitignore | 4 + .../plans/2026-04-20-web-ui-library-reads.md | 2124 --------- ...026-04-20-web-ui-server-auth-foundation.md | 1458 ------ .../2026-04-21-web-ui-media-endpoints.md | 1066 ----- .../plans/2026-04-21-web-ui-scaffold.md | 954 ---- .../plans/2026-04-22-web-ui-auth.md | 1245 ----- .../plans/2026-04-23-web-ui-library-views.md | 1815 -------- .../plans/2026-04-24-web-ui-player.md | 1698 ------- .../superpowers/plans/2026-04-25-m2-events.md | 2390 ---------- .../plans/2026-04-25-web-ui-search.md | 2152 --------- docs/superpowers/plans/2026-04-26-m2-likes.md | 2495 ---------- .../plans/2026-04-27-m3-shuffle.md | 1588 ------- .../plans/2026-04-27-m3-similarity.md | 1414 ------ .../plans/2026-04-27-m3-vectors.md | 1491 ------ .../plans/2026-04-28-m4a-scrobble.md | 2523 ---------- .../plans/2026-04-28-m4b-similarity.md | 1551 ------- .../superpowers/plans/2026-04-28-m4c-radio.md | 1391 ------ .../plans/2026-04-29-m5a-lidarr.md | 1979 -------- .../plans/2026-04-30-m5b-quarantine.md | 2916 ------------ .../2026-04-30-m5c-suggested-additions.md | 1838 -------- .../plans/2026-04-30-m6a-home-page.md | 3341 ------------- ...2026-05-02-m7-flutter-mobile-foundation.md | 4131 ----------------- .../plans/2026-05-03-m7-362-theme-toggle.md | 860 ---- .../plans/2026-05-03-m7-playlists-crud.md | 3508 -------------- .../plans/2026-05-03-m7-track-actions-menu.md | 1515 ------ .../2026-04-20-web-ui-library-reads-design.md | 319 -- .../2026-04-20-web-ui-scaffold-design.md | 268 -- ...026-04-21-web-ui-media-endpoints-design.md | 177 - .../specs/2026-04-22-web-ui-auth-design.md | 256 - .../2026-04-23-web-ui-library-views-design.md | 320 -- .../specs/2026-04-24-web-ui-player-design.md | 423 -- .../specs/2026-04-25-m2-events-design.md | 221 - .../specs/2026-04-25-web-ui-search-design.md | 189 - .../specs/2026-04-26-m2-likes-design.md | 242 - .../specs/2026-04-27-m3-shuffle-design.md | 230 - .../specs/2026-04-27-m3-similarity-design.md | 301 -- .../specs/2026-04-27-m3-vectors-design.md | 251 - .../specs/2026-04-28-m4a-scrobble-design.md | 403 -- .../specs/2026-04-28-m4b-similarity-design.md | 338 -- .../specs/2026-04-28-m4c-radio-design.md | 444 -- .../specs/2026-04-29-m5a-lidarr-design.md | 345 -- .../specs/2026-04-30-m5b-quarantine-design.md | 389 -- ...26-04-30-m5c-suggested-additions-design.md | 360 -- .../specs/2026-04-30-m6a-home-page-design.md | 410 -- ...-05-02-flutter-mobile-foundation-design.md | 313 -- .../2026-05-03-m7-362-theme-toggle-design.md | 256 - ...026-05-03-m7-363-branding-polish-design.md | 253 - .../2026-05-03-m7-playlists-crud-design.md | 371 -- ...2026-05-03-m7-track-actions-menu-design.md | 250 - 49 files changed, 4 insertions(+), 54772 deletions(-) delete mode 100644 docs/superpowers/plans/2026-04-20-web-ui-library-reads.md delete mode 100644 docs/superpowers/plans/2026-04-20-web-ui-server-auth-foundation.md delete mode 100644 docs/superpowers/plans/2026-04-21-web-ui-media-endpoints.md delete mode 100644 docs/superpowers/plans/2026-04-21-web-ui-scaffold.md delete mode 100644 docs/superpowers/plans/2026-04-22-web-ui-auth.md delete mode 100644 docs/superpowers/plans/2026-04-23-web-ui-library-views.md delete mode 100644 docs/superpowers/plans/2026-04-24-web-ui-player.md delete mode 100644 docs/superpowers/plans/2026-04-25-m2-events.md delete mode 100644 docs/superpowers/plans/2026-04-25-web-ui-search.md delete mode 100644 docs/superpowers/plans/2026-04-26-m2-likes.md delete mode 100644 docs/superpowers/plans/2026-04-27-m3-shuffle.md delete mode 100644 docs/superpowers/plans/2026-04-27-m3-similarity.md delete mode 100644 docs/superpowers/plans/2026-04-27-m3-vectors.md delete mode 100644 docs/superpowers/plans/2026-04-28-m4a-scrobble.md delete mode 100644 docs/superpowers/plans/2026-04-28-m4b-similarity.md delete mode 100644 docs/superpowers/plans/2026-04-28-m4c-radio.md delete mode 100644 docs/superpowers/plans/2026-04-29-m5a-lidarr.md delete mode 100644 docs/superpowers/plans/2026-04-30-m5b-quarantine.md delete mode 100644 docs/superpowers/plans/2026-04-30-m5c-suggested-additions.md delete mode 100644 docs/superpowers/plans/2026-04-30-m6a-home-page.md delete mode 100644 docs/superpowers/plans/2026-05-02-m7-flutter-mobile-foundation.md delete mode 100644 docs/superpowers/plans/2026-05-03-m7-362-theme-toggle.md delete mode 100644 docs/superpowers/plans/2026-05-03-m7-playlists-crud.md delete mode 100644 docs/superpowers/plans/2026-05-03-m7-track-actions-menu.md delete mode 100644 docs/superpowers/specs/2026-04-20-web-ui-library-reads-design.md delete mode 100644 docs/superpowers/specs/2026-04-20-web-ui-scaffold-design.md delete mode 100644 docs/superpowers/specs/2026-04-21-web-ui-media-endpoints-design.md delete mode 100644 docs/superpowers/specs/2026-04-22-web-ui-auth-design.md delete mode 100644 docs/superpowers/specs/2026-04-23-web-ui-library-views-design.md delete mode 100644 docs/superpowers/specs/2026-04-24-web-ui-player-design.md delete mode 100644 docs/superpowers/specs/2026-04-25-m2-events-design.md delete mode 100644 docs/superpowers/specs/2026-04-25-web-ui-search-design.md delete mode 100644 docs/superpowers/specs/2026-04-26-m2-likes-design.md delete mode 100644 docs/superpowers/specs/2026-04-27-m3-shuffle-design.md delete mode 100644 docs/superpowers/specs/2026-04-27-m3-similarity-design.md delete mode 100644 docs/superpowers/specs/2026-04-27-m3-vectors-design.md delete mode 100644 docs/superpowers/specs/2026-04-28-m4a-scrobble-design.md delete mode 100644 docs/superpowers/specs/2026-04-28-m4b-similarity-design.md delete mode 100644 docs/superpowers/specs/2026-04-28-m4c-radio-design.md delete mode 100644 docs/superpowers/specs/2026-04-29-m5a-lidarr-design.md delete mode 100644 docs/superpowers/specs/2026-04-30-m5b-quarantine-design.md delete mode 100644 docs/superpowers/specs/2026-04-30-m5c-suggested-additions-design.md delete mode 100644 docs/superpowers/specs/2026-04-30-m6a-home-page-design.md delete mode 100644 docs/superpowers/specs/2026-05-02-flutter-mobile-foundation-design.md delete mode 100644 docs/superpowers/specs/2026-05-03-m7-362-theme-toggle-design.md delete mode 100644 docs/superpowers/specs/2026-05-03-m7-363-branding-polish-design.md delete mode 100644 docs/superpowers/specs/2026-05-03-m7-playlists-crud-design.md delete mode 100644 docs/superpowers/specs/2026-05-03-m7-track-actions-menu-design.md diff --git a/.gitignore b/.gitignore index f0f72eb4..f680b66b 100644 --- a/.gitignore +++ b/.gitignore @@ -29,6 +29,10 @@ go.work.sum # Superpowers brainstorming companion sessions .superpowers/ +# Agent-authored design specs and implementation plans (kept local; the +# canonical record lives in commit messages and the running code). +docs/superpowers/ + # Flutter flutter_client/.dart_tool/ flutter_client/.flutter-plugins diff --git a/docs/superpowers/plans/2026-04-20-web-ui-library-reads.md b/docs/superpowers/plans/2026-04-20-web-ui-library-reads.md deleted file mode 100644 index 2cee0f8d..00000000 --- a/docs/superpowers/plans/2026-04-20-web-ui-library-reads.md +++ /dev/null @@ -1,2124 +0,0 @@ -# Web UI Library Reads Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Add five read-only JSON endpoints to `/api/*` — paged artist list (alpha + newest), artist/album/track detail, and unified search — so the SvelteKit SPA can render browse and search views. - -**Architecture:** Handlers live in `internal/api` alongside existing auth code, gated by `RequireUser`. New SQL queries added to `internal/db/queries/` and regenerated via sqlc. Response DTOs follow a Ref/Detail split; lists use a generic `Page[T]` envelope; forward-reference stream/cover URLs are embedded now so Plan 3 can land transparently. Tiny conversion helpers are duplicated inline rather than shared with `internal/subsonic` — the two packages shape responses differently and the helpers are ~5 lines each. - -**Tech Stack:** Go 1.23, chi/v5, pgx/v5, sqlc 1.27, pgxpool, pgtype. - -**Spec:** `docs/superpowers/specs/2026-04-20-web-ui-library-reads-design.md` - ---- - -## File Structure - -New files: - -| File | Responsibility | -|---|---| -| `internal/api/library.go` | Artist/album/track list + detail handlers | -| `internal/api/search.go` | Unified search handler | -| `internal/api/convert.go` | UUID↔string, year-from-pgdate, URL builders, dbq→ref projections | -| `internal/api/library_fixtures_test.go` | Shared seed helpers for integration tests | -| `internal/api/library_test.go` | Integration tests for library endpoints | -| `internal/api/search_test.go` | Integration tests for search | - -Modified files: - -| File | Change | -|---|---| -| `internal/api/api.go` | Register new routes inside the `authed` group | -| `internal/api/types.go` | Add `Page[T]`, `ArtistRef`, `AlbumRef`, `TrackRef`, `ArtistDetail`, `AlbumDetail`, `SearchResponse` | -| `internal/db/queries/artists.sql` | Add `ListArtistsAlpha`, `ListArtistsNewest`, `CountArtists`, `CountArtistsMatching` | -| `internal/db/queries/albums.sql` | Add `CountAlbumsMatching` | -| `internal/db/queries/tracks.sql` | Add `CountTracksMatching` | -| `internal/db/dbq/artists.sql.go`, `albums.sql.go`, `tracks.sql.go` | sqlc-regenerated; do not hand-edit | - ---- - -## Working Conventions - -- **Commit after each green test step.** One task = one commit minimum; split further if a task produces multiple logical units. -- **Integration tests require a live Postgres** at `MINSTREL_TEST_DATABASE_URL`. The existing `docker-compose.yml` spins one up; tests skip cleanly when the env var is unset. -- **sqlc regeneration:** run `make generate` after changing any `*.sql` file. Never hand-edit `internal/db/dbq/*.go`. -- **Run the full test suite before committing any task:** `MINSTREL_TEST_DATABASE_URL=postgres://... go test ./...`. Use `-short` only when explicitly noted. -- **`pgtype` imports:** add `"github.com/jackc/pgx/v5/pgtype"` only where needed; Go won't compile with unused imports. - ---- - -## Task 1: Add SQL queries - -**Files:** -- Modify: `internal/db/queries/artists.sql` -- Modify: `internal/db/queries/albums.sql` -- Modify: `internal/db/queries/tracks.sql` -- Regenerate: `internal/db/dbq/artists.sql.go`, `albums.sql.go`, `tracks.sql.go` - -- [ ] **Step 1: Append to `internal/db/queries/artists.sql`** - -Append these four queries (keep existing `ListArtists` untouched — subsonic's `buildArtistIndexes` still uses it): - -```sql - --- name: ListArtistsAlpha :many -SELECT * FROM artists ORDER BY sort_name, name, id LIMIT $1 OFFSET $2; - --- name: ListArtistsNewest :many --- Secondary id tiebreaker keeps pagination stable when created_at ties. -SELECT * FROM artists ORDER BY created_at DESC, id DESC LIMIT $1 OFFSET $2; - --- name: CountArtists :one -SELECT COUNT(*) FROM artists; - --- name: CountArtistsMatching :one -SELECT COUNT(*) FROM artists WHERE name ILIKE '%' || $1::text || '%'; -``` - -- [ ] **Step 2: Append to `internal/db/queries/albums.sql`** - -```sql - --- name: CountAlbumsMatching :one -SELECT COUNT(*) FROM albums WHERE title ILIKE '%' || $1::text || '%'; -``` - -- [ ] **Step 3: Append to `internal/db/queries/tracks.sql`** - -```sql - --- name: CountTracksMatching :one -SELECT COUNT(*) FROM tracks WHERE title ILIKE '%' || $1::text || '%'; -``` - -- [ ] **Step 4: Regenerate sqlc code** - -Run: `make generate` -Expected: no errors; `internal/db/dbq/artists.sql.go`, `albums.sql.go`, `tracks.sql.go` are updated with the new functions `ListArtistsAlpha`, `ListArtistsNewest`, `CountArtists`, `CountArtistsMatching`, `CountAlbumsMatching`, `CountTracksMatching`. - -- [ ] **Step 5: Verify the package builds** - -Run: `go build ./internal/db/...` -Expected: exit 0, no output. - -- [ ] **Step 6: Commit** - -```bash -git add internal/db/queries/*.sql internal/db/dbq/*.sql.go -git commit -m "feat(db): add paged + count queries for library reads" -``` - ---- - -## Task 2: Add DTO types - -**Files:** -- Modify: `internal/api/types.go` - -- [ ] **Step 1: Extend `internal/api/types.go`** - -Append to the existing `internal/api/types.go` (keep `LoginRequest`, `LoginResponse`, `UserView`): - -```go - -// Page is the generic pagination envelope used by list and search endpoints. -// Items is always non-nil at JSON encode time — handlers must initialize -// empty slices explicitly so absent results render as [] rather than null. -type Page[T any] struct { - Items []T `json:"items"` - Total int `json:"total"` - Limit int `json:"limit"` - Offset int `json:"offset"` -} - -// ArtistRef is the lightweight artist shape used in lists and search results. -type ArtistRef struct { - ID string `json:"id"` - Name string `json:"name"` - AlbumCount int `json:"album_count"` -} - -// AlbumRef is the lightweight album shape used in lists, artist details, -// and search results. CoverURL points to /api/albums/{id}/cover which -// Plan 3 implements. -type AlbumRef struct { - ID string `json:"id"` - Title string `json:"title"` - ArtistID string `json:"artist_id"` - ArtistName string `json:"artist_name"` - Year int `json:"year,omitempty"` - TrackCount int `json:"track_count"` - DurationSec int `json:"duration_sec"` - CoverURL string `json:"cover_url"` -} - -// TrackRef is the lightweight track shape used in album details and search. -// StreamURL points to /api/tracks/{id}/stream which Plan 3 implements. -type TrackRef struct { - ID string `json:"id"` - Title string `json:"title"` - AlbumID string `json:"album_id"` - AlbumTitle string `json:"album_title"` - ArtistID string `json:"artist_id"` - ArtistName string `json:"artist_name"` - TrackNumber int `json:"track_number,omitempty"` - DiscNumber int `json:"disc_number,omitempty"` - DurationSec int `json:"duration_sec"` - StreamURL string `json:"stream_url"` -} - -// ArtistDetail is the response body of GET /api/artists/{id}. Embeds the ref -// so callers read id/name/album_count from the same path as list entries. -type ArtistDetail struct { - ArtistRef - Albums []AlbumRef `json:"albums"` -} - -// AlbumDetail is the response body of GET /api/albums/{id}. -type AlbumDetail struct { - AlbumRef - Tracks []TrackRef `json:"tracks"` -} - -// SearchResponse is the body of GET /api/search. Each facet carries its own -// total + limit + offset; the client sends one shared limit/offset and the -// server applies it uniformly to each facet's LIMIT/OFFSET query. -type SearchResponse struct { - Artists Page[ArtistRef] `json:"artists"` - Albums Page[AlbumRef] `json:"albums"` - Tracks Page[TrackRef] `json:"tracks"` -} -``` - -- [ ] **Step 2: Verify build** - -Run: `go build ./internal/api/...` -Expected: exit 0. - -- [ ] **Step 3: Commit** - -```bash -git add internal/api/types.go -git commit -m "feat(api): add DTOs for library reads and search" -``` - ---- - -## Task 3: Add conversion helpers - -**Files:** -- Create: `internal/api/convert.go` -- Create: `internal/api/convert_test.go` - -- [ ] **Step 1: Write failing test** - -Create `internal/api/convert_test.go`: - -```go -package api - -import ( - "testing" - - "github.com/jackc/pgx/v5/pgtype" -) - -func TestUUIDToString(t *testing.T) { - var u pgtype.UUID - if err := u.Scan("00112233-4455-6677-8899-aabbccddeeff"); err != nil { - t.Fatalf("scan: %v", err) - } - got := uuidToString(u) - want := "00112233-4455-6677-8899-aabbccddeeff" - if got != want { - t.Errorf("uuidToString = %q, want %q", got, want) - } - - var zero pgtype.UUID - if s := uuidToString(zero); s != "" { - t.Errorf("uuidToString(invalid) = %q, want empty", s) - } -} - -func TestParseUUID(t *testing.T) { - u, ok := parseUUID("00112233-4455-6677-8899-aabbccddeeff") - if !ok || !u.Valid { - t.Fatalf("parseUUID valid: ok=%v valid=%v", ok, u.Valid) - } - if _, ok := parseUUID("not-a-uuid"); ok { - t.Error("parseUUID accepted garbage") - } - if _, ok := parseUUID(""); ok { - t.Error("parseUUID accepted empty string") - } -} - -func TestYearFromDate(t *testing.T) { - var d pgtype.Date - if y := yearFromDate(d); y != 0 { - t.Errorf("yearFromDate(invalid) = %d, want 0", y) - } -} - -func TestDurationMsToSec(t *testing.T) { - cases := []struct { - ms int32 - want int - }{ - {0, 0}, - {500, 1}, // rounds up - {499, 0}, // rounds down - {1500, 2}, // rounds up - {60000, 60}, - } - for _, c := range cases { - if got := durationMsToSec(c.ms); got != c.want { - t.Errorf("durationMsToSec(%d) = %d, want %d", c.ms, got, c.want) - } - } -} - -func TestCoverURLAndStreamURL(t *testing.T) { - var u pgtype.UUID - if err := u.Scan("00112233-4455-6677-8899-aabbccddeeff"); err != nil { - t.Fatalf("scan: %v", err) - } - if got := coverURL(u); got != "/api/albums/00112233-4455-6677-8899-aabbccddeeff/cover" { - t.Errorf("coverURL = %q", got) - } - if got := streamURL(u); got != "/api/tracks/00112233-4455-6677-8899-aabbccddeeff/stream" { - t.Errorf("streamURL = %q", got) - } -} -``` - -- [ ] **Step 2: Run — expect FAIL** - -Run: `go test ./internal/api/ -run 'TestUUIDToString|TestParseUUID|TestYearFromDate|TestDurationMsToSec|TestCoverURLAndStreamURL' -count=1` -Expected: compile error — `undefined: uuidToString` etc. - -- [ ] **Step 3: Create `internal/api/convert.go`** - -```go -package api - -import ( - "github.com/jackc/pgx/v5/pgtype" -) - -// uuidToString renders a pgtype.UUID as a canonical hyphenated string. -// Returns "" when the UUID is invalid (unset). Duplicated from the subsonic -// package intentionally — the two packages must not depend on each other. -func uuidToString(u pgtype.UUID) string { - if !u.Valid { - return "" - } - b := u.Bytes - const hex = "0123456789abcdef" - out := make([]byte, 36) - j := 0 - for i, x := range b { - if i == 4 || i == 6 || i == 8 || i == 10 { - out[j] = '-' - j++ - } - out[j] = hex[x>>4] - out[j+1] = hex[x&0x0f] - j += 2 - } - return string(out) -} - -// parseUUID accepts the canonical hyphenated form produced by uuidToString. -// Returns ok=false on any malformed input so handlers can 400 cleanly. -func parseUUID(s string) (pgtype.UUID, bool) { - var u pgtype.UUID - if err := u.Scan(s); err != nil { - return pgtype.UUID{}, false - } - return u, u.Valid -} - -// yearFromDate returns the 4-digit year from a pgtype.Date, or 0 when the -// date is unset. Album release years are optional at scan time. -func yearFromDate(d pgtype.Date) int { - if !d.Valid { - return 0 - } - return d.Time.Year() -} - -// durationMsToSec converts millisecond durations (as stored in tracks.duration_ms) -// to seconds, rounding to nearest. Callers display seconds; sub-second precision -// is not useful in UI. -func durationMsToSec(ms int32) int { - return int((ms + 500) / 1000) -} - -// coverURL returns the /api relative URL for an album's cover art. The -// endpoint ships in Plan 3; until then it 404s, and the SPA does not wire -// to it. -func coverURL(albumID pgtype.UUID) string { - return "/api/albums/" + uuidToString(albumID) + "/cover" -} - -// streamURL returns the /api relative URL for a track's audio stream. The -// endpoint ships in Plan 3. -func streamURL(trackID pgtype.UUID) string { - return "/api/tracks/" + uuidToString(trackID) + "/stream" -} -``` - -- [ ] **Step 4: Run — expect PASS** - -Run: `go test ./internal/api/ -run 'TestUUIDToString|TestParseUUID|TestYearFromDate|TestDurationMsToSec|TestCoverURLAndStreamURL' -count=1 -v` -Expected: all five tests PASS. - -- [ ] **Step 5: Commit** - -```bash -git add internal/api/convert.go internal/api/convert_test.go -git commit -m "feat(api): add conversion helpers for library DTOs" -``` - ---- - -## Task 4: Add ref/detail projection helpers - -These project dbq rows into the response DTOs. Keeping them next to the DTO types and conversion helpers means handlers stay thin. - -**Files:** -- Modify: `internal/api/convert.go` (append) -- Modify: `internal/api/convert_test.go` (append) - -- [ ] **Step 1: Append failing tests to `internal/api/convert_test.go`** - -```go - -func TestArtistRefFrom(t *testing.T) { - var id pgtype.UUID - _ = id.Scan("00112233-4455-6677-8899-aabbccddeeff") - a := dbq.Artist{ID: id, Name: "Beatles", SortName: "Beatles"} - got := artistRefFrom(a, 7) - if got.ID != "00112233-4455-6677-8899-aabbccddeeff" { - t.Errorf("ID = %q", got.ID) - } - if got.Name != "Beatles" || got.AlbumCount != 7 { - t.Errorf("ref = %+v", got) - } -} - -func TestAlbumRefFrom(t *testing.T) { - var aid, artID pgtype.UUID - _ = aid.Scan("00000000-0000-0000-0000-000000000001") - _ = artID.Scan("00000000-0000-0000-0000-000000000002") - var rel pgtype.Date - _ = rel.Scan("1969-09-26") - a := dbq.Album{ID: aid, Title: "Abbey Road", ArtistID: artID, ReleaseDate: rel} - got := albumRefFrom(a, "Beatles", 17, 2820) - if got.Title != "Abbey Road" || got.ArtistName != "Beatles" { - t.Errorf("ref = %+v", got) - } - if got.TrackCount != 17 || got.DurationSec != 2820 { - t.Errorf("counts = %+v", got) - } - if got.Year != 1969 { - t.Errorf("year = %d", got.Year) - } - if got.CoverURL != "/api/albums/00000000-0000-0000-0000-000000000001/cover" { - t.Errorf("cover_url = %q", got.CoverURL) - } -} - -func TestTrackRefFrom(t *testing.T) { - var tid, aid, artID pgtype.UUID - _ = tid.Scan("00000000-0000-0000-0000-000000000010") - _ = aid.Scan("00000000-0000-0000-0000-000000000001") - _ = artID.Scan("00000000-0000-0000-0000-000000000002") - trackNum := int32(3) - discNum := int32(1) - t2 := dbq.Track{ - ID: tid, Title: "Something", AlbumID: aid, ArtistID: artID, - TrackNumber: &trackNum, DiscNumber: &discNum, - DurationMs: 183_000, - } - got := trackRefFrom(t2, "Abbey Road", "Beatles") - if got.Title != "Something" || got.AlbumTitle != "Abbey Road" || got.ArtistName != "Beatles" { - t.Errorf("ref = %+v", got) - } - if got.TrackNumber != 3 || got.DiscNumber != 1 { - t.Errorf("positions = %+v", got) - } - if got.DurationSec != 183 { - t.Errorf("duration = %d, want 183", got.DurationSec) - } - if got.StreamURL != "/api/tracks/00000000-0000-0000-0000-000000000010/stream" { - t.Errorf("stream_url = %q", got.StreamURL) - } -} - -func TestTrackRefFromNilPositions(t *testing.T) { - var tid, aid, artID pgtype.UUID - _ = tid.Scan("00000000-0000-0000-0000-000000000010") - _ = aid.Scan("00000000-0000-0000-0000-000000000001") - _ = artID.Scan("00000000-0000-0000-0000-000000000002") - t2 := dbq.Track{ID: tid, AlbumID: aid, ArtistID: artID, DurationMs: 1000} - got := trackRefFrom(t2, "A", "B") - if got.TrackNumber != 0 || got.DiscNumber != 0 { - t.Errorf("nil positions should be 0, got %+v", got) - } -} -``` - -Add these imports to `internal/api/convert_test.go` (or merge into existing import block): - -```go -import ( - "testing" - - "github.com/jackc/pgx/v5/pgtype" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) -``` - -- [ ] **Step 2: Run — expect FAIL** - -Run: `go test ./internal/api/ -run 'TestArtistRefFrom|TestAlbumRefFrom|TestTrackRefFrom' -count=1` -Expected: compile error — `undefined: artistRefFrom` etc. - -- [ ] **Step 3: Append to `internal/api/convert.go`** - -Extend the existing import block to include `dbq`: - -```go -import ( - "github.com/jackc/pgx/v5/pgtype" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) -``` - -Append to the file: - -```go - -// artistRefFrom projects a dbq.Artist into an ArtistRef. albumCount must be -// pre-computed by the caller (one query per artist in lists). -func artistRefFrom(a dbq.Artist, albumCount int) ArtistRef { - return ArtistRef{ - ID: uuidToString(a.ID), - Name: a.Name, - AlbumCount: albumCount, - } -} - -// albumRefFrom projects a dbq.Album into an AlbumRef. artistName must be -// pre-resolved because Album only carries artist_id. trackCount and -// durationSec may be 0 when the caller does not compute them. -func albumRefFrom(a dbq.Album, artistName string, trackCount, durationSec int) AlbumRef { - return AlbumRef{ - ID: uuidToString(a.ID), - Title: a.Title, - ArtistID: uuidToString(a.ArtistID), - ArtistName: artistName, - Year: yearFromDate(a.ReleaseDate), - TrackCount: trackCount, - DurationSec: durationSec, - CoverURL: coverURL(a.ID), - } -} - -// trackRefFrom projects a dbq.Track into a TrackRef. Parent names must be -// pre-resolved because Track only carries album_id and artist_id. -func trackRefFrom(t dbq.Track, albumTitle, artistName string) TrackRef { - ref := TrackRef{ - ID: uuidToString(t.ID), - Title: t.Title, - AlbumID: uuidToString(t.AlbumID), - AlbumTitle: albumTitle, - ArtistID: uuidToString(t.ArtistID), - ArtistName: artistName, - DurationSec: durationMsToSec(t.DurationMs), - StreamURL: streamURL(t.ID), - } - if t.TrackNumber != nil { - ref.TrackNumber = int(*t.TrackNumber) - } - if t.DiscNumber != nil { - ref.DiscNumber = int(*t.DiscNumber) - } - return ref -} -``` - -- [ ] **Step 4: Run — expect PASS** - -Run: `go test ./internal/api/ -run 'TestArtistRefFrom|TestAlbumRefFrom|TestTrackRefFrom' -count=1 -v` -Expected: four tests PASS. - -- [ ] **Step 5: Full package builds clean** - -Run: `go build ./internal/api/...` -Expected: exit 0, no output. - -- [ ] **Step 6: Commit** - -```bash -git add internal/api/convert.go internal/api/convert_test.go -git commit -m "feat(api): add dbq→ref projection helpers" -``` - ---- - -## Task 5: Test fixtures helper - -Shared seed helpers that library_test.go and search_test.go both use. Putting them in `library_fixtures_test.go` means they compile only in the test binary and don't pollute the production package. - -**Files:** -- Create: `internal/api/library_fixtures_test.go` - -- [ ] **Step 1: Create `internal/api/library_fixtures_test.go`** - -```go -package api - -import ( - "context" - "testing" - "time" - - "github.com/jackc/pgx/v5/pgtype" - "github.com/jackc/pgx/v5/pgxpool" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) - -// truncateLibrary clears the music tables between tests. Sessions/users are -// cleared by testHandlers. CASCADE covers tracks→albums→artists FK chain. -func truncateLibrary(t *testing.T, pool *pgxpool.Pool) { - t.Helper() - if _, err := pool.Exec(context.Background(), - "TRUNCATE tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } -} - -// seedArtist inserts an artist with deterministic sort_name = name. -func seedArtist(t *testing.T, pool *pgxpool.Pool, name string) dbq.Artist { - t.Helper() - a, err := dbq.New(pool).UpsertArtist(context.Background(), dbq.UpsertArtistParams{ - Name: name, - SortName: name, - Mbid: nil, - }) - if err != nil { - t.Fatalf("UpsertArtist(%s): %v", name, err) - } - return a -} - -// seedAlbum inserts an album belonging to artistID. releaseYear=0 leaves -// release_date unset. -func seedAlbum(t *testing.T, pool *pgxpool.Pool, artistID pgtype.UUID, title string, releaseYear int) dbq.Album { - t.Helper() - var rel pgtype.Date - if releaseYear > 0 { - rel = pgtype.Date{Time: time.Date(releaseYear, 1, 1, 0, 0, 0, 0, time.UTC), Valid: true} - } - a, err := dbq.New(pool).UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{ - Title: title, - SortTitle: title, - ArtistID: artistID, - ReleaseDate: rel, - Mbid: nil, - CoverArtPath: nil, - }) - if err != nil { - t.Fatalf("UpsertAlbum(%s): %v", title, err) - } - return a -} - -// seedTrack inserts a track. trackNumber<=0 leaves it NULL. filePath is -// synthesized from title to stay unique across seeds. -func seedTrack(t *testing.T, pool *pgxpool.Pool, albumID, artistID pgtype.UUID, title string, trackNumber int, durationMs int32) dbq.Track { - t.Helper() - var tn *int32 - if trackNumber > 0 { - v := int32(trackNumber) - tn = &v - } - tr, err := dbq.New(pool).UpsertTrack(context.Background(), dbq.UpsertTrackParams{ - Title: title, - AlbumID: albumID, - ArtistID: artistID, - TrackNumber: tn, - DiscNumber: nil, - DurationMs: durationMs, - FilePath: "/seed/" + title + ".flac", - FileSize: 1024, - FileFormat: "flac", - Bitrate: nil, - Mbid: nil, - Genre: nil, - }) - if err != nil { - t.Fatalf("UpsertTrack(%s): %v", title, err) - } - return tr -} -``` - -- [ ] **Step 2: Verify test binary compiles** - -Run: `go test -run '^$' ./internal/api/... -count=1` -Expected: no output and exit 0 (no tests match `^$`, but the package must compile). - -- [ ] **Step 3: Commit** - -```bash -git add internal/api/library_fixtures_test.go -git commit -m "test(api): add library seed fixtures" -``` - ---- - -## Task 6: Implement GET /api/tracks/{id} - -Starting with the simplest detail endpoint so the route-wiring pattern is in place before the more complex ones. TDD: write tests, verify fail, implement. - -**Files:** -- Create: `internal/api/library.go` -- Create: `internal/api/library_test.go` -- Modify: `internal/api/api.go` - -- [ ] **Step 1: Write failing test in `internal/api/library_test.go`** - -```go -package api - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "testing" - - "github.com/go-chi/chi/v5" -) - -// newLibraryRouter builds a test-only chi router with the library handlers -// mounted at their path-style routes. Tests hit this router rather than -// calling handler methods directly so chi URL params populate correctly. -// RequireUser is NOT applied — library handlers don't read user context. -func newLibraryRouter(h *handlers) chi.Router { - r := chi.NewRouter() - r.Get("/api/artists", h.handleListArtists) - r.Get("/api/artists/{id}", h.handleGetArtist) - r.Get("/api/albums/{id}", h.handleGetAlbum) - r.Get("/api/tracks/{id}", h.handleGetTrack) - r.Get("/api/search", h.handleSearch) - return r -} - -func TestHandleGetTrack_HappyPath(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "Beatles") - album := seedAlbum(t, pool, artist.ID, "Abbey Road", 1969) - track := seedTrack(t, pool, album.ID, artist.ID, "Something", 3, 183_000) - - req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(track.ID), nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - var got TrackRef - if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { - t.Fatalf("decode: %v body=%s", err, w.Body.String()) - } - if got.Title != "Something" || got.AlbumTitle != "Abbey Road" || got.ArtistName != "Beatles" { - t.Errorf("ref = %+v", got) - } - if got.TrackNumber != 3 || got.DurationSec != 183 { - t.Errorf("positions = %+v", got) - } - want := "/api/tracks/" + uuidToString(track.ID) + "/stream" - if got.StreamURL != want { - t.Errorf("stream_url = %q, want %q", got.StreamURL, want) - } -} - -func TestHandleGetTrack_BadUUID(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodGet, "/api/tracks/not-a-uuid", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - -func TestHandleGetTrack_NotFound(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - req := httptest.NewRequest(http.MethodGet, - "/api/tracks/00000000-0000-0000-0000-000000000001", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusNotFound { - t.Errorf("status = %d, want 404", w.Code) - } -} -``` - -- [ ] **Step 2: Run — expect FAIL** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleGetTrack' -count=1` -Expected: compile error — `h.handleGetTrack` undefined, plus sibling methods referenced in `newLibraryRouter`. - -- [ ] **Step 3: Create `internal/api/library.go` with all route method stubs + `handleGetTrack` implemented** - -Stubbing sibling methods (`handleListArtists`, `handleGetArtist`, `handleGetAlbum`, `handleSearch`) lets `newLibraryRouter` compile even though subsequent tasks flesh them out. - -```go -package api - -import ( - "errors" - "net/http" - - "github.com/go-chi/chi/v5" - "github.com/jackc/pgx/v5" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) - -// handleGetTrack implements GET /api/tracks/{id}. Resolves parent album + -// artist names so the response is self-contained — clients should not need -// a follow-up request to render a track outside an album view. -func (h *handlers) handleGetTrack(w http.ResponseWriter, r *http.Request) { - id, ok := parseUUID(chi.URLParam(r, "id")) - if !ok { - writeErr(w, http.StatusBadRequest, "bad_request", "invalid track id") - return - } - q := dbq.New(h.pool) - track, err := q.GetTrackByID(r.Context(), id) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - writeErr(w, http.StatusNotFound, "not_found", "track not found") - return - } - h.logger.Error("api: get track failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - album, err := q.GetAlbumByID(r.Context(), track.AlbumID) - if err != nil { - h.logger.Error("api: get track album failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - artist, err := q.GetArtistByID(r.Context(), track.ArtistID) - if err != nil { - h.logger.Error("api: get track artist failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - writeJSON(w, http.StatusOK, trackRefFrom(track, album.Title, artist.Name)) -} - -// handleGetAlbum is implemented in Task 7. -func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "stub") -} - -// handleGetArtist is implemented in Task 8. -func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "stub") -} - -// handleListArtists is implemented in Task 9. -func (h *handlers) handleListArtists(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "stub") -} - -// handleSearch is implemented in Task 10 (file: search.go). -func (h *handlers) handleSearch(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "stub") -} -``` - -- [ ] **Step 4: Add `writeJSON` helper to `internal/api/errors.go`** - -Open `internal/api/errors.go` and append: - -```go - -// writeJSON emits a 2xx response with the given body JSON-encoded. Mirrors -// writeErr's shape so handlers have one shape for success and one for -// failure. -func writeJSON(w http.ResponseWriter, status int, body any) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(body) -} -``` - -- [ ] **Step 5: Run — expect PASS** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleGetTrack' -count=1 -v` -Expected: three tests PASS (`_HappyPath`, `_BadUUID`, `_NotFound`). - -- [ ] **Step 6: Full test suite stays green** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -count=1` -Expected: all existing and new tests PASS. - -- [ ] **Step 7: Commit** - -```bash -git add internal/api/library.go internal/api/library_test.go internal/api/errors.go -git commit -m "feat(api): GET /api/tracks/{id} handler with parent metadata" -``` - ---- - -## Task 7: Implement GET /api/albums/{id} - -**Files:** -- Modify: `internal/api/library.go` (replace the `handleGetAlbum` stub) -- Modify: `internal/api/library_test.go` (append tests) - -- [ ] **Step 1: Append failing tests to `internal/api/library_test.go`** - -```go - -func TestHandleGetAlbum_HappyPath(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "Beatles") - album := seedAlbum(t, pool, artist.ID, "Abbey Road", 1969) - seedTrack(t, pool, album.ID, artist.ID, "Come Together", 1, 259_000) - seedTrack(t, pool, album.ID, artist.ID, "Something", 2, 183_000) - - req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID), nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - var got AlbumDetail - if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { - t.Fatalf("decode: %v body=%s", err, w.Body.String()) - } - if got.Title != "Abbey Road" || got.ArtistName != "Beatles" || got.Year != 1969 { - t.Errorf("album = %+v", got) - } - if len(got.Tracks) != 2 { - t.Fatalf("tracks len = %d, want 2", len(got.Tracks)) - } - if got.Tracks[0].Title != "Come Together" || got.Tracks[1].Title != "Something" { - t.Errorf("tracks = %+v", got.Tracks) - } - if got.TrackCount != 2 || got.DurationSec != 442 { // 259 + 183 - t.Errorf("counts = track=%d duration=%d", got.TrackCount, got.DurationSec) - } - want := "/api/albums/" + uuidToString(album.ID) + "/cover" - if got.CoverURL != want { - t.Errorf("cover_url = %q", got.CoverURL) - } - // StreamURL on nested tracks must be set - if got.Tracks[0].StreamURL == "" { - t.Error("nested track missing stream_url") - } -} - -func TestHandleGetAlbum_NoTracks(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "Beatles") - album := seedAlbum(t, pool, artist.ID, "Empty", 0) - - req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID), nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - var got AlbumDetail - _ = json.Unmarshal(w.Body.Bytes(), &got) - if got.Tracks == nil { - t.Error("tracks = nil, want [] for JSON encoding") - } - if len(got.Tracks) != 0 { - t.Errorf("tracks len = %d, want 0", len(got.Tracks)) - } - // Year omitempty: releaseYear=0 => Year=0 => field should be absent from JSON - if got.Year != 0 { - t.Errorf("year = %d, want 0", got.Year) - } -} - -func TestHandleGetAlbum_BadUUID(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodGet, "/api/albums/not-a-uuid", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - -func TestHandleGetAlbum_NotFound(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - req := httptest.NewRequest(http.MethodGet, - "/api/albums/00000000-0000-0000-0000-000000000001", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusNotFound { - t.Errorf("status = %d, want 404", w.Code) - } -} -``` - -- [ ] **Step 2: Run — expect FAIL (stub returns 501)** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleGetAlbum' -count=1` -Expected: all four fail because the stub returns 501. - -- [ ] **Step 3: Replace `handleGetAlbum` stub in `internal/api/library.go`** - -Find the stub: - -```go -// handleGetAlbum is implemented in Task 7. -func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "stub") -} -``` - -Replace with: - -```go -// handleGetAlbum implements GET /api/albums/{id}. Returns the album plus its -// tracks (ordered by disc/track number via the underlying query) with -// duration summed from the track list — keeps one source of truth. -func (h *handlers) handleGetAlbum(w http.ResponseWriter, r *http.Request) { - id, ok := parseUUID(chi.URLParam(r, "id")) - if !ok { - writeErr(w, http.StatusBadRequest, "bad_request", "invalid album id") - return - } - q := dbq.New(h.pool) - album, err := q.GetAlbumByID(r.Context(), id) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - writeErr(w, http.StatusNotFound, "not_found", "album not found") - return - } - h.logger.Error("api: get album failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - artist, err := q.GetArtistByID(r.Context(), album.ArtistID) - if err != nil { - h.logger.Error("api: get album artist failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - tracks, err := q.ListTracksByAlbum(r.Context(), id) - if err != nil { - h.logger.Error("api: list tracks failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - refs := make([]TrackRef, 0, len(tracks)) - durSec := 0 - for _, t := range tracks { - ref := trackRefFrom(t, album.Title, artist.Name) - refs = append(refs, ref) - durSec += ref.DurationSec - } - detail := AlbumDetail{ - AlbumRef: albumRefFrom(album, artist.Name, len(tracks), durSec), - Tracks: refs, - } - writeJSON(w, http.StatusOK, detail) -} -``` - -- [ ] **Step 4: Run — expect PASS** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleGetAlbum' -count=1 -v` -Expected: four tests PASS. - -- [ ] **Step 5: Full api suite stays green** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -count=1` -Expected: all tests PASS. - -- [ ] **Step 6: Commit** - -```bash -git add internal/api/library.go internal/api/library_test.go -git commit -m "feat(api): GET /api/albums/{id} with nested tracks" -``` - ---- - -## Task 8: Implement GET /api/artists/{id} - -**Files:** -- Modify: `internal/api/library.go` (replace the `handleGetArtist` stub) -- Modify: `internal/api/library_test.go` (append tests) - -- [ ] **Step 1: Append failing tests to `internal/api/library_test.go`** - -```go - -func TestHandleGetArtist_HappyPath(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "Beatles") - album1 := seedAlbum(t, pool, artist.ID, "Abbey Road", 1969) - album2 := seedAlbum(t, pool, artist.ID, "Let It Be", 1970) - seedTrack(t, pool, album1.ID, artist.ID, "Something", 3, 183_000) - seedTrack(t, pool, album2.ID, artist.ID, "Get Back", 1, 189_000) - - req := httptest.NewRequest(http.MethodGet, "/api/artists/"+uuidToString(artist.ID), nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - var got ArtistDetail - if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { - t.Fatalf("decode: %v body=%s", err, w.Body.String()) - } - if got.Name != "Beatles" { - t.Errorf("name = %q", got.Name) - } - if got.AlbumCount != 2 { - t.Errorf("album_count = %d, want 2", got.AlbumCount) - } - if len(got.Albums) != 2 { - t.Fatalf("albums len = %d, want 2", len(got.Albums)) - } - // ListAlbumsByArtist orders by release_date then sort_title; Abbey Road (1969) first. - if got.Albums[0].Title != "Abbey Road" { - t.Errorf("first album = %q, want Abbey Road", got.Albums[0].Title) - } - for _, a := range got.Albums { - if a.ArtistName != "Beatles" { - t.Errorf("album.artist_name = %q", a.ArtistName) - } - if a.CoverURL == "" { - t.Error("album missing cover_url") - } - if a.TrackCount != 1 { - t.Errorf("album.track_count = %d, want 1", a.TrackCount) - } - } -} - -func TestHandleGetArtist_NoAlbums(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "Solo") - - req := httptest.NewRequest(http.MethodGet, "/api/artists/"+uuidToString(artist.ID), nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d", w.Code) - } - var got ArtistDetail - _ = json.Unmarshal(w.Body.Bytes(), &got) - if got.Albums == nil { - t.Error("albums = nil, want []") - } - if got.AlbumCount != 0 { - t.Errorf("album_count = %d, want 0", got.AlbumCount) - } -} - -func TestHandleGetArtist_BadUUID(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodGet, "/api/artists/not-a-uuid", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - -func TestHandleGetArtist_NotFound(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - req := httptest.NewRequest(http.MethodGet, - "/api/artists/00000000-0000-0000-0000-000000000001", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusNotFound { - t.Errorf("status = %d, want 404", w.Code) - } -} -``` - -- [ ] **Step 2: Run — expect FAIL (stub 501)** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleGetArtist' -count=1` -Expected: four fail against the stub. - -- [ ] **Step 3: Replace `handleGetArtist` stub in `internal/api/library.go`** - -Replace with: - -```go -// handleGetArtist implements GET /api/artists/{id}. Returns artist + albums; -// each album carries its own track_count (one count query per album, same -// pattern as subsonic). At M1 sizes (albums-per-artist << 100) this is fine. -func (h *handlers) handleGetArtist(w http.ResponseWriter, r *http.Request) { - id, ok := parseUUID(chi.URLParam(r, "id")) - if !ok { - writeErr(w, http.StatusBadRequest, "bad_request", "invalid artist id") - return - } - q := dbq.New(h.pool) - artist, err := q.GetArtistByID(r.Context(), id) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - writeErr(w, http.StatusNotFound, "not_found", "artist not found") - return - } - h.logger.Error("api: get artist failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - albums, err := q.ListAlbumsByArtist(r.Context(), id) - if err != nil { - h.logger.Error("api: list albums by artist failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - refs := make([]AlbumRef, 0, len(albums)) - for _, a := range albums { - count, cerr := q.CountTracksByAlbum(r.Context(), a.ID) - if cerr != nil { - h.logger.Error("api: count tracks failed", "err", cerr) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - refs = append(refs, albumRefFrom(a, artist.Name, int(count), 0)) - } - detail := ArtistDetail{ - ArtistRef: artistRefFrom(artist, len(albums)), - Albums: refs, - } - writeJSON(w, http.StatusOK, detail) -} -``` - -- [ ] **Step 4: Run — expect PASS** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleGetArtist' -count=1 -v` -Expected: four tests PASS. - -- [ ] **Step 5: Full api suite stays green** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -count=1` -Expected: all PASS. - -- [ ] **Step 6: Commit** - -```bash -git add internal/api/library.go internal/api/library_test.go -git commit -m "feat(api): GET /api/artists/{id} with nested albums" -``` - ---- - -## Task 9: Implement GET /api/artists (paged list with sort) - -**Files:** -- Modify: `internal/api/library.go` (replace the `handleListArtists` stub; add pagination helper) -- Modify: `internal/api/library_test.go` (append tests) - -- [ ] **Step 1: Append failing tests to `internal/api/library_test.go`** - -```go - -func TestHandleListArtists_DefaultAlpha(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - seedArtist(t, pool, "Beatles") - seedArtist(t, pool, "ABBA") - seedArtist(t, pool, "Zeppelin") - - req := httptest.NewRequest(http.MethodGet, "/api/artists", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - var got Page[ArtistRef] - if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { - t.Fatalf("decode: %v", err) - } - if got.Total != 3 { - t.Errorf("total = %d, want 3", got.Total) - } - if got.Limit != 50 || got.Offset != 0 { - t.Errorf("pagination defaults = limit=%d offset=%d, want 50/0", got.Limit, got.Offset) - } - if len(got.Items) != 3 { - t.Fatalf("items len = %d", len(got.Items)) - } - if got.Items[0].Name != "ABBA" || got.Items[2].Name != "Zeppelin" { - t.Errorf("alpha order wrong: %q, %q, %q", got.Items[0].Name, got.Items[1].Name, got.Items[2].Name) - } -} - -func TestHandleListArtists_SortNewest(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - // Seeded in this order; created_at DESC means Third first. - seedArtist(t, pool, "First") - seedArtist(t, pool, "Second") - seedArtist(t, pool, "Third") - - req := httptest.NewRequest(http.MethodGet, "/api/artists?sort=newest", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d", w.Code) - } - var got Page[ArtistRef] - _ = json.Unmarshal(w.Body.Bytes(), &got) - if len(got.Items) != 3 { - t.Fatalf("items len = %d", len(got.Items)) - } - if got.Items[0].Name != "Third" || got.Items[2].Name != "First" { - t.Errorf("newest order wrong: %q, %q, %q", got.Items[0].Name, got.Items[1].Name, got.Items[2].Name) - } -} - -func TestHandleListArtists_InvalidSort(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - seedArtist(t, pool, "Only") - - req := httptest.NewRequest(http.MethodGet, "/api/artists?sort=garbage", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - -func TestHandleListArtists_Pagination(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - for _, n := range []string{"A", "B", "C", "D", "E"} { - seedArtist(t, pool, n) - } - - req := httptest.NewRequest(http.MethodGet, "/api/artists?limit=2&offset=2", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d", w.Code) - } - var got Page[ArtistRef] - _ = json.Unmarshal(w.Body.Bytes(), &got) - if got.Total != 5 { - t.Errorf("total = %d, want 5", got.Total) - } - if got.Limit != 2 || got.Offset != 2 { - t.Errorf("page = limit=%d offset=%d", got.Limit, got.Offset) - } - if len(got.Items) != 2 { - t.Fatalf("items len = %d", len(got.Items)) - } - if got.Items[0].Name != "C" || got.Items[1].Name != "D" { - t.Errorf("slice = %q, %q", got.Items[0].Name, got.Items[1].Name) - } -} - -func TestHandleListArtists_LimitClamped(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - seedArtist(t, pool, "One") - - req := httptest.NewRequest(http.MethodGet, "/api/artists?limit=9999", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d", w.Code) - } - var got Page[ArtistRef] - _ = json.Unmarshal(w.Body.Bytes(), &got) - if got.Limit != 200 { - t.Errorf("limit = %d, want 200 (max clamp)", got.Limit) - } -} - -func TestHandleListArtists_LimitNonNumeric(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodGet, "/api/artists?limit=abc", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - -func TestHandleListArtists_AlbumCount(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - seedAlbum(t, pool, artist.ID, "X", 0) - seedAlbum(t, pool, artist.ID, "Y", 0) - - req := httptest.NewRequest(http.MethodGet, "/api/artists", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - var got Page[ArtistRef] - _ = json.Unmarshal(w.Body.Bytes(), &got) - if len(got.Items) != 1 || got.Items[0].AlbumCount != 2 { - t.Errorf("items = %+v", got.Items) - } -} - -func TestHandleListArtists_EmptyDB(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - - req := httptest.NewRequest(http.MethodGet, "/api/artists", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d", w.Code) - } - var got Page[ArtistRef] - _ = json.Unmarshal(w.Body.Bytes(), &got) - if got.Items == nil { - t.Error("items = nil, want []") - } - if got.Total != 0 { - t.Errorf("total = %d", got.Total) - } -} -``` - -- [ ] **Step 2: Run — expect FAIL** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleListArtists' -count=1` -Expected: all eight fail against the stub. - -- [ ] **Step 3: Add pagination parsing helper to `internal/api/convert.go`** - -Append: - -```go - -// parsePaging reads limit/offset from the query string, applying defaults -// and clamping. Returns a 400-worthy error when the values are non-numeric -// or negative; out-of-range values silently clamp (deliberate — UX). -func parsePaging(raw url.Values) (limit, offset int, err error) { - const ( - defLimit = 50 - maxLimit = 200 - ) - limit = defLimit - if s := strings.TrimSpace(raw.Get("limit")); s != "" { - n, perr := strconv.Atoi(s) - if perr != nil { - return 0, 0, errBadPaging - } - if n < 1 { - n = 1 - } - if n > maxLimit { - n = maxLimit - } - limit = n - } - if s := strings.TrimSpace(raw.Get("offset")); s != "" { - n, perr := strconv.Atoi(s) - if perr != nil { - return 0, 0, errBadPaging - } - if n < 0 { - return 0, 0, errBadPaging - } - offset = n - } - return limit, offset, nil -} -``` - -Extend the import block at the top of `internal/api/convert.go` with the new imports + `errBadPaging`: - -```go -import ( - "errors" - "net/url" - "strconv" - "strings" - - "github.com/jackc/pgx/v5/pgtype" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) - -var errBadPaging = errors.New("invalid limit or offset") -``` - -- [ ] **Step 4: Replace `handleListArtists` stub in `internal/api/library.go`** - -Replace the stub with: - -```go -// handleListArtists implements GET /api/artists with two sort modes -// (alpha|newest) and enveloped pagination. album_count per artist is -// computed via an N+1 — acceptable at limit=200. -func (h *handlers) handleListArtists(w http.ResponseWriter, r *http.Request) { - sort := r.URL.Query().Get("sort") - if sort == "" { - sort = "alpha" - } - if sort != "alpha" && sort != "newest" { - writeErr(w, http.StatusBadRequest, "bad_request", "sort must be alpha or newest") - return - } - limit, offset, err := parsePaging(r.URL.Query()) - if err != nil { - writeErr(w, http.StatusBadRequest, "bad_request", err.Error()) - return - } - - q := dbq.New(h.pool) - var artists []dbq.Artist - switch sort { - case "newest": - artists, err = q.ListArtistsNewest(r.Context(), dbq.ListArtistsNewestParams{ - Limit: int32(limit), Offset: int32(offset), - }) - default: // alpha - artists, err = q.ListArtistsAlpha(r.Context(), dbq.ListArtistsAlphaParams{ - Limit: int32(limit), Offset: int32(offset), - }) - } - if err != nil { - h.logger.Error("api: list artists failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - total, err := q.CountArtists(r.Context()) - if err != nil { - h.logger.Error("api: count artists failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "count failed") - return - } - - items := make([]ArtistRef, 0, len(artists)) - for _, a := range artists { - albums, aerr := q.ListAlbumsByArtist(r.Context(), a.ID) - if aerr != nil { - h.logger.Error("api: list albums for artist failed", "err", aerr) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - items = append(items, artistRefFrom(a, len(albums))) - } - writeJSON(w, http.StatusOK, Page[ArtistRef]{ - Items: items, - Total: int(total), - Limit: limit, - Offset: offset, - }) -} -``` - -- [ ] **Step 5: Run — expect PASS** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleListArtists' -count=1 -v` -Expected: all eight tests PASS. - -- [ ] **Step 6: Full api suite stays green** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -count=1` -Expected: all PASS. - -- [ ] **Step 7: Commit** - -```bash -git add internal/api/library.go internal/api/library_test.go internal/api/convert.go -git commit -m "feat(api): GET /api/artists with alpha/newest sort and paging" -``` - ---- - -## Task 10: Implement GET /api/search - -**Files:** -- Create: `internal/api/search.go` -- Create: `internal/api/search_test.go` -- Modify: `internal/api/library.go` (remove `handleSearch` stub) - -- [ ] **Step 1: Write failing tests in `internal/api/search_test.go`** - -```go -package api - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "testing" -) - -func TestHandleSearch_ThreeFacets(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "Beatles") - album := seedAlbum(t, pool, artist.ID, "Beatles Greatest", 1982) - seedTrack(t, pool, album.ID, artist.ID, "Beatles Jam", 1, 60_000) - - req := httptest.NewRequest(http.MethodGet, "/api/search?q=beatles", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - var got SearchResponse - if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { - t.Fatalf("decode: %v", err) - } - if got.Artists.Total != 1 || len(got.Artists.Items) != 1 { - t.Errorf("artists = %+v", got.Artists) - } - if got.Albums.Total != 1 || len(got.Albums.Items) != 1 { - t.Errorf("albums = %+v", got.Albums) - } - if got.Tracks.Total != 1 || len(got.Tracks.Items) != 1 { - t.Errorf("tracks = %+v", got.Tracks) - } - if got.Artists.Items[0].Name != "Beatles" { - t.Errorf("artist name = %q", got.Artists.Items[0].Name) - } - if got.Albums.Items[0].ArtistName != "Beatles" { - t.Errorf("album.artist_name = %q", got.Albums.Items[0].ArtistName) - } - if got.Tracks.Items[0].AlbumTitle != "Beatles Greatest" { - t.Errorf("track.album_title = %q", got.Tracks.Items[0].AlbumTitle) - } -} - -func TestHandleSearch_NoMatches(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - seedArtist(t, pool, "Beatles") - - req := httptest.NewRequest(http.MethodGet, "/api/search?q=nothinghere", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d", w.Code) - } - var got SearchResponse - _ = json.Unmarshal(w.Body.Bytes(), &got) - if got.Artists.Total != 0 || got.Albums.Total != 0 || got.Tracks.Total != 0 { - t.Errorf("totals = %+v", got) - } - // All three item slices must JSON-encode as [] not null. - if got.Artists.Items == nil || got.Albums.Items == nil || got.Tracks.Items == nil { - t.Errorf("nil item slice: %+v", got) - } -} - -func TestHandleSearch_EmptyQuery(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodGet, "/api/search", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - -func TestHandleSearch_BlankQuery(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodGet, "/api/search?q=%20%20", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - -func TestHandleSearch_PagingAppliedPerFacet(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - // Create three matching artists so paging shows. - a1 := seedArtist(t, pool, "Match One") - a2 := seedArtist(t, pool, "Match Two") - a3 := seedArtist(t, pool, "Match Three") - _ = a1 - _ = a2 - _ = a3 - - req := httptest.NewRequest(http.MethodGet, "/api/search?q=Match&limit=2&offset=1", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d", w.Code) - } - var got SearchResponse - _ = json.Unmarshal(w.Body.Bytes(), &got) - if got.Artists.Total != 3 { - t.Errorf("total = %d, want 3", got.Artists.Total) - } - if got.Artists.Limit != 2 || got.Artists.Offset != 1 { - t.Errorf("page = limit=%d offset=%d", got.Artists.Limit, got.Artists.Offset) - } - if len(got.Artists.Items) != 2 { - t.Errorf("items len = %d, want 2", len(got.Artists.Items)) - } -} -``` - -- [ ] **Step 2: Run — expect FAIL (stub returns 501)** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleSearch' -count=1` -Expected: five fail against stub. - -- [ ] **Step 3: Create `internal/api/search.go`** - -```go -package api - -import ( - "context" - "net/http" - "strings" - - "github.com/jackc/pgx/v5/pgtype" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) - -// handleSearch implements GET /api/search. Runs three paged facets sharing -// one limit/offset. Each facet returns its own total reflecting the full -// match count (not just the page). Nil slices are explicitly [] so JSON -// doesn't emit null. -func (h *handlers) handleSearch(w http.ResponseWriter, r *http.Request) { - q := strings.TrimSpace(r.URL.Query().Get("q")) - if q == "" { - writeErr(w, http.StatusBadRequest, "bad_request", "q is required") - return - } - limit, offset, err := parsePaging(r.URL.Query()) - if err != nil { - writeErr(w, http.StatusBadRequest, "bad_request", err.Error()) - return - } - - dbQ := dbq.New(h.pool) - needle := &q - - artists, err := dbQ.SearchArtists(r.Context(), dbq.SearchArtistsParams{ - Column1: needle, Limit: int32(limit), Offset: int32(offset), - }) - if err != nil { - h.logger.Error("api: search artists failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "search failed") - return - } - artistTotal, err := dbQ.CountArtistsMatching(r.Context(), q) - if err != nil { - h.logger.Error("api: count artists matching failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "search failed") - return - } - artistItems := make([]ArtistRef, 0, len(artists)) - for _, a := range artists { - albums, aerr := dbQ.ListAlbumsByArtist(r.Context(), a.ID) - if aerr != nil { - h.logger.Error("api: list albums for search artist failed", "err", aerr) - writeErr(w, http.StatusInternalServerError, "server_error", "search failed") - return - } - artistItems = append(artistItems, artistRefFrom(a, len(albums))) - } - - albums, err := dbQ.SearchAlbums(r.Context(), dbq.SearchAlbumsParams{ - Column1: needle, Limit: int32(limit), Offset: int32(offset), - }) - if err != nil { - h.logger.Error("api: search albums failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "search failed") - return - } - albumTotal, err := dbQ.CountAlbumsMatching(r.Context(), q) - if err != nil { - h.logger.Error("api: count albums matching failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "search failed") - return - } - albumItems, err := h.resolveAlbumRefs(r.Context(), dbQ, albums) - if err != nil { - writeErr(w, http.StatusInternalServerError, "server_error", "search failed") - return - } - - tracks, err := dbQ.SearchTracks(r.Context(), dbq.SearchTracksParams{ - Column1: needle, Limit: int32(limit), Offset: int32(offset), - }) - if err != nil { - h.logger.Error("api: search tracks failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "search failed") - return - } - trackTotal, err := dbQ.CountTracksMatching(r.Context(), q) - if err != nil { - h.logger.Error("api: count tracks matching failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "search failed") - return - } - trackItems, err := h.resolveTrackRefs(r.Context(), dbQ, tracks) - if err != nil { - writeErr(w, http.StatusInternalServerError, "server_error", "search failed") - return - } - - writeJSON(w, http.StatusOK, SearchResponse{ - Artists: Page[ArtistRef]{Items: artistItems, Total: int(artistTotal), Limit: limit, Offset: offset}, - Albums: Page[AlbumRef]{Items: albumItems, Total: int(albumTotal), Limit: limit, Offset: offset}, - Tracks: Page[TrackRef]{Items: trackItems, Total: int(trackTotal), Limit: limit, Offset: offset}, - }) -} - -// resolveAlbumRefs builds AlbumRefs from dbq.Album rows: resolves artist -// name (one query per distinct artist) and track count per album (one -// query each). Acceptable at limit=200 page size. -func (h *handlers) resolveAlbumRefs(ctx context.Context, q *dbq.Queries, albums []dbq.Album) ([]AlbumRef, error) { - items := make([]AlbumRef, 0, len(albums)) - names, err := resolveArtistNames(ctx, q, collectArtistIDs(albums)) - if err != nil { - h.logger.Error("api: resolve artist names failed", "err", err) - return nil, err - } - for _, a := range albums { - count, cerr := q.CountTracksByAlbum(ctx, a.ID) - if cerr != nil { - h.logger.Error("api: count tracks for album failed", "err", cerr) - return nil, cerr - } - items = append(items, albumRefFrom(a, names[uuidToString(a.ArtistID)], int(count), 0)) - } - return items, nil -} - -// resolveTrackRefs builds TrackRefs from dbq.Track rows: looks up the -// parent album + artist by id for each track (pair of queries per track). -// At limit=200 this is 400 queries worst case — acceptable for M1.5. -func (h *handlers) resolveTrackRefs(ctx context.Context, q *dbq.Queries, tracks []dbq.Track) ([]TrackRef, error) { - items := make([]TrackRef, 0, len(tracks)) - for _, t := range tracks { - album, aerr := q.GetAlbumByID(ctx, t.AlbumID) - if aerr != nil { - h.logger.Error("api: get album for track failed", "err", aerr) - return nil, aerr - } - artist, aerr := q.GetArtistByID(ctx, t.ArtistID) - if aerr != nil { - h.logger.Error("api: get artist for track failed", "err", aerr) - return nil, aerr - } - items = append(items, trackRefFrom(t, album.Title, artist.Name)) - } - return items, nil -} - -// collectArtistIDs extracts the distinct-preserving ordered list of artist -// ids from a slice of albums. resolveArtistNames dedupes internally. -func collectArtistIDs(albums []dbq.Album) []pgtype.UUID { - ids := make([]pgtype.UUID, 0, len(albums)) - for _, a := range albums { - ids = append(ids, a.ArtistID) - } - return ids -} - -// resolveArtistNames fetches each distinct artist id and returns a -// uuid-string → name map. Duplicated from subsonic intentionally. -func resolveArtistNames(ctx context.Context, q *dbq.Queries, ids []pgtype.UUID) (map[string]string, error) { - seen := make(map[string]struct{}, len(ids)) - out := make(map[string]string, len(ids)) - for _, id := range ids { - key := uuidToString(id) - if _, ok := seen[key]; ok { - continue - } - seen[key] = struct{}{} - a, err := q.GetArtistByID(ctx, id) - if err != nil { - return nil, err - } - out[key] = a.Name - } - return out, nil -} -``` - -- [ ] **Step 4: Remove the `handleSearch` stub from `internal/api/library.go`** - -Delete this block from `library.go`: - -```go -// handleSearch is implemented in Task 10 (file: search.go). -func (h *handlers) handleSearch(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "stub") -} -``` - -- [ ] **Step 5: Run — expect PASS** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestHandleSearch' -count=1 -v` -Expected: five tests PASS. - -- [ ] **Step 6: Full api suite stays green** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -count=1` -Expected: all PASS. - -- [ ] **Step 7: Commit** - -```bash -git add internal/api/search.go internal/api/search_test.go internal/api/library.go -git commit -m "feat(api): GET /api/search with three paged facets" -``` - ---- - -## Task 11: Register routes in production mount - -The tests exercise a test-only router; the production `api.Mount` must also wire all five routes so the SPA can hit them. Once wired, `RequireUser` gates them. - -**Files:** -- Modify: `internal/api/api.go` - -- [ ] **Step 1: Write failing test for production route registration** - -Append to `internal/api/library_test.go`: - -```go - -// TestRoutesRegisteredInMount verifies the production Mount() wires every -// library route. We hit each path through the real Mount (no RequireUser -// stripped — we expect 401 from the middleware, which is fine — that -// proves the route reached the authenticated group). -func TestRoutesRegisteredInMount(t *testing.T) { - h, _ := testHandlers(t) - r := chi.NewRouter() - Mount(r, h.pool, h.logger) - - paths := []string{ - "/api/artists", - "/api/artists/00000000-0000-0000-0000-000000000001", - "/api/albums/00000000-0000-0000-0000-000000000001", - "/api/tracks/00000000-0000-0000-0000-000000000001", - "/api/search?q=x", - } - for _, p := range paths { - req := httptest.NewRequest(http.MethodGet, p, nil) - w := httptest.NewRecorder() - r.ServeHTTP(w, req) - // Unauthenticated → 401. What we must NOT see is 404 (route missing). - if w.Code == http.StatusNotFound { - t.Errorf("path %s returned 404 — route not registered", p) - } - if w.Code != http.StatusUnauthorized { - t.Errorf("path %s status = %d, want 401 (unauth)", p, w.Code) - } - } -} -``` - -- [ ] **Step 2: Run — expect FAIL** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestRoutesRegisteredInMount' -count=1` -Expected: 404s — routes not in Mount yet. - -- [ ] **Step 3: Update `internal/api/api.go`** - -Replace the existing `Mount` function with: - -```go -// Mount attaches /api/* handlers to r. Public endpoints (login) are outside -// RequireUser; everything else is gated by the middleware. -func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger) { - h := &handlers{pool: pool, logger: logger} - - r.Route("/api", func(api chi.Router) { - api.Post("/auth/login", h.handleLogin) - api.Group(func(authed chi.Router) { - authed.Use(auth.RequireUser(pool)) - authed.Post("/auth/logout", h.handleLogout) - authed.Get("/me", h.handleGetMe) - - authed.Get("/artists", h.handleListArtists) - authed.Get("/artists/{id}", h.handleGetArtist) - authed.Get("/albums/{id}", h.handleGetAlbum) - authed.Get("/tracks/{id}", h.handleGetTrack) - authed.Get("/search", h.handleSearch) - }) - }) -} -``` - -- [ ] **Step 4: Run — expect PASS** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -run 'TestRoutesRegisteredInMount' -count=1 -v` -Expected: all paths return 401, test PASSes. - -- [ ] **Step 5: api package suite stays green** - -Run: `MINSTREL_TEST_DATABASE_URL=$MINSTREL_TEST_DATABASE_URL go test ./internal/api/ -count=1` -Expected: all api tests PASS. - -Optionally run `go test ./... -count=1` to catch cross-package regressions. Known flaky integration tests outside `internal/api/` (bootstrap, scanner) may fail independently — those are pre-existing and out of scope for this plan. - -- [ ] **Step 6: Commit** - -```bash -git add internal/api/api.go internal/api/library_test.go -git commit -m "feat(api): register library + search routes under RequireUser" -``` - ---- - -## Task 12: Manual smoke + PR - -Live-binary verification: start the server and hit the endpoints end-to-end, then open a PR. - -- [ ] **Step 1: Build and run the server** - -Run: `make build && ./bin/minstrel server` (or whatever the invocation is on your branch — check the Makefile's `build` target). - -Expected: server listens on the configured port without errors in the log. - -- [ ] **Step 2: Log in to capture a session cookie + token** - -Run: - -```bash -curl -s -i -X POST http://localhost:8080/api/auth/login \ - -H 'content-type: application/json' \ - -d '{"username":"admin","password":""}' -``` - -Expected: `200 OK`, `Set-Cookie: minstrel_session=...`, JSON body with `token` and `user`. - -Copy the token from the JSON body; call it `$TOKEN` below. - -- [ ] **Step 3: Smoke each endpoint** - -Run: - -```bash -curl -s -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/artists | jq . -curl -s -H "Authorization: Bearer $TOKEN" 'http://localhost:8080/api/artists?sort=newest&limit=5' | jq . -curl -s -H "Authorization: Bearer $TOKEN" 'http://localhost:8080/api/search?q=a&limit=3' | jq . -# Grab a real artist id from /api/artists output, substitute below: -curl -s -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/artists/ | jq . -# Grab a real album id from the artist detail output: -curl -s -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/albums/ | jq . -# Grab a real track id from the album detail output: -curl -s -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/tracks/ | jq . -``` - -Expected: -- Each returns 200 with valid JSON in the documented shape. -- List response contains `items`, `total`, `limit`, `offset`. -- Album detail's `cover_url` ends in `/cover` and tracks' `stream_url` end in `/stream`. -- Error path: `curl -s -H "Authorization: Bearer $TOKEN" http://localhost:8080/api/albums/not-a-uuid | jq .` returns 400 with `{error:{code:"bad_request",...}}`. - -- [ ] **Step 4: Verify unauth paths are 401** - -Run: `curl -s -o /dev/null -w '%{http_code}\n' http://localhost:8080/api/artists` -Expected: `401`. - -- [ ] **Step 5: Push branch and open PR** - -Run: - -```bash -git push -u origin dev -``` - -Then open a PR against `main` via `gh` (or whatever the project uses; the memory notes Forgejo MCP): - -PR title: `feat: library reads + search API (/api/artists, albums, tracks, search)` - -PR body: - -``` -Implements Plan 2 of the web UI rollout: paginated artist list with alpha/newest sort, artist/album/track detail, and unified search. - -Spec: docs/superpowers/specs/2026-04-20-web-ui-library-reads-design.md - -Highlights: -- Enveloped pagination: {items, total, limit, offset} -- Ref/Detail response split mirroring subsonic -- Forward-reference stream/cover URLs embedded (Plan 3 implements the endpoints) -- Empty slices always render as [] (never null) - -Test plan: -- [x] Unit tests for conversion helpers -- [x] Integration tests per endpoint (happy path, 400/404 paths, paging, sort) -- [x] Route-registration test verifies all five routes wired under RequireUser -- [x] Manual smoke verified against running server -``` - -- [ ] **Step 6: Wait for CI** - -Check Forgejo Actions; fix any lint/test failures surfaced there and push. Once green, merge per project workflow (dev → protected main). - ---- - -## Self-Review Checklist (run before handing off) - -1. **Spec coverage:** - - Routes: Tasks 6 (tracks), 7 (albums), 8 (artist detail), 9 (artist list), 10 (search), 11 (mount) ✓ - - Pagination envelope `Page[T]`: Task 2 ✓ - - Sort modes alpha/newest: Task 1 (SQL), Task 9 (handler + tests) ✓ - - URL embedding: Task 3 (helpers), projection in Task 4 ✓ - - Error taxonomy (400/404/401/500): exercised per-endpoint test ✓ - - Empty-slice-never-null rule: Tasks 7, 8, 9, 10 assert explicitly ✓ - -2. **Placeholder scan:** no TBD/TODO/"add appropriate error handling" — every step shows code or exact commands. - -3. **Type consistency:** Helpers named `artistRefFrom`, `albumRefFrom`, `trackRefFrom` consistently across Tasks 4–10. `parsePaging` signature `(raw map[string][]string) (limit, offset int, err error)` used identically in Tasks 9 and 10. `newLibraryRouter` and `withUserCtx` helpers defined in Task 6 are reused in Tasks 7–11. diff --git a/docs/superpowers/plans/2026-04-20-web-ui-server-auth-foundation.md b/docs/superpowers/plans/2026-04-20-web-ui-server-auth-foundation.md deleted file mode 100644 index e43e3351..00000000 --- a/docs/superpowers/plans/2026-04-20-web-ui-server-auth-foundation.md +++ /dev/null @@ -1,1458 +0,0 @@ -# Web UI Scaffold — Server Auth Foundation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Land the `/api/*` authentication foundation that the upcoming SvelteKit SPA and the future Flutter client will share. Adds a sessions table, a `POST /api/auth/login` / `POST /api/auth/logout` / `GET /api/me` surface, and a `RequireUser` middleware that accepts either a session cookie or an `Authorization: Bearer` header. - -**Architecture:** New `internal/api` package holds native JSON handlers (no Subsonic envelope). Sessions are opaque 32-byte tokens, stored as sha256 hashes in a new `sessions` table. `POST /api/auth/login` sets an `httpOnly; SameSite=Strict` cookie and returns the same token in the JSON body so Flutter can use `Authorization: Bearer` later. Middleware tries cookie first, bearer second. `/rest/*` is untouched. - -**Tech Stack:** Go 1.23, pgx/v5, sqlc, chi router, golang-migrate, bcrypt, stdlib `crypto/sha256` + `crypto/rand`. - -**Scope guard:** This plan intentionally stops at auth + `/api/me`. Library browse / search / stream / cover-art endpoints and the SvelteKit project are separate plans that land after this one. - ---- - -## File Structure - -**New files:** - -- `internal/db/migrations/0004_sessions.up.sql` — create sessions table -- `internal/db/migrations/0004_sessions.down.sql` — drop sessions table -- `internal/db/queries/sessions.sql` — sqlc queries for sessions -- `internal/db/dbq/sessions.sql.go` — **generated** by `make generate` -- `internal/auth/session.go` — session mint / hash / lookup helpers + `VerifyPassword` + `RequireUser` -- `internal/auth/session_test.go` — unit tests for the helpers -- `internal/api/api.go` — package doc, route mounting (`Mount(chi.Router, pool, logger)`) -- `internal/api/types.go` — shared request/response types -- `internal/api/auth.go` — `handleLogin`, `handleLogout` -- `internal/api/auth_test.go` — handler tests -- `internal/api/me.go` — `handleGetMe` -- `internal/api/me_test.go` — handler test -- `internal/api/errors.go` — `writeErr(w, status, code, message)` helper + tests in `auth_test.go` - -**Modified files:** - -- `internal/server/server.go:35-54` — mount `api.Mount(...)` inside the `/api` route group -- `internal/auth/middleware.go` — no edits; `RequireUser` lives in the new `session.go` alongside cookie helpers - -**Guiding principle:** files stay focused. `session.go` owns the session lifecycle; each `handle*` file owns one HTTP surface. Helpers that would be reused from other `/api/*` handlers (error writer, JSON envelope) live in `api/` for future siblings to import. - ---- - -## Conventions (apply to every task unless a task says otherwise) - -- **Always run `gofmt -w` on any file you touch before committing.** -- **Test commands** are run from repo root (`/home/bvandeusen/Nextcloud/Projects/Minstrel/minstrel`). -- **`sqlc generate` is run via `make generate`.** This requires Docker; if Docker isn't available, fail the task — don't skip regeneration. -- **Commit messages follow existing repo style:** `type(scope): subject` (`feat(api): ...`, `fix(auth): ...`), one-line subject ≤ 72 chars, body explains *why*, trailer is `Co-Authored-By: Claude Opus 4.7 `. -- **Every `git commit` step below uses a HEREDOC** so multi-line messages survive shell quoting. - ---- - -### Task 1: Migration — create `sessions` table - -**Files:** -- Create: `internal/db/migrations/0004_sessions.up.sql` -- Create: `internal/db/migrations/0004_sessions.down.sql` - -- [ ] **Step 1: Write the up migration** - -File: `internal/db/migrations/0004_sessions.up.sql` - -```sql --- Session tokens authenticate /api/* requests. We store sha256(token) so a --- read-only DB leak doesn't grant active sessions; the raw token lives only --- in the client's cookie (web) or bearer header (Flutter). --- --- last_seen_at enables an "active sessions" UI later (not wired in this plan) --- without schema churn. user_agent is captured at issue time for the same --- reason — free metadata now, no migration later. - -CREATE TABLE sessions ( - id uuid PRIMARY KEY DEFAULT gen_random_uuid(), - user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, - token_hash bytea NOT NULL UNIQUE, - user_agent text NOT NULL DEFAULT '', - created_at timestamptz NOT NULL DEFAULT now(), - last_seen_at timestamptz NOT NULL DEFAULT now() -); - -CREATE INDEX sessions_user_id_idx ON sessions (user_id); -``` - -- [ ] **Step 2: Write the down migration** - -File: `internal/db/migrations/0004_sessions.down.sql` - -```sql -DROP INDEX IF EXISTS sessions_user_id_idx; -DROP TABLE IF EXISTS sessions; -``` - -- [ ] **Step 3: Run the migration against a disposable DB to prove it's well-formed** - -Command: -```bash -docker compose exec -T postgres psql -U minstrel -d minstrel -c "SELECT version FROM schema_migrations;" -docker compose restart minstrel -docker compose logs minstrel --since=30s | grep -i migrat -``` - -Expected: logs show "applied 0004_sessions" (or equivalent); `SELECT * FROM sessions LIMIT 0;` works afterwards. - -If the stack isn't running, start it: `docker compose up -d` and repeat. - -- [ ] **Step 4: Commit** - -```bash -git add internal/db/migrations/0004_sessions.up.sql internal/db/migrations/0004_sessions.down.sql -git commit -m "$(cat <<'EOF' -feat(db): add sessions table for /api/* auth - -Stores sha256(token) plus user_agent + last_seen_at so future -active-sessions UI doesn't need another migration. - -Co-Authored-By: Claude Opus 4.7 -EOF -)" -``` - ---- - -### Task 2: sqlc queries for sessions - -**Files:** -- Create: `internal/db/queries/sessions.sql` -- Regenerate: `internal/db/dbq/sessions.sql.go` (via `make generate`) - -- [ ] **Step 1: Write the queries** - -File: `internal/db/queries/sessions.sql` - -```sql --- name: InsertSession :one -INSERT INTO sessions (user_id, token_hash, user_agent) -VALUES ($1, $2, $3) -RETURNING *; - --- name: GetSessionByTokenHash :one -SELECT * FROM sessions WHERE token_hash = $1; - --- name: TouchSessionLastSeen :exec -UPDATE sessions SET last_seen_at = now() WHERE id = $1; - --- name: DeleteSession :exec -DELETE FROM sessions WHERE id = $1; - --- name: DeleteSessionByTokenHash :exec -DELETE FROM sessions WHERE token_hash = $1; -``` - -- [ ] **Step 2: Regenerate sqlc output** - -Run: `make generate` - -Expected: `internal/db/dbq/sessions.sql.go` is created with `InsertSession`, `GetSessionByTokenHash`, `TouchSessionLastSeen`, `DeleteSession`, `DeleteSessionByTokenHash`. - -- [ ] **Step 3: Verify generated code compiles** - -Run: `go build ./...` -Expected: exit 0, no output. - -- [ ] **Step 4: Commit** - -```bash -git add internal/db/queries/sessions.sql internal/db/dbq/sessions.sql.go -git commit -m "$(cat <<'EOF' -feat(dbq): generate session queries - -Adds Insert/Get/Touch/Delete helpers over the sessions table. - -Co-Authored-By: Claude Opus 4.7 -EOF -)" -``` - ---- - -### Task 3: Session helpers — mint / hash / verify password - -**Files:** -- Create: `internal/auth/session.go` -- Create: `internal/auth/session_test.go` - -- [ ] **Step 1: Write the failing test** - -File: `internal/auth/session_test.go` - -```go -package auth - -import ( - "crypto/sha256" - "encoding/base64" - "strings" - "testing" - - "golang.org/x/crypto/bcrypt" -) - -func TestMintSessionToken_ReturnsUrlSafeBase64(t *testing.T) { - token, err := MintSessionToken() - if err != nil { - t.Fatalf("MintSessionToken: %v", err) - } - if len(token) < 40 { - t.Errorf("token length = %d, want >= 40 (32B base64 url-safe)", len(token)) - } - if strings.ContainsAny(token, "+/=") { - t.Errorf("token %q contains non-url-safe chars", token) - } -} - -func TestHashSessionToken_IsDeterministicSHA256(t *testing.T) { - token := "test-token-xyz" - want := sha256.Sum256([]byte(token)) - - got := HashSessionToken(token) - if len(got) != sha256.Size { - t.Fatalf("hash length = %d, want %d", len(got), sha256.Size) - } - if base64.StdEncoding.EncodeToString(got) != base64.StdEncoding.EncodeToString(want[:]) { - t.Errorf("hash = %x, want %x", got, want) - } -} - -func TestVerifyPassword(t *testing.T) { - hash, err := bcrypt.GenerateFromPassword([]byte("hunter2"), bcrypt.MinCost) - if err != nil { - t.Fatalf("GenerateFromPassword: %v", err) - } - if !VerifyPassword(string(hash), "hunter2") { - t.Error("correct password rejected") - } - if VerifyPassword(string(hash), "wrong") { - t.Error("wrong password accepted") - } - if VerifyPassword("not-a-hash", "hunter2") { - t.Error("malformed hash accepted") - } -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `go test ./internal/auth/ -run 'TestMintSessionToken|TestHashSessionToken|TestVerifyPassword' -v` -Expected: FAIL with `undefined: MintSessionToken` / `HashSessionToken` / `VerifyPassword`. - -- [ ] **Step 3: Implement** - -File: `internal/auth/session.go` - -```go -package auth - -import ( - "crypto/rand" - "crypto/sha256" - "encoding/base64" - - "golang.org/x/crypto/bcrypt" -) - -// sessionTokenBytes is the raw entropy per session token. 32 bytes of -// crypto/rand gives ~256 bits; after base64 url-safe encoding the cookie -// value is 43 chars with no padding. -const sessionTokenBytes = 32 - -// MintSessionToken returns a freshly-generated, url-safe opaque token. -// The token is what the client carries; the DB only ever sees its sha256. -func MintSessionToken() (string, error) { - b := make([]byte, sessionTokenBytes) - if _, err := rand.Read(b); err != nil { - return "", err - } - return base64.RawURLEncoding.EncodeToString(b), nil -} - -// HashSessionToken is the single source of truth for mapping a raw token to -// the `sessions.token_hash` column. sha256 is fine here — we're not guarding -// against offline brute force (the token has 256 bits of entropy); we only -// want "leaked DB row can't be replayed without also having the raw token." -func HashSessionToken(token string) []byte { - sum := sha256.Sum256([]byte(token)) - return sum[:] -} - -// VerifyPassword is the canonical bcrypt comparison. Returns false on a -// malformed hash so callers don't need to distinguish "hash invalid" from -// "password wrong" — both are auth failures from the client's perspective. -func VerifyPassword(hash, plaintext string) bool { - return bcrypt.CompareHashAndPassword([]byte(hash), []byte(plaintext)) == nil -} -``` - -- [ ] **Step 4: Run tests to verify they pass** - -Run: `go test ./internal/auth/ -run 'TestMintSessionToken|TestHashSessionToken|TestVerifyPassword' -v` -Expected: PASS (3 tests). - -- [ ] **Step 5: Commit** - -```bash -git add internal/auth/session.go internal/auth/session_test.go -git commit -m "$(cat <<'EOF' -feat(auth): session token + password verification helpers - -Shared primitives for /api/* auth: mint a url-safe opaque token, -hash it for storage, verify a bcrypt password hash. - -Co-Authored-By: Claude Opus 4.7 -EOF -)" -``` - ---- - -### Task 4: `RequireUser` middleware (cookie or bearer) - -**Files:** -- Modify: `internal/auth/session.go` — add `RequireUser`, `sessionCookieName`, extract helper -- Modify: `internal/auth/session_test.go` — add middleware tests - -- [ ] **Step 1: Write the failing tests** - -Append to `internal/auth/session_test.go`: - -```go -import "net/http" -import "net/http/httptest" -import "context" -// (merge these imports into the existing import block) - -func TestRequireUser_RejectsWhenNoCookieOrBearer(t *testing.T) { - next := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { - t.Fatal("handler must not be called") - }) - h := RequireUser(nil)(next) - - req := httptest.NewRequest(http.MethodGet, "/api/me", nil) - w := httptest.NewRecorder() - h.ServeHTTP(w, req) - - if w.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", w.Code) - } -} - -func TestExtractBearerToken(t *testing.T) { - cases := []struct { - header string - want string - }{ - {"", ""}, - {"Bearer abc", "abc"}, - {"bearer abc", "abc"}, - {"Token abc", ""}, - {"Bearer", ""}, - {"Bearer whitespace-token ", "whitespace-token"}, - } - for _, c := range cases { - got := extractBearerToken(c.header) - if got != c.want { - t.Errorf("extractBearerToken(%q) = %q, want %q", c.header, got, c.want) - } - } -} -``` - -- [ ] **Step 2: Run to verify they fail** - -Run: `go test ./internal/auth/ -run 'TestRequireUser|TestExtractBearerToken' -v` -Expected: FAIL (`undefined: RequireUser`, `undefined: extractBearerToken`). - -- [ ] **Step 3: Add `GetUserByID` sqlc query (needed by the middleware)** - -Append to `internal/db/queries/users.sql`: - -```sql --- name: GetUserByID :one -SELECT * FROM users WHERE id = $1; -``` - -Run: `make generate` -Expected: `internal/db/dbq/users.sql.go` now contains `GetUserByID(ctx, id pgtype.UUID)`. - -- [ ] **Step 4: Implement the middleware** - -Append to `internal/auth/session.go`: - -```go -import ( - "context" - "errors" - "log/slog" - "net/http" - "strings" - - "github.com/jackc/pgx/v5" - "github.com/jackc/pgx/v5/pgxpool" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) -// (merge these imports into the existing import block at the top of session.go) - -// SessionCookieName is the cookie the web SPA rides. Exported because handlers -// that issue/clear the cookie (handleLogin / handleLogout) need to match it. -const SessionCookieName = "minstrel_session" - -// RequireUser resolves the caller from a session cookie OR Authorization -// bearer header and puts the dbq.User in request context via userCtxKey. -// Requests without a valid session return 401 with no body so callers don't -// leak whether the username existed (matches the /rest/* auth posture). -func RequireUser(pool *pgxpool.Pool) func(http.Handler) http.Handler { - return func(next http.Handler) http.Handler { - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - token := sessionTokenFromRequest(r) - if token == "" { - http.Error(w, "unauthenticated", http.StatusUnauthorized) - return - } - if pool == nil { - // Test-only path: the test at the top of this file constructs - // the middleware with nil pool to prove the no-token case - // short-circuits without a DB call. Any real token here is - // programmer error. - http.Error(w, "unauthenticated", http.StatusUnauthorized) - return - } - q := dbq.New(pool) - sess, err := q.GetSessionByTokenHash(r.Context(), HashSessionToken(token)) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - http.Error(w, "unauthenticated", http.StatusUnauthorized) - return - } - slog.Error("api: session lookup failed", "err", err) - http.Error(w, "auth lookup failed", http.StatusInternalServerError) - return - } - user, err := q.GetUserByID(r.Context(), sess.UserID) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - // Session points at a deleted user — treat as unauth, - // best-effort cleanup. - _ = q.DeleteSession(r.Context(), sess.ID) - http.Error(w, "unauthenticated", http.StatusUnauthorized) - return - } - slog.Error("api: user lookup failed", "err", err) - http.Error(w, "auth lookup failed", http.StatusInternalServerError) - return - } - // Best-effort last-seen update. A failure here shouldn't fail the - // request; the session is still valid and this is observability. - if err := q.TouchSessionLastSeen(r.Context(), sess.ID); err != nil { - slog.Warn("api: touch session last_seen failed", "err", err) - } - ctx := context.WithValue(r.Context(), userCtxKey, user) - next.ServeHTTP(w, r.WithContext(ctx)) - }) - } -} - -func sessionTokenFromRequest(r *http.Request) string { - if c, err := r.Cookie(SessionCookieName); err == nil && c.Value != "" { - return c.Value - } - return extractBearerToken(r.Header.Get("Authorization")) -} - -// extractBearerToken pulls the token out of an Authorization header in -// either "Bearer xyz" or "bearer xyz" form. Returns "" when the header is -// missing, malformed, or uses a different scheme. -func extractBearerToken(header string) string { - if header == "" { - return "" - } - const prefix = "bearer " - lower := strings.ToLower(header) - if !strings.HasPrefix(lower, prefix) { - return "" - } - return strings.TrimSpace(header[len(prefix):]) -} -``` - -- [ ] **Step 5: Run middleware tests** - -Run: `go test ./internal/auth/ -v` -Expected: all prior tests plus `TestRequireUser_RejectsWhenNoCookieOrBearer` and `TestExtractBearerToken` PASS. - -- [ ] **Step 6: Commit** - -```bash -git add internal/auth/session.go internal/auth/session_test.go internal/db/queries/users.sql internal/db/dbq/users.sql.go -git commit -m "$(cat <<'EOF' -feat(auth): RequireUser middleware (cookie or bearer) - -Resolves /api/* callers from session cookie first, Authorization -bearer second. Touches last_seen on success for future active- -sessions UI. Adds GetUserByID query used by the middleware. - -Co-Authored-By: Claude Opus 4.7 -EOF -)" -``` - ---- - -### Task 5: `internal/api` package skeleton + error helper - -**Files:** -- Create: `internal/api/api.go` -- Create: `internal/api/errors.go` -- Create: `internal/api/errors_test.go` - -- [ ] **Step 1: Write the failing test** - -File: `internal/api/errors_test.go` - -```go -package api - -import ( - "encoding/json" - "net/http" - "net/http/httptest" - "testing" -) - -func TestWriteErr_EmitsJSONEnvelope(t *testing.T) { - w := httptest.NewRecorder() - writeErr(w, http.StatusBadRequest, "bad_request", "field x required") - - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } - if got := w.Header().Get("Content-Type"); got != "application/json" { - t.Errorf("content-type = %q, want application/json", got) - } - var body struct { - Error struct { - Code string `json:"code"` - Message string `json:"message"` - } `json:"error"` - } - if err := json.Unmarshal(w.Body.Bytes(), &body); err != nil { - t.Fatalf("decode: %v\nbody=%s", err, w.Body.String()) - } - if body.Error.Code != "bad_request" || body.Error.Message != "field x required" { - t.Errorf("body = %+v", body) - } -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `go test ./internal/api/ -v` -Expected: FAIL — package doesn't exist yet. - -- [ ] **Step 3: Create the package + error helper** - -File: `internal/api/api.go` - -```go -// Package api implements Minstrel's native JSON surface under /api. It is -// consumed by the built-in web SPA and (eventually) the Flutter client. -// Subsonic-compatible endpoints under /rest are intentionally separate — -// see internal/subsonic — and the two packages must not depend on each other. -package api - -import ( - "log/slog" - "net/http" - - "github.com/go-chi/chi/v5" - "github.com/jackc/pgx/v5/pgxpool" - - "git.fabledsword.com/bvandeusen/minstrel/internal/auth" -) - -// Mount attaches /api/* handlers to r. Public endpoints (login) are outside -// RequireUser; everything else is gated by the middleware. -func Mount(r chi.Router, pool *pgxpool.Pool, logger *slog.Logger) { - h := &handlers{pool: pool, logger: logger} - - r.Route("/api", func(api chi.Router) { - api.Post("/auth/login", h.handleLogin) - api.Group(func(authed chi.Router) { - authed.Use(auth.RequireUser(pool)) - authed.Post("/auth/logout", h.handleLogout) - authed.Get("/me", h.handleGetMe) - }) - }) -} - -type handlers struct { - pool *pgxpool.Pool - logger *slog.Logger -} - -// Stub handlers so Mount() compiles. Real implementations in later tasks -// replace these in place (Task 6 login, Task 7 logout, Task 8 me). -func (h *handlers) handleLogin(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "login not wired") -} - -func (h *handlers) handleLogout(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "logout not wired") -} - -func (h *handlers) handleGetMe(w http.ResponseWriter, r *http.Request) { - writeErr(w, http.StatusNotImplemented, "not_implemented", "me not wired") -} -``` - -**Reviewer note:** Real `handleLogin` / `handleLogout` / `handleGetMe` implementations in Tasks 6–8 *move* into their own files (`auth.go`, `me.go`) and the stubs here get deleted as part of each task. - -File: `internal/api/errors.go` - -```go -package api - -import ( - "encoding/json" - "net/http" -) - -// errorBody is the JSON envelope for failures on /api/*. Kept separate from -// the Subsonic envelope so native clients don't have to parse two shapes. -type errorBody struct { - Error errorPayload `json:"error"` -} - -type errorPayload struct { - Code string `json:"code"` - Message string `json:"message"` -} - -// writeErr is the canonical way to emit a failure. Code is a stable -// short-string clients can switch on; message is human-readable. -func writeErr(w http.ResponseWriter, status int, code, message string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(errorBody{Error: errorPayload{Code: code, Message: message}}) -} -``` - -- [ ] **Step 4: Run tests and verify build** - -Run: `go build ./... && go test ./internal/api/ -v` -Expected: build exits 0; `TestWriteErr_EmitsJSONEnvelope` PASSes. - -- [ ] **Step 5: Commit** - -```bash -git add internal/api/api.go internal/api/errors.go internal/api/errors_test.go -git commit -m "$(cat <<'EOF' -feat(api): package skeleton + error envelope - -Introduces internal/api with Mount(), the {error:{code,message}} -response shape, and stubbed login/logout/me so later tasks can -land one handler at a time without breaking the build. - -Co-Authored-By: Claude Opus 4.7 -EOF -)" -``` - ---- - -### Task 6: `POST /api/auth/login` - -**Files:** -- Create: `internal/api/types.go` -- Modify: `internal/api/api.go` — replace login stub with real implementation -- Create: `internal/api/auth_test.go` - -- [ ] **Step 1: Write the failing test** - -File: `internal/api/auth_test.go` - -```go -package api - -import ( - "bytes" - "context" - "encoding/json" - "io" - "log/slog" - "net/http" - "net/http/httptest" - "os" - "strings" - "testing" - - "github.com/jackc/pgx/v5/pgxpool" - "golang.org/x/crypto/bcrypt" - - "git.fabledsword.com/bvandeusen/minstrel/internal/auth" - "git.fabledsword.com/bvandeusen/minstrel/internal/db" - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) - -// testHandlers spins up a handlers instance against MINSTREL_TEST_DATABASE_URL. -// Skips in -short mode or when the env var is missing, matching the pattern -// used elsewhere (scanner_test.go, etc.). -func testHandlers(t *testing.T) (*handlers, *pgxpool.Pool) { - t.Helper() - if testing.Short() { - t.Skip("skipping api integration in -short mode") - } - dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") - if dsn == "" { - t.Skip("MINSTREL_TEST_DATABASE_URL not set") - } - logger := slog.New(slog.NewTextHandler(io.Discard, nil)) - if err := db.Migrate(dsn, logger); err != nil { - t.Fatalf("migrate: %v", err) - } - pool, err := pgxpool.New(context.Background(), dsn) - if err != nil { - t.Fatalf("pool: %v", err) - } - t.Cleanup(pool.Close) - if _, err := pool.Exec(context.Background(), - "TRUNCATE sessions, users RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } - return &handlers{pool: pool, logger: logger}, pool -} - -func seedUser(t *testing.T, pool *pgxpool.Pool, username, password string, isAdmin bool) dbq.User { - t.Helper() - hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.MinCost) - if err != nil { - t.Fatalf("bcrypt: %v", err) - } - u, err := dbq.New(pool).CreateUser(context.Background(), dbq.CreateUserParams{ - Username: username, - PasswordHash: string(hash), - ApiToken: "test-api-token-" + username, - IsAdmin: isAdmin, - }) - if err != nil { - t.Fatalf("CreateUser: %v", err) - } - return u -} - -func TestHandleLogin_SuccessSetsCookieAndReturnsToken(t *testing.T) { - h, pool := testHandlers(t) - seedUser(t, pool, "alice", "hunter2", false) - - body := strings.NewReader(`{"username":"alice","password":"hunter2"}`) - req := httptest.NewRequest(http.MethodPost, "/api/auth/login", body) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - h.handleLogin(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - - var resp struct { - Token string `json:"token"` - User struct { - Username string `json:"username"` - IsAdmin bool `json:"is_admin"` - } `json:"user"` - } - if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { - t.Fatalf("decode: %v\nbody=%s", err, w.Body.String()) - } - if resp.Token == "" { - t.Error("token empty") - } - if resp.User.Username != "alice" || resp.User.IsAdmin { - t.Errorf("user = %+v, want alice/non-admin", resp.User) - } - - var cookieFound bool - for _, c := range w.Result().Cookies() { - if c.Name == auth.SessionCookieName { - cookieFound = true - if !c.HttpOnly { - t.Error("session cookie missing HttpOnly") - } - if c.SameSite != http.SameSiteStrictMode { - t.Errorf("SameSite = %v, want Strict", c.SameSite) - } - if c.Value != resp.Token { - t.Error("cookie value does not match response token") - } - } - } - if !cookieFound { - t.Error("session cookie not set") - } -} - -func TestHandleLogin_WrongPasswordReturns401(t *testing.T) { - h, pool := testHandlers(t) - seedUser(t, pool, "alice", "hunter2", false) - - body := strings.NewReader(`{"username":"alice","password":"wrong"}`) - req := httptest.NewRequest(http.MethodPost, "/api/auth/login", body) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - h.handleLogin(w, req) - - if w.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", w.Code) - } -} - -func TestHandleLogin_UnknownUserReturns401(t *testing.T) { - h, _ := testHandlers(t) - body := strings.NewReader(`{"username":"ghost","password":"whatever"}`) - req := httptest.NewRequest(http.MethodPost, "/api/auth/login", body) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - h.handleLogin(w, req) - - if w.Code != http.StatusUnauthorized { - t.Errorf("status = %d, want 401", w.Code) - } -} - -func TestHandleLogin_MalformedBodyReturns400(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodPost, "/api/auth/login", - bytes.NewReader([]byte("not-json"))) - req.Header.Set("Content-Type", "application/json") - w := httptest.NewRecorder() - h.handleLogin(w, req) - - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel go test ./internal/api/ -v -run TestHandleLogin` - -Expected: tests run (integration DSN reachable) and FAIL with "not_implemented" responses. - -If your local stack uses a different DSN, set `MINSTREL_TEST_DATABASE_URL` accordingly. If you're developing without the stack up, skip this step — the CI pipeline runs it. - -- [ ] **Step 3: Define shared types** - -File: `internal/api/types.go` - -```go -package api - -import "github.com/jackc/pgx/v5/pgtype" - -// LoginRequest is the POST /api/auth/login body. Kept boring on purpose — -// web and Flutter both send the same payload. -type LoginRequest struct { - Username string `json:"username"` - Password string `json:"password"` -} - -// LoginResponse carries the opaque session token AND the authenticated user -// so the SPA can hydrate its auth store in one round-trip. The token is also -// set as a cookie; SPAs ignore the body token, Flutter uses it as bearer. -type LoginResponse struct { - Token string `json:"token"` - User UserView `json:"user"` -} - -// UserView is the /api/* view of a user. Narrower than dbq.User — no hash, -// no api_token, no subsonic_password. -type UserView struct { - ID pgtype.UUID `json:"id"` - Username string `json:"username"` - IsAdmin bool `json:"is_admin"` -} -``` - -- [ ] **Step 4: Implement `handleLogin`** - -File: `internal/api/auth.go` - -```go -package api - -import ( - "encoding/json" - "errors" - "net/http" - "time" - - "github.com/jackc/pgx/v5" - - "git.fabledsword.com/bvandeusen/minstrel/internal/auth" - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) - -// sessionCookieMaxAge is the cookie lifetime. Sessions don't auto-expire -// server-side yet (future work); the cookie still caps browser-side lifetime -// so an abandoned laptop doesn't stay logged in forever. -const sessionCookieMaxAge = 30 * 24 * time.Hour - -func (h *handlers) handleLogin(w http.ResponseWriter, r *http.Request) { - var req LoginRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { - writeErr(w, http.StatusBadRequest, "bad_request", "invalid JSON body") - return - } - if req.Username == "" || req.Password == "" { - writeErr(w, http.StatusBadRequest, "bad_request", "username and password required") - return - } - - q := dbq.New(h.pool) - user, err := q.GetUserByUsername(r.Context(), req.Username) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - writeErr(w, http.StatusUnauthorized, "invalid_credentials", "invalid username or password") - return - } - h.logger.Error("api: user lookup failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "lookup failed") - return - } - if !auth.VerifyPassword(user.PasswordHash, req.Password) { - writeErr(w, http.StatusUnauthorized, "invalid_credentials", "invalid username or password") - return - } - - token, err := auth.MintSessionToken() - if err != nil { - h.logger.Error("api: mint session token failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "mint failed") - return - } - if _, err := q.InsertSession(r.Context(), dbq.InsertSessionParams{ - UserID: user.ID, - TokenHash: auth.HashSessionToken(token), - UserAgent: r.UserAgent(), - }); err != nil { - h.logger.Error("api: insert session failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "insert failed") - return - } - - http.SetCookie(w, &http.Cookie{ - Name: auth.SessionCookieName, - Value: token, - Path: "/", - HttpOnly: true, - Secure: r.TLS != nil, // dev over http stays functional; prod over https gets Secure - SameSite: http.SameSiteStrictMode, - MaxAge: int(sessionCookieMaxAge.Seconds()), - }) - - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(LoginResponse{ - Token: token, - User: UserView{ - ID: user.ID, - Username: user.Username, - IsAdmin: user.IsAdmin, - }, - }) -} -``` - -Delete the `handleLogin` stub from `internal/api/api.go` — the real one in `auth.go` replaces it. - -- [ ] **Step 5: Run tests** - -Run: `MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel go test ./internal/api/ -v -run TestHandleLogin` - -Expected: all four `TestHandleLogin_*` tests PASS. - -- [ ] **Step 6: Commit** - -```bash -git add internal/api/auth.go internal/api/api.go internal/api/types.go internal/api/auth_test.go -git commit -m "$(cat <<'EOF' -feat(api): POST /api/auth/login - -Verifies bcrypt password hash, mints a session token, stores its -sha256 in the sessions table, and returns the token in both the -response body (for Flutter) and an httpOnly/SameSite=Strict -cookie (for the web SPA). - -Co-Authored-By: Claude Opus 4.7 -EOF -)" -``` - ---- - -### Task 7: `POST /api/auth/logout` - -**Files:** -- Modify: `internal/api/auth.go` — add `handleLogout` -- Modify: `internal/api/api.go` — remove logout stub -- Modify: `internal/api/auth_test.go` — add logout tests - -- [ ] **Step 1: Write the failing tests** - -Append to `internal/api/auth_test.go`: - -```go -func TestHandleLogout_DeletesSessionAndClearsCookie(t *testing.T) { - h, pool := testHandlers(t) - user := seedUser(t, pool, "alice", "hunter2", false) - - // Manually create a session to log out of. - token, err := auth.MintSessionToken() - if err != nil { - t.Fatalf("mint: %v", err) - } - if _, err := dbq.New(pool).InsertSession(context.Background(), dbq.InsertSessionParams{ - UserID: user.ID, - TokenHash: auth.HashSessionToken(token), - }); err != nil { - t.Fatalf("insert: %v", err) - } - - req := httptest.NewRequest(http.MethodPost, "/api/auth/logout", nil) - req.AddCookie(&http.Cookie{Name: auth.SessionCookieName, Value: token}) - // handleLogout runs behind RequireUser in real routing; simulate that by - // putting the user into context here. - req = req.WithContext(context.WithValue(req.Context(), userCtxKeyForTest(), user)) - w := httptest.NewRecorder() - h.handleLogout(w, req) - - if w.Code != http.StatusNoContent { - t.Errorf("status = %d, want 204", w.Code) - } - - // Cookie should be cleared. - var cleared bool - for _, c := range w.Result().Cookies() { - if c.Name == auth.SessionCookieName && c.MaxAge < 0 { - cleared = true - } - } - if !cleared { - t.Error("session cookie not cleared") - } - - // Session row should be gone. - _, err = dbq.New(pool).GetSessionByTokenHash(context.Background(), auth.HashSessionToken(token)) - if err == nil { - t.Error("session row still present after logout") - } -} -``` - -**Reviewer note:** `userCtxKeyForTest()` is a test-only shim that exposes `auth.userCtxKey` to the api package, needed because context keys are unexported. Added in the next step. - -- [ ] **Step 2: Expose a test-only context key accessor** - -Append to `internal/auth/session.go`: - -```go -// UserCtxKeyForTest is exported ONLY for tests in sibling packages that need -// to inject a dbq.User into request context without going through the -// middleware. Do not use this outside _test.go files. -func UserCtxKeyForTest() any { return userCtxKey } -``` - -Append to `internal/api/auth_test.go` (top-level, after imports): - -```go -func userCtxKeyForTest() any { return auth.UserCtxKeyForTest() } -``` - -- [ ] **Step 3: Run tests to verify they fail** - -Run: `MINSTREL_TEST_DATABASE_URL=... go test ./internal/api/ -v -run TestHandleLogout` -Expected: FAIL (stub returns 501, not 204). - -- [ ] **Step 4: Implement `handleLogout`** - -Append to `internal/api/auth.go`: - -```go -func (h *handlers) handleLogout(w http.ResponseWriter, r *http.Request) { - // The session token can be on the cookie OR bearer header — RequireUser - // accepted either. Re-resolve it here so we can delete the row. - token := sessionTokenFromHTTP(r) - if token != "" { - if err := dbq.New(h.pool).DeleteSessionByTokenHash(r.Context(), auth.HashSessionToken(token)); err != nil { - h.logger.Warn("api: delete session failed", "err", err) - // Continue — logout is best-effort; the client still gets the - // cookie cleared. - } - } - http.SetCookie(w, &http.Cookie{ - Name: auth.SessionCookieName, - Value: "", - Path: "/", - HttpOnly: true, - SameSite: http.SameSiteStrictMode, - MaxAge: -1, - }) - w.WriteHeader(http.StatusNoContent) -} - -// sessionTokenFromHTTP duplicates the internal helper in internal/auth -// because that one is unexported. Cheap to repeat here; keeping the auth -// package's internal helper package-private is worth more than DRY. -func sessionTokenFromHTTP(r *http.Request) string { - if c, err := r.Cookie(auth.SessionCookieName); err == nil && c.Value != "" { - return c.Value - } - h := r.Header.Get("Authorization") - const prefix = "bearer " - if len(h) > len(prefix) && (h[:7] == "Bearer " || h[:7] == "bearer ") { - return h[len(prefix):] - } - return "" -} -``` - -Delete the logout stub from `internal/api/api.go`. - -- [ ] **Step 5: Run tests** - -Run: `MINSTREL_TEST_DATABASE_URL=... go test ./internal/api/ -v -run TestHandleLogout` -Expected: PASS. - -- [ ] **Step 6: Commit** - -```bash -git add internal/api/auth.go internal/api/api.go internal/api/auth_test.go internal/auth/session.go -git commit -m "$(cat <<'EOF' -feat(api): POST /api/auth/logout - -Deletes the session row keyed by the cookie/bearer token and -clears the cookie on the client. Best-effort DB delete — logout -still succeeds for the client if the row's already gone. - -Co-Authored-By: Claude Opus 4.7 -EOF -)" -``` - ---- - -### Task 8: `GET /api/me` - -**Files:** -- Create: `internal/api/me.go` -- Modify: `internal/api/api.go` — remove `handleGetMe` stub -- Create: `internal/api/me_test.go` - -- [ ] **Step 1: Write the failing test** - -File: `internal/api/me_test.go` - -```go -package api - -import ( - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "testing" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) - -func TestHandleGetMe_ReturnsAuthenticatedUser(t *testing.T) { - h, pool := testHandlers(t) - user := seedUser(t, pool, "alice", "hunter2", true) - - req := httptest.NewRequest(http.MethodGet, "/api/me", nil) - req = req.WithContext(context.WithValue(req.Context(), userCtxKeyForTest(), user)) - w := httptest.NewRecorder() - h.handleGetMe(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - var got UserView - if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { - t.Fatalf("decode: %v", err) - } - if got.Username != "alice" || !got.IsAdmin { - t.Errorf("user = %+v, want alice/admin", got) - } -} - -func TestHandleGetMe_MissingContextReturns500(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodGet, "/api/me", nil) - w := httptest.NewRecorder() - h.handleGetMe(w, req) - - if w.Code != http.StatusInternalServerError { - t.Errorf("status = %d, want 500 (context should be populated by RequireUser)", w.Code) - } - _ = dbq.User{} // keep the import used -} -``` - -- [ ] **Step 2: Run tests to verify they fail** - -Run: `MINSTREL_TEST_DATABASE_URL=... go test ./internal/api/ -v -run TestHandleGetMe` -Expected: FAIL (stub returns 501). - -- [ ] **Step 3: Implement `handleGetMe`** - -File: `internal/api/me.go` - -```go -package api - -import ( - "encoding/json" - "net/http" - - "git.fabledsword.com/bvandeusen/minstrel/internal/auth" -) - -func (h *handlers) handleGetMe(w http.ResponseWriter, r *http.Request) { - user, ok := auth.UserFromContext(r.Context()) - if !ok { - // Hitting /me without RequireUser in front of it is a routing bug; - // it can't happen in real traffic. - h.logger.Error("api: /me reached without authenticated user") - writeErr(w, http.StatusInternalServerError, "server_error", "missing auth context") - return - } - w.Header().Set("Content-Type", "application/json") - _ = json.NewEncoder(w).Encode(UserView{ - ID: user.ID, - Username: user.Username, - IsAdmin: user.IsAdmin, - }) -} -``` - -Delete the `handleGetMe` stub from `internal/api/api.go`. - -- [ ] **Step 4: Run tests** - -Run: `MINSTREL_TEST_DATABASE_URL=... go test ./internal/api/ -v -run TestHandleGetMe` -Expected: PASS (both tests). - -- [ ] **Step 5: Run the full api package tests to confirm nothing regressed** - -Run: `MINSTREL_TEST_DATABASE_URL=... go test ./internal/api/ -v` -Expected: every test PASSes. - -- [ ] **Step 6: Commit** - -```bash -git add internal/api/me.go internal/api/me_test.go internal/api/api.go -git commit -m "$(cat <<'EOF' -feat(api): GET /api/me - -Returns the authenticated user (id/username/is_admin). Shape -matches UserView used in the login response so SPA stores stay -typed against one interface. - -Co-Authored-By: Claude Opus 4.7 -EOF -)" -``` - ---- - -### Task 9: Mount `/api/*` in the root router - -**Files:** -- Modify: `internal/server/server.go` - -- [ ] **Step 1: Update the router** - -Replace the existing `Router()` body at `internal/server/server.go:35-54`. The edit is: after the existing `r.Route("/api", ...)` admin block, call `api.Mount(r, s.Pool, s.Logger)` at the same level. But `api.Mount` *also* opens `/api`; chi allows multiple `Route("/api", ...)` calls — but to avoid confusion, we inline the admin routes into the api package structure in a single `Route("/api", ...)` is cleanest. - -Concretely — final shape: - -```go -func (s *Server) Router() http.Handler { - r := chi.NewRouter() - r.Use(middleware.RequestID) - r.Use(middleware.Recoverer) - - r.Get("/healthz", s.handleHealthz) - - if s.Pool != nil { - api.Mount(r, s.Pool, s.Logger) - r.Route("/api/admin", func(admin chi.Router) { - admin.Use(auth.RequireAdmin(s.Pool)) - if s.Scanner != nil { - admin.Post("/scan", s.handleAdminScan) - } - }) - subsonic.Mount(r, s.Pool, s.Logger, s.SubsonicCfg) - } - return r -} -``` - -Add the import for `internal/api`: - -```go -import ( - // existing imports... - "git.fabledsword.com/bvandeusen/minstrel/internal/api" -) -``` - -**Reviewer note:** chi permits both `r.Route("/api", func(api chi.Router) {...})` opened by `api.Mount` and a sibling `r.Route("/api/admin", func...)` because the paths don't overlap at the exact level — chi's internal tree merges prefixes. If you see "handler overlaps" at startup, convert `api.Mount` to accept an optional admin wiring callback and move scan into it; flag this up to the reviewer rather than hacking around it. - -- [ ] **Step 2: Build and verify the server still starts** - -Run: `go build ./...` -Expected: exit 0. - -Run (if stack running): `docker compose restart minstrel && docker compose logs minstrel --since=30s` -Expected: "listening on :4533" and no panics. - -- [ ] **Step 3: Commit** - -```bash -git add internal/server/server.go -git commit -m "$(cat <<'EOF' -feat(server): mount /api/* (login, logout, me) - -Wires the native JSON surface alongside the existing /api/admin -and /rest/* routes. Subsonic compatibility remains untouched. - -Co-Authored-By: Claude Opus 4.7 -EOF -)" -``` - ---- - -### Task 10: End-to-end smoke (manual, recorded in the plan) - -**Files:** none (verification only) - -- [ ] **Step 1: Bring the full stack up** - -```bash -docker compose up --build -d -docker compose logs minstrel --since=60s | grep -iE 'listening|error|panic' -``` - -Expected: "listening on :4533", no errors. - -- [ ] **Step 2: Login with the bootstrap admin** - -```bash -# Replace PASSWORD with the bootstrap password from startup logs. -curl -i -X POST http://localhost:4533/api/auth/login \ - -H 'Content-Type: application/json' \ - -d '{"username":"admin","password":"PASSWORD"}' -``` - -Expected: HTTP/1.1 200 OK; body contains `{"token":"...","user":{...}}`; `Set-Cookie: minstrel_session=...; HttpOnly; SameSite=Strict`. - -- [ ] **Step 3: Call `/api/me` with the cookie** - -```bash -curl -i http://localhost:4533/api/me \ - -H "Cookie: minstrel_session=" -``` - -Expected: HTTP/1.1 200 OK; body is `{"id":"...","username":"admin","is_admin":true}`. - -- [ ] **Step 4: Call `/api/me` with bearer instead** - -```bash -curl -i http://localhost:4533/api/me \ - -H "Authorization: Bearer " -``` - -Expected: HTTP/1.1 200 OK; same body. - -- [ ] **Step 5: Verify 401 without auth** - -```bash -curl -i http://localhost:4533/api/me -``` - -Expected: HTTP/1.1 401 Unauthorized. - -- [ ] **Step 6: Verify 401 with wrong password on login** - -```bash -curl -i -X POST http://localhost:4533/api/auth/login \ - -H 'Content-Type: application/json' \ - -d '{"username":"admin","password":"nope"}' -``` - -Expected: HTTP/1.1 401 Unauthorized; body `{"error":{"code":"invalid_credentials","message":"..."}}`. - -- [ ] **Step 7: Logout clears the session** - -```bash -curl -i -X POST http://localhost:4533/api/auth/logout \ - -H "Cookie: minstrel_session=" -``` - -Expected: HTTP/1.1 204 No Content; `Set-Cookie: minstrel_session=; Max-Age=0`. - -Then reuse that token against `/api/me` and expect 401. - -- [ ] **Step 8: Open the PR** - -After all smoke steps pass: - -```bash -git push origin dev -``` - -Then create a PR from `dev` → `main` using the Forgejo MCP (see git workflow memory). Poll CI; merge when green; `git pull --ff-only origin dev` locally. - -Done. - ---- - -## Self-Review - -**Spec coverage check** (against `docs/superpowers/specs/2026-04-20-web-ui-scaffold-design.md`): - -- Sessions table (spec §Authentication → Sessions table) — Task 1. -- sha256(token) at rest (spec §Authentication → Sessions table) — Task 3. -- Login issues both cookie and bearer token (spec §Authentication → Login flow) — Task 6. -- `httpOnly; SameSite=Strict` cookie (spec §Authentication → Login flow) — Task 6. -- Middleware resolves cookie → bearer (spec §Authentication → Middleware) — Task 4. -- `401` on unauthenticated `/api/*` (spec §Authentication → Middleware) — Task 4 + Task 10. -- `/api/me` returns `{id, username, is_admin}` (spec §API surface → first-cut endpoints) — Task 8. -- `/rest/*` untouched (spec §Non-goals and §API surface) — no task modifies `internal/subsonic`, no test imports it. -- Logout deletes session + clears cookie — spec §Authentication → Login flow mentions "expire via Set-Cookie" and sessions table Task 2 includes `DeleteSessionByTokenHash`. — Task 7. -- OIDC headroom (spec §Open questions resolved) — design preserved: login endpoint is the only auth-material touchpoint, so OIDC callback can mint the same cookie+token pair without schema change. - -**Not yet landed** (other plans): -- `/api/artists`, `/api/albums/{id}`, `/api/tracks/{id}`, `/api/search`, `/api/stream/{id}`, `/api/cover/{id}` — Plan 2 (server library reads). -- SvelteKit project, Dockerfile multi-stage, `embed.FS` wiring — Plan 3 (web SPA + packaging). - -**Placeholder scan:** No TBDs, TODOs, or "implement later" markers. Each step has the exact code or command. - -**Type consistency check:** -- `UserView` defined in `types.go` (Task 6), used in `LoginResponse` (Task 6), returned from `handleGetMe` (Task 8) — names match. -- `LoginRequest`/`LoginResponse` — defined once, referenced only by `handleLogin`. -- `SessionCookieName` exported from `internal/auth/session.go` (Task 4), referenced from `internal/api/auth.go` (Task 6), `internal/api/auth_test.go` (Task 6), `internal/api/me.go` (nope — `me.go` uses `UserFromContext`), and the manual curl in Task 10. Consistent. -- `HashSessionToken` / `MintSessionToken` / `VerifyPassword` — defined Task 3, used Tasks 4, 6, 7. Consistent. -- `sessionTokenFromHTTP` (api package, Task 7) vs `sessionTokenFromRequest` (auth package, Task 4) — separate helpers, documented as such. diff --git a/docs/superpowers/plans/2026-04-21-web-ui-media-endpoints.md b/docs/superpowers/plans/2026-04-21-web-ui-media-endpoints.md deleted file mode 100644 index 14a069f2..00000000 --- a/docs/superpowers/plans/2026-04-21-web-ui-media-endpoints.md +++ /dev/null @@ -1,1066 +0,0 @@ -# Web UI Media Endpoints Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Ship `/api/albums/{id}/cover` and `/api/tracks/{id}/stream` so Plan 2's forward-reference `cover_url` and `stream_url` fields resolve in the SPA. - -**Architecture:** One new file `internal/api/media.go` holds both handlers plus locally-duplicated filesystem/MIME helpers — no shared package with `internal/subsonic`, per the subsonic-is-long-term-legacy direction. Byte-serving uses `http.ServeContent`, which handles Range / If-Modified-Since / ETag automatically. Auth is the existing `RequireUser` middleware (cookie or bearer token). - -**Tech Stack:** Go 1.23, chi/v5, pgx/v5, pgxpool, stdlib `net/http`, `os`, `path/filepath`. - -**Spec:** `docs/superpowers/specs/2026-04-21-web-ui-media-endpoints-design.md` - ---- - -## File Structure - -New files: - -| File | Responsibility | -|---|---| -| `internal/api/media.go` | Both handlers + helpers (`resolveAlbumCoverPath`, `findSidecarCover`, `audioContentType`, `imageContentType`) | -| `internal/api/media_test.go` | Unit tests for MIME helpers + integration tests for both endpoints | - -Modified files: - -| File | Change | -|---|---| -| `internal/api/library_fixtures_test.go` | Add `seedTrackWithFile` helper that materializes bytes on disk | -| `internal/api/library_test.go` | Extend `newLibraryRouter` (register `/cover` + `/stream`) and `TestRoutesRegisteredInMount` (assert production Mount wires both) | -| `internal/api/api.go` | Register both routes inside the existing `authed` chi group | - -No migrations. No new sqlc queries — `GetAlbumByID`, `GetTrackByID`, and `ListTracksByAlbum` already exist. - ---- - -## Working Conventions - -- **Commit after each green step.** One task = at least one commit. Multiple commits per task if convenient. -- **Integration tests require a live Postgres** at `MINSTREL_TEST_DATABASE_URL`. `docker-compose.yml` spins one up; tests skip cleanly when the env var is unset. Run the full suite with `MINSTREL_TEST_DATABASE_URL=postgres://… go test ./...` before committing. -- **Do NOT touch `internal/subsonic/*`**. If you notice the helpers here duplicate code in `internal/subsonic/stream.go`, that's intentional — see the memory note `project_subsonic_legacy.md`. -- **Go unused-import rule:** only add an import once the code actually uses it; the compiler rejects unused imports. -- **File-format and extension tables** in this plan are authoritative — they are the ones the spec approved, which differ slightly from subsonic's table. Do not "fix" them to match subsonic. - ---- - -## Task 1: Scaffold `media.go` with helpers + unit tests - -**Files:** -- Create: `internal/api/media.go` -- Create: `internal/api/media_test.go` -- Modify: `internal/api/library_fixtures_test.go` - -This task produces the four package-private helpers — no handlers yet. The handlers land in Tasks 2 and 3. Separating them keeps each commit's diff tight. - -- [ ] **Step 1: Create `internal/api/media.go` with helpers only** - -```go -// Package api — media endpoints serve raw bytes (cover art, audio streams). -// The helpers below duplicate shape with internal/subsonic/stream.go by -// design; the two packages are kept independent so subsonic can freeze as a -// compatibility surface while /api evolves. See project memory -// `project_subsonic_legacy.md` for the rationale. - -package api - -import ( - "context" - "os" - "path/filepath" - "strings" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) - -// sidecarNames is the lookup order for cover art living next to audio files. -// Matches common library conventions: cover.* wins over folder.*; JPEG wins -// over PNG when both exist. -var sidecarNames = []string{ - "cover.jpg", "cover.jpeg", "cover.png", - "folder.jpg", "folder.jpeg", "folder.png", -} - -// findSidecarCover looks for a conventional cover image in the directory that -// contains trackPath. Returns "" if no sidecar exists. -func findSidecarCover(trackPath string) string { - dir := filepath.Dir(trackPath) - for _, name := range sidecarNames { - candidate := filepath.Join(dir, name) - if info, err := os.Stat(candidate); err == nil && !info.IsDir() { - return candidate - } - } - return "" -} - -// resolveAlbumCoverPath returns the filesystem path to the album's cover art. -// It prefers an explicit cover_art_path (set by the scanner in a future -// milestone) and falls back to a sidecar next to the first track in the -// album's directory. "" means no art was found. -func resolveAlbumCoverPath(ctx context.Context, q *dbq.Queries, album dbq.Album) string { - if album.CoverArtPath != nil && *album.CoverArtPath != "" { - if _, err := os.Stat(*album.CoverArtPath); err == nil { - return *album.CoverArtPath - } - } - tracks, err := q.ListTracksByAlbum(ctx, album.ID) - if err != nil || len(tracks) == 0 { - return "" - } - return findSidecarCover(tracks[0].FilePath) -} - -// audioContentType maps the short file_format recorded on tracks (mp3, flac, -// ogg, opus, m4a, aac, wav) to a MIME type for the Content-Type header. -// Unknown formats fall back to octet-stream so the browser downloads them -// rather than attempting to decode. -func audioContentType(format string) string { - switch strings.ToLower(format) { - case "mp3": - return "audio/mpeg" - case "flac": - return "audio/flac" - case "ogg", "opus": - return "audio/ogg" - case "m4a": - return "audio/mp4" - case "aac": - return "audio/aac" - case "wav": - return "audio/wav" - } - return "application/octet-stream" -} - -// imageContentType maps a file extension to a MIME type for cover art. -// Unknown extensions fall back to octet-stream. -func imageContentType(path string) string { - switch strings.ToLower(filepath.Ext(path)) { - case ".jpg", ".jpeg": - return "image/jpeg" - case ".png": - return "image/png" - case ".webp": - return "image/webp" - case ".gif": - return "image/gif" - } - return "application/octet-stream" -} -``` - -- [ ] **Step 2: Append `seedTrackWithFile` to `internal/api/library_fixtures_test.go`** - -Inside the same `package api` file, add (alongside the existing helpers): - -```go -// seedTrackWithFile creates a fresh temp directory, writes fileBody to -// /.<ext>, and inserts a Track row whose file_path points at it. -// Returns the inserted Track and the directory (callers drop sidecar covers -// into this dir when a test needs them). FileFormat is ext lowercased. -func seedTrackWithFile(t *testing.T, pool *pgxpool.Pool, albumID, artistID pgtype.UUID, title string, fileBody []byte, ext string) (dbq.Track, string) { - t.Helper() - dir := t.TempDir() - path := filepath.Join(dir, title+"."+ext) - if err := os.WriteFile(path, fileBody, 0o644); err != nil { - t.Fatalf("WriteFile(%s): %v", path, err) - } - one := int32(1) - tr, err := dbq.New(pool).UpsertTrack(context.Background(), dbq.UpsertTrackParams{ - Title: title, - AlbumID: albumID, - ArtistID: artistID, - TrackNumber: &one, - DiscNumber: nil, - DurationMs: int32(len(fileBody)), - FilePath: path, - FileSize: int64(len(fileBody)), - FileFormat: strings.ToLower(ext), - Bitrate: nil, - Mbid: nil, - Genre: nil, - }) - if err != nil { - t.Fatalf("UpsertTrack(%s): %v", title, err) - } - return tr, dir -} -``` - -Add these imports to `library_fixtures_test.go` (keep existing imports; add only the new ones): - -```go -import ( - "context" - "os" - "path/filepath" - "strings" - "testing" - "time" - - "github.com/jackc/pgx/v5/pgtype" - "github.com/jackc/pgx/v5/pgxpool" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) -``` - -(`os`, `path/filepath`, `strings` are the additions.) - -- [ ] **Step 3: Create `internal/api/media_test.go` with unit tests for the MIME helpers** - -```go -package api - -import ( - "context" - "os" - "path/filepath" - "testing" -) - -func TestAudioContentType(t *testing.T) { - cases := map[string]string{ - "mp3": "audio/mpeg", - "MP3": "audio/mpeg", - "flac": "audio/flac", - "ogg": "audio/ogg", - "opus": "audio/ogg", - "m4a": "audio/mp4", - "aac": "audio/aac", - "wav": "audio/wav", - "unknown": "application/octet-stream", - "": "application/octet-stream", - } - for in, want := range cases { - if got := audioContentType(in); got != want { - t.Errorf("audioContentType(%q) = %q, want %q", in, got, want) - } - } -} - -func TestImageContentType(t *testing.T) { - cases := map[string]string{ - "/a/cover.jpg": "image/jpeg", - "/a/cover.JPG": "image/jpeg", - "/a/cover.jpeg": "image/jpeg", - "/a/cover.png": "image/png", - "/a/cover.webp": "image/webp", - "/a/cover.gif": "image/gif", - "/a/cover.bmp": "application/octet-stream", - "/a/cover": "application/octet-stream", - } - for in, want := range cases { - if got := imageContentType(in); got != want { - t.Errorf("imageContentType(%q) = %q, want %q", in, got, want) - } - } -} - -func TestFindSidecarCover_PrefersCoverOverFolder(t *testing.T) { - dir := t.TempDir() - // Both present — cover.jpg wins. - if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), []byte("c"), 0o644); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(filepath.Join(dir, "folder.jpg"), []byte("f"), 0o644); err != nil { - t.Fatal(err) - } - got := findSidecarCover(filepath.Join(dir, "any-track.flac")) - if got != filepath.Join(dir, "cover.jpg") { - t.Errorf("got %q, want cover.jpg path", got) - } -} - -func TestFindSidecarCover_FallsBackToFolder(t *testing.T) { - dir := t.TempDir() - if err := os.WriteFile(filepath.Join(dir, "folder.png"), []byte("f"), 0o644); err != nil { - t.Fatal(err) - } - got := findSidecarCover(filepath.Join(dir, "t.flac")) - if got != filepath.Join(dir, "folder.png") { - t.Errorf("got %q, want folder.png path", got) - } -} - -func TestFindSidecarCover_NoneFound(t *testing.T) { - dir := t.TempDir() - if got := findSidecarCover(filepath.Join(dir, "t.flac")); got != "" { - t.Errorf("got %q, want empty string", got) - } -} - -func TestResolveAlbumCoverPath_ExplicitPresent(t *testing.T) { - _, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - - // Put an explicit cover file on disk, then write its path to album.cover_art_path. - dir := t.TempDir() - explicit := filepath.Join(dir, "explicit.jpg") - if err := os.WriteFile(explicit, []byte("e"), 0o644); err != nil { - t.Fatal(err) - } - if _, err := pool.Exec(context.Background(), - `UPDATE albums SET cover_art_path = $1 WHERE id = $2`, explicit, album.ID); err != nil { - t.Fatalf("UPDATE albums: %v", err) - } - // Refetch so album.CoverArtPath reflects the update. - got := resolveAlbumCoverPath(context.Background(), dbq.New(pool), fetchAlbum(t, pool, album.ID)) - if got != explicit { - t.Errorf("got %q, want %q", got, explicit) - } -} - -func TestResolveAlbumCoverPath_ExplicitMissingFallsThroughToSidecar(t *testing.T) { - _, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - - // Seed track first (helper owns the temp dir), then drop the sidecar in - // that same dir. Explicit cover_art_path points at a file we never create. - _, dir := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", []byte("audio"), "mp3") - sidecar := filepath.Join(dir, "cover.jpg") - if err := os.WriteFile(sidecar, []byte("s"), 0o644); err != nil { - t.Fatal(err) - } - if _, err := pool.Exec(context.Background(), - `UPDATE albums SET cover_art_path = $1 WHERE id = $2`, - filepath.Join(dir, "does-not-exist.jpg"), album.ID); err != nil { - t.Fatalf("UPDATE albums: %v", err) - } - - got := resolveAlbumCoverPath(context.Background(), dbq.New(pool), fetchAlbum(t, pool, album.ID)) - if got != sidecar { - t.Errorf("got %q, want %q", got, sidecar) - } -} - -func TestResolveAlbumCoverPath_NoArt(t *testing.T) { - _, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - seedTrack(t, pool, album.ID, artist.ID, "t", 1, 1000) // synthetic /seed path, no real file - got := resolveAlbumCoverPath(context.Background(), dbq.New(pool), album) - if got != "" { - t.Errorf("got %q, want empty string", got) - } -} - -// fetchAlbum reloads the album row so tests see updates made via raw SQL. -func fetchAlbum(t *testing.T, pool *pgxpool.Pool, id pgtype.UUID) dbq.Album { - t.Helper() - a, err := dbq.New(pool).GetAlbumByID(context.Background(), id) - if err != nil { - t.Fatalf("GetAlbumByID: %v", err) - } - return a -} -``` - -Add to the imports at top of `media_test.go` as they're used: - -```go -import ( - "context" - "os" - "path/filepath" - "testing" - - "github.com/jackc/pgx/v5/pgtype" - "github.com/jackc/pgx/v5/pgxpool" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) -``` - -- [ ] **Step 4: Run the new tests; all must pass** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./internal/api/ -run 'TestAudioContentType|TestImageContentType|TestFindSidecarCover|TestResolveAlbumCoverPath' -v -``` - -Expected: all 7 tests PASS. - -- [ ] **Step 5: Run the full api test suite to catch regressions in shared fixtures** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./internal/api/... -count=1 -``` - -Expected: all tests PASS (existing + new). - -- [ ] **Step 6: Commit** - -``` -git add internal/api/media.go internal/api/media_test.go internal/api/library_fixtures_test.go -git commit -m "feat(api): scaffold media helpers for cover + stream endpoints" -``` - ---- - -## Task 2: Implement `GET /api/albums/{id}/cover` - -**Files:** -- Modify: `internal/api/media.go` (add handler) -- Modify: `internal/api/media_test.go` (add integration tests) -- Modify: `internal/api/library_test.go` (register route in `newLibraryRouter`) - -TDD order — tests first, then handler. Tests fail first because the handler doesn't exist yet; they pass after it's added. - -- [ ] **Step 1: Register `/cover` in `newLibraryRouter`** - -Edit `internal/api/library_test.go:16-24` and add the cover route: - -```go -func newLibraryRouter(h *handlers) chi.Router { - r := chi.NewRouter() - r.Get("/api/artists", h.handleListArtists) - r.Get("/api/artists/{id}", h.handleGetArtist) - r.Get("/api/albums/{id}", h.handleGetAlbum) - r.Get("/api/albums/{id}/cover", h.handleGetCover) - r.Get("/api/tracks/{id}", h.handleGetTrack) - r.Get("/api/search", h.handleSearch) - return r -} -``` - -At this point the package won't compile — `handleGetCover` is undefined. That's expected; the integration tests need the route registered so they can fail with "handler missing" in the same compile unit as the test router. (Implementer hint: if the compile error bothers you, add the method body as `func (h *handlers) handleGetCover(w http.ResponseWriter, r *http.Request) {}` as a temporary stub for Step 2. Remove the stub before Step 4.) - -- [ ] **Step 2: Append integration tests to `internal/api/media_test.go`** - -```go -func TestHandleGetCover_SidecarHappyPath(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - - _, dir := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", []byte("audio"), "mp3") - coverBytes := []byte{0xFF, 0xD8, 0xFF, 0xE0, 0x01, 0x02, 0x03} // small JPEG-ish payload - if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), coverBytes, 0o644); err != nil { - t.Fatal(err) - } - - req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID)+"/cover", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - if ct := w.Header().Get("Content-Type"); ct != "image/jpeg" { - t.Errorf("Content-Type = %q", ct) - } - if !bytes.Equal(w.Body.Bytes(), coverBytes) { - t.Errorf("body = %x, want %x", w.Body.Bytes(), coverBytes) - } -} - -func TestHandleGetCover_ExplicitPathHappyPath(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - - dir := t.TempDir() - explicit := filepath.Join(dir, "art.png") - pngBytes := []byte{0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A} // PNG magic - if err := os.WriteFile(explicit, pngBytes, 0o644); err != nil { - t.Fatal(err) - } - if _, err := pool.Exec(context.Background(), - `UPDATE albums SET cover_art_path = $1 WHERE id = $2`, explicit, album.ID); err != nil { - t.Fatal(err) - } - // No track needed — explicit path wins before we ever check the track dir. - - req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID)+"/cover", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - if ct := w.Header().Get("Content-Type"); ct != "image/png" { - t.Errorf("Content-Type = %q", ct) - } - if !bytes.Equal(w.Body.Bytes(), pngBytes) { - t.Errorf("body mismatch") - } -} - -func TestHandleGetCover_ExplicitMissingFallsThroughToSidecar(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - - _, dir := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", []byte("audio"), "mp3") - sidecarBytes := []byte("sidecar") - if err := os.WriteFile(filepath.Join(dir, "cover.jpg"), sidecarBytes, 0o644); err != nil { - t.Fatal(err) - } - if _, err := pool.Exec(context.Background(), - `UPDATE albums SET cover_art_path = $1 WHERE id = $2`, - filepath.Join(dir, "does-not-exist.jpg"), album.ID); err != nil { - t.Fatal(err) - } - - req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID)+"/cover", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d", w.Code) - } - if !bytes.Equal(w.Body.Bytes(), sidecarBytes) { - t.Errorf("body = %q, want %q", w.Body.Bytes(), sidecarBytes) - } -} - -func TestHandleGetCover_NoArt(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - seedTrack(t, pool, album.ID, artist.ID, "t", 1, 1000) // synthetic path, no sidecar - - req := httptest.NewRequest(http.MethodGet, "/api/albums/"+uuidToString(album.ID)+"/cover", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusNotFound { - t.Fatalf("status = %d", w.Code) - } - var body errorBody - _ = json.Unmarshal(w.Body.Bytes(), &body) - if body.Error.Code != "not_found" || body.Error.Message != "cover not found" { - t.Errorf("error body = %+v", body) - } -} - -func TestHandleGetCover_BadUUID(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodGet, "/api/albums/not-a-uuid/cover", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - -func TestHandleGetCover_UnknownAlbum(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - req := httptest.NewRequest(http.MethodGet, - "/api/albums/00000000-0000-0000-0000-000000000001/cover", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusNotFound { - t.Fatalf("status = %d", w.Code) - } - var body errorBody - _ = json.Unmarshal(w.Body.Bytes(), &body) - if body.Error.Message != "album not found" { - t.Errorf("message = %q, want \"album not found\"", body.Error.Message) - } -} -``` - -Update the imports at the top of `media_test.go` to add `"bytes"`, `"encoding/json"`, `"net/http"`, `"net/http/httptest"`: - -```go -import ( - "bytes" - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "os" - "path/filepath" - "testing" - - "github.com/jackc/pgx/v5/pgtype" - "github.com/jackc/pgx/v5/pgxpool" - - "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" -) -``` - -- [ ] **Step 3: Run the cover tests — expect FAIL (handler not implemented)** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./internal/api/ -run TestHandleGetCover -v -``` - -Expected: compile error OR 6 FAILS. - -- [ ] **Step 4: Append `handleGetCover` to `internal/api/media.go`** - -Add these imports to media.go (alongside existing): - -```go -"errors" -"net/http" - -"github.com/go-chi/chi/v5" -"github.com/jackc/pgx/v5" -``` - -Append the handler: - -```go -// handleGetCover implements GET /api/albums/{id}/cover. Resolves the cover -// path (explicit column, else sidecar next to first track), then delegates -// byte-serving to http.ServeContent so clients get Range / If-Modified-Since -// / ETag for free. -func (h *handlers) handleGetCover(w http.ResponseWriter, r *http.Request) { - id, ok := parseUUID(chi.URLParam(r, "id")) - if !ok { - writeErr(w, http.StatusBadRequest, "bad_request", "invalid album id") - return - } - q := dbq.New(h.pool) - album, err := q.GetAlbumByID(r.Context(), id) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - writeErr(w, http.StatusNotFound, "not_found", "album not found") - return - } - h.logger.Error("api: get album for cover failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "server error") - return - } - path := resolveAlbumCoverPath(r.Context(), q, album) - if path == "" { - writeErr(w, http.StatusNotFound, "not_found", "cover not found") - return - } - f, err := os.Open(path) - if err != nil { - // resolveAlbumCoverPath saw the file a moment ago; if it vanished - // between stat and open, treat it the same as no-art — the user - // experience (404) matches, and 500 would misleadingly imply a server - // bug rather than a filesystem race. - writeErr(w, http.StatusNotFound, "not_found", "cover not found") - return - } - defer func() { _ = f.Close() }() - info, err := f.Stat() - if err != nil { - h.logger.Error("api: stat cover failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "server error") - return - } - w.Header().Set("Content-Type", imageContentType(path)) - http.ServeContent(w, r, filepath.Base(path), info.ModTime(), f) -} -``` - -- [ ] **Step 5: Run the cover tests — expect all PASS** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./internal/api/ -run TestHandleGetCover -v -``` - -Expected: 6 PASS. - -- [ ] **Step 6: Run the full api suite** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./internal/api/... -count=1 -``` - -Expected: all PASS. - -- [ ] **Step 7: Commit** - -``` -git add internal/api/media.go internal/api/media_test.go internal/api/library_test.go -git commit -m "feat(api): GET /api/albums/{id}/cover with sidecar fallback" -``` - ---- - -## Task 3: Implement `GET /api/tracks/{id}/stream` - -**Files:** -- Modify: `internal/api/media.go` (add handler) -- Modify: `internal/api/media_test.go` (add tests) -- Modify: `internal/api/library_test.go` (register route in `newLibraryRouter`) - -Same TDD shape as Task 2. - -- [ ] **Step 1: Register `/stream` in `newLibraryRouter`** - -Edit `internal/api/library_test.go` so the helper also knows the stream route: - -```go -func newLibraryRouter(h *handlers) chi.Router { - r := chi.NewRouter() - r.Get("/api/artists", h.handleListArtists) - r.Get("/api/artists/{id}", h.handleGetArtist) - r.Get("/api/albums/{id}", h.handleGetAlbum) - r.Get("/api/albums/{id}/cover", h.handleGetCover) - r.Get("/api/tracks/{id}", h.handleGetTrack) - r.Get("/api/tracks/{id}/stream", h.handleGetStream) - r.Get("/api/search", h.handleSearch) - return r -} -``` - -(Same compile-time hint as Task 2 — add a stub if that compile error is in your way while writing the tests.) - -- [ ] **Step 2: Append integration tests to `internal/api/media_test.go`** - -```go -// deterministicBytes returns n bytes whose values cycle 0..255. Using a -// fixed pattern (not crypto/rand) makes assertions easy and the test -// reproducible. -func deterministicBytes(n int) []byte { - b := make([]byte, n) - for i := range b { - b[i] = byte(i % 256) - } - return b -} - -func TestHandleGetStream_HappyPath(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - body := deterministicBytes(32 * 1024) - track, _ := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", body, "mp3") - - req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(track.ID)+"/stream", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("status = %d body = %s", w.Code, w.Body.String()) - } - if ct := w.Header().Get("Content-Type"); ct != "audio/mpeg" { - t.Errorf("Content-Type = %q", ct) - } - if ar := w.Header().Get("Accept-Ranges"); ar != "bytes" { - t.Errorf("Accept-Ranges = %q", ar) - } - if cl := w.Header().Get("Content-Length"); cl != "32768" { - t.Errorf("Content-Length = %q, want 32768", cl) - } - if !bytes.Equal(w.Body.Bytes(), body) { - t.Errorf("body len = %d, want %d", len(w.Body.Bytes()), len(body)) - } -} - -func TestHandleGetStream_RangeFromStart(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - body := deterministicBytes(32 * 1024) - track, _ := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", body, "mp3") - - req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(track.ID)+"/stream", nil) - req.Header.Set("Range", "bytes=0-99") - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusPartialContent { - t.Fatalf("status = %d, want 206", w.Code) - } - if cr := w.Header().Get("Content-Range"); cr != "bytes 0-99/32768" { - t.Errorf("Content-Range = %q", cr) - } - if got := w.Body.Bytes(); !bytes.Equal(got, body[:100]) { - t.Errorf("body mismatch: len=%d, want 100", len(got)) - } -} - -func TestHandleGetStream_RangeFromMiddleToEnd(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - body := deterministicBytes(32 * 1024) - track, _ := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", body, "mp3") - - req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(track.ID)+"/stream", nil) - req.Header.Set("Range", "bytes=32000-") - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusPartialContent { - t.Fatalf("status = %d", w.Code) - } - if cr := w.Header().Get("Content-Range"); cr != "bytes 32000-32767/32768" { - t.Errorf("Content-Range = %q", cr) - } - if got := w.Body.Bytes(); !bytes.Equal(got, body[32000:]) { - t.Errorf("body mismatch: len=%d, want %d", len(got), len(body)-32000) - } -} - -func TestHandleGetStream_IfModifiedSinceReturns304(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - body := deterministicBytes(1024) - track, _ := seedTrackWithFile(t, pool, album.ID, artist.ID, "t", body, "mp3") - - info, err := os.Stat(track.FilePath) - if err != nil { - t.Fatal(err) - } - req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(track.ID)+"/stream", nil) - req.Header.Set("If-Modified-Since", info.ModTime().UTC().Format(http.TimeFormat)) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusNotModified { - t.Fatalf("status = %d, want 304", w.Code) - } - if got := w.Body.Len(); got != 0 { - t.Errorf("body len = %d, want 0 for 304", got) - } -} - -func TestHandleGetStream_BadUUID(t *testing.T) { - h, _ := testHandlers(t) - req := httptest.NewRequest(http.MethodGet, "/api/tracks/not-a-uuid/stream", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusBadRequest { - t.Errorf("status = %d, want 400", w.Code) - } -} - -func TestHandleGetStream_UnknownTrack(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - req := httptest.NewRequest(http.MethodGet, - "/api/tracks/00000000-0000-0000-0000-000000000001/stream", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - if w.Code != http.StatusNotFound { - t.Fatalf("status = %d", w.Code) - } - var body errorBody - _ = json.Unmarshal(w.Body.Bytes(), &body) - if body.Error.Message != "track not found" { - t.Errorf("message = %q, want \"track not found\"", body.Error.Message) - } -} - -func TestHandleGetStream_VanishedFile(t *testing.T) { - h, pool := testHandlers(t) - truncateLibrary(t, pool) - artist := seedArtist(t, pool, "A") - album := seedAlbum(t, pool, artist.ID, "X", 0) - dir := t.TempDir() - // Insert a track whose file_path points somewhere we never create. - gone := filepath.Join(dir, "does-not-exist.mp3") - if _, err := dbq.New(pool).UpsertTrack(context.Background(), dbq.UpsertTrackParams{ - Title: "gone", AlbumID: album.ID, ArtistID: artist.ID, - DurationMs: 1, FilePath: gone, FileSize: 1, FileFormat: "mp3", - }); err != nil { - t.Fatal(err) - } - // Look up the just-inserted track so we can address it by id. - tr, err := dbq.New(pool).GetTrackByPath(context.Background(), gone) - if err != nil { - t.Fatal(err) - } - - req := httptest.NewRequest(http.MethodGet, "/api/tracks/"+uuidToString(tr.ID)+"/stream", nil) - w := httptest.NewRecorder() - newLibraryRouter(h).ServeHTTP(w, req) - - if w.Code != http.StatusNotFound { - t.Fatalf("status = %d", w.Code) - } - var body errorBody - _ = json.Unmarshal(w.Body.Bytes(), &body) - if body.Error.Message != "track file not found" { - t.Errorf("message = %q, want \"track file not found\"", body.Error.Message) - } -} -``` - -- [ ] **Step 3: Run the stream tests — expect FAIL (handler not implemented)** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./internal/api/ -run TestHandleGetStream -v -``` - -Expected: compile error OR 7 FAILS. - -- [ ] **Step 4: Append `handleGetStream` to `internal/api/media.go`** - -```go -// handleGetStream implements GET /api/tracks/{id}/stream. Opens the file on -// disk and delegates byte-serving to http.ServeContent, which handles Range, -// If-Modified-Since, and ETag based on the file's mod time. -func (h *handlers) handleGetStream(w http.ResponseWriter, r *http.Request) { - id, ok := parseUUID(chi.URLParam(r, "id")) - if !ok { - writeErr(w, http.StatusBadRequest, "bad_request", "invalid track id") - return - } - q := dbq.New(h.pool) - track, err := q.GetTrackByID(r.Context(), id) - if err != nil { - if errors.Is(err, pgx.ErrNoRows) { - writeErr(w, http.StatusNotFound, "not_found", "track not found") - return - } - h.logger.Error("api: get track for stream failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "server error") - return - } - f, err := os.Open(track.FilePath) - if err != nil { - // File vanished (scanner indexed it, filesystem lost it). Treat as - // 404 so clients can fall back to the next item in a queue. - writeErr(w, http.StatusNotFound, "not_found", "track file not found") - return - } - defer func() { _ = f.Close() }() - info, err := f.Stat() - if err != nil { - h.logger.Error("api: stat track failed", "err", err) - writeErr(w, http.StatusInternalServerError, "server_error", "server error") - return - } - w.Header().Set("Content-Type", audioContentType(track.FileFormat)) - w.Header().Set("Accept-Ranges", "bytes") - http.ServeContent(w, r, filepath.Base(track.FilePath), info.ModTime(), f) -} -``` - -- [ ] **Step 5: Run the stream tests — expect all PASS** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./internal/api/ -run TestHandleGetStream -v -``` - -Expected: 7 PASS. - -- [ ] **Step 6: Run the full suite** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./... -count=1 -``` - -Expected: all PASS. - -- [ ] **Step 7: Commit** - -``` -git add internal/api/media.go internal/api/media_test.go internal/api/library_test.go -git commit -m "feat(api): GET /api/tracks/{id}/stream with Range support" -``` - ---- - -## Task 4: Wire routes into production `Mount` + route-registration regression test - -**Files:** -- Modify: `internal/api/api.go` -- Modify: `internal/api/library_test.go` - -- [ ] **Step 1: Extend the existing `TestRoutesRegisteredInMount` paths list** - -Edit `internal/api/library_test.go:436-441` (the `paths := []string{...}` slice inside `TestRoutesRegisteredInMount`) so it becomes: - -```go -paths := []string{ - "/api/artists", - "/api/artists/00000000-0000-0000-0000-000000000001", - "/api/albums/00000000-0000-0000-0000-000000000001", - "/api/albums/00000000-0000-0000-0000-000000000001/cover", - "/api/tracks/00000000-0000-0000-0000-000000000001", - "/api/tracks/00000000-0000-0000-0000-000000000001/stream", - "/api/search?q=x", -} -``` - -- [ ] **Step 2: Run the regression test — expect FAIL (routes not in Mount)** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./internal/api/ -run TestRoutesRegisteredInMount -v -``` - -Expected: FAIL — the two new paths return 404 because `Mount` doesn't wire them yet. - -- [ ] **Step 3: Register both routes in production `Mount`** - -Edit `internal/api/api.go:23-34` (the authed chi.Group body) to add the two new routes. The full authed block becomes: - -```go -api.Group(func(authed chi.Router) { - authed.Use(auth.RequireUser(pool)) - authed.Post("/auth/logout", h.handleLogout) - authed.Get("/me", h.handleGetMe) - - authed.Get("/artists", h.handleListArtists) - authed.Get("/artists/{id}", h.handleGetArtist) - authed.Get("/albums/{id}", h.handleGetAlbum) - authed.Get("/albums/{id}/cover", h.handleGetCover) - authed.Get("/tracks/{id}", h.handleGetTrack) - authed.Get("/tracks/{id}/stream", h.handleGetStream) - authed.Get("/search", h.handleSearch) -}) -``` - -- [ ] **Step 4: Run the regression test — expect PASS** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./internal/api/ -run TestRoutesRegisteredInMount -v -``` - -Expected: PASS — both new paths now return 401 (auth required), not 404. - -- [ ] **Step 5: Run the full suite** - -``` -MINSTREL_TEST_DATABASE_URL=postgres://minstrel:minstrel@localhost:5432/minstrel_test?sslmode=disable \ - go test ./... -count=1 -``` - -Expected: all PASS. - -- [ ] **Step 6: Commit** - -``` -git add internal/api/api.go internal/api/library_test.go -git commit -m "feat(api): register cover + stream routes under RequireUser" -``` - ---- - -## Post-merge smoke checklist - -After the PR merges, run this sanity pass against a live build to confirm end-to-end behavior — unit and integration tests cover correctness, but browsers reveal surprises. - -- Log in through the SPA and hit `GET /api/albums/<id>/cover` for an album with a sidecar — expect the image bytes and `Content-Type: image/jpeg`. -- `GET /api/albums/<id>/cover` for an album with no art — expect a JSON 404 body. -- `GET /api/tracks/<id>/stream` in a browser `<audio>` tag — expect playback and seekable timeline (Range support). -- `curl -I -H 'Range: bytes=0-99'` against a stream — expect `206 Partial Content` and `Content-Range: bytes 0-99/<size>`. -- `curl` a second time with `If-Modified-Since` set to the first response's `Last-Modified` — expect `304 Not Modified`. diff --git a/docs/superpowers/plans/2026-04-21-web-ui-scaffold.md b/docs/superpowers/plans/2026-04-21-web-ui-scaffold.md deleted file mode 100644 index 5171952d..00000000 --- a/docs/superpowers/plans/2026-04-21-web-ui-scaffold.md +++ /dev/null @@ -1,954 +0,0 @@ -# Web UI Scaffold Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Scaffold a SvelteKit SPA at `web/`, embed its build output into the Go binary, and update the Dockerfile + CI so a single container image serves the SPA at `/` alongside the existing `/api/*` and `/rest/*` surfaces. - -**Architecture:** SvelteKit with `@sveltejs/adapter-static` (SPA fallback to `index.html`) builds to `web/build/`. A thin `web` Go package `//go:embed`s that directory and returns an `http.Handler` that serves static assets and falls back to `index.html` for unknown paths. The handler is wired as the server's `NotFound` route so explicit routes (`/api`, `/rest`, `/healthz`) are unaffected; the `NotFound` wrapper also returns a JSON 404 for unmatched `/api/*` and `/rest/*` paths so the SPA fallback never produces HTML bodies for API consumers. A hand-crafted placeholder `web/build/index.html` is committed (via negated gitignore) so `go build` works in clean checkouts before anyone runs `npm run build`. - -**Tech Stack:** SvelteKit 2.x, Svelte 5, TypeScript 5, Vite 5, Tailwind CSS 3, Vitest, Go 1.23, chi/v5, Docker multi-stage builds, Forgejo Actions. - -**Reference spec:** `docs/superpowers/specs/2026-04-20-web-ui-scaffold-design.md`. This plan implements only the Stack / Packaging / Dev-workflow / Build sections. Feature UI (login, library views, player) is out of scope and lands in follow-up plans. - ---- - -## File structure - -| File | Action | Purpose | -|---|---|---| -| `web/package.json` | Create | npm config + scripts | -| `web/tsconfig.json` | Create | TypeScript config extending SvelteKit's generated one | -| `web/svelte.config.js` | Create | `adapter-static` with `fallback: 'index.html'` | -| `web/vite.config.ts` | Create | Vite config + dev proxy for `/api` and `/rest` | -| `web/vitest.config.ts` | Create | Vitest + jsdom config | -| `web/tailwind.config.js` | Create | Tailwind content globs + dark palette tokens | -| `web/postcss.config.cjs` | Create | PostCSS pipeline (tailwindcss + autoprefixer) | -| `web/src/app.html` | Create | HTML template | -| `web/src/app.css` | Create | Tailwind directives + CSS custom properties | -| `web/src/routes/+layout.svelte` | Create | Imports `app.css`, renders outlet | -| `web/src/routes/+page.svelte` | Create | Placeholder landing page | -| `web/src/lib/example.test.ts` | Create | Vitest smoke test | -| `web/static/favicon.png` | Create | 1×1 transparent PNG placeholder | -| `web/build/index.html` | Create | Hand-crafted placeholder committed so `go:embed` works pre-build | -| `web/.gitignore` | Create | Ignore `node_modules/`, `.svelte-kit/`, `build/*` with `!build/index.html` | -| `web/embed.go` | Create | `//go:embed` build tree + `Handler()` with SPA fallback (must live next to `web/build/` — `go:embed` cannot reach parent directories) | -| `web/embed_test.go` | Create | Unit tests for the handler | -| `internal/server/server.go` | Modify | Wire `web.Handler()` into `NotFound`; JSON-404 for `/api/*` and `/rest/*` | -| `internal/server/server_test.go` | Modify | Assert `/` returns HTML; `/api/anything` does not return HTML | -| `Dockerfile` | Rewrite | Three-stage build: `node` → `golang` → `debian:slim` | -| `.forgejo/workflows/test.yml` | Modify | Install Node, build web, run `npm test` before Go steps | -| `README.md` | Modify | Document two-process dev workflow | - ---- - -## Task 1: SvelteKit project skeleton - -**Files:** -- Create: `web/package.json`, `web/tsconfig.json`, `web/svelte.config.js`, `web/vite.config.ts`, `web/tailwind.config.js`, `web/postcss.config.cjs` -- Create: `web/src/app.html`, `web/src/app.css`, `web/src/routes/+layout.svelte`, `web/src/routes/+page.svelte` -- Create: `web/.gitignore`, `web/static/favicon.png`, `web/build/index.html` - -- [ ] **Step 1: Create `web/package.json`** - -```json -{ - "name": "minstrel-web", - "version": "0.0.0", - "private": true, - "type": "module", - "scripts": { - "dev": "vite dev", - "build": "vite build", - "preview": "vite preview", - "check": "svelte-kit sync && svelte-check --tsconfig ./tsconfig.json", - "test": "svelte-kit sync && vitest run" - }, - "devDependencies": { - "@sveltejs/adapter-static": "^3.0.6", - "@sveltejs/kit": "^2.8.0", - "@sveltejs/vite-plugin-svelte": "^4.0.0", - "autoprefixer": "^10.4.20", - "jsdom": "^25.0.1", - "postcss": "^8.4.49", - "svelte": "^5.1.9", - "svelte-check": "^4.0.5", - "tailwindcss": "^3.4.14", - "tslib": "^2.8.0", - "typescript": "^5.6.3", - "vite": "^5.4.10", - "vitest": "^2.1.4" - } -} -``` - -- [ ] **Step 2: Create `web/tsconfig.json`** - -```json -{ - "extends": "./.svelte-kit/tsconfig.json", - "compilerOptions": { - "allowJs": true, - "checkJs": true, - "esModuleInterop": true, - "forceConsistentCasingInFileNames": true, - "resolveJsonModule": true, - "skipLibCheck": true, - "sourceMap": true, - "strict": true, - "moduleResolution": "bundler" - } -} -``` - -- [ ] **Step 3: Create `web/svelte.config.js`** - -```js -import adapter from '@sveltejs/adapter-static'; -import { vitePreprocess } from '@sveltejs/vite-plugin-svelte'; - -export default { - preprocess: vitePreprocess(), - kit: { - adapter: adapter({ - pages: 'build', - assets: 'build', - fallback: 'index.html', - precompress: false, - strict: false - }), - files: { - assets: 'static' - } - } -}; -``` - -- [ ] **Step 4: Create `web/vite.config.ts`** - -```ts -import { sveltekit } from '@sveltejs/kit/vite'; -import { defineConfig } from 'vite'; - -export default defineConfig({ - plugins: [sveltekit()], - server: { - port: 5173, - proxy: { - '/api': { - target: 'http://localhost:4533', - changeOrigin: true - }, - '/rest': { - target: 'http://localhost:4533', - changeOrigin: true - } - } - } -}); -``` - -- [ ] **Step 5: Create `web/tailwind.config.js`** - -```js -export default { - content: ['./src/**/*.{html,js,ts,svelte}'], - darkMode: 'class', - theme: { - extend: { - colors: { - surface: { - 900: '#14161a', - 800: '#1a1d22', - 700: '#2a2f36' - }, - text: { - primary: '#e8ecf2', - secondary: '#cdd3db' - } - } - } - }, - plugins: [] -}; -``` - -- [ ] **Step 6: Create `web/postcss.config.cjs`** - -```js -module.exports = { - plugins: { - tailwindcss: {}, - autoprefixer: {} - } -}; -``` - -- [ ] **Step 7: Create `web/src/app.html`** - -```html -<!doctype html> -<html lang="en" class="dark"> - <head> - <meta charset="utf-8" /> - <link rel="icon" href="%sveltekit.assets%/favicon.png" /> - <meta name="viewport" content="width=device-width, initial-scale=1" /> - <title>Minstrel - %sveltekit.head% - - -
%sveltekit.body%
- - -``` - -- [ ] **Step 8: Create `web/src/app.css`** - -```css -@tailwind base; -@tailwind components; -@tailwind utilities; - -:root { - color-scheme: dark; -} - -html, -body { - height: 100%; - margin: 0; -} -``` - -- [ ] **Step 9: Create `web/src/routes/+layout.svelte`** - -```svelte - - - -``` - -- [ ] **Step 10: Create `web/src/routes/+page.svelte`** - -```svelte -
-
-

Minstrel

-

- Scaffold — UI features land in subsequent plans. -

-
-
-``` - -- [ ] **Step 11: Create `web/.gitignore`** - -``` -node_modules/ -.svelte-kit/ -build/* -!build/index.html -``` - -- [ ] **Step 12: Create placeholder `web/static/favicon.png`** - -Run: -```bash -mkdir -p web/static -python3 -c "import base64,sys; sys.stdout.buffer.write(base64.b64decode('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8/5+hHgAHggJ/PchI7wAAAABJRU5ErkJggg=='))" > web/static/favicon.png -``` - -Expected: `web/static/favicon.png` is a valid 1×1 PNG (67 bytes). - -- [ ] **Step 13: Create placeholder `web/build/index.html`** - -This placeholder lets `//go:embed build` compile before anyone runs `npm run build`. Real `npm run build` overwrites it locally; only this hand-written version is committed. - -```html - - - - - Minstrel - - -

- Minstrel SPA has not been built. Run - cd web && npm install && npm run build or - build the container image, which runs the web build during - docker build. -

- - -``` - -- [ ] **Step 14: Install dependencies** - -Run: `cd web && npm install` -Expected: `node_modules/` populated, `package-lock.json` written, exit 0. - -- [ ] **Step 15: Verify SvelteKit sync + build** - -Run: `cd web && npm run build` -Expected: exit 0; `web/build/` now contains `index.html`, `_app/`, `favicon.png`. (Local `index.html` is overwritten but not committed — `.gitignore` keeps only the committed placeholder under version control.) - -Undo the local overwrite so the committed placeholder remains: - -Run: `git checkout -- web/build/index.html` -Expected: placeholder restored. - -- [ ] **Step 16: Commit** - -```bash -git add web/.gitignore web/package.json web/package-lock.json web/tsconfig.json \ - web/svelte.config.js web/vite.config.ts web/tailwind.config.js web/postcss.config.cjs \ - web/src/ web/static/ web/build/index.html -git commit -m "feat(web): scaffold SvelteKit SPA with Tailwind + adapter-static - -- SvelteKit 2.x + Svelte 5 + TypeScript + Vite 5 -- adapter-static with fallback:'index.html' for SPA deep-link routing -- Tailwind configured with dark-only palette matching spec -- Placeholder landing page at / -- Committed web/build/index.html placeholder (via .gitignore negation) so - go:embed works before the SvelteKit build has been run" -``` - ---- - -## Task 2: Vitest smoke test - -**Files:** -- Create: `web/vitest.config.ts` -- Create: `web/src/lib/example.test.ts` - -- [ ] **Step 1: Create `web/vitest.config.ts`** - -```ts -import { sveltekit } from '@sveltejs/kit/vite'; -import { defineConfig } from 'vitest/config'; - -export default defineConfig({ - plugins: [sveltekit()], - test: { - environment: 'jsdom', - include: ['src/**/*.test.ts'] - } -}); -``` - -- [ ] **Step 2: Write failing smoke test** - -Create `web/src/lib/example.test.ts`: - -```ts -import { describe, expect, it } from 'vitest'; - -describe('scaffold smoke', () => { - it('runs vitest', () => { - expect(1 + 1).toBe(2); - }); -}); -``` - -- [ ] **Step 3: Run tests** - -Run: `cd web && npm test` -Expected: 1 test passes. Exit 0. - -- [ ] **Step 4: Commit** - -```bash -git add web/vitest.config.ts web/src/lib/example.test.ts -git commit -m "test(web): add Vitest smoke test and jsdom config" -``` - ---- - -## Task 3: Go embed package + SPA fallback handler - -**Files:** -- Create: `web/embed.go` -- Create: `web/embed_test.go` - -- [ ] **Step 1: Write failing tests** - -Create `web/embed_test.go`: - -```go -package web - -import ( - "io" - "net/http" - "net/http/httptest" - "strings" - "testing" -) - -func TestHandler_ServesIndexAtRoot(t *testing.T) { - req := httptest.NewRequest(http.MethodGet, "/", nil) - rec := httptest.NewRecorder() - Handler().ServeHTTP(rec, req) - - if rec.Code != http.StatusOK { - t.Fatalf("status = %d, want 200", rec.Code) - } - if ct := rec.Header().Get("Content-Type"); !strings.Contains(ct, "text/html") { - t.Errorf("Content-Type = %q, want text/html*", ct) - } - body, _ := io.ReadAll(rec.Body) - if !strings.Contains(strings.ToLower(string(body)), " build/index.html -// - GET / -> that file, via http.FileServer -// - GET / -> build/index.html (SPA fallback) -// -// Callers are expected to register this as the router's NotFound/catch-all so -// explicitly-routed paths (like /api/* and /rest/*) take precedence. -func Handler() http.Handler { - sub, err := fs.Sub(buildFS, "build") - if err != nil { - // fs.Sub on a valid embed path can only fail if the embed directive - // is malformed, which the compile would have caught. - panic("web: fs.Sub(build) failed: " + err.Error()) - } - - index, err := fs.ReadFile(sub, "index.html") - if err != nil { - panic("web: embedded build/index.html missing: " + err.Error()) - } - - fileServer := http.FileServer(http.FS(sub)) - - return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - clean := path.Clean(r.URL.Path) - if clean == "/" || clean == "." { - serveIndex(w, index) - return - } - name := strings.TrimPrefix(clean, "/") - if info, err := fs.Stat(sub, name); err == nil && !info.IsDir() { - fileServer.ServeHTTP(w, r) - return - } - serveIndex(w, index) - }) -} - -func serveIndex(w http.ResponseWriter, index []byte) { - w.Header().Set("Content-Type", "text/html; charset=utf-8") - w.Header().Set("Cache-Control", "no-cache") - _, _ = w.Write(index) -} -``` - -- [ ] **Step 4: Run tests — expect PASS** - -Run: `go test ./web/ -v` -Expected: `TestHandler_ServesIndexAtRoot`, `TestHandler_UnknownPathFallsBackToIndex`, `TestHandler_MissingAssetFallsBackToIndex` all PASS. - -- [ ] **Step 5: Commit** - -```bash -git add web/ -git commit -m "feat(web): embed SvelteKit build output with SPA fallback handler" -``` - ---- - -## Task 4: Wire web handler into server router - -**Files:** -- Modify: `internal/server/server.go` -- Modify: `internal/server/server_test.go` - -- [ ] **Step 1: Write failing tests** - -Add to `internal/server/server_test.go` (also add `"strings"` to the imports): - -```go -func TestRouter_ServesSPAAtRoot(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}) - ts := httptest.NewServer(s.Router()) - defer ts.Close() - - resp, err := http.Get(ts.URL + "/") - if err != nil { - t.Fatalf("GET /: %v", err) - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - t.Errorf("status = %d, want 200", resp.StatusCode) - } - if ct := resp.Header.Get("Content-Type"); !strings.Contains(ct, "text/html") { - t.Errorf("Content-Type = %q, want text/html*", ct) - } -} - -func TestRouter_DeepLinkFallbackReturnsSPA(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}) - ts := httptest.NewServer(s.Router()) - defer ts.Close() - - resp, err := http.Get(ts.URL + "/artists/00000000-0000-0000-0000-000000000001") - if err != nil { - t.Fatalf("GET deep link: %v", err) - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusOK { - t.Errorf("status = %d, want 200", resp.StatusCode) - } - if ct := resp.Header.Get("Content-Type"); !strings.Contains(ct, "text/html") { - t.Errorf("Content-Type = %q, want text/html*", ct) - } -} - -func TestRouter_APIPathNotSwallowedBySPA(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}) - ts := httptest.NewServer(s.Router()) - defer ts.Close() - - resp, err := http.Get(ts.URL + "/api/does-not-exist") - if err != nil { - t.Fatalf("GET /api/does-not-exist: %v", err) - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusNotFound { - t.Errorf("status = %d, want 404", resp.StatusCode) - } - if ct := resp.Header.Get("Content-Type"); strings.Contains(ct, "text/html") { - t.Errorf("Content-Type = %q; /api/* must never return text/html", ct) - } -} - -func TestRouter_RestPathNotSwallowedBySPA(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}) - ts := httptest.NewServer(s.Router()) - defer ts.Close() - - resp, err := http.Get(ts.URL + "/rest/does-not-exist") - if err != nil { - t.Fatalf("GET /rest/does-not-exist: %v", err) - } - defer func() { _ = resp.Body.Close() }() - - if resp.StatusCode != http.StatusNotFound { - t.Errorf("status = %d, want 404", resp.StatusCode) - } - if ct := resp.Header.Get("Content-Type"); strings.Contains(ct, "text/html") { - t.Errorf("Content-Type = %q; /rest/* must never return text/html", ct) - } -} -``` - -- [ ] **Step 2: Run to verify failure** - -Run: `go test ./internal/server/ -v` -Expected: FAIL on all four new tests (chi returns its own 404 with empty/plain body for unmatched routes). - -- [ ] **Step 3: Wire `web.Handler()` into `Router()`** - -Modify `internal/server/server.go`: - -a) Add imports (preserving alphabetical order within the group): - -```go -import ( - "context" - "encoding/json" - "log/slog" - "net/http" - "strings" - - "github.com/go-chi/chi/v5" - "github.com/go-chi/chi/v5/middleware" - "github.com/jackc/pgx/v5/pgxpool" - - "git.fabledsword.com/bvandeusen/minstrel/internal/api" - "git.fabledsword.com/bvandeusen/minstrel/internal/auth" - "git.fabledsword.com/bvandeusen/minstrel/internal/library" - "git.fabledsword.com/bvandeusen/minstrel/internal/subsonic" - "git.fabledsword.com/bvandeusen/minstrel/web" -) -``` - -b) Replace the `Router()` method with: - -```go -func (s *Server) Router() http.Handler { - r := chi.NewRouter() - r.Use(middleware.RequestID) - r.Use(middleware.Recoverer) - - r.Get("/healthz", s.handleHealthz) - - if s.Pool != nil { - api.Mount(r, s.Pool, s.Logger) - r.Route("/api/admin", func(admin chi.Router) { - admin.Use(auth.RequireAdmin(s.Pool)) - if s.Scanner != nil { - admin.Post("/scan", s.handleAdminScan) - } - }) - subsonic.Mount(r, s.Pool, s.Logger, s.SubsonicCfg) - } - - spa := web.Handler() - r.NotFound(func(w http.ResponseWriter, req *http.Request) { - p := req.URL.Path - if strings.HasPrefix(p, "/api/") || strings.HasPrefix(p, "/rest/") { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusNotFound) - _, _ = w.Write([]byte(`{"error":{"code":"not_found","message":"not found"}}`)) - return - } - spa.ServeHTTP(w, req) - }) - return r -} -``` - -- [ ] **Step 4: Run tests — expect PASS** - -Run: `go test ./internal/server/ -v` -Expected: all tests PASS, including the existing `TestHealthz` and the four new ones. - -- [ ] **Step 5: Run full test suite** - -Run: `go test -short -race ./...` -Expected: PASS. - -- [ ] **Step 6: Run linter** - -Run: `golangci-lint run ./...` -Expected: no issues. - -- [ ] **Step 7: Commit** - -```bash -git add internal/server/ -git commit -m "feat(server): serve embedded SPA at root with /api,/rest carve-out - -- NotFound wrapper routes unknown /api/* and /rest/* to a JSON 404 -- Everything else falls through to the embedded web handler, which - resolves static assets or returns index.html for SPA deep links" -``` - ---- - -## Task 5: Multi-stage Dockerfile - -**Files:** -- Rewrite: `Dockerfile` - -- [ ] **Step 1: Replace `Dockerfile` contents** - -```dockerfile -# syntax=docker/dockerfile:1.6 - -FROM node:22-bookworm-slim AS web -WORKDIR /web -COPY web/package.json web/package-lock.json ./ -RUN npm ci -COPY web/ ./ -RUN npm run build - -FROM golang:1.23-bookworm AS builder -WORKDIR /src -COPY go.mod go.sum ./ -RUN go mod download -COPY . . -# Overwrite the committed placeholder with the freshly-built SPA assets. -COPY --from=web /web/build ./web/build -ENV CGO_ENABLED=0 -RUN go build -trimpath -ldflags="-s -w" -o /out/minstrel ./cmd/minstrel - -FROM debian:bookworm-slim -RUN apt-get update \ - && apt-get install -y --no-install-recommends ca-certificates ffmpeg \ - && rm -rf /var/lib/apt/lists/* - -RUN groupadd --system --gid 1000 minstrel \ - && useradd --system --uid 1000 --gid minstrel --shell /usr/sbin/nologin minstrel - -COPY --from=builder /out/minstrel /usr/local/bin/minstrel -COPY config.example.yaml /etc/smartmusic/config.yaml - -USER minstrel -EXPOSE 4533 -ENTRYPOINT ["/usr/local/bin/minstrel"] -CMD ["--config", "/etc/smartmusic/config.yaml"] -``` - -- [ ] **Step 2: Build image locally** - -Run: `docker build -t minstrel:scaffold-smoke .` -Expected: all three stages succeed. The final layer is `FROM debian:bookworm-slim`, the binary exists at `/usr/local/bin/minstrel`. - -- [ ] **Step 3: Inspect image for SPA assets** - -Run: `docker run --rm --entrypoint /bin/sh minstrel:scaffold-smoke -c 'test -x /usr/local/bin/minstrel && echo ok'` -Expected: output `ok` (verifies the binary was copied in and is executable). - -- [ ] **Step 4: Commit** - -```bash -git add Dockerfile -git commit -m "build: multi-stage Dockerfile with node->go->slim for embedded SPA - -Web assets are built in the node stage, copied into the go stage before -go build so //go:embed picks them up, then the minimal slim runtime -carries only the final binary and ffmpeg." -``` - ---- - -## Task 6: CI — install Node and build web before Go - -**Files:** -- Modify: `.forgejo/workflows/test.yml` - -- [ ] **Step 1: Replace `.forgejo/workflows/test.yml` contents** - -```yaml -name: test - -# Lint + short unit tests. Integration tests needing Postgres/ffmpeg run -# locally via docker-compose; they should guard with testing.Short(). - -on: - push: - branches: [dev] - pull_request: - branches: [main] - -jobs: - test: - runs-on: go-ci - - steps: - - name: Checkout - uses: actions/checkout@v4 - - - name: Setup Node - uses: actions/setup-node@v4 - with: - node-version: '22' - cache: 'npm' - cache-dependency-path: web/package-lock.json - - - name: Install web deps - working-directory: web - run: npm ci - - - name: Web build (populates web/build/ for go:embed) - working-directory: web - run: npm run build - - - name: Web test (vitest) - working-directory: web - run: npm test - - - name: Detect Go project - id: gomod - shell: bash - run: | - if [ -f go.mod ]; then - echo "present=true" >> "$GITHUB_OUTPUT" - else - echo "present=false" >> "$GITHUB_OUTPUT" - echo "::notice::No go.mod yet — Go steps will be skipped until the skeleton lands" - fi - - - name: Toolchain versions - if: steps.gomod.outputs.present == 'true' - run: | - go version - golangci-lint --version - - - name: go vet - if: steps.gomod.outputs.present == 'true' - run: go vet ./... - - - name: golangci-lint - if: steps.gomod.outputs.present == 'true' - run: golangci-lint run ./... - - - name: go test (short, race) - if: steps.gomod.outputs.present == 'true' - run: go test -short -race ./... -``` - -- [ ] **Step 2: Commit** - -```bash -git add .forgejo/workflows/test.yml -git commit -m "ci: install Node + build web before Go steps - -Go's //go:embed needs web/build/ to exist at compile time. CI now -runs 'npm ci && npm run build && npm test' ahead of the Go steps -so the embed directive sees real, freshly-built assets." -``` - ---- - -## Task 7: README dev-workflow section - -**Files:** -- Modify: `README.md` - -- [ ] **Step 1: Read current README** - -Run: `cat README.md` -Note the existing structure (headings, sections). - -- [ ] **Step 2: Append or replace a "Development" section** - -Use the Edit tool to insert the following section. Place it after any existing "Getting started" or "Quickstart" section; if none exists, append to the end of the file. - -```markdown -## Development - -Minstrel has two concurrent dev processes: - -1. **Backend:** `docker compose up` — starts Postgres and Minstrel on `:4533`. -2. **Frontend:** `cd web && npm install && npm run dev` — Vite dev server on `:5173` with HMR. - -The Vite dev server proxies `/api/*` and `/rest/*` to the backend on `:4533`, -so session cookies work correctly when the SPA is loaded from the Vite origin. - -### Production build - -`docker build -t minstrel .` runs the SvelteKit build inside a `node` stage, -copies the output into the `golang` stage, and `//go:embed`s it into the -final binary. The resulting container serves the SPA from `/` alongside the -API surfaces. No separate static-file server is required. -``` - -- [ ] **Step 3: Commit** - -```bash -git add README.md -git commit -m "docs: document two-process dev workflow with Vite proxy" -``` - ---- - -## Task 8: Final verification - -**Files:** none (verification only). - -- [ ] **Step 1: Full Go suite** - -Run: `go test -short -race ./...` -Expected: PASS. - -- [ ] **Step 2: Go linter** - -Run: `golangci-lint run ./...` -Expected: no issues. - -- [ ] **Step 3: Web tests + build** - -Run: `cd web && npm test && npm run build` -Expected: PASS. Verify `web/build/index.html` was regenerated; then: - -Run: `git checkout -- web/build/index.html` -Expected: committed placeholder restored (the fresh build output stays out of version control). - -- [ ] **Step 4: Docker build smoke** - -Run: `docker build -t minstrel:scaffold-final . && docker image inspect minstrel:scaffold-final --format '{{.Size}}'` -Expected: image builds; size printed (bytes). - -- [ ] **Step 5: Local end-to-end** - -Bring up the stack: -```bash -docker compose up --build -d -``` - -Then in another terminal: -```bash -curl -sI http://localhost:4533/ -curl -sI http://localhost:4533/artists/00000000-0000-0000-0000-000000000001 -curl -sI http://localhost:4533/healthz -curl -sI http://localhost:4533/api/nonexistent -``` - -Expected responses: -- `GET /` → `200 OK`, `Content-Type: text/html; charset=utf-8`. -- `GET /artists/…` → `200 OK`, `Content-Type: text/html; charset=utf-8` (SPA fallback). -- `GET /healthz` → `200 OK`, `Content-Type: application/json`. -- `GET /api/nonexistent` → `401 Unauthorized` (RequireUser kicks in before the NotFound wrapper for mounted `/api/*` routes) OR `404 Not Found` with `Content-Type: application/json` (not `text/html`). Either is acceptable — the key requirement is **no `text/html` body**. - -Tear down: -```bash -docker compose down -``` - -- [ ] **Step 6: Finish the branch** - -Follow `superpowers:finishing-a-development-branch`: present the four-option menu and proceed per the user's choice. The expected path is Option 2 — push `dev`, open a PR to `main`, wait for Forgejo CI, merge once green. diff --git a/docs/superpowers/plans/2026-04-22-web-ui-auth.md b/docs/superpowers/plans/2026-04-22-web-ui-auth.md deleted file mode 100644 index 1375969b..00000000 --- a/docs/superpowers/plans/2026-04-22-web-ui-auth.md +++ /dev/null @@ -1,1245 +0,0 @@ -# Web UI Auth Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Ship the browser-side auth flow — login page, session store, persistent Shell, and typed fetch wrapper — on top of the existing SvelteKit scaffold, so every subsequent frontend plan can assume a known-authenticated user and use `api.get(path)` out of the box. - -**Architecture:** Three layers under SvelteKit. `lib/api/client.ts` is a typed wrapper around `fetch` that parses JSON, throws typed `ApiError`, and silently logs the user out on 401. `lib/auth/store.svelte.ts` is a Svelte 5 rune-based store holding the current user, bootstrapped once from `/api/me` in the root `+layout.ts`. `lib/query/client.ts` is a TanStack Query `QueryClient` installed via `QueryClientProvider` at the root layout. Route guarding and shell rendering happen in `+layout.svelte`. - -**Tech Stack:** SvelteKit 2 + Svelte 5 (runes), TypeScript, TanStack Query 5 (`@tanstack/svelte-query`), Vitest + jsdom + Testing Library (`@testing-library/svelte`, `@testing-library/jest-dom`), Tailwind. - -**Reference:** design spec at `docs/superpowers/specs/2026-04-22-web-ui-auth-design.md`. - ---- - -## File Structure - -**New files under `web/`:** - -| File | Responsibility | -|---|---| -| `src/lib/api/client.ts` | `apiFetch`, `api.{get,post,del}`, types `User`, `LoginResponse`, `ApiError` | -| `src/lib/api/client.test.ts` | Unit tests for the transport layer | -| `src/lib/auth/store.svelte.ts` | `$user` rune, `bootstrap`, `login`, `logout` | -| `src/lib/auth/store.test.ts` | Unit tests for auth store | -| `src/lib/query/client.ts` | Singleton `QueryClient` export | -| `src/lib/components/Shell.svelte` | Header + sidebar + main slot | -| `src/lib/components/Shell.test.ts` | Shell component tests | -| `src/routes/+layout.ts` | `load()` → `await bootstrap()` | -| `src/routes/login/+page.svelte` | Login form UI + behavior | -| `src/routes/login/login.test.ts` | Login page tests (note: NOT `+page.test.ts` — the `+` prefix would trigger SvelteKit's route walker) | -| `src/routes/search/+page.svelte` | "Coming soon" placeholder | -| `src/routes/playlists/+page.svelte` | "Coming soon" placeholder | -| `vitest.setup.ts` | jest-dom matcher registration | - -**Modified files:** - -| File | Change | -|---|---| -| `web/package.json` | Add `@tanstack/svelte-query` (runtime) + `@testing-library/svelte`, `@testing-library/jest-dom` (dev) | -| `web/vitest.config.ts` | `setupFiles: ['./vitest.setup.ts']` | -| `web/src/routes/+layout.svelte` | Mount `QueryClientProvider`, run route guard, render `` or bare `` | -| `web/src/routes/+page.svelte` | Replace scaffold text with a minimal "Library" placeholder | - ---- - -## Task 1: Add dependencies + jest-dom setup - -**Files:** -- Modify: `web/package.json` -- Modify: `web/vitest.config.ts` -- Create: `web/vitest.setup.ts` - -- [ ] **Step 1: Install packages** - -Run: -```bash -cd web && npm install --save @tanstack/svelte-query@^5 && npm install --save-dev @testing-library/svelte@^5 @testing-library/jest-dom@^6 -``` - -Expected: three packages added to `package.json`; `package-lock.json` updated. No errors. - -- [ ] **Step 2: Create `web/vitest.setup.ts`** - -```ts -import '@testing-library/jest-dom/vitest'; -``` - -- [ ] **Step 3: Update `web/vitest.config.ts`** - -Replace the whole file with: - -```ts -import { sveltekit } from '@sveltejs/kit/vite'; -import { defineConfig } from 'vitest/config'; - -export default defineConfig({ - plugins: [sveltekit()], - test: { - environment: 'jsdom', - include: ['src/**/*.test.ts'], - setupFiles: ['./vitest.setup.ts'] - } -}); -``` - -- [ ] **Step 4: Sanity-check the existing test still passes** - -Run: `cd web && npm test` -Expected: PASS — `src/lib/example.test.ts (1 test)` passes. The new setup file doesn't break anything. - -- [ ] **Step 5: Commit** - -```bash -git add web/package.json web/package-lock.json web/vitest.config.ts web/vitest.setup.ts -git commit -m "build(web): add TanStack Query + Testing Library deps - -Wires @tanstack/svelte-query for the data layer and -@testing-library/{svelte,jest-dom} for component-level tests. Registers -the jest-dom matchers via a new vitest setup file." -``` - ---- - -## Task 2: Types and the `ApiError` shape - -**Files:** -- Create: `web/src/lib/api/client.ts` (types only so far) - -- [ ] **Step 1: Write `web/src/lib/api/client.ts` with types only** - -```ts -export type ApiError = { - code: string; - message: string; - status: number; -}; - -export type User = { - id: string; - username: string; - is_admin: boolean; -}; - -export type LoginResponse = { - token: string; - user: User; -}; -``` - -- [ ] **Step 2: Verify tsc is happy** - -Run: `cd web && npm run check` -Expected: `svelte-check found 0 errors and 0 warnings`. - -- [ ] **Step 3: Commit** - -```bash -git add web/src/lib/api/client.ts -git commit -m "feat(web): add api client types (User, LoginResponse, ApiError)" -``` - ---- - -## Task 3: `apiFetch` happy path + error envelope - -**Files:** -- Modify: `web/src/lib/api/client.ts` -- Create: `web/src/lib/api/client.test.ts` - -- [ ] **Step 1: Write the failing test** - -Create `web/src/lib/api/client.test.ts`: - -```ts -import { afterEach, describe, expect, test, vi } from 'vitest'; -import { apiFetch } from './client'; - -afterEach(() => { - vi.unstubAllGlobals(); -}); - -function stubFetch(status: number, body: unknown, init: Partial = {}) { - const res = new Response( - body === null ? null : JSON.stringify(body), - { status, headers: { 'Content-Type': 'application/json' }, ...init } - ); - const spy = vi.fn().mockResolvedValue(res); - vi.stubGlobal('fetch', spy); - return spy; -} - -describe('apiFetch', () => { - test('resolves with parsed JSON on 200', async () => { - stubFetch(200, { hello: 'world' }); - const result = await apiFetch('/api/ping'); - expect(result).toEqual({ hello: 'world' }); - }); - - test('sends Content-Type application/json by default', async () => { - const spy = stubFetch(200, {}); - await apiFetch('/api/ping'); - const init = spy.mock.calls[0][1] as RequestInit; - expect((init.headers as Record)['Content-Type']).toBe('application/json'); - }); - - test('resolves to null on 204', async () => { - stubFetch(204, null); - const result = await apiFetch('/api/ping', { method: 'DELETE' }); - expect(result).toBeNull(); - }); - - test('throws ApiError from error envelope on 4xx', async () => { - stubFetch(404, { error: { code: 'not_found', message: 'track not found' } }); - await expect(apiFetch('/api/tracks/x')).rejects.toMatchObject({ - code: 'not_found', - message: 'track not found', - status: 404 - }); - }); - - test('degrades to {code:unknown, message:statusText} on non-JSON error body', async () => { - vi.stubGlobal('fetch', vi.fn().mockResolvedValue( - new Response('500', { status: 500, statusText: 'Internal Server Error' }) - )); - await expect(apiFetch('/api/ping')).rejects.toMatchObject({ - code: 'unknown', - message: 'Internal Server Error', - status: 500 - }); - }); -}); -``` - -- [ ] **Step 2: Run the test, confirm it fails** - -Run: `cd web && npm test -- src/lib/api/client.test.ts` -Expected: FAIL — `apiFetch` not exported from `./client`. - -- [ ] **Step 3: Implement `apiFetch`** - -Append to `web/src/lib/api/client.ts`: - -```ts -export async function apiFetch(path: string, init?: RequestInit): Promise { - const res = await fetch(path, { - credentials: 'same-origin', - headers: { 'Content-Type': 'application/json', ...(init?.headers ?? {}) }, - ...init - }); - const body = res.status === 204 ? null : await res.json().catch(() => null); - if (!res.ok) { - const envelope = body && (body as { error?: { code?: string; message?: string } }).error; - const err: ApiError = { - code: envelope?.code ?? 'unknown', - message: envelope?.message ?? res.statusText, - status: res.status - }; - throw err; - } - return body; -} -``` - -- [ ] **Step 4: Run the test again, confirm it passes** - -Run: `cd web && npm test -- src/lib/api/client.test.ts` -Expected: PASS — 5 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/api/client.ts web/src/lib/api/client.test.ts -git commit -m "feat(web): add apiFetch with typed error envelope - -Wraps fetch, parses JSON, and throws ApiError{code,message,status} -on non-2xx. Degrades to {code:'unknown'} when the error body is not -the expected envelope shape." -``` - ---- - -## Task 4: `api.get` / `api.post` / `api.del` facade - -**Files:** -- Modify: `web/src/lib/api/client.ts` -- Modify: `web/src/lib/api/client.test.ts` - -- [ ] **Step 1: Add failing tests** - -Append inside `web/src/lib/api/client.test.ts`: - -```ts -import { api } from './client'; - -describe('api.get/post/del', () => { - test('api.get returns typed JSON on 200', async () => { - stubFetch(200, { id: 'abc', name: 'A' }); - type T = { id: string; name: string }; - const result = await api.get('/api/artists/abc'); - expect(result).toEqual({ id: 'abc', name: 'A' }); - }); - - test('api.post serializes body and sets method', async () => { - const spy = stubFetch(200, { ok: true }); - await api.post('/api/auth/login', { username: 'u', password: 'p' }); - const init = spy.mock.calls[0][1] as RequestInit; - expect(init.method).toBe('POST'); - expect(init.body).toBe(JSON.stringify({ username: 'u', password: 'p' })); - }); - - test('api.del sends DELETE and resolves to null on 204', async () => { - const spy = stubFetch(204, null); - const result = await api.del('/api/sessions/xyz'); - expect(spy.mock.calls[0][1]).toMatchObject({ method: 'DELETE' }); - expect(result).toBeNull(); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/api/client.test.ts` -Expected: FAIL — `api` not exported. - -- [ ] **Step 3: Implement the facade** - -Append to `web/src/lib/api/client.ts`: - -```ts -export const api = { - get: (path: string): Promise => apiFetch(path) as Promise, - post: (path: string, body: unknown): Promise => - apiFetch(path, { method: 'POST', body: JSON.stringify(body) }) as Promise, - del: (path: string): Promise => - apiFetch(path, { method: 'DELETE' }) as Promise -}; -``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/api/client.test.ts` -Expected: PASS — 8 tests total (5 from Task 3 + 3 new). - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/api/client.ts web/src/lib/api/client.test.ts -git commit -m "feat(web): add api.{get,post,del} typed facade over apiFetch" -``` - ---- - -## Task 5: `QueryClient` singleton - -**Files:** -- Create: `web/src/lib/query/client.ts` - -- [ ] **Step 1: Write the module** - -Create `web/src/lib/query/client.ts`: - -```ts -import { QueryClient } from '@tanstack/svelte-query'; - -export const queryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - staleTime: 30_000, - refetchOnWindowFocus: false - } - } -}); -``` - -- [ ] **Step 2: Verify it type-checks** - -Run: `cd web && npm run check` -Expected: `svelte-check found 0 errors and 0 warnings`. - -- [ ] **Step 3: Commit** - -```bash -git add web/src/lib/query/client.ts -git commit -m "feat(web): add TanStack Query client singleton - -Tuned defaults: retry disabled (we throw typed errors), 30s staleTime -to make tab-switch re-navigation feel instant, no refetchOnWindowFocus -(users on a music app don't expect spurious network when they tab back)." -``` - ---- - -## Task 6: `auth` store — `bootstrap`, `login`, `logout` - -**Files:** -- Create: `web/src/lib/auth/store.svelte.ts` -- Create: `web/src/lib/auth/store.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/auth/store.test.ts`: - -```ts -import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest'; - -vi.mock('$lib/api/client', () => ({ - api: { - get: vi.fn(), - post: vi.fn(), - del: vi.fn() - } -})); - -vi.mock('$lib/query/client', () => ({ - queryClient: { clear: vi.fn() } -})); - -import { api } from '$lib/api/client'; -import { queryClient } from '$lib/query/client'; -import { bootstrap, login, logout, user } from './store.svelte'; - -beforeEach(() => { - vi.resetAllMocks(); -}); - -afterEach(() => { - // Force-clear the store between tests by stubbing api.get to reject, - // then calling bootstrap. -}); - -describe('auth store', () => { - test('bootstrap populates user on 200', async () => { - (api.get as ReturnType).mockResolvedValue({ - id: '1', username: 'alice', is_admin: false - }); - await bootstrap(); - expect(user.value).toEqual({ id: '1', username: 'alice', is_admin: false }); - }); - - test('bootstrap leaves user null on failure', async () => { - (api.get as ReturnType).mockRejectedValue( - { code: 'unauthorized', message: 'no session', status: 401 } - ); - await bootstrap(); - expect(user.value).toBeNull(); - }); - - test('login sets user from LoginResponse.user', async () => { - (api.post as ReturnType).mockResolvedValue({ - token: 't', user: { id: '2', username: 'bob', is_admin: true } - }); - await login('bob', 'pw'); - expect(user.value).toEqual({ id: '2', username: 'bob', is_admin: true }); - expect(api.post).toHaveBeenCalledWith('/api/auth/login', { - username: 'bob', password: 'pw' - }); - }); - - test('logout clears user, calls /api/auth/logout, and clears query cache', async () => { - (api.post as ReturnType).mockResolvedValue(null); - await logout(); - expect(user.value).toBeNull(); - expect(api.post).toHaveBeenCalledWith('/api/auth/logout', {}); - expect(queryClient.clear).toHaveBeenCalledTimes(1); - }); - - test('logout with silent:true skips the POST but still clears state', async () => { - await logout({ silent: true }); - expect(user.value).toBeNull(); - expect(api.post).not.toHaveBeenCalled(); - expect(queryClient.clear).toHaveBeenCalledTimes(1); - }); - - test('logout swallows POST errors (best-effort)', async () => { - (api.post as ReturnType).mockRejectedValue( - { code: 'server_error', message: 'boom', status: 500 } - ); - await expect(logout()).resolves.toBeUndefined(); - expect(user.value).toBeNull(); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/auth/store.test.ts` -Expected: FAIL — module `./store.svelte` does not exist. - -- [ ] **Step 3: Implement the store** - -Create `web/src/lib/auth/store.svelte.ts`: - -```ts -import { api, type User, type LoginResponse } from '$lib/api/client'; -import { queryClient } from '$lib/query/client'; - -let _user = $state(null); - -export const user = { - get value(): User | null { - return _user; - } -}; - -export async function bootstrap(): Promise { - try { - _user = await api.get('/api/me'); - } catch { - _user = null; - } -} - -export async function login(username: string, password: string): Promise { - const res = await api.post('/api/auth/login', { username, password }); - _user = res.user; -} - -export async function logout(opts: { silent?: boolean } = {}): Promise { - if (!opts.silent) { - try { - await api.post('/api/auth/logout', {}); - } catch { - // best effort — server-side session may already be gone - } - } - _user = null; - queryClient.clear(); -} -``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/auth/store.test.ts` -Expected: PASS — 6 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/auth/store.svelte.ts web/src/lib/auth/store.test.ts -git commit -m "feat(web): add rune-based auth store with bootstrap/login/logout - -Central synchronous source of truth for 'who is signed in'. bootstrap() -runs once in +layout.ts load(); login/logout mutate _user and the -TanStack query cache. The silent option on logout is used by the 401 -interceptor (next commit) to avoid POSTing /logout against an already- -invalid session." -``` - ---- - -## Task 7: Wire 401 → `logout({silent:true})` into `apiFetch` - -**Files:** -- Modify: `web/src/lib/api/client.ts` -- Modify: `web/src/lib/api/client.test.ts` - -**Why a separate task:** the 401 interceptor requires `auth/store` to exist, and `auth/store` imports from `api/client`. To break the cycle, `apiFetch` uses a dynamic `import()` — introducing this logic only after the store is in place. - -- [ ] **Step 1: Add the failing test** - -Append inside `web/src/lib/api/client.test.ts`: - -```ts -describe('apiFetch 401 interceptor', () => { - test('401 response triggers auth.logout({silent:true}) and still throws', async () => { - const logoutSpy = vi.fn(); - vi.doMock('$lib/auth/store.svelte', () => ({ - logout: logoutSpy, - login: vi.fn(), - bootstrap: vi.fn(), - user: { value: null } - })); - // Re-import to pick up the mock. - const { apiFetch: apiFetchFresh } = await import('./client'); - stubFetch(401, { error: { code: 'unauthorized', message: 'session expired' } }); - await expect(apiFetchFresh('/api/me')).rejects.toMatchObject({ - code: 'unauthorized', - status: 401 - }); - expect(logoutSpy).toHaveBeenCalledWith({ silent: true }); - vi.doUnmock('$lib/auth/store.svelte'); - }); -}); -``` - -- [ ] **Step 2: Run, confirm the new test fails** - -Run: `cd web && npm test -- src/lib/api/client.test.ts` -Expected: FAIL — `logoutSpy` not called. - -- [ ] **Step 3: Add the interceptor in `apiFetch`** - -Replace the `if (!res.ok)` block in `web/src/lib/api/client.ts` with: - -```ts - if (!res.ok) { - if (res.status === 401) { - // Lazy import: auth/store imports from this file, so a top-level - // import would be circular. By the time any 401 actually happens - // at runtime, both modules have finished loading. - const { logout } = await import('$lib/auth/store.svelte'); - await logout({ silent: true }); - } - const envelope = body && (body as { error?: { code?: string; message?: string } }).error; - const err: ApiError = { - code: envelope?.code ?? 'unknown', - message: envelope?.message ?? res.statusText, - status: res.status - }; - throw err; - } -``` - -- [ ] **Step 4: Run all client tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/api/client.test.ts` -Expected: PASS — 9 tests total. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/api/client.ts web/src/lib/api/client.test.ts -git commit -m "feat(web): auto-logout on 401 inside apiFetch - -Dynamic import breaks the apiFetch <-> auth/store cycle. silent:true -prevents re-POSTing /logout against an already-dead session." -``` - ---- - -## Task 8: Shell component - -**Files:** -- Create: `web/src/lib/components/Shell.svelte` -- Create: `web/src/lib/components/Shell.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/components/Shell.test.ts`: - -```ts -import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/svelte'; - -vi.mock('$app/state', () => ({ - page: { url: new URL('http://localhost/') } -})); - -vi.mock('$app/navigation', () => ({ - goto: vi.fn() -})); - -vi.mock('$lib/auth/store.svelte', () => ({ - user: { value: { id: '1', username: 'alice', is_admin: false } }, - logout: vi.fn().mockResolvedValue(undefined) -})); - -import Shell from './Shell.svelte'; -import { logout } from '$lib/auth/store.svelte'; -import { goto } from '$app/navigation'; - -afterEach(() => { - vi.clearAllMocks(); -}); - -describe('Shell', () => { - test('renders the username in the header', () => { - render(Shell); - expect(screen.getByText('alice')).toBeInTheDocument(); - }); - - test('renders three nav items: Library, Search, Playlists', () => { - render(Shell); - expect(screen.getByRole('link', { name: 'Library' })).toHaveAttribute('href', '/'); - expect(screen.getByRole('link', { name: 'Search' })).toHaveAttribute('href', '/search'); - expect(screen.getByRole('link', { name: 'Playlists' })).toHaveAttribute('href', '/playlists'); - }); - - test('user-menu "Log out" calls logout() and navigates to /login', async () => { - render(Shell); - await fireEvent.click(screen.getByRole('button', { name: /alice/i })); - await fireEvent.click(screen.getByRole('button', { name: /log out/i })); - expect(logout).toHaveBeenCalledTimes(1); - expect(goto).toHaveBeenCalledWith('/login', { replaceState: true }); - }); -}); -``` - -- [ ] **Step 2: Run the tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/components/Shell.test.ts` -Expected: FAIL — component does not exist. - -- [ ] **Step 3: Implement the component** - -Create `web/src/lib/components/Shell.svelte`: - -```svelte - - - (menuOpen = false)} onkeydown={(e) => e.key === 'Escape' && (menuOpen = false)} /> - -
-
-
Minstrel
-
- - {#if menuOpen} -
e.stopPropagation()} - role="menu" - > - -
- {/if} -
-
- - - -
- {@render children()} -
-
-``` - -- [ ] **Step 4: Run the tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/components/Shell.test.ts` -Expected: PASS — 3 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/components/Shell.svelte web/src/lib/components/Shell.test.ts -git commit -m "feat(web): add Shell component with header, sidebar, and user menu - -Persistent chrome used by every authenticated route. Sidebar hides -below md:; mobile nav is a separate concern for a later plan." -``` - ---- - -## Task 9: Placeholder pages for `/`, `/search`, `/playlists` - -**Files:** -- Modify: `web/src/routes/+page.svelte` -- Create: `web/src/routes/search/+page.svelte` -- Create: `web/src/routes/playlists/+page.svelte` - -- [ ] **Step 1: Replace `web/src/routes/+page.svelte`** - -Overwrite the file with: - -```svelte -

Library

-

Library views land in a subsequent plan.

-``` - -- [ ] **Step 2: Create `web/src/routes/search/+page.svelte`** - -```svelte -

Search

-

Search lands in a subsequent plan.

-``` - -- [ ] **Step 3: Create `web/src/routes/playlists/+page.svelte`** - -```svelte -

Playlists

-

Playlists land in a subsequent plan.

-``` - -- [ ] **Step 4: Verify nothing breaks** - -Run: `cd web && npm run check && npm run build` -Expected: check passes with 0 errors; build emits `web/build/` including `index.html`. - -Then restore the committed placeholder (the build regenerates it; we don't want the real build output in git): - -Run: `git checkout -- web/build/index.html` -Expected: `git status` shows no changes under `web/build/`. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/routes/+page.svelte web/src/routes/search/+page.svelte web/src/routes/playlists/+page.svelte -git commit -m "feat(web): add Library/Search/Playlists placeholder pages - -Library replaces the scaffold splash. Search and Playlists are routable -so the Shell's sidebar nav works end-to-end before the real features land." -``` - ---- - -## Task 10: Login page - -**Files:** -- Create: `web/src/routes/login/+page.svelte` -- Create: `web/src/routes/login/login.test.ts` - -- [ ] **Step 1: Write the failing test** - -Create `web/src/routes/login/login.test.ts`: - -```ts -import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; - -// vi.hoisted avoids the temporal-dead-zone hazard: mock factories are lazy -// and can run before top-level `let` is initialized, since imports are -// hoisted above variable declarations. -const state = vi.hoisted(() => ({ pageUrl: new URL('http://localhost/login') })); - -vi.mock('$app/state', () => ({ - page: { - get url() { return state.pageUrl; } - } -})); - -vi.mock('$app/navigation', () => ({ - goto: vi.fn() -})); - -vi.mock('$lib/auth/store.svelte', () => ({ - login: vi.fn(), - user: { value: null } -})); - -import LoginPage from './+page.svelte'; -import { login } from '$lib/auth/store.svelte'; -import { goto } from '$app/navigation'; - -afterEach(() => { - vi.clearAllMocks(); - state.pageUrl = new URL('http://localhost/login'); -}); - -async function submit(username = 'alice', password = 'pw') { - const u = screen.getByLabelText(/username/i) as HTMLInputElement; - const p = screen.getByLabelText(/password/i) as HTMLInputElement; - await fireEvent.input(u, { target: { value: username } }); - await fireEvent.input(p, { target: { value: password } }); - await fireEvent.click(screen.getByRole('button', { name: /sign in/i })); -} - -describe('login page', () => { - test('renders username, password, and sign-in button', () => { - render(LoginPage); - expect(screen.getByLabelText(/username/i)).toBeInTheDocument(); - expect(screen.getByLabelText(/password/i)).toBeInTheDocument(); - expect(screen.getByRole('button', { name: /sign in/i })).toBeInTheDocument(); - }); - - test('invalid credentials show inline error and clear password', async () => { - (login as ReturnType).mockRejectedValue({ - code: 'invalid_credentials', message: 'bad creds', status: 401 - }); - render(LoginPage); - await submit(); - await waitFor(() => { - expect(screen.getByRole('alert')).toHaveTextContent(/invalid username or password/i); - }); - expect((screen.getByLabelText(/password/i) as HTMLInputElement).value).toBe(''); - expect((screen.getByLabelText(/username/i) as HTMLInputElement).value).toBe('alice'); - }); - - test('server error shows generic message and preserves both fields', async () => { - (login as ReturnType).mockRejectedValue({ - code: 'server_error', message: 'boom', status: 500 - }); - render(LoginPage); - await submit('alice', 'pw'); - await waitFor(() => { - expect(screen.getByRole('alert')).toHaveTextContent(/try again in a moment/i); - }); - expect((screen.getByLabelText(/password/i) as HTMLInputElement).value).toBe('pw'); - expect((screen.getByLabelText(/username/i) as HTMLInputElement).value).toBe('alice'); - }); - - test('success navigates to returnTo when safe', async () => { - state.pageUrl = new URL('http://localhost/login?returnTo=/artists/abc'); - (login as ReturnType).mockResolvedValue(undefined); - render(LoginPage); - await submit(); - await waitFor(() => { - expect(goto).toHaveBeenCalledWith('/artists/abc', { replaceState: true }); - }); - }); - - test('success falls back to "/" when returnTo is missing', async () => { - (login as ReturnType).mockResolvedValue(undefined); - render(LoginPage); - await submit(); - await waitFor(() => { - expect(goto).toHaveBeenCalledWith('/', { replaceState: true }); - }); - }); - - test.each([ - ['//evil.com', 'protocol-relative'], - ['http://evil.com', 'absolute URL'], - ['/login', 'self-redirect'], - ['../admin', 'parent traversal'] - ])('rejects unsafe returnTo %s (%s) and falls back to "/"', async (value) => { - state.pageUrl = new URL(`http://localhost/login?returnTo=${encodeURIComponent(value)}`); - (login as ReturnType).mockResolvedValue(undefined); - render(LoginPage); - await submit(); - await waitFor(() => { - expect(goto).toHaveBeenCalledWith('/', { replaceState: true }); - }); - }); - - test('submit button disables and marks aria-busy while pending', async () => { - let release!: () => void; - (login as ReturnType).mockReturnValue( - new Promise((resolve) => { release = () => resolve(); }) - ); - render(LoginPage); - await submit(); - const btn = screen.getByRole('button', { name: /sign in/i }); - expect(btn).toBeDisabled(); - expect(btn).toHaveAttribute('aria-busy', 'true'); - release(); - await waitFor(() => expect(btn).not.toBeDisabled()); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/routes/login/+page.test.ts` -Expected: FAIL — component does not exist. - -- [ ] **Step 3: Implement `web/src/routes/login/+page.svelte`** - -```svelte - - -
-
-

Minstrel

-
- - - - {#if error} - - {/if} -
-
-
-``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/routes/login/+page.test.ts` -Expected: PASS — 10 tests (6 `test(...)` blocks + 4 from `test.each` with 4 rows). - -- [ ] **Step 5: Commit** - -```bash -git add web/src/routes/login/+page.svelte web/src/routes/login/login.test.ts -git commit -m "feat(web): add login page with returnTo support and typed error UX - -Inline errors for invalid creds vs. server-side failures. Validates -returnTo to block open-redirect vectors (//, http://, /login, -parent-traversal)." -``` - ---- - -## Task 11: Root layout wiring — bootstrap, QueryClientProvider, guard, Shell - -**Files:** -- Create: `web/src/routes/+layout.ts` -- Modify: `web/src/routes/+layout.svelte` - -- [ ] **Step 1: Create `web/src/routes/+layout.ts`** - -```ts -import type { LayoutLoad } from './$types'; -import { bootstrap } from '$lib/auth/store.svelte'; - -export const ssr = false; // adapter-static fallback; we're SPA-only -export const prerender = false; - -export const load: LayoutLoad = async () => { - await bootstrap(); - return {}; -}; -``` - -- [ ] **Step 2: Overwrite `web/src/routes/+layout.svelte`** - -```svelte - - - - {#if user.value !== null && page.url.pathname !== '/login'} - {@render children()} - {:else} - {@render children()} - {/if} - -``` - -- [ ] **Step 3: Type-check** - -Run: `cd web && npm run check` -Expected: `svelte-check found 0 errors and 0 warnings`. - -- [ ] **Step 4: Build** - -Run: `cd web && npm run build` -Expected: build succeeds. Then: - -Run: `git checkout -- web/build/index.html` -Expected: committed placeholder restored. - -- [ ] **Step 5: Full test suite** - -Run: `cd web && npm test` -Expected: all test files pass. - -- [ ] **Step 6: Commit** - -```bash -git add web/src/routes/+layout.ts web/src/routes/+layout.svelte -git commit -m "feat(web): wire root layout — bootstrap, guard, Shell, QueryProvider - -+layout.ts runs auth.bootstrap() in load() so the shell sees the user -synchronously. +layout.svelte installs the TanStack QueryClientProvider, -runs the route guard as a \$effect, and swaps Shell vs. bare slot -based on auth state." -``` - ---- - -## Task 12: Final verification + branch finish - -**Files:** none (verification only). - -- [ ] **Step 1: Full Go suite (make sure the frontend work didn't accidentally touch Go files)** - -Run: `go test -short -race ./...` -Expected: PASS. - -- [ ] **Step 2: Go linter** - -Run: `golangci-lint run ./...` -Expected: no issues. - -- [ ] **Step 3: Web check + tests + build** - -Run: `cd web && npm run check && npm test && npm run build` -Expected: check 0 errors; all vitest files pass; build emits to `web/build/`. - -Run: `git checkout -- web/build/index.html` -Expected: committed placeholder restored. - -- [ ] **Step 4: Docker build smoke** - -Run: `docker build -t minstrel:auth-smoke .` -Expected: image builds. - -Run: `docker run --rm --entrypoint /bin/sh minstrel:auth-smoke -c 'test -x /usr/local/bin/minstrel && echo ok'` -Expected: `ok`. - -- [ ] **Step 5: Local end-to-end manual test** - -Bring up the stack: - -```bash -docker compose up --build -d -``` - -Seed a user into Postgres if one does not already exist (reuse whatever bootstrap the auth-foundation plan established; e.g., `psql` insert with a bcrypt hash). Record the username/password used. - -Then in a browser, verify these five flows: - -1. **Deep-link gated:** visit `http://localhost:4533/artists/abc`. Expected: URL changes to `/login?returnTo=%2Fartists%2Fabc`; login form is visible. -2. **Login + deep-link return:** sign in with seeded creds. Expected: URL changes to `/artists/abc`; page is blank (SPA fallback — library plan fills this in) but the Shell header shows your username and sidebar is visible. -3. **Refresh does not flash:** press F5. Expected: Shell renders without the login form appearing first. -4. **Logout:** click username menu → "Log out". Expected: URL changes to `/login`; visiting `/` redirects back to `/login`. -5. **Cross-tab session death:** log in again, open a second tab to `/`. In dev tools, clear the `minstrel_session` cookie. Click "Search" in the nav of the second tab. Expected: after the first API call in the tab (which will 401), the tab redirects to `/login`. - -Tear down: - -```bash -docker compose down -``` - -- [ ] **Step 6: Finish the branch** - -Follow `superpowers:finishing-a-development-branch`: present the four-option menu. The expected path is Option 2 — push `dev`, open a PR to `main`, wait for Forgejo CI, merge once green. - ---- - -## Self-Review Notes - -**Spec coverage check:** -- `apiFetch` contract → Tasks 3, 7 -- `api` facade → Task 4 -- `auth` store → Task 6 -- QueryClient → Task 5 -- Shell component → Task 8 -- Login page → Task 10 -- Placeholder pages → Task 9 -- Root layout bootstrap + guard → Task 11 -- Testing coverage → each feature task has its own test step -- Dependencies → Task 1 -- Final verification & branch finish → Task 12 - -All spec sections map to tasks. No gaps. - -**Type consistency:** -- `User` shape `{id, username, is_admin}` — used identically in Tasks 2, 6, 8. -- `ApiError` shape `{code, message, status}` — used identically in Tasks 2, 3, 7, 10. -- `LoginResponse` shape `{token, user}` — used in Tasks 2, 6. -- `user.value` getter — Tasks 6 (definition), 8, 11 (consumption). -- `logout(opts?: {silent?: boolean})` — Tasks 6 (definition), 7 (silent call), 8 (plain call). -- `page` from `$app/state` (not `$app/stores`) — consistent across Tasks 8, 10, 11. diff --git a/docs/superpowers/plans/2026-04-23-web-ui-library-views.md b/docs/superpowers/plans/2026-04-23-web-ui-library-views.md deleted file mode 100644 index 30031e08..00000000 --- a/docs/superpowers/plans/2026-04-23-web-ui-library-views.md +++ /dev/null @@ -1,1815 +0,0 @@ -# Web UI Library Views Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Wire up the three library browse routes — artists list at `/`, artist detail at `/artists/:id`, album detail at `/albums/:id` — consuming the existing `/api/artists*`, `/api/albums/:id`, and `/api/albums/:id/cover` endpoints, so an authenticated user can navigate the music library entirely in the SPA. - -**Architecture:** TanStack Query handles all fetches and caching via a thin `lib/api/queries.ts` helper. Each route is a tiny `+page.svelte` that imports a `create*Query` helper and delegates repeating visual units to small shared components (`ArtistRow`, `AlbumCard`, `TrackRow`, `LibrarySkeleton`, `ApiErrorBanner`). Loading/error/empty states follow the same pattern on all three routes. - -**Tech Stack:** SvelteKit 2 + Svelte 5 (runes), TypeScript, TanStack Query 5 (`@tanstack/svelte-query`), Vitest + jsdom + Testing Library, Tailwind. - -**Reference:** design spec at `docs/superpowers/specs/2026-04-23-web-ui-library-views-design.md`. - ---- - -## File Structure - -**New files under `web/`:** - -| File | Responsibility | -|---|---| -| `src/lib/api/types.ts` | Mirrors backend `internal/api/types.go` — `ArtistRef`, `AlbumRef`, `TrackRef`, `ArtistDetail`, `AlbumDetail`, `Page` | -| `src/lib/api/queries.ts` | `qk.*` key helpers and `createArtistsQuery`/`createArtistQuery`/`createAlbumQuery` wrappers | -| `src/lib/api/queries.test.ts` | Tests the pure `qk` key generators | -| `src/lib/media/covers.ts` | `FALLBACK_COVER` data URL + `coverUrl(id)` helper | -| `src/lib/media/duration.ts` | `formatDuration(seconds)` returning `mm:ss` or `h:mm:ss` | -| `src/lib/media/duration.test.ts` | Unit tests for `formatDuration` | -| `src/lib/utils/useDelayed.svelte.ts` | `useDelayed(getValue, delayMs)` rune helper for skeleton suppression | -| `src/lib/utils/useDelayed.svelte.test.ts` | Unit tests for `useDelayed` | -| `src/lib/components/ArtistRow.svelte` | Artists list row — avatar initial, name, album count, chevron, wrapping `` | -| `src/lib/components/ArtistRow.test.ts` | Component test | -| `src/lib/components/AlbumCard.svelte` | Album grid card — cover, title, year, wrapping `` | -| `src/lib/components/AlbumCard.test.ts` | Component test | -| `src/lib/components/TrackRow.svelte` | Album track row — number, title, duration | -| `src/lib/components/TrackRow.test.ts` | Component test | -| `src/lib/components/LibrarySkeleton.svelte` | Skeleton shimmer with variants `list`/`grid`/`album` | -| `src/lib/components/LibrarySkeleton.test.ts` | Component test | -| `src/lib/components/ApiErrorBanner.svelte` | Retry-capable error card | -| `src/lib/components/ApiErrorBanner.test.ts` | Component test | -| `src/test-utils/query.ts` | Shared `mockQuery` / `mockInfiniteQuery` factories for page tests | -| `src/routes/artists/[id]/+page.svelte` | Artist detail page | -| `src/routes/artists/[id]/artist.test.ts` | Page test (NOT `+page.test.ts` — route walker collision) | -| `src/routes/albums/[id]/+page.svelte` | Album detail page | -| `src/routes/albums/[id]/album.test.ts` | Page test | -| `src/routes/artists.test.ts` | Page test for the artists list (lives at `src/routes/`, not route-colocated in a `+` file) | - -**Modified files:** - -| File | Change | -|---|---| -| `src/routes/+page.svelte` | Replace the "Library" placeholder with the real artists list | - ---- - -## Task 1: API types mirroring `internal/api/types.go` - -**Files:** -- Create: `web/src/lib/api/types.ts` - -- [ ] **Step 1: Write `web/src/lib/api/types.ts`** - -```ts -// Mirrors internal/api/types.go. Kept minimal — only the shapes the SPA -// actually reads today (no LoginRequest/LoginResponse here; those live in -// client.ts alongside the auth plumbing). - -export type Page = { - items: T[]; - total: number; - limit: number; - offset: number; -}; - -export type ArtistRef = { - id: string; - name: string; - album_count: number; -}; - -export type AlbumRef = { - id: string; - title: string; - artist_id: string; - artist_name: string; - year?: number; - track_count: number; - duration_sec: number; - cover_url: string; -}; - -export type TrackRef = { - id: string; - title: string; - album_id: string; - album_title: string; - artist_id: string; - artist_name: string; - track_number?: number; - disc_number?: number; - duration_sec: number; - stream_url: string; -}; - -export type ArtistDetail = ArtistRef & { - albums: AlbumRef[]; -}; - -export type AlbumDetail = AlbumRef & { - tracks: TrackRef[]; -}; -``` - -- [ ] **Step 2: Type-check** - -Run: `cd web && npm run check` -Expected: `0 ERRORS 0 WARNINGS`. - -- [ ] **Step 3: Commit** - -```bash -git add web/src/lib/api/types.ts -git commit -m "feat(web): add library API types mirroring internal/api/types.go" -``` - ---- - -## Task 2: `formatDuration` helper + tests - -**Files:** -- Create: `web/src/lib/media/duration.ts` -- Create: `web/src/lib/media/duration.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/media/duration.test.ts`: - -```ts -import { describe, expect, test } from 'vitest'; -import { formatDuration } from './duration'; - -describe('formatDuration', () => { - test('under a minute renders 0:SS', () => { - expect(formatDuration(42)).toBe('0:42'); - expect(formatDuration(5)).toBe('0:05'); - }); - - test('under an hour renders M:SS', () => { - expect(formatDuration(242)).toBe('4:02'); - expect(formatDuration(600)).toBe('10:00'); - }); - - test('at or above an hour renders H:MM:SS', () => { - expect(formatDuration(3600)).toBe('1:00:00'); - expect(formatDuration(3723)).toBe('1:02:03'); - expect(formatDuration(36000)).toBe('10:00:00'); - }); - - test('zero renders 0:00', () => { - expect(formatDuration(0)).toBe('0:00'); - }); - - test('negative values clamp to 0:00', () => { - expect(formatDuration(-1)).toBe('0:00'); - expect(formatDuration(-100)).toBe('0:00'); - }); - - test('fractional seconds floor', () => { - expect(formatDuration(61.9)).toBe('1:01'); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/media/duration.test.ts` -Expected: FAIL — `formatDuration` not exported. - -- [ ] **Step 3: Implement `web/src/lib/media/duration.ts`** - -```ts -// Formats a non-negative number of seconds as `mm:ss` when under an hour, -// `h:mm:ss` otherwise. Negative values clamp to zero (callers shouldn't -// pass them, but the UI shouldn't crash if they do). -export function formatDuration(seconds: number): string { - const total = Math.max(0, Math.floor(seconds)); - const h = Math.floor(total / 3600); - const m = Math.floor((total % 3600) / 60); - const s = total % 60; - const ss = String(s).padStart(2, '0'); - if (h > 0) { - const mm = String(m).padStart(2, '0'); - return `${h}:${mm}:${ss}`; - } - return `${m}:${ss}`; -} -``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/media/duration.test.ts` -Expected: PASS — 6 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/media/duration.ts web/src/lib/media/duration.test.ts -git commit -m "feat(web): add formatDuration helper - -Renders seconds as mm:ss or h:mm:ss. Negative inputs clamp to 0:00 -so a bad backend value doesn't crash the UI." -``` - ---- - -## Task 3: `covers.ts` helper - -**Files:** -- Create: `web/src/lib/media/covers.ts` - -- [ ] **Step 1: Write `web/src/lib/media/covers.ts`** - -```ts -// Fallback cover — an inline SVG data URL showing a music-note glyph on the -// surface color. Used by AlbumCard's onerror handler when the real cover -// fails (e.g. scanner didn't pick up an embedded image and there's no -// sidecar). Data URL avoids an extra HTTP round-trip. -export const FALLBACK_COVER = - 'data:image/svg+xml;utf8,' + - encodeURIComponent( - ` - - - ` - ); - -export function coverUrl(albumId: string): string { - return `/api/albums/${albumId}/cover`; -} -``` - -- [ ] **Step 2: Type-check** - -Run: `cd web && npm run check` -Expected: `0 ERRORS 0 WARNINGS`. - -- [ ] **Step 3: Commit** - -```bash -git add web/src/lib/media/covers.ts -git commit -m "feat(web): add cover URL helper + fallback SVG data URL" -``` - ---- - -## Task 4: `useDelayed` rune helper - -**Files:** -- Create: `web/src/lib/utils/useDelayed.svelte.ts` -- Create: `web/src/lib/utils/useDelayed.svelte.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/utils/useDelayed.svelte.test.ts`: - -```ts -import { describe, expect, test, vi } from 'vitest'; -import { flushSync } from 'svelte'; -import { useDelayed } from './useDelayed.svelte'; - -describe('useDelayed', () => { - test('value stays false until delay elapses while source is true', () => { - vi.useFakeTimers(); - try { - let source = $state(false); - let delayed!: { readonly value: boolean }; - const cleanup = $effect.root(() => { - delayed = useDelayed(() => source, 100); - }); - - expect(delayed.value).toBe(false); - - source = true; - flushSync(); - expect(delayed.value).toBe(false); - - vi.advanceTimersByTime(50); - expect(delayed.value).toBe(false); - - vi.advanceTimersByTime(60); - expect(delayed.value).toBe(true); - - cleanup(); - } finally { - vi.useRealTimers(); - } - }); - - test('source flipping false before delay cancels the pending timeout', () => { - vi.useFakeTimers(); - try { - let source = $state(false); - let delayed!: { readonly value: boolean }; - const cleanup = $effect.root(() => { - delayed = useDelayed(() => source, 100); - }); - - source = true; - flushSync(); - vi.advanceTimersByTime(40); - source = false; - flushSync(); - vi.advanceTimersByTime(200); - expect(delayed.value).toBe(false); - - cleanup(); - } finally { - vi.useRealTimers(); - } - }); - - test('value resets to false immediately when source becomes false', () => { - vi.useFakeTimers(); - try { - let source = $state(true); - let delayed!: { readonly value: boolean }; - const cleanup = $effect.root(() => { - delayed = useDelayed(() => source, 100); - }); - flushSync(); // run the initial $effect so the setTimeout is scheduled - - vi.advanceTimersByTime(200); - expect(delayed.value).toBe(true); - - source = false; - flushSync(); - expect(delayed.value).toBe(false); - - cleanup(); - } finally { - vi.useRealTimers(); - } - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/utils/useDelayed.svelte.test.ts` -Expected: FAIL — module not found. - -- [ ] **Step 3: Implement `web/src/lib/utils/useDelayed.svelte.ts`** - -```ts -// Returns a rune-wrapped boolean that mirrors the source getter, but only -// flips to true after the source has been true for delayMs continuously. -// Flips back to false immediately when the source becomes false. -// -// Used to suppress flash-of-skeleton on cache-hit navigation: skeletons -// only render for loads that take longer than ~100ms. -export function useDelayed( - source: () => boolean, - delayMs = 100 -): { readonly value: boolean } { - let delayed = $state(false); - let timeout: ReturnType | null = null; - - $effect(() => { - const v = source(); - if (timeout) { - clearTimeout(timeout); - timeout = null; - } - if (v) { - timeout = setTimeout(() => { - delayed = true; - timeout = null; - }, delayMs); - } else { - delayed = false; - } - return () => { - if (timeout) { - clearTimeout(timeout); - timeout = null; - } - }; - }); - - return { get value() { return delayed; } }; -} -``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/utils/useDelayed.svelte.test.ts` -Expected: PASS — 3 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/utils/useDelayed.svelte.ts web/src/lib/utils/useDelayed.svelte.test.ts -git commit -m "feat(web): add useDelayed rune helper - -Boolean mirror that flips true only after the source stays true for -delayMs. Used to hide the skeleton on sub-100ms cache hits so -back-navigation feels instant." -``` - ---- - -## Task 5: `queries.ts` + key-generator tests - -**Files:** -- Create: `web/src/lib/api/queries.ts` -- Create: `web/src/lib/api/queries.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/api/queries.test.ts`: - -```ts -import { describe, expect, test } from 'vitest'; -import { qk } from './queries'; - -describe('qk key generators', () => { - test('artists key includes sort discriminator', () => { - expect(qk.artists('alpha')).toEqual(['artists', { sort: 'alpha' }]); - expect(qk.artists('newest')).toEqual(['artists', { sort: 'newest' }]); - }); - - test('artist key tuple', () => { - expect(qk.artist('abc')).toEqual(['artist', 'abc']); - }); - - test('album key tuple', () => { - expect(qk.album('xyz')).toEqual(['album', 'xyz']); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/api/queries.test.ts` -Expected: FAIL — module not found. - -- [ ] **Step 3: Implement `web/src/lib/api/queries.ts`** - -```ts -import { createQuery, createInfiniteQuery } from '@tanstack/svelte-query'; -import { api } from './client'; -import type { ArtistRef, ArtistDetail, AlbumDetail, Page } from './types'; - -export type ArtistSort = 'alpha' | 'newest'; -export const ARTIST_PAGE_SIZE = 50; - -export const qk = { - artists: (sort: ArtistSort) => ['artists', { sort }] as const, - artist: (id: string) => ['artist', id] as const, - album: (id: string) => ['album', id] as const, -}; - -export function createArtistsQuery(sort: ArtistSort) { - return createInfiniteQuery({ - queryKey: qk.artists(sort), - queryFn: ({ pageParam = 0 }) => - api.get>( - `/api/artists?sort=${sort}&limit=${ARTIST_PAGE_SIZE}&offset=${pageParam}` - ), - initialPageParam: 0, - getNextPageParam: (last) => { - const loaded = last.offset + last.items.length; - return loaded >= last.total ? undefined : loaded; - }, - }); -} - -export function createArtistQuery(id: string) { - return createQuery({ - queryKey: qk.artist(id), - queryFn: () => api.get(`/api/artists/${id}`), - }); -} - -export function createAlbumQuery(id: string) { - return createQuery({ - queryKey: qk.album(id), - queryFn: () => api.get(`/api/albums/${id}`), - }); -} -``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/api/queries.test.ts` -Expected: PASS — 3 tests. - -- [ ] **Step 5: Type-check** - -Run: `cd web && npm run check` -Expected: `0 ERRORS 0 WARNINGS`. - -- [ ] **Step 6: Commit** - -```bash -git add web/src/lib/api/queries.ts web/src/lib/api/queries.test.ts -git commit -m "feat(web): add TanStack Query helpers for library endpoints - -qk.{artists,artist,album} key generators and create*Query wrappers -with shared page size, sort-aware keys, and infinite-query plumbing -for /api/artists." -``` - ---- - -## Task 6: `ArtistRow` component - -**Files:** -- Create: `web/src/lib/components/ArtistRow.svelte` -- Create: `web/src/lib/components/ArtistRow.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/components/ArtistRow.test.ts`: - -```ts -import { describe, expect, test } from 'vitest'; -import { render, screen } from '@testing-library/svelte'; -import ArtistRow from './ArtistRow.svelte'; -import type { ArtistRef } from '$lib/api/types'; - -const artist: ArtistRef = { id: 'abc', name: 'alice', album_count: 12 }; - -describe('ArtistRow', () => { - test('renders initial, name, album count inside a link to /artists/:id', () => { - render(ArtistRow, { props: { artist } }); - - const link = screen.getByRole('link'); - expect(link).toHaveAttribute('href', '/artists/abc'); - expect(link).toHaveTextContent('alice'); - expect(link).toHaveTextContent('12 albums'); - expect(link).toHaveTextContent('A'); // initial letter - }); - - test('singular "1 album" when album_count is 1', () => { - render(ArtistRow, { props: { artist: { ...artist, album_count: 1 } } }); - expect(screen.getByRole('link')).toHaveTextContent('1 album'); - }); - - test('empty name still renders a safe initial', () => { - render(ArtistRow, { props: { artist: { ...artist, name: '' } } }); - // No crash; initial container present but empty or a placeholder char. - expect(screen.getByRole('link')).toBeInTheDocument(); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/components/ArtistRow.test.ts` -Expected: FAIL — component not found. - -- [ ] **Step 3: Implement `web/src/lib/components/ArtistRow.svelte`** - -```svelte - - - - - {artist.name} - {countLabel} - - -``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/components/ArtistRow.test.ts` -Expected: PASS — 3 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/components/ArtistRow.svelte web/src/lib/components/ArtistRow.test.ts -git commit -m "feat(web): add ArtistRow component - -One row in the artists list: avatar initial, name, album count, -chevron. Entire row is an for click + keyboard activation." -``` - ---- - -## Task 7: `AlbumCard` component - -**Files:** -- Create: `web/src/lib/components/AlbumCard.svelte` -- Create: `web/src/lib/components/AlbumCard.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/components/AlbumCard.test.ts`: - -```ts -import { describe, expect, test } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/svelte'; -import AlbumCard from './AlbumCard.svelte'; -import type { AlbumRef } from '$lib/api/types'; -import { FALLBACK_COVER } from '$lib/media/covers'; - -const album: AlbumRef = { - id: 'xyz', - title: 'Kind of Blue', - artist_id: 'm-davis', - artist_name: 'Miles Davis', - year: 1959, - track_count: 5, - duration_sec: 2630, - cover_url: '/api/albums/xyz/cover', -}; - -describe('AlbumCard', () => { - test('renders cover, title, year inside a link to /albums/:id', () => { - render(AlbumCard, { props: { album } }); - const link = screen.getByRole('link'); - expect(link).toHaveAttribute('href', '/albums/xyz'); - expect(link).toHaveTextContent('Kind of Blue'); - expect(link).toHaveTextContent('1959'); - - const img = screen.getByRole('img') as HTMLImageElement; - expect(img.src).toContain('/api/albums/xyz/cover'); - }); - - test('year is omitted when not present', () => { - render(AlbumCard, { props: { album: { ...album, year: undefined } } }); - // No crash; no year text. - expect(screen.queryByText(/1959/)).not.toBeInTheDocument(); - }); - - test('broken cover falls back to FALLBACK_COVER data URL', async () => { - render(AlbumCard, { props: { album } }); - const img = screen.getByRole('img') as HTMLImageElement; - await fireEvent.error(img); - expect(img.src).toBe(FALLBACK_COVER); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/components/AlbumCard.test.ts` -Expected: FAIL — component not found. - -- [ ] **Step 3: Implement `web/src/lib/components/AlbumCard.svelte`** - -```svelte - - - - -
{album.title}
- {#if album.year} -
{album.year}
- {/if} - -``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/components/AlbumCard.test.ts` -Expected: PASS — 3 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/components/AlbumCard.svelte web/src/lib/components/AlbumCard.test.ts -git commit -m "feat(web): add AlbumCard component - -Grid card used on the artist detail page. Cover image, title, year. -Broken covers swap to FALLBACK_COVER via onerror." -``` - ---- - -## Task 8: `TrackRow` component - -**Files:** -- Create: `web/src/lib/components/TrackRow.svelte` -- Create: `web/src/lib/components/TrackRow.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/components/TrackRow.test.ts`: - -```ts -import { describe, expect, test } from 'vitest'; -import { render, screen } from '@testing-library/svelte'; -import TrackRow from './TrackRow.svelte'; -import type { TrackRef } from '$lib/api/types'; - -const track: TrackRef = { - id: 't1', - title: 'So What', - album_id: 'xyz', - album_title: 'Kind of Blue', - artist_id: 'm-davis', - artist_name: 'Miles Davis', - track_number: 1, - disc_number: 1, - duration_sec: 545, - stream_url: '/api/tracks/t1/stream', -}; - -describe('TrackRow', () => { - test('renders track number, title, formatted duration', () => { - render(TrackRow, { props: { track } }); - expect(screen.getByText('1')).toBeInTheDocument(); - expect(screen.getByText('So What')).toBeInTheDocument(); - expect(screen.getByText('9:05')).toBeInTheDocument(); - }); - - test('track number falls back to dash when missing', () => { - render(TrackRow, { props: { track: { ...track, track_number: undefined } } }); - expect(screen.getByText('—')).toBeInTheDocument(); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/components/TrackRow.test.ts` -Expected: FAIL — component not found. - -- [ ] **Step 3: Implement `web/src/lib/components/TrackRow.svelte`** - -```svelte - - -
- - {track.track_number ?? '—'} - - {track.title} - {formatDuration(track.duration_sec)} -
-``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/components/TrackRow.test.ts` -Expected: PASS — 2 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/components/TrackRow.svelte web/src/lib/components/TrackRow.test.ts -git commit -m "feat(web): add TrackRow component - -Non-interactive album track row with number, title, duration. -Player plan will add click-to-play." -``` - ---- - -## Task 9: `LibrarySkeleton` component - -**Files:** -- Create: `web/src/lib/components/LibrarySkeleton.svelte` -- Create: `web/src/lib/components/LibrarySkeleton.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/components/LibrarySkeleton.test.ts`: - -```ts -import { describe, expect, test } from 'vitest'; -import { render, screen } from '@testing-library/svelte'; -import LibrarySkeleton from './LibrarySkeleton.svelte'; - -describe('LibrarySkeleton', () => { - test('variant="list" renders count placeholder rows', () => { - render(LibrarySkeleton, { props: { variant: 'list', count: 5 } }); - const container = screen.getByTestId('skeleton-list'); - expect(container.querySelectorAll('[data-skeleton-row]').length).toBe(5); - }); - - test('variant="grid" renders count placeholder cards', () => { - render(LibrarySkeleton, { props: { variant: 'grid', count: 6 } }); - const container = screen.getByTestId('skeleton-grid'); - expect(container.querySelectorAll('[data-skeleton-card]').length).toBe(6); - }); - - test('variant="album" renders a hero placeholder with cover block + bars', () => { - render(LibrarySkeleton, { props: { variant: 'album' } }); - expect(screen.getByTestId('skeleton-album')).toBeInTheDocument(); - expect(screen.getByTestId('skeleton-album-cover')).toBeInTheDocument(); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/components/LibrarySkeleton.test.ts` -Expected: FAIL — component not found. - -- [ ] **Step 3: Implement `web/src/lib/components/LibrarySkeleton.svelte`** - -```svelte - - -{#if variant === 'list'} -
- {#each Array.from({ length: count }) as _, i (i)} -
- - - -
- {/each} -
-{:else if variant === 'grid'} -
- {#each Array.from({ length: count }) as _, i (i)} -
-
-
-
-
- {/each} -
-{:else} -
-
-
-
-
-
-
-
-
-
- {#each Array.from({ length: 10 }) as _, i (i)} -
- {/each} -
-
-{/if} -``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/components/LibrarySkeleton.test.ts` -Expected: PASS — 3 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/components/LibrarySkeleton.svelte web/src/lib/components/LibrarySkeleton.test.ts -git commit -m "feat(web): add LibrarySkeleton with list/grid/album variants - -Shimmer placeholders that mirror the layout of each library page so -the real content slides in without shifting." -``` - ---- - -## Task 10: `ApiErrorBanner` component - -**Files:** -- Create: `web/src/lib/components/ApiErrorBanner.svelte` -- Create: `web/src/lib/components/ApiErrorBanner.test.ts` - -- [ ] **Step 1: Write the failing tests** - -Create `web/src/lib/components/ApiErrorBanner.test.ts`: - -```ts -import { describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/svelte'; -import ApiErrorBanner from './ApiErrorBanner.svelte'; - -describe('ApiErrorBanner', () => { - test('renders error.message when present', () => { - render(ApiErrorBanner, { - props: { - error: { code: 'server_error', message: 'database down', status: 500 }, - onRetry: () => {} - } - }); - expect(screen.getByRole('alert')).toHaveTextContent('database down'); - }); - - test('falls back to generic text when error lacks message', () => { - render(ApiErrorBanner, { props: { error: undefined, onRetry: () => {} } }); - expect(screen.getByRole('alert')).toHaveTextContent(/something went wrong/i); - }); - - test('clicking the button calls onRetry', async () => { - const onRetry = vi.fn(); - render(ApiErrorBanner, { - props: { - error: { code: 'server_error', message: 'x', status: 500 }, - onRetry - } - }); - await fireEvent.click(screen.getByRole('button', { name: /try again/i })); - expect(onRetry).toHaveBeenCalledTimes(1); - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/lib/components/ApiErrorBanner.test.ts` -Expected: FAIL — component not found. - -- [ ] **Step 3: Implement `web/src/lib/components/ApiErrorBanner.svelte`** - -```svelte - - - -``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/lib/components/ApiErrorBanner.test.ts` -Expected: PASS — 3 tests. - -- [ ] **Step 5: Commit** - -```bash -git add web/src/lib/components/ApiErrorBanner.svelte web/src/lib/components/ApiErrorBanner.test.ts -git commit -m "feat(web): add ApiErrorBanner retry-capable error card" -``` - ---- - -## Task 11: Artists list page at `/` - -**Files:** -- Modify: `web/src/routes/+page.svelte` -- Create: `web/src/test-utils/query.ts` -- Create: `web/src/routes/artists.test.ts` - -- [ ] **Step 1: Create `web/src/test-utils/query.ts`** - -```ts -import type { Page } from '$lib/api/types'; - -// Synthetic infinite-query object shape matching what the SPA reads from -// @tanstack/svelte-query. Enough surface area for component tests; -// real behavior is covered by the TanStack library's own tests. -export function mockInfiniteQuery(opts: { - pages?: Page[]; - isPending?: boolean; - isError?: boolean; - error?: unknown; - hasNextPage?: boolean; - isFetchingNextPage?: boolean; - fetchNextPage?: () => void; - refetch?: () => void; -} = {}) { - return { - data: { pages: opts.pages ?? [] }, - isPending: opts.isPending ?? false, - isError: opts.isError ?? false, - error: opts.error, - hasNextPage: opts.hasNextPage ?? false, - isFetchingNextPage: opts.isFetchingNextPage ?? false, - fetchNextPage: opts.fetchNextPage ?? (() => {}), - refetch: opts.refetch ?? (() => {}) - }; -} - -export function mockQuery(opts: { - data?: T; - isPending?: boolean; - isError?: boolean; - error?: unknown; - refetch?: () => void; -} = {}) { - return { - data: opts.data, - isPending: opts.isPending ?? false, - isError: opts.isError ?? false, - error: opts.error, - refetch: opts.refetch ?? (() => {}) - }; -} -``` - -- [ ] **Step 2: Write the failing page test** - -Create `web/src/routes/artists.test.ts`: - -```ts -import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent } from '@testing-library/svelte'; -import { flushSync } from 'svelte'; -import { mockInfiniteQuery } from '../test-utils/query'; -import type { ArtistRef, Page } from '$lib/api/types'; - -const state = vi.hoisted(() => ({ pageUrl: new URL('http://localhost/') })); - -vi.mock('$app/state', () => ({ - page: { get url() { return state.pageUrl; } } -})); - -vi.mock('$app/navigation', () => ({ - goto: vi.fn() -})); - -vi.mock('$lib/api/queries', () => ({ - qk: { artists: (sort: string) => ['artists', { sort }] }, - createArtistsQuery: vi.fn() -})); - -import ArtistsPage from './+page.svelte'; -import { createArtistsQuery } from '$lib/api/queries'; -import { goto } from '$app/navigation'; - -function page(items: T[], total: number, offset = 0, limit = 50): Page { - return { items, total, offset, limit }; -} - -afterEach(() => { - vi.clearAllMocks(); - state.pageUrl = new URL('http://localhost/'); -}); - -describe('artists list page', () => { - test('renders a row per artist', () => { - const alice: ArtistRef = { id: 'a', name: 'Alice', album_count: 3 }; - const bob: ArtistRef = { id: 'b', name: 'Bob', album_count: 1 }; - (createArtistsQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([alice, bob], 2)] }) - ); - render(ArtistsPage); - expect(screen.getByRole('link', { name: /Alice/ })).toHaveAttribute('href', '/artists/a'); - expect(screen.getByRole('link', { name: /Bob/ })).toHaveAttribute('href', '/artists/b'); - }); - - test('renders the total count from the first page', () => { - (createArtistsQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 1247)] }) - ); - render(ArtistsPage); - expect(screen.getByText(/1247 artists/i)).toBeInTheDocument(); - }); - - test('Load more button calls fetchNextPage when hasNextPage', async () => { - const fetchNextPage = vi.fn(); - (createArtistsQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ - pages: [page([], 100)], - hasNextPage: true, - fetchNextPage - }) - ); - render(ArtistsPage); - await fireEvent.click(screen.getByRole('button', { name: /load more/i })); - expect(fetchNextPage).toHaveBeenCalledTimes(1); - }); - - test('"End of library" when hasNextPage is false and at least one page loaded', () => { - (createArtistsQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([{ id: 'a', name: 'A', album_count: 1 }], 1)] }) - ); - render(ArtistsPage); - expect(screen.getByText(/end of library/i)).toBeInTheDocument(); - }); - - test('empty total renders empty-library message', () => { - (createArtistsQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 0)] }) - ); - render(ArtistsPage); - expect(screen.getByText(/no artists yet/i)).toBeInTheDocument(); - }); - - test('changing the sort dropdown calls goto with ?sort=newest', async () => { - (createArtistsQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ pages: [page([], 0)] }) - ); - render(ArtistsPage); - const select = screen.getByLabelText(/sort/i) as HTMLSelectElement; - await fireEvent.change(select, { target: { value: 'newest' } }); - expect(goto).toHaveBeenCalledWith('?sort=newest', { replaceState: true }); - }); - - test('pending state renders the list skeleton after the useDelayed window', () => { - (createArtistsQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ isPending: true }) - ); - vi.useFakeTimers(); - try { - render(ArtistsPage); - vi.advanceTimersByTime(200); - flushSync(); // flush the $effect that reads delayed.value - expect(screen.getByTestId('skeleton-list')).toBeInTheDocument(); - } finally { - vi.useRealTimers(); - } - }); - - test('error state renders the error banner with retry', async () => { - const refetch = vi.fn(); - (createArtistsQuery as ReturnType).mockReturnValue( - mockInfiniteQuery({ - isError: true, - error: { code: 'server_error', message: 'db down', status: 500 }, - refetch - }) - ); - render(ArtistsPage); - expect(screen.getByRole('alert')).toHaveTextContent('db down'); - await fireEvent.click(screen.getByRole('button', { name: /try again/i })); - expect(refetch).toHaveBeenCalledTimes(1); - }); -}); -``` - -- [ ] **Step 3: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/routes/artists.test.ts` -Expected: FAIL — page still renders the placeholder, assertions don't match. - -- [ ] **Step 4: Replace `web/src/routes/+page.svelte`** - -```svelte - - -
-
-
-

Library

- {#if !query.isPending && !query.isError} -

- {total} {total === 1 ? 'artist' : 'artists'} -

- {/if} -
- -
- - {#if query.isError} - - {:else if showSkeleton.value && artists.length === 0} - - {:else if !query.isPending && total === 0} -

- No artists yet — scan a library folder via the server's config. -

- {:else} -
- {#each artists as a (a.id)} - - {/each} -
- - {#if query.hasNextPage} - - {:else if artists.length > 0} -

End of library

- {/if} - {/if} -
-``` - -- [ ] **Step 5: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/routes/artists.test.ts` -Expected: PASS — 8 tests. - -- [ ] **Step 6: Type-check** - -Run: `cd web && npm run check` -Expected: `0 ERRORS 0 WARNINGS`. - -- [ ] **Step 7: Commit** - -```bash -git add web/src/routes/+page.svelte web/src/routes/artists.test.ts web/src/test-utils/query.ts -git commit -m "feat(web): replace / placeholder with real artists list - -Uses createArtistsQuery (infinite), URL-driven sort dropdown, Load -more pagination, delayed skeleton, and shared error banner. Also -introduces the test-utils/query.ts helpers for subsequent page tests." -``` - ---- - -## Task 12: Artist detail page at `/artists/:id` - -**Files:** -- Create: `web/src/routes/artists/[id]/+page.svelte` -- Create: `web/src/routes/artists/[id]/artist.test.ts` - -- [ ] **Step 1: Write the failing test** - -Create `web/src/routes/artists/[id]/artist.test.ts`: - -```ts -import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen } from '@testing-library/svelte'; -import { flushSync } from 'svelte'; -import { mockQuery } from '../../../test-utils/query'; -import type { ArtistDetail, AlbumRef } from '$lib/api/types'; - -const state = vi.hoisted(() => ({ pageParams: { id: 'abc' } as Record })); - -vi.mock('$app/state', () => ({ - page: { - get params() { return state.pageParams; }, - get url() { return new URL('http://localhost/artists/' + state.pageParams.id); } - } -})); - -vi.mock('$lib/api/queries', () => ({ - qk: { artist: (id: string) => ['artist', id] }, - createArtistQuery: vi.fn() -})); - -import ArtistPage from './+page.svelte'; -import { createArtistQuery } from '$lib/api/queries'; - -function album(id: string, title: string, year?: number): AlbumRef { - return { - id, title, - artist_id: 'abc', artist_name: 'Alice', - year, track_count: 10, duration_sec: 2400, - cover_url: `/api/albums/${id}/cover` - }; -} - -afterEach(() => { - vi.clearAllMocks(); - state.pageParams = { id: 'abc' }; -}); - -describe('artist detail page', () => { - test('renders artist name, subtitle, and one AlbumCard per album', () => { - const detail: ArtistDetail = { - id: 'abc', name: 'Alice', album_count: 2, - albums: [album('a1', 'First', 2020), album('a2', 'Second')] - }; - (createArtistQuery as ReturnType).mockReturnValue(mockQuery({ data: detail })); - render(ArtistPage); - expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Alice'); - expect(screen.getByText(/2 albums/i)).toBeInTheDocument(); - expect(screen.getByRole('link', { name: /First/ })).toHaveAttribute('href', '/albums/a1'); - expect(screen.getByRole('link', { name: /Second/ })).toHaveAttribute('href', '/albums/a2'); - }); - - test('back link points to Library', () => { - (createArtistQuery as ReturnType).mockReturnValue(mockQuery({ - data: { id: 'abc', name: 'Alice', album_count: 0, albums: [] } - })); - render(ArtistPage); - const back = screen.getByRole('link', { name: /library/i }); - expect(back).toHaveAttribute('href', '/'); - }); - - test('404 renders non-retryable "Artist not found" with back link', () => { - (createArtistQuery as ReturnType).mockReturnValue(mockQuery({ - isError: true, - error: { code: 'not_found', message: 'nope', status: 404 } - })); - render(ArtistPage); - expect(screen.getByText(/artist not found/i)).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: /try again/i })).not.toBeInTheDocument(); - }); - - test('non-404 error renders the retry banner', () => { - (createArtistQuery as ReturnType).mockReturnValue(mockQuery({ - isError: true, - error: { code: 'server_error', message: 'boom', status: 500 } - })); - render(ArtistPage); - expect(screen.getByRole('alert')).toHaveTextContent('boom'); - expect(screen.getByRole('button', { name: /try again/i })).toBeInTheDocument(); - }); - - test('pending (after delay) renders the grid skeleton', () => { - (createArtistQuery as ReturnType).mockReturnValue(mockQuery({ isPending: true })); - vi.useFakeTimers(); - try { - render(ArtistPage); - vi.advanceTimersByTime(200); - flushSync(); - expect(screen.getByTestId('skeleton-grid')).toBeInTheDocument(); - } finally { - vi.useRealTimers(); - } - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/routes/artists/[id]/artist.test.ts` -Expected: FAIL — component not found. - -- [ ] **Step 3: Implement `web/src/routes/artists/[id]/+page.svelte`** - -```svelte - - -
- ← Library - - {#if notFound} - - {:else if query.isError} - - {:else if showSkeleton.value && !query.data} - - {:else if query.data} - {@const detail = query.data} -
-

{detail.name}

-

- {detail.album_count} {detail.album_count === 1 ? 'album' : 'albums'} -

-
- - {#if detail.albums.length === 0} -

This artist has no albums in the library.

- {:else} -
- {#each detail.albums as album (album.id)} - - {/each} -
- {/if} - {/if} -
-``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/routes/artists/[id]/artist.test.ts` -Expected: PASS — 5 tests. - -- [ ] **Step 5: Type-check** - -Run: `cd web && npm run check` -Expected: `0 ERRORS 0 WARNINGS`. - -- [ ] **Step 6: Commit** - -```bash -git add web/src/routes/artists/[id]/+page.svelte web/src/routes/artists/[id]/artist.test.ts -git commit -m "feat(web): add artist detail page at /artists/:id - -Renders the artist name + album count and a responsive AlbumCard grid -from the nested ArtistDetail response. 404 surfaces a distinct -non-retryable 'not found' state; other errors share the retry banner." -``` - ---- - -## Task 13: Album detail page at `/albums/:id` - -**Files:** -- Create: `web/src/routes/albums/[id]/+page.svelte` -- Create: `web/src/routes/albums/[id]/album.test.ts` - -- [ ] **Step 1: Write the failing test** - -Create `web/src/routes/albums/[id]/album.test.ts`: - -```ts -import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen } from '@testing-library/svelte'; -import { flushSync } from 'svelte'; -import { mockQuery } from '../../../test-utils/query'; -import type { AlbumDetail, TrackRef } from '$lib/api/types'; - -const state = vi.hoisted(() => ({ pageParams: { id: 'xyz' } as Record })); - -vi.mock('$app/state', () => ({ - page: { - get params() { return state.pageParams; }, - get url() { return new URL('http://localhost/albums/' + state.pageParams.id); } - } -})); - -vi.mock('$lib/api/queries', () => ({ - qk: { album: (id: string) => ['album', id] }, - createAlbumQuery: vi.fn() -})); - -import AlbumPage from './+page.svelte'; -import { createAlbumQuery } from '$lib/api/queries'; - -function track(id: string, title: string, number: number, dur: number): TrackRef { - return { - id, title, - album_id: 'xyz', album_title: 'Kind of Blue', - artist_id: 'md', artist_name: 'Miles Davis', - track_number: number, disc_number: 1, - duration_sec: dur, - stream_url: `/api/tracks/${id}/stream` - }; -} - -afterEach(() => { - vi.clearAllMocks(); - state.pageParams = { id: 'xyz' }; -}); - -describe('album detail page', () => { - test('renders hero metadata + one TrackRow per track', () => { - const detail: AlbumDetail = { - id: 'xyz', title: 'Kind of Blue', - artist_id: 'md', artist_name: 'Miles Davis', - year: 1959, track_count: 2, duration_sec: 544 + 565, - cover_url: '/api/albums/xyz/cover', - tracks: [track('t1', 'So What', 1, 544), track('t2', 'Freddie Freeloader', 2, 565)] - }; - (createAlbumQuery as ReturnType).mockReturnValue(mockQuery({ data: detail })); - render(AlbumPage); - - expect(screen.getByRole('heading', { level: 1 })).toHaveTextContent('Kind of Blue'); - expect(screen.getByRole('link', { name: /Miles Davis/ })).toHaveAttribute('href', '/artists/md'); - expect(screen.getByText(/1959/)).toBeInTheDocument(); - expect(screen.getByText(/2 tracks/i)).toBeInTheDocument(); - - expect(screen.getByText('So What')).toBeInTheDocument(); - expect(screen.getByText('Freddie Freeloader')).toBeInTheDocument(); - - const img = screen.getByRole('img') as HTMLImageElement; - expect(img.src).toContain('/api/albums/xyz/cover'); - }); - - test('back link points to /artists/:artistId with the artist name', () => { - const detail: AlbumDetail = { - id: 'xyz', title: 'T', - artist_id: 'md', artist_name: 'Miles Davis', - track_count: 0, duration_sec: 0, - cover_url: '/api/albums/xyz/cover', - tracks: [] - }; - (createAlbumQuery as ReturnType).mockReturnValue(mockQuery({ data: detail })); - render(AlbumPage); - const back = screen.getByRole('link', { name: /Miles Davis/ }); - expect(back).toHaveAttribute('href', '/artists/md'); - }); - - test('404 renders non-retryable "Album not found" with back link', () => { - (createAlbumQuery as ReturnType).mockReturnValue(mockQuery({ - isError: true, - error: { code: 'not_found', message: 'nope', status: 404 } - })); - render(AlbumPage); - expect(screen.getByText(/album not found/i)).toBeInTheDocument(); - expect(screen.queryByRole('button', { name: /try again/i })).not.toBeInTheDocument(); - }); - - test('non-404 error renders the retry banner', () => { - (createAlbumQuery as ReturnType).mockReturnValue(mockQuery({ - isError: true, - error: { code: 'server_error', message: 'boom', status: 500 } - })); - render(AlbumPage); - expect(screen.getByRole('alert')).toHaveTextContent('boom'); - expect(screen.getByRole('button', { name: /try again/i })).toBeInTheDocument(); - }); - - test('pending (after delay) renders the album skeleton', () => { - (createAlbumQuery as ReturnType).mockReturnValue(mockQuery({ isPending: true })); - vi.useFakeTimers(); - try { - render(AlbumPage); - vi.advanceTimersByTime(200); - flushSync(); - expect(screen.getByTestId('skeleton-album')).toBeInTheDocument(); - } finally { - vi.useRealTimers(); - } - }); -}); -``` - -- [ ] **Step 2: Run tests, confirm they fail** - -Run: `cd web && npm test -- src/routes/albums/[id]/album.test.ts` -Expected: FAIL — component not found. - -- [ ] **Step 3: Implement `web/src/routes/albums/[id]/+page.svelte`** - -```svelte - - -
- {#if notFound} - - {:else if query.isError} - - {:else if showSkeleton.value && !query.data} - - {:else if query.data} - {@const album = query.data} - - ← {album.artist_name} - - -
- -
-

{album.title}

-

- {album.artist_name} -

- {#if album.year} -

{album.year}

- {/if} -

- {album.track_count} {album.track_count === 1 ? 'track' : 'tracks'} - · {formatDuration(album.duration_sec)} -

-
-
- - {#if album.tracks.length === 0} -

This album has no tracks.

- {:else} -
- {#each album.tracks as track (track.id)} - - {/each} -
- {/if} - {/if} -
-``` - -- [ ] **Step 4: Run tests, confirm they pass** - -Run: `cd web && npm test -- src/routes/albums/[id]/album.test.ts` -Expected: PASS — 5 tests. - -- [ ] **Step 5: Type-check** - -Run: `cd web && npm run check` -Expected: `0 ERRORS 0 WARNINGS`. - -- [ ] **Step 6: Commit** - -```bash -git add web/src/routes/albums/[id]/+page.svelte web/src/routes/albums/[id]/album.test.ts -git commit -m "feat(web): add album detail page at /albums/:id - -Hero with cover + title + linked artist + metadata; TrackRow list -below. Back link targets the artist page (not Library) so drilling -up feels correct. 404 renders a distinct non-retryable state." -``` - ---- - -## Task 14: Final verification + branch finish - -**Files:** none (verification only). - -- [ ] **Step 1: Full Go suite (sanity — backend untouched)** - -Run: `go test -short -race ./...` -Expected: PASS. - -- [ ] **Step 2: Go linter** - -Run: `golangci-lint run ./...` -Expected: no issues. - -- [ ] **Step 3: Web check + full tests + build** - -Run: `cd web && npm run check && npm test && npm run build` -Expected: check 0 errors / 0 warnings; all vitest files pass; build emits `web/build/`. - -Run: `git checkout -- web/build/index.html` -Expected: committed placeholder restored. - -- [ ] **Step 4: Docker build smoke** - -Run: `docker build -t minstrel:library-smoke .` -Expected: image builds. - -Run: `docker run --rm --entrypoint /bin/sh minstrel:library-smoke -c 'test -x /usr/local/bin/minstrel && echo ok'` -Expected: `ok`. - -- [ ] **Step 5: Local end-to-end manual check** - -Bring up the stack: - -```bash -docker compose up --build -d -``` - -In a browser: - -1. Sign in with the seeded admin user. -2. `/` renders the artists list with counts and "Load more" if more than 50 artists exist. -3. Change sort dropdown → URL shows `?sort=newest`, list reorders. -4. Click an artist → albums grid renders with covers (some may show the fallback glyph — that's expected for albums without embedded cover art). -5. Click an album → hero + track list. Duration format looks right (`3:42` for short, `1:02:03` for long). -6. Click the back link on the album page → returns to the artist (not to `/`). -7. Visit `/artists/00000000-0000-0000-0000-000000000000` → "Artist not found" card. -8. Visit `/albums/00000000-0000-0000-0000-000000000000` → "Album not found" card. -9. Refresh `/artists/` → Shell renders without login flash; data refetches and renders. - -Tear down: - -```bash -docker compose down -``` - -- [ ] **Step 6: Finish the branch** - -Follow `superpowers:finishing-a-development-branch`. Expected path: Option 2 — push `dev`, open a PR to `main`, wait for Forgejo CI, merge once green. - ---- - -## Self-Review Notes - -**Spec coverage check:** -- API types → Task 1 -- `queries.ts` helpers + query-key convention → Task 5 -- Cover URL + fallback → Task 3 -- Duration formatter → Task 2 -- Delayed skeleton helper → Task 4 -- ArtistRow / AlbumCard / TrackRow / LibrarySkeleton / ApiErrorBanner → Tasks 6–10 -- Test-utils mocks → Task 11 (introduced alongside the first page that needs them) -- Artists list page (replace `/` placeholder) → Task 11 -- Artist detail page → Task 12 -- Album detail page → Task 13 -- Final verification + branch finish → Task 14 - -All spec sections map to tasks. - -**Type consistency:** -- `ArtistRef`, `AlbumRef`, `TrackRef`, `ArtistDetail`, `AlbumDetail`, `Page` — defined in Task 1, used identically in Tasks 5–13. -- `ArtistSort` — exported from `queries.ts` (Task 5), consumed in Task 11. -- `ApiError` — re-imported from `$lib/api/client` (auth plan) in Tasks 10, 12, 13. -- `createArtistsQuery` / `createArtistQuery` / `createAlbumQuery` signatures consistent between Task 5 and consuming tasks. -- `qk.artists(sort)` / `qk.artist(id)` / `qk.album(id)` — mocked-out in page tests exactly as defined. -- `FALLBACK_COVER` — defined in Task 3, used in Tasks 7 (AlbumCard) and 13 (album page hero). -- `formatDuration(seconds)` — defined in Task 2, used in Task 8 (TrackRow test and component) and Task 13. -- `useDelayed(getValue, delayMs?)` — defined in Task 4, used in Tasks 11, 12, 13. - -**Filename hazards addressed:** No page test is named `+page.test.ts` (auth plan's lesson). Route-colocated tests use `artist.test.ts` / `album.test.ts`; the artists list test lives at `src/routes/artists.test.ts` (outside the `+` route-walker). diff --git a/docs/superpowers/plans/2026-04-24-web-ui-player.md b/docs/superpowers/plans/2026-04-24-web-ui-player.md deleted file mode 100644 index 32caee1e..00000000 --- a/docs/superpowers/plans/2026-04-24-web-ui-player.md +++ /dev/null @@ -1,1698 +0,0 @@ -# Web UI Player Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Ship queue-based audio playback in the SPA — click a track on an album, the rest of the album queues, the bottom bar shows progress + controls, MediaSession populates OS media UI, and shuffle/repeat/volume all work. - -**Architecture:** A single Svelte 5 rune store in `lib/player/store.svelte.ts` holds all player state. A single `