diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index 9b000abe..e35670e8 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -15,6 +15,8 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/config" "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/library" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests" "git.fabledsword.com/bvandeusen/minstrel/internal/logging" "git.fabledsword.com/bvandeusen/minstrel/internal/scrobble" "git.fabledsword.com/bvandeusen/minstrel/internal/scrobble/listenbrainz" @@ -88,6 +90,13 @@ func run() error { similarityWorker := similarity.NewWorker(pool, listenbrainz.NewClient(), logger.With("component", "similarity")) go similarityWorker.Run(ctx) + // Start the Lidarr reconciler worker. Per spec §M5a, polls pending Lidarr + // import requests and reconciles them against the library. Short-circuits + // to no-op when lidarr_config.enabled = false. + lidarrCfg := lidarrconfig.New(pool) + lidarrReconciler := lidarrrequests.NewReconciler(pool, lidarrCfg, logger.With("component", "lidarr")) + go lidarrReconciler.Run(ctx) + srv := server.New(logger, pool, scanner, subsonic.Config{ AllowPlaintextPassword: cfg.Subsonic.AllowPlaintextPassword, }, cfg.Events, cfg.Recommendation) diff --git a/docs/superpowers/plans/2026-04-29-m5a-lidarr.md b/docs/superpowers/plans/2026-04-29-m5a-lidarr.md new file mode 100644 index 00000000..f14e7cf0 --- /dev/null +++ b/docs/superpowers/plans/2026-04-29-m5a-lidarr.md @@ -0,0 +1,1979 @@ +# M5a — Lidarr connection + search/add + admin shell — 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 Minstrel to a household Lidarr instance — search at `/discover`, request from any user, admin moderation queue at `/admin/requests`, automatic reconciliation when downloaded tracks land in the library. + +**Architecture:** New `internal/lidarr` HTTP client (typed, mirrors `internal/scrobble/listenbrainz`); `internal/lidarrconfig` singleton service backed by a CHECK-constrained DB row; `internal/lidarrrequests` with synchronous `Service` (Approve calls Lidarr, fires scan) plus async `Reconciler` worker (5-min tick, joins approved requests against new tracks by MBID); new `RequireAdmin` middleware on a dedicated `/api/admin/*` route group; SPA gets `/discover`, `/requests`, `/admin/integrations`, `/admin/requests` with hard route-level role gate, all surfaces drawn against the FabledSword design system. + +**Tech Stack:** Go 1.23 · chi router · pgx/v5 + sqlc · Postgres + golang-migrate · SvelteKit 2 / Svelte 5 (runes) · TanStack Query · Vitest · golangci-lint · FabledSword design tokens (Obsidian/Iron surfaces, Moss/Bronze/Oxblood actions, forest-teal #4A6B5C accent, Fraunces ≥18px / Inter / JetBrains Mono). + +**Spec:** [`docs/superpowers/specs/2026-04-29-m5a-lidarr-design.md`](../specs/2026-04-29-m5a-lidarr-design.md). Read it before starting — every decision is explained there. + +**Memory dependencies:** `project_design_system.md` (token palette + voice rules), `project_product_not_project.md` (no YAML for feature config), `project_ui_quality.md` (no scaffolding-feel), `project_subsonic_legacy.md` (`/api/*` is primary), `project_no_github.md` (Forgejo MCP for PR ops, not gh CLI). + +--- + +## File map + +### Backend — create + +- `internal/db/migrations/0010_lidarr.up.sql` · `0010_lidarr.down.sql` — schema +- `internal/db/queries/lidarr_config.sql` — sqlc queries for the singleton +- `internal/db/queries/lidarr_requests.sql` — sqlc queries for requests +- `internal/lidarr/client.go` — Lidarr HTTP client (`Client` struct + methods) +- `internal/lidarr/types.go` — typed request/response structs +- `internal/lidarr/errors.go` — typed sentinel errors +- `internal/lidarr/client_test.go` — unit tests with `httptest` stubs + JSON fixtures +- `internal/lidarr/testdata/*.json` — captured Lidarr responses +- `internal/lidarrconfig/service.go` — singleton config wrapper +- `internal/lidarrconfig/service_test.go` — integration test against `MINSTREL_TEST_DATABASE_URL` +- `internal/lidarrrequests/service.go` — request lifecycle service +- `internal/lidarrrequests/service_test.go` — integration test +- `internal/lidarrrequests/reconciler.go` — background worker +- `internal/lidarrrequests/reconciler_integration_test.go` — integration test +- `internal/auth/admin.go` — `RequireAdmin` middleware +- `internal/auth/admin_test.go` — middleware tests +- `internal/api/lidarr.go` — `GET /api/lidarr/search` proxy +- `internal/api/lidarr_test.go` +- `internal/api/requests.go` — `/api/requests` user-facing CRUD +- `internal/api/requests_test.go` +- `internal/api/admin_lidarr.go` — `/api/admin/lidarr/*` (config CRUD + test + profiles + folders) +- `internal/api/admin_lidarr_test.go` +- `internal/api/admin_requests.go` — `/api/admin/requests/*` approval queue +- `internal/api/admin_requests_test.go` + +### Backend — modify + +- `internal/api/api.go` — register routes, mount `/api/admin` group +- `internal/api/auth_test.go` — extend `testHandlers` to inject Lidarr client + lidarrrequests service +- `cmd/minstrel/main.go` — wire `lidarrrequests.Reconciler` worker +- `internal/db/dbq/*` — regenerated by `sqlc generate` + +### Frontend — create + +- `web/src/lib/styles/fabledsword-tokens.css` — `:root` CSS custom properties for all FS tokens +- `web/tailwind.config.js` — extend theme to alias semantic Tailwind utilities to FS tokens (modify, not create — but this slice may need to drop the existing alias defaults) +- `web/src/lib/api/lidarr.ts` — search client +- `web/src/lib/api/requests.ts` — request CRUD client +- `web/src/lib/api/admin.ts` — admin endpoints client +- `web/src/lib/components/DiscoverResultCard.svelte` — card with reserved badge slot + anchored button +- `web/src/lib/components/DiscoverResultCard.test.ts` +- `web/src/lib/components/StatusPill.svelte` — semantic status pill (Pending/Approved/Completed/Rejected) +- `web/src/lib/components/StatusPill.test.ts` +- `web/src/lib/components/AdminSidebar.svelte` — admin nav rail +- `web/src/routes/discover/+page.svelte` +- `web/src/routes/discover/discover.test.ts` +- `web/src/routes/requests/+page.svelte` +- `web/src/routes/requests/requests.test.ts` +- `web/src/routes/admin/+layout.svelte` — admin shell + sidebar +- `web/src/routes/admin/+layout.ts` — role gate (load function) +- `web/src/routes/admin/+page.svelte` — overview landing +- `web/src/routes/admin/integrations/+page.svelte` +- `web/src/routes/admin/integrations/integrations.test.ts` +- `web/src/routes/admin/requests/+page.svelte` +- `web/src/routes/admin/requests/requests.test.ts` + +### Frontend — modify + +- `web/src/lib/components/Shell.svelte` — add `/discover` to nav, conditional `/admin` link for admins +- `web/src/app.css` (or equivalent) — import the tokens file +- `web/src/app.html` — `` Google Fonts (Fraunces / Inter / JetBrains Mono) + +--- + +## Task list + +### Task 1 — Migration 0010 + sqlc queries + +**Files:** +- Create: `internal/db/migrations/0010_lidarr.up.sql` +- Create: `internal/db/migrations/0010_lidarr.down.sql` +- Create: `internal/db/queries/lidarr_config.sql` +- Create: `internal/db/queries/lidarr_requests.sql` +- Modify: `internal/db/dbq/*` (regenerated by `sqlc generate`) + +- [ ] **Step 1.1: Write the up migration** + +`internal/db/migrations/0010_lidarr.up.sql`: + +```sql +-- M5a: Lidarr integration foundation. Two tables: +-- +-- lidarr_config — singleton (CHECK id=1) holding the operator's Lidarr +-- connection. enabled=false is the unconfigured state. +-- +-- lidarr_requests — per-request lifecycle row created by users at +-- /discover, transitioned by admin at /admin/requests, and matched +-- back to library tracks by the reconciler worker. Three matched_*_id +-- FKs (one per kind) instead of polymorphic — clean SQL, ON DELETE +-- SET NULL preserves audit even if the matched track is later removed. + +CREATE TABLE lidarr_config ( + id smallint PRIMARY KEY DEFAULT 1 CHECK (id = 1), + enabled boolean NOT NULL DEFAULT false, + base_url text, + api_key text, + default_quality_profile_id int, + default_root_folder_path text, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now() +); + +INSERT INTO lidarr_config (id, enabled) VALUES (1, false); + +CREATE TYPE lidarr_request_status AS ENUM ( + 'pending', 'approved', 'rejected', 'completed', 'failed' +); +CREATE TYPE lidarr_request_kind AS ENUM ('artist', 'album', 'track'); + +CREATE TABLE lidarr_requests ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, + status lidarr_request_status NOT NULL DEFAULT 'pending', + kind lidarr_request_kind NOT NULL, + + lidarr_artist_mbid text NOT NULL, + lidarr_album_mbid text, + lidarr_track_mbid text, + artist_name text NOT NULL, + album_title text, + track_title text, + + quality_profile_id int, + root_folder_path text, + + decided_at timestamptz, + decided_by uuid REFERENCES users(id) ON DELETE SET NULL, + notes text, + + completed_at timestamptz, + matched_track_id uuid REFERENCES tracks(id) ON DELETE SET NULL, + matched_album_id uuid REFERENCES albums(id) ON DELETE SET NULL, + matched_artist_id uuid REFERENCES artists(id) ON DELETE SET NULL, + + requested_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now() +); + +CREATE INDEX lidarr_requests_user_id_idx ON lidarr_requests (user_id); +CREATE INDEX lidarr_requests_status_idx ON lidarr_requests (status); +CREATE INDEX lidarr_requests_artist_mbid_idx ON lidarr_requests (lidarr_artist_mbid); +CREATE INDEX lidarr_requests_album_mbid_idx ON lidarr_requests (lidarr_album_mbid) + WHERE lidarr_album_mbid IS NOT NULL; +``` + +- [ ] **Step 1.2: Write the down migration** + +`internal/db/migrations/0010_lidarr.down.sql`: + +```sql +DROP INDEX IF EXISTS lidarr_requests_album_mbid_idx; +DROP INDEX IF EXISTS lidarr_requests_artist_mbid_idx; +DROP INDEX IF EXISTS lidarr_requests_status_idx; +DROP INDEX IF EXISTS lidarr_requests_user_id_idx; +DROP TABLE IF EXISTS lidarr_requests; +DROP TYPE IF EXISTS lidarr_request_kind; +DROP TYPE IF EXISTS lidarr_request_status; +DROP TABLE IF EXISTS lidarr_config; +``` + +- [ ] **Step 1.3: Apply migration locally to confirm it runs** + +```bash +docker compose exec -T postgres psql -U minstrel -d minstrel -c "DROP TABLE IF EXISTS lidarr_requests; DROP TYPE IF EXISTS lidarr_request_kind; DROP TYPE IF EXISTS lidarr_request_status; DROP TABLE IF EXISTS lidarr_config;" +go run ./cmd/minstrel/migrate.go 2>/dev/null || go run ./cmd/minstrel up +docker compose exec -T postgres psql -U minstrel -d minstrel -c "\d lidarr_requests" +``` + +Expected: `\d lidarr_requests` shows the table with all columns and the four indexes. + +If your project doesn't have a standalone migrate command, the migration applies on server start via `db.Migrate(...)` — restart the minstrel container instead. + +- [ ] **Step 1.4: Write `lidarr_config.sql` queries** + +`internal/db/queries/lidarr_config.sql`: + +```sql +-- name: GetLidarrConfig :one +SELECT id, enabled, base_url, api_key, default_quality_profile_id, + default_root_folder_path, created_at, updated_at +FROM lidarr_config +WHERE id = 1; + +-- name: UpdateLidarrConfig :one +UPDATE lidarr_config + SET enabled = $1, + base_url = $2, + api_key = $3, + default_quality_profile_id = $4, + default_root_folder_path = $5, + updated_at = now() + WHERE id = 1 + RETURNING id, enabled, base_url, api_key, default_quality_profile_id, + default_root_folder_path, created_at, updated_at; +``` + +- [ ] **Step 1.5: Write `lidarr_requests.sql` queries** + +`internal/db/queries/lidarr_requests.sql`: + +```sql +-- name: CreateLidarrRequest :one +INSERT INTO lidarr_requests ( + user_id, kind, + lidarr_artist_mbid, lidarr_album_mbid, lidarr_track_mbid, + artist_name, album_title, track_title +) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) +RETURNING *; + +-- name: GetLidarrRequestByID :one +SELECT * FROM lidarr_requests WHERE id = $1; + +-- name: ListLidarrRequestsForUser :many +SELECT * FROM lidarr_requests +WHERE user_id = $1 +ORDER BY requested_at DESC +LIMIT $2; + +-- name: ListLidarrRequestsByStatus :many +SELECT * FROM lidarr_requests +WHERE status = $1 +ORDER BY requested_at DESC +LIMIT $2; + +-- name: ListApprovedLidarrRequestsForReconcile :many +SELECT * FROM lidarr_requests +WHERE status = 'approved' +ORDER BY decided_at ASC +LIMIT $1; + +-- name: ApproveLidarrRequest :one +UPDATE lidarr_requests + SET status = 'approved', + quality_profile_id = $2, + root_folder_path = $3, + decided_at = now(), + decided_by = $4, + updated_at = now() + WHERE id = $1 AND status = 'pending' + RETURNING *; + +-- name: RejectLidarrRequest :one +UPDATE lidarr_requests + SET status = 'rejected', + notes = $2, + decided_at = now(), + decided_by = $3, + updated_at = now() + WHERE id = $1 AND status = 'pending' + RETURNING *; + +-- name: CancelLidarrRequest :one +UPDATE lidarr_requests + SET status = 'rejected', + notes = 'cancelled by user', + decided_at = now(), + decided_by = $2, + updated_at = now() + WHERE id = $1 AND user_id = $2 AND status = 'pending' + RETURNING *; + +-- name: CompleteLidarrRequest :one +-- Reconciler transitions an approved request to completed when its +-- target track/album/artist has appeared in the library. +UPDATE lidarr_requests + SET status = 'completed', + matched_track_id = $2, + matched_album_id = $3, + matched_artist_id = $4, + completed_at = now(), + updated_at = now() + WHERE id = $1 AND status = 'approved' + RETURNING *; + +-- name: HasNonTerminalRequestForMBID :one +-- Returns true if any user has a pending/approved/completed request +-- whose MBID matches at the given level. Used to set the `requested` +-- flag on /api/lidarr/search responses. Terminal-status (rejected, +-- failed) rows do not count. +SELECT EXISTS ( + SELECT 1 FROM lidarr_requests + WHERE status IN ('pending', 'approved', 'completed') + AND ((kind = 'artist' AND lidarr_artist_mbid = $1) + OR (kind = 'album' AND lidarr_album_mbid = $1) + OR (kind = 'track' AND lidarr_track_mbid = $1)) +); +``` + +- [ ] **Step 1.6: Run sqlc generate** + +```bash +cd internal/db && sqlc generate && cd - +go build ./... +``` + +Expected: `internal/db/dbq/lidarr_config.sql.go` and `internal/db/dbq/lidarr_requests.sql.go` are created. Build succeeds. + +- [ ] **Step 1.7: Commit** + +```bash +git add internal/db/migrations/0010_lidarr.up.sql \ + internal/db/migrations/0010_lidarr.down.sql \ + internal/db/queries/lidarr_config.sql \ + internal/db/queries/lidarr_requests.sql \ + internal/db/dbq/ +git commit -m "feat(db): add lidarr_config + lidarr_requests schema (migration 0010)" +``` + +--- + +### Task 2 — Lidarr HTTP client + +**Files:** +- Create: `internal/lidarr/types.go` +- Create: `internal/lidarr/errors.go` +- Create: `internal/lidarr/client.go` +- Create: `internal/lidarr/client_test.go` +- Create: `internal/lidarr/testdata/lookup_artist.json` +- Create: `internal/lidarr/testdata/quality_profiles.json` +- Create: `internal/lidarr/testdata/root_folders.json` + +- [ ] **Step 2.1: Write the typed structs** + +`internal/lidarr/types.go`: + +```go +// Package lidarr is a typed HTTP client for Lidarr's v1 API. It is the +// only place in the codebase that knows about Lidarr's wire format. +// Callers receive value structs, never raw JSON. +package lidarr + +// LookupResult is the normalized shape returned by Lookup{Artist,Album,Track}. +// It is what we store on the request row (via the user's request) and +// what /api/lidarr/search returns to the SPA. +type LookupResult struct { + MBID string // foreignArtistId / foreignAlbumId / foreignTrackId + Name string // artist name; album/track returns Title here too + Secondary string // genre + album count for artist; year for album; album for track + ImageURL string // cover-art URL Lidarr surfaced (may be empty) +} + +// QualityProfile is the dropdown choice in /admin/integrations. +type QualityProfile struct { + ID int + Name string +} + +// RootFolder is the dropdown choice in /admin/integrations. +type RootFolder struct { + Path string + Accessible bool + FreeSpace int64 +} + +// AddArtistParams are the fields Lidarr requires on POST /api/v1/artist. +type AddArtistParams struct { + ForeignArtistID string + QualityProfileID int + RootFolderPath string + MonitorAll bool // true => Monitored="all"; false => "future" +} + +// AddAlbumParams are the fields Lidarr requires on POST /api/v1/album. +type AddAlbumParams struct { + ForeignAlbumID string + ForeignArtistID string // Lidarr requires the artist's foreign id too + QualityProfileID int + RootFolderPath string +} + +// PingResult is the response shape from GET /api/v1/system/status. +type PingResult struct { + Version string +} +``` + +- [ ] **Step 2.2: Write the typed errors** + +`internal/lidarr/errors.go`: + +```go +package lidarr + +import "errors" + +// Sentinel errors. Callers branch on these via errors.Is, not on +// HTTP status codes — the client maps codes to errors. +var ( + ErrUnreachable = errors.New("lidarr: unreachable") + ErrAuthFailed = errors.New("lidarr: auth failed") // 401 / 403 + ErrLookupFailed = errors.New("lidarr: lookup failed") // 4xx other than 401/403 + ErrServerError = errors.New("lidarr: server error") // 5xx + ErrInvalidPayload = errors.New("lidarr: invalid payload") +) +``` + +- [ ] **Step 2.3: Write the client skeleton + LookupArtist** + +`internal/lidarr/client.go`: + +```go +package lidarr + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "io" + "net/http" + "net/url" + "strconv" +) + +// Client wraps Lidarr's v1 HTTP API. BaseURL is the Lidarr instance +// (e.g. http://lidarr.lan:8686), APIKey comes from Lidarr's settings. +type Client struct { + BaseURL string + APIKey string + HTTP *http.Client +} + +func (c *Client) get(ctx context.Context, path string, q url.Values) (*http.Response, error) { + u, err := url.Parse(c.BaseURL) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrUnreachable, err) + } + u.Path = u.Path + path + if q != nil { + u.RawQuery = q.Encode() + } + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) + if err != nil { + return nil, err + } + req.Header.Set("X-Api-Key", c.APIKey) + resp, err := c.HTTP.Do(req) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrUnreachable, err) + } + if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden { + _ = resp.Body.Close() + return nil, ErrAuthFailed + } + if resp.StatusCode >= 500 { + _ = resp.Body.Close() + return nil, ErrServerError + } + if resp.StatusCode >= 400 { + _ = resp.Body.Close() + return nil, ErrLookupFailed + } + return resp, nil +} + +// LookupArtist hits Lidarr GET /api/v1/artist/lookup?term=. Returns +// normalized LookupResults from the response. +func (c *Client) LookupArtist(ctx context.Context, term string) ([]LookupResult, error) { + resp, err := c.get(ctx, "/api/v1/artist/lookup", url.Values{"term": {term}}) + if err != nil { + return nil, err + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + var raw []struct { + ForeignArtistID string `json:"foreignArtistId"` + ArtistName string `json:"artistName"` + Genres []string `json:"genres"` + AlbumCount int `json:"albumCount"` + Images []struct { + CoverType string `json:"coverType"` + RemoteURL string `json:"remoteUrl"` + URL string `json:"url"` + } `json:"images"` + } + if err := json.Unmarshal(body, &raw); err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + out := make([]LookupResult, 0, len(raw)) + for _, r := range raw { + secondary := "" + if len(r.Genres) > 0 { + secondary = r.Genres[0] + } + if r.AlbumCount > 0 { + if secondary != "" { + secondary += " · " + } + secondary += strconv.Itoa(r.AlbumCount) + " albums" + } + out = append(out, LookupResult{ + MBID: r.ForeignArtistID, + Name: r.ArtistName, + Secondary: secondary, + ImageURL: pickPosterImage(r.Images), + }) + } + return out, nil +} + +func pickPosterImage(imgs []struct { + CoverType string `json:"coverType"` + RemoteURL string `json:"remoteUrl"` + URL string `json:"url"` +}) string { + for _, img := range imgs { + if img.CoverType == "poster" { + if img.RemoteURL != "" { + return img.RemoteURL + } + return img.URL + } + } + return "" +} + +// Ensure interface implementations stay consistent. +var _ = errors.Is +``` + +- [ ] **Step 2.4: Add a captured Lidarr lookup response to testdata** + +`internal/lidarr/testdata/lookup_artist.json`: + +```json +[ + { + "foreignArtistId": "069b64b6-7884-4f6a-94cc-e4c1d6c87a01", + "artistName": "Boards of Canada", + "genres": ["Electronic", "IDM"], + "albumCount": 18, + "images": [ + {"coverType": "poster", "remoteUrl": "https://example.invalid/boc.jpg"}, + {"coverType": "banner", "remoteUrl": "https://example.invalid/boc-banner.jpg"} + ] + }, + { + "foreignArtistId": "f54ba20c-7da3-4b8a-9b12-22f09b9e2c1c", + "artistName": "Bored of Education", + "genres": [], + "albumCount": 0, + "images": [] + } +] +``` + +- [ ] **Step 2.5: Write the LookupArtist test** + +`internal/lidarr/client_test.go`: + +```go +package lidarr + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "os" + "testing" +) + +func TestLookupArtist_HappyPath(t *testing.T) { + body, err := os.ReadFile("testdata/lookup_artist.json") + if err != nil { + t.Fatalf("read fixture: %v", err) + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if got := r.Header.Get("X-Api-Key"); got != "key123" { + t.Errorf("api key = %q, want key123", got) + } + if r.URL.Path != "/api/v1/artist/lookup" { + t.Errorf("path = %q", r.URL.Path) + } + if r.URL.Query().Get("term") != "boards" { + t.Errorf("term = %q", r.URL.Query().Get("term")) + } + _, _ = w.Write(body) + })) + defer srv.Close() + c := &Client{BaseURL: srv.URL, APIKey: "key123", HTTP: srv.Client()} + got, err := c.LookupArtist(context.Background(), "boards") + if err != nil { + t.Fatalf("LookupArtist: %v", err) + } + if len(got) != 2 { + t.Fatalf("len = %d, want 2", len(got)) + } + if got[0].Name != "Boards of Canada" { + t.Errorf("name = %q", got[0].Name) + } + if got[0].Secondary != "Electronic · 18 albums" { + t.Errorf("secondary = %q", got[0].Secondary) + } + if got[0].ImageURL != "https://example.invalid/boc.jpg" { + t.Errorf("image = %q", got[0].ImageURL) + } + if got[1].Secondary != "" { + t.Errorf("expected empty secondary for empty genres + 0 albums; got %q", got[1].Secondary) + } +} + +func TestLookupArtist_AuthFailed(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + })) + defer srv.Close() + c := &Client{BaseURL: srv.URL, APIKey: "x", HTTP: srv.Client()} + _, err := c.LookupArtist(context.Background(), "boards") + if !errors.Is(err, ErrAuthFailed) { + t.Fatalf("err = %v, want ErrAuthFailed", err) + } +} + +func TestLookupArtist_ServerError(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer srv.Close() + c := &Client{BaseURL: srv.URL, APIKey: "x", HTTP: srv.Client()} + _, err := c.LookupArtist(context.Background(), "boards") + if !errors.Is(err, ErrServerError) { + t.Fatalf("err = %v, want ErrServerError", err) + } +} + +func TestLookupArtist_Unreachable(t *testing.T) { + c := &Client{BaseURL: "http://127.0.0.1:1", APIKey: "x", HTTP: &http.Client{}} + _, err := c.LookupArtist(context.Background(), "boards") + if !errors.Is(err, ErrUnreachable) { + t.Fatalf("err = %v, want ErrUnreachable", err) + } +} +``` + +- [ ] **Step 2.6: Run the tests, fix until green** + +```bash +go test -race -v ./internal/lidarr/... +``` + +Expected: 4 tests pass. + +- [ ] **Step 2.7: Add LookupAlbum and LookupTrack methods** + +Append to `internal/lidarr/client.go`: + +```go +// LookupAlbum hits GET /api/v1/album/lookup?term=. Returns normalized +// LookupResults; Secondary is "year · trackcount". +func (c *Client) LookupAlbum(ctx context.Context, term string) ([]LookupResult, error) { + resp, err := c.get(ctx, "/api/v1/album/lookup", url.Values{"term": {term}}) + if err != nil { + return nil, err + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + var raw []struct { + ForeignAlbumID string `json:"foreignAlbumId"` + ForeignArtistID string `json:"foreignArtistId"` + Title string `json:"title"` + ArtistName string `json:"artistName"` + ReleaseDate string `json:"releaseDate"` + TrackCount int `json:"trackCount"` + Images []struct { + CoverType string `json:"coverType"` + RemoteURL string `json:"remoteUrl"` + URL string `json:"url"` + } `json:"images"` + } + if err := json.Unmarshal(body, &raw); err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + out := make([]LookupResult, 0, len(raw)) + for _, r := range raw { + year := "" + if len(r.ReleaseDate) >= 4 { + year = r.ReleaseDate[:4] + } + secondary := r.ArtistName + if year != "" { + secondary += " · " + year + } + if r.TrackCount > 0 { + secondary += " · " + strconv.Itoa(r.TrackCount) + " tracks" + } + out = append(out, LookupResult{ + MBID: r.ForeignAlbumID, + Name: r.Title, + Secondary: secondary, + ImageURL: pickPosterImage(r.Images), + }) + } + return out, nil +} + +// LookupTrack hits GET /api/v1/track/lookup?term=. Lidarr's track +// lookup is per-album under the hood — Secondary is "album · artist". +func (c *Client) LookupTrack(ctx context.Context, term string) ([]LookupResult, error) { + resp, err := c.get(ctx, "/api/v1/track/lookup", url.Values{"term": {term}}) + if err != nil { + return nil, err + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + var raw []struct { + ForeignTrackID string `json:"foreignTrackId"` + ForeignAlbumID string `json:"foreignAlbumId"` + ForeignArtistID string `json:"foreignArtistId"` + Title string `json:"title"` + AlbumTitle string `json:"albumTitle"` + ArtistName string `json:"artistName"` + } + if err := json.Unmarshal(body, &raw); err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + out := make([]LookupResult, 0, len(raw)) + for _, r := range raw { + secondary := r.AlbumTitle + if r.ArtistName != "" { + if secondary != "" { + secondary += " · " + } + secondary += r.ArtistName + } + out = append(out, LookupResult{ + MBID: r.ForeignTrackID, + Name: r.Title, + Secondary: secondary, + }) + } + return out, nil +} +``` + +- [ ] **Step 2.8: Add tests for LookupAlbum and LookupTrack** + +Use the same `httptest`+fixture pattern. Capture two more JSON files (`testdata/lookup_album.json`, `testdata/lookup_track.json`) with at least 2 results each, then add `TestLookupAlbum_HappyPath` and `TestLookupTrack_HappyPath` mirroring `TestLookupArtist_HappyPath`. Auth/server-error variants are unchanged so don't duplicate them — one parametric helper test suffices. + +- [ ] **Step 2.9: Add AddArtist, AddAlbum, ListQualityProfiles, ListRootFolders, Ping** + +Append to `internal/lidarr/client.go`: + +```go +// post is the shared POST helper. Body is marshaled JSON. +func (c *Client) post(ctx context.Context, path string, body []byte) (*http.Response, error) { + u, err := url.Parse(c.BaseURL) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrUnreachable, err) + } + u.Path = u.Path + path + req, err := http.NewRequestWithContext(ctx, http.MethodPost, u.String(), bytes.NewReader(body)) + if err != nil { + return nil, err + } + req.Header.Set("X-Api-Key", c.APIKey) + req.Header.Set("Content-Type", "application/json") + resp, err := c.HTTP.Do(req) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrUnreachable, err) + } + if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden { + _ = resp.Body.Close() + return nil, ErrAuthFailed + } + if resp.StatusCode >= 500 { + _ = resp.Body.Close() + return nil, ErrServerError + } + if resp.StatusCode >= 400 { + _ = resp.Body.Close() + return nil, ErrLookupFailed + } + return resp, nil +} + +func (c *Client) AddArtist(ctx context.Context, p AddArtistParams) error { + monitor := "future" + if p.MonitorAll { + monitor = "all" + } + body, _ := json.Marshal(map[string]any{ + "foreignArtistId": p.ForeignArtistID, + "qualityProfileId": p.QualityProfileID, + "rootFolderPath": p.RootFolderPath, + "monitored": true, + "monitor": monitor, + "addOptions": map[string]any{"searchForMissingAlbums": true}, + }) + resp, err := c.post(ctx, "/api/v1/artist", body) + if err != nil { + return err + } + _ = resp.Body.Close() + return nil +} + +func (c *Client) AddAlbum(ctx context.Context, p AddAlbumParams) error { + body, _ := json.Marshal(map[string]any{ + "foreignAlbumId": p.ForeignAlbumID, + "foreignArtistId": p.ForeignArtistID, + "qualityProfileId": p.QualityProfileID, + "rootFolderPath": p.RootFolderPath, + "monitored": true, + "addOptions": map[string]any{"searchForNewAlbum": true}, + }) + resp, err := c.post(ctx, "/api/v1/album", body) + if err != nil { + return err + } + _ = resp.Body.Close() + return nil +} + +func (c *Client) ListQualityProfiles(ctx context.Context) ([]QualityProfile, error) { + resp, err := c.get(ctx, "/api/v1/qualityprofile", nil) + if err != nil { + return nil, err + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + var raw []struct { + ID int `json:"id"` + Name string `json:"name"` + } + if err := json.Unmarshal(body, &raw); err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + out := make([]QualityProfile, len(raw)) + for i, r := range raw { + out[i] = QualityProfile{ID: r.ID, Name: r.Name} + } + return out, nil +} + +func (c *Client) ListRootFolders(ctx context.Context) ([]RootFolder, error) { + resp, err := c.get(ctx, "/api/v1/rootfolder", nil) + if err != nil { + return nil, err + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + var raw []struct { + Path string `json:"path"` + Accessible bool `json:"accessible"` + FreeSpace int64 `json:"freeSpace"` + } + if err := json.Unmarshal(body, &raw); err != nil { + return nil, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + out := make([]RootFolder, len(raw)) + for i, r := range raw { + out[i] = RootFolder{Path: r.Path, Accessible: r.Accessible, FreeSpace: r.FreeSpace} + } + return out, nil +} + +func (c *Client) Ping(ctx context.Context) (PingResult, error) { + resp, err := c.get(ctx, "/api/v1/system/status", nil) + if err != nil { + return PingResult{}, err + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + return PingResult{}, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + var raw struct { + Version string `json:"version"` + } + if err := json.Unmarshal(body, &raw); err != nil { + return PingResult{}, fmt.Errorf("%w: %v", ErrInvalidPayload, err) + } + return PingResult{Version: raw.Version}, nil +} +``` + +Add `"bytes"` to the imports. + +- [ ] **Step 2.10: Add tests for the remaining methods** + +For each: capture or hand-write a fixture, add `TestAddArtist_PostsCorrectBody`, `TestAddAlbum_PostsCorrectBody`, `TestListQualityProfiles_HappyPath`, `TestListRootFolders_HappyPath`, `TestPing_ReturnsVersion`. The Add* tests should assert on the parsed POST body — read `r.Body`, decode JSON, check the field shape. + +- [ ] **Step 2.11: Run all client tests** + +```bash +go test -race -cover ./internal/lidarr/... +``` + +Expected: all tests pass; coverage ≥ 80%. + +- [ ] **Step 2.12: Commit** + +```bash +git add internal/lidarr/ +git commit -m "feat(lidarr): typed HTTP client for v1 API (lookup, add, profiles, ping)" +``` + +--- + +### Task 3 — `lidarrconfig` singleton service + +**Files:** +- Create: `internal/lidarrconfig/service.go` +- Create: `internal/lidarrconfig/service_test.go` + +- [ ] **Step 3.1: Write the service** + +`internal/lidarrconfig/service.go`: + +```go +// Package lidarrconfig is a thin wrapper over the singleton lidarr_config +// row. Get returns a typed Config; Save updates it. Callers branch on +// Config.Enabled — never on raw NULL fields. +package lidarrconfig + +import ( + "context" + "fmt" + + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// Config is the typed projection of lidarr_config (no NULL fields exposed +// to callers — empty strings / zero ints carry the "unset" meaning). +type Config struct { + Enabled bool + BaseURL string + APIKey string + DefaultQualityProfileID int + DefaultRootFolderPath string +} + +// Service reads and writes the singleton. +type Service struct { + pool *pgxpool.Pool +} + +func New(pool *pgxpool.Pool) *Service { return &Service{pool: pool} } + +func (s *Service) Get(ctx context.Context) (Config, error) { + row, err := dbq.New(s.pool).GetLidarrConfig(ctx) + if err != nil { + return Config{}, fmt.Errorf("lidarrconfig: %w", err) + } + cfg := Config{Enabled: row.Enabled} + if row.BaseUrl != nil { + cfg.BaseURL = *row.BaseUrl + } + if row.ApiKey != nil { + cfg.APIKey = *row.ApiKey + } + if row.DefaultQualityProfileID != nil { + cfg.DefaultQualityProfileID = int(*row.DefaultQualityProfileID) + } + if row.DefaultRootFolderPath != nil { + cfg.DefaultRootFolderPath = *row.DefaultRootFolderPath + } + return cfg, nil +} + +// Save writes the entire row. Callers pass the full Config they want +// stored — this is not a partial update. +func (s *Service) Save(ctx context.Context, cfg Config) error { + var ( + baseURL *string = strPtr(cfg.BaseURL) + apiKey *string = strPtr(cfg.APIKey) + qpID *int32 = int32Ptr(cfg.DefaultQualityProfileID) + rootPath *string = strPtr(cfg.DefaultRootFolderPath) + ) + _, err := dbq.New(s.pool).UpdateLidarrConfig(ctx, dbq.UpdateLidarrConfigParams{ + Enabled: cfg.Enabled, + BaseUrl: baseURL, + ApiKey: apiKey, + DefaultQualityProfileID: qpID, + DefaultRootFolderPath: rootPath, + }) + if err != nil { + return fmt.Errorf("lidarrconfig: %w", err) + } + return nil +} + +func strPtr(s string) *string { + if s == "" { + return nil + } + return &s +} + +func int32Ptr(i int) *int32 { + if i == 0 { + return nil + } + v := int32(i) + return &v +} +``` + +Note: the exact field names on `dbq.UpdateLidarrConfigParams` depend on sqlc's generation. After `sqlc generate` ran in Task 1, you'll see them — adjust the literal field names here to match. Pointer-vs-value also depends on how sqlc treats nullable text columns. + +- [ ] **Step 3.2: Write the integration test** + +`internal/lidarrconfig/service_test.go`: + +```go +package lidarrconfig + +import ( + "context" + "io" + "log/slog" + "os" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db" +) + +func newTestPool(t *testing.T) *pgxpool.Pool { + t.Helper() + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); 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) + // Reset singleton to default state before each test by replacing + // the row contents via UPDATE (TRUNCATE would violate the CHECK). + if _, err := pool.Exec(context.Background(), + "UPDATE lidarr_config SET enabled=false, base_url=NULL, api_key=NULL, default_quality_profile_id=NULL, default_root_folder_path=NULL WHERE id=1", + ); err != nil { + t.Fatalf("reset: %v", err) + } + return pool +} + +func TestGet_DefaultRowReturnsZeroValueConfig(t *testing.T) { + pool := newTestPool(t) + cfg, err := New(pool).Get(context.Background()) + if err != nil { + t.Fatalf("Get: %v", err) + } + if cfg.Enabled || cfg.BaseURL != "" || cfg.APIKey != "" { + t.Errorf("expected zero-value Config, got %+v", cfg) + } +} + +func TestSaveThenGet_RoundTrip(t *testing.T) { + pool := newTestPool(t) + s := New(pool) + want := Config{ + Enabled: true, + BaseURL: "http://lidarr.lan:8686", + APIKey: "secret", + DefaultQualityProfileID: 4, + DefaultRootFolderPath: "/music", + } + if err := s.Save(context.Background(), want); err != nil { + t.Fatalf("Save: %v", err) + } + got, err := s.Get(context.Background()) + if err != nil { + t.Fatalf("Get: %v", err) + } + if got != want { + t.Errorf("round-trip mismatch:\n got = %+v\nwant = %+v", got, want) + } +} + +func TestSave_EmptyValuesPersistAsNULL(t *testing.T) { + pool := newTestPool(t) + s := New(pool) + if err := s.Save(context.Background(), Config{Enabled: false}); err != nil { + t.Fatalf("Save: %v", err) + } + got, err := s.Get(context.Background()) + if err != nil { + t.Fatalf("Get: %v", err) + } + if got.BaseURL != "" || got.APIKey != "" || got.DefaultRootFolderPath != "" { + t.Errorf("expected empty strings on round-trip; got %+v", got) + } +} +``` + +- [ ] **Step 3.3: Run the tests inside the docker network** + +```bash +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race ./internal/lidarrconfig/... +``` + +Expected: 3 tests pass. + +- [ ] **Step 3.4: Commit** + +```bash +git add internal/lidarrconfig/ +git commit -m "feat(lidarrconfig): typed singleton config wrapper" +``` + +--- + +### Task 4 — `lidarrrequests` Service (lifecycle, no reconciler) + +**Files:** +- Create: `internal/lidarrrequests/service.go` +- Create: `internal/lidarrrequests/service_test.go` + +- [ ] **Step 4.1: Write the Service** + +`internal/lidarrrequests/service.go`: + +```go +// Package lidarrrequests owns the lifecycle of user requests to add +// music via Lidarr. The synchronous Service handles Create/List/ +// Approve/Reject/Cancel; the async Reconciler (separate file) closes +// approved requests once their target track lands in the library. +package lidarrrequests + +import ( + "context" + "errors" + "fmt" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarr" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" +) + +// Public errors. Handlers map these to API error codes. +var ( + ErrInvalidKindFields = errors.New("lidarrrequests: missing required fields for kind") + ErrNotPending = errors.New("lidarrrequests: request is not pending") + ErrNotFound = errors.New("lidarrrequests: request not found") + ErrLidarrDisabled = errors.New("lidarrrequests: lidarr not configured") +) + +// CreateParams is the input for a new request from a user. +type CreateParams struct { + Kind string // "artist", "album", or "track" + LidarrArtistMBID string + LidarrAlbumMBID string // required for kind=album/track + LidarrTrackMBID string // required for kind=track + ArtistName string + AlbumTitle string // required for kind=album/track + TrackTitle string // required for kind=track +} + +// ApproveOverrides lets the admin override the snapshot defaults for one +// approval. Zero values mean "use config default." +type ApproveOverrides struct { + QualityProfileID int + RootFolderPath string +} + +type Service struct { + pool *pgxpool.Pool + lidarrCfg *lidarrconfig.Service + client *lidarr.Client + scanFn func() // injected; called after Approve to trigger a library scan +} + +func NewService(pool *pgxpool.Pool, cfg *lidarrconfig.Service, client *lidarr.Client, scanFn func()) *Service { + if scanFn == nil { + scanFn = func() {} + } + return &Service{pool: pool, lidarrCfg: cfg, client: client, scanFn: scanFn} +} + +// Create validates the kind→required-fields invariant and inserts a +// pending row. +func (s *Service) Create(ctx context.Context, userID pgtype.UUID, p CreateParams) (dbq.LidarrRequest, error) { + if err := validateKindFields(p); err != nil { + return dbq.LidarrRequest{}, err + } + q := dbq.New(s.pool) + row, err := q.CreateLidarrRequest(ctx, dbq.CreateLidarrRequestParams{ + UserID: userID, + Kind: dbq.LidarrRequestKind(p.Kind), + LidarrArtistMbid: p.LidarrArtistMBID, + LidarrAlbumMbid: strPtr(p.LidarrAlbumMBID), + LidarrTrackMbid: strPtr(p.LidarrTrackMBID), + ArtistName: p.ArtistName, + AlbumTitle: strPtr(p.AlbumTitle), + TrackTitle: strPtr(p.TrackTitle), + }) + if err != nil { + return dbq.LidarrRequest{}, fmt.Errorf("create: %w", err) + } + return row, nil +} + +func validateKindFields(p CreateParams) error { + if p.LidarrArtistMBID == "" || p.ArtistName == "" { + return fmt.Errorf("%w: artist_mbid and artist_name are always required", ErrInvalidKindFields) + } + switch p.Kind { + case "artist": + // fine + case "album": + if p.LidarrAlbumMBID == "" || p.AlbumTitle == "" { + return fmt.Errorf("%w: album kind requires album_mbid and album_title", ErrInvalidKindFields) + } + case "track": + if p.LidarrAlbumMBID == "" || p.AlbumTitle == "" { + return fmt.Errorf("%w: track kind requires album_mbid and album_title (track will be promoted)", ErrInvalidKindFields) + } + if p.LidarrTrackMBID == "" || p.TrackTitle == "" { + return fmt.Errorf("%w: track kind requires track_mbid and track_title", ErrInvalidKindFields) + } + default: + return fmt.Errorf("%w: unknown kind %q", ErrInvalidKindFields, p.Kind) + } + return nil +} + +func (s *Service) ListPending(ctx context.Context, limit int32) ([]dbq.LidarrRequest, error) { + return dbq.New(s.pool).ListLidarrRequestsByStatus(ctx, dbq.ListLidarrRequestsByStatusParams{ + Status: dbq.LidarrRequestStatusPending, Limit: limit, + }) +} + +func (s *Service) ListByStatus(ctx context.Context, status string, limit int32) ([]dbq.LidarrRequest, error) { + return dbq.New(s.pool).ListLidarrRequestsByStatus(ctx, dbq.ListLidarrRequestsByStatusParams{ + Status: dbq.LidarrRequestStatus(status), Limit: limit, + }) +} + +func (s *Service) ListForUser(ctx context.Context, userID pgtype.UUID, limit int32) ([]dbq.LidarrRequest, error) { + return dbq.New(s.pool).ListLidarrRequestsForUser(ctx, dbq.ListLidarrRequestsForUserParams{ + UserID: userID, Limit: limit, + }) +} + +// Approve transitions a pending request to approved, snapshotting the +// chosen quality profile + root folder, then calls Lidarr to actually +// add the artist/album, then triggers a library scan. If Lidarr returns +// an error, the request stays pending — the admin sees the error and +// can retry without losing the request. +func (s *Service) Approve(ctx context.Context, requestID pgtype.UUID, adminID pgtype.UUID, ov ApproveOverrides) (dbq.LidarrRequest, error) { + cfg, err := s.lidarrCfg.Get(ctx) + if err != nil { + return dbq.LidarrRequest{}, fmt.Errorf("approve: load config: %w", err) + } + if !cfg.Enabled || s.client == nil { + return dbq.LidarrRequest{}, ErrLidarrDisabled + } + row, err := dbq.New(s.pool).GetLidarrRequestByID(ctx, requestID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return dbq.LidarrRequest{}, ErrNotFound + } + return dbq.LidarrRequest{}, fmt.Errorf("approve: get: %w", err) + } + if row.Status != dbq.LidarrRequestStatusPending { + return dbq.LidarrRequest{}, ErrNotPending + } + + qp := cfg.DefaultQualityProfileID + if ov.QualityProfileID != 0 { + qp = ov.QualityProfileID + } + rf := cfg.DefaultRootFolderPath + if ov.RootFolderPath != "" { + rf = ov.RootFolderPath + } + + switch row.Kind { + case dbq.LidarrRequestKindArtist: + err = s.client.AddArtist(ctx, lidarr.AddArtistParams{ + ForeignArtistID: row.LidarrArtistMbid, QualityProfileID: qp, RootFolderPath: rf, MonitorAll: true, + }) + case dbq.LidarrRequestKindAlbum, dbq.LidarrRequestKindTrack: + // Track-kind requests promote to album-add; the spec is explicit. + albumMBID := "" + if row.LidarrAlbumMbid != nil { + albumMBID = *row.LidarrAlbumMbid + } + err = s.client.AddAlbum(ctx, lidarr.AddAlbumParams{ + ForeignAlbumID: albumMBID, ForeignArtistID: row.LidarrArtistMbid, + QualityProfileID: qp, RootFolderPath: rf, + }) + } + if err != nil { + return dbq.LidarrRequest{}, fmt.Errorf("approve: lidarr add: %w", err) + } + + approved, err := dbq.New(s.pool).ApproveLidarrRequest(ctx, dbq.ApproveLidarrRequestParams{ + ID: requestID, + QualityProfileID: int32Ptr(qp), + RootFolderPath: strPtr(rf), + DecidedBy: uuidPtr(adminID), + }) + if err != nil { + // Lidarr accepted but our DB update failed; admin should retry. + return dbq.LidarrRequest{}, fmt.Errorf("approve: persist: %w", err) + } + s.scanFn() + return approved, nil +} + +func (s *Service) Reject(ctx context.Context, requestID pgtype.UUID, adminID pgtype.UUID, notes string) (dbq.LidarrRequest, error) { + row, err := dbq.New(s.pool).RejectLidarrRequest(ctx, dbq.RejectLidarrRequestParams{ + ID: requestID, Notes: strPtr(notes), DecidedBy: uuidPtr(adminID), + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + // Either not found OR not pending — caller can't distinguish from + // the SQL alone, so check after. + cur, gerr := dbq.New(s.pool).GetLidarrRequestByID(ctx, requestID) + if gerr != nil { + return dbq.LidarrRequest{}, ErrNotFound + } + if cur.Status != dbq.LidarrRequestStatusPending { + return dbq.LidarrRequest{}, ErrNotPending + } + return dbq.LidarrRequest{}, ErrNotFound + } + return dbq.LidarrRequest{}, fmt.Errorf("reject: %w", err) + } + return row, nil +} + +func (s *Service) Cancel(ctx context.Context, requestID pgtype.UUID, userID pgtype.UUID) (dbq.LidarrRequest, error) { + row, err := dbq.New(s.pool).CancelLidarrRequest(ctx, dbq.CancelLidarrRequestParams{ + ID: requestID, UserID: userID, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return dbq.LidarrRequest{}, ErrNotPending + } + return dbq.LidarrRequest{}, fmt.Errorf("cancel: %w", err) + } + return row, nil +} + +func strPtr(s string) *string { + if s == "" { + return nil + } + return &s +} +func int32Ptr(i int) *int32 { + if i == 0 { + return nil + } + v := int32(i) + return &v +} +func uuidPtr(u pgtype.UUID) pgtype.UUID { return u } +``` + +(Field names like `dbq.LidarrRequestStatusPending`, `dbq.LidarrRequestKindArtist` come from sqlc — verify after `sqlc generate`. Pointer-vs-value for nullable columns also from sqlc.) + +- [ ] **Step 4.2: Write the integration test** + +`internal/lidarrrequests/service_test.go`: + +```go +package lidarrrequests + +import ( + "context" + "errors" + "io" + "log/slog" + "os" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" +) + +func newPool(t *testing.T) *pgxpool.Pool { + t.Helper() + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); 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) + dbtest.ResetDB(t, pool) + if _, err := pool.Exec(context.Background(), + "DELETE FROM lidarr_requests; UPDATE lidarr_config SET enabled=false, base_url=NULL, api_key=NULL WHERE id=1", + ); err != nil { + t.Fatalf("reset lidarr tables: %v", err) + } + return pool +} + +func seedUser(t *testing.T, pool *pgxpool.Pool) pgtype.UUID { + t.Helper() + u, err := dbq.New(pool).CreateUser(context.Background(), dbq.CreateUserParams{ + Username: dbtest.TestUserPrefix + "rqtester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, + }) + if err != nil { + t.Fatalf("seed user: %v", err) + } + return u.ID +} + +func TestCreate_HappyPath_Artist(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool) + svc := NewService(pool, lidarrconfig.New(pool), nil, nil) + r, err := svc.Create(context.Background(), user, CreateParams{ + Kind: "artist", + LidarrArtistMBID: "069b64b6-7884-4f6a-94cc-e4c1d6c87a01", + ArtistName: "Boards of Canada", + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + if r.Status != dbq.LidarrRequestStatusPending { + t.Errorf("status = %v", r.Status) + } +} + +func TestCreate_TrackKindRequiresAlbumMBID(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool) + svc := NewService(pool, lidarrconfig.New(pool), nil, nil) + _, err := svc.Create(context.Background(), user, CreateParams{ + Kind: "track", + LidarrArtistMBID: "a-mbid", ArtistName: "X", + LidarrTrackMBID: "t-mbid", TrackTitle: "Y", + // missing album fields + }) + if !errors.Is(err, ErrInvalidKindFields) { + t.Fatalf("err = %v, want ErrInvalidKindFields", err) + } +} + +func TestApprove_NotConfigured(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool) + svc := NewService(pool, lidarrconfig.New(pool), nil, nil) + r, _ := svc.Create(context.Background(), user, CreateParams{ + Kind: "artist", LidarrArtistMBID: "a-mbid", ArtistName: "X", + }) + _, err := svc.Approve(context.Background(), r.ID, user, ApproveOverrides{}) + if !errors.Is(err, ErrLidarrDisabled) { + t.Fatalf("err = %v, want ErrLidarrDisabled", err) + } +} + +func TestReject_TransitionsToRejected(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool) + svc := NewService(pool, lidarrconfig.New(pool), nil, nil) + r, _ := svc.Create(context.Background(), user, CreateParams{ + Kind: "artist", LidarrArtistMBID: "a-mbid", ArtistName: "X", + }) + rejected, err := svc.Reject(context.Background(), r.ID, user, "low quality") + if err != nil { + t.Fatalf("Reject: %v", err) + } + if rejected.Status != dbq.LidarrRequestStatusRejected { + t.Errorf("status = %v", rejected.Status) + } +} + +func TestReject_AlreadyRejectedReturnsErrNotPending(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool) + svc := NewService(pool, lidarrconfig.New(pool), nil, nil) + r, _ := svc.Create(context.Background(), user, CreateParams{ + Kind: "artist", LidarrArtistMBID: "a-mbid", ArtistName: "X", + }) + _, _ = svc.Reject(context.Background(), r.ID, user, "first") + _, err := svc.Reject(context.Background(), r.ID, user, "second") + if !errors.Is(err, ErrNotPending) { + t.Fatalf("err = %v, want ErrNotPending", err) + } +} + +func TestCancel_OwnPendingOnly(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool) + svc := NewService(pool, lidarrconfig.New(pool), nil, nil) + r, _ := svc.Create(context.Background(), user, CreateParams{ + Kind: "artist", LidarrArtistMBID: "a-mbid", ArtistName: "X", + }) + if _, err := svc.Cancel(context.Background(), r.ID, user); err != nil { + t.Fatalf("Cancel: %v", err) + } + // Second cancel hits "not pending" because we just rejected it. + if _, err := svc.Cancel(context.Background(), r.ID, user); !errors.Is(err, ErrNotPending) { + t.Errorf("err = %v, want ErrNotPending", err) + } +} +``` + +- [ ] **Step 4.3: Run the tests** + +```bash +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race ./internal/lidarrrequests/... +``` + +Expected: 6 tests pass. + +- [ ] **Step 4.4: Commit** + +```bash +git add internal/lidarrrequests/service.go internal/lidarrrequests/service_test.go +git commit -m "feat(lidarrrequests): request lifecycle service (create/list/approve/reject/cancel)" +``` + +--- + +This plan continues in the same shape for the remaining tasks. Subsequent tasks are sketched below at one-paragraph-per-task density to keep the plan navigable; expand each into the same step-level TDD detail (write test → run → implement → run → commit) when you reach it. Each task references the spec sections that drive it. + +--- + +### Task 5 — `lidarrrequests` Reconciler worker + +**Files:** `internal/lidarrrequests/reconciler.go`, `internal/lidarrrequests/reconciler_integration_test.go`, plus a sqlc query `MatchTrackForRequest` in `internal/db/queries/lidarr_requests.sql`. + +The reconciler mirrors `internal/similarity.Worker`: a `Run(ctx)` loop that calls `tickOnce(ctx)` every 5 minutes. `tickOnce`: +1. `ListApprovedLidarrRequestsForReconcile(limit=50)`. +2. For each row, look up the matching local row via MBID: + - `kind=artist` → `SELECT id FROM artists WHERE mbid = $1` + - `kind=album` → `SELECT id FROM albums WHERE mbid = $1` + - `kind=track` → `SELECT id FROM tracks WHERE album_id = (SELECT id FROM albums WHERE mbid = $1) LIMIT 1` (track-kind matched by parent album per spec §3 reconciler note) +3. If a match is found, call `CompleteLidarrRequest` with the matched IDs. + +Reconciler short-circuits to no-op when `lidarrconfig.Get(...).Enabled == false`. Errors logged at WARN, never propagated. + +**Tests** (per spec §8 — five integration scenarios): +- `TestReconciler_MatchesArtistByMBID` — seed artist, seed approved artist-kind request with same MBID, run `tickOnce`, expect status=completed and `matched_artist_id` set. +- `TestReconciler_MatchesAlbumByMBID` — same shape for album. +- `TestReconciler_MatchesTrackViaAlbumMBID` — track-kind request matches when ANY track of the parent album appears. +- `TestReconciler_NoMatchLeavesPending` — approved request with MBID not in library → row unchanged after `tickOnce`. +- `TestReconciler_AlreadyCompletedRowNotReprocessed` — pre-set status=completed, ensure `tickOnce` doesn't touch it. +- `TestReconciler_DisabledIsNoOp` — `lidarr_config.enabled=false` → `tickOnce` short-circuits even with approved rows present. + +Commit: `feat(lidarrrequests): add Reconciler worker matching approved requests to library`. + +--- + +### Task 6 — `RequireAdmin` middleware + +**Files:** `internal/auth/admin.go`, `internal/auth/admin_test.go`. + +Mirror `RequireUser`'s shape. After `RequireUser` puts the user in context, `RequireAdmin` reads the user from context, returns 403 with `{"error":"not_authorized"}` JSON envelope if `IsAdmin == false`. Test cases: admin passes through; non-admin returns 403; missing context (programmer error) returns 500. + +Commit: `feat(auth): add RequireAdmin middleware for /api/admin/* routes`. + +--- + +### Task 7 — `/api/lidarr/search` proxy handler + +**Files:** `internal/api/lidarr.go`, `internal/api/lidarr_test.go`. Modify: `internal/api/api.go` to inject the Lidarr client + lidarrconfig service into `handlers`. + +Handler reads `q` and `kind` from query params, validates `kind ∈ {artist, album, track}`, checks `lidarrconfig.Get().Enabled` — if false, returns `503 {"error":"lidarr_disabled"}`. Calls the matching `client.Lookup*`, then per-result enriches with: +- `in_library` — by joining against `artists.mbid` / `albums.mbid` / `tracks.mbid`. Add a small `IsMBIDInLibrary` sqlc query for each kind. +- `requested` — via `HasNonTerminalRequestForMBID` (already in queries from Task 1). + +Maps `lidarr.ErrUnreachable`/`ErrAuthFailed` to `503 lidarr_unreachable` / `503 lidarr_auth_failed`. Other errors → `500`. + +**Tests:** +- `TestHandleLidarrSearch_HappyPath` — stubs the Client to return one in-library + one requestable + one already-requested, asserts the JSON shape. +- `TestHandleLidarrSearch_DisabledReturns503` — `lidarr_config.enabled=false`. +- `TestHandleLidarrSearch_LidarrUnreachable` — stubbed Client returns `ErrUnreachable`. +- `TestHandleLidarrSearch_BadKind400`. +- `TestHandleLidarrSearch_RequiresAuth` — anonymous request rejected. + +Commit: `feat(api): add /api/lidarr/search proxy with library/request enrichment`. + +--- + +### Task 8 — `/api/requests` user-facing CRUD handlers + +**Files:** `internal/api/requests.go`, `internal/api/requests_test.go`. Modify: `internal/api/api.go` to register routes inside the `RequireUser` group. + +Five handlers: `POST /api/requests` (Create), `GET /api/requests` (ListForUser), `GET /api/requests/:id`, `DELETE /api/requests/:id` (Cancel). + +Each handler delegates to `lidarrrequests.Service`. Map `ErrInvalidKindFields → 400 mbid_required`, `ErrNotPending → 409 request_not_pending`, `ErrNotFound → 404 request_not_found`. `GET /:id` returns 404 if the row isn't the caller's own AND caller isn't admin. + +**Tests** (extend `testHandlers` to inject `lidarrrequests.Service`): +- Create with valid artist/album/track payloads → 201. +- Create with each invalid kind→fields combination → 400. +- List returns only caller's rows; cross-user scoped out. +- Get-own returns row; get-other-user 404; get-other-user-as-admin 200. +- Cancel pending → 200 with status=rejected; cancel non-pending → 409. + +Commit: `feat(api): add /api/requests user-facing CRUD`. + +--- + +### Task 9 — `/api/admin/lidarr/*` config + profiles + folders + test + +**Files:** `internal/api/admin_lidarr.go`, `internal/api/admin_lidarr_test.go`. Modify: `internal/api/api.go` to mount a `RequireAdmin` group under `/api/admin`. + +Handlers: `GET /api/admin/lidarr/config` (mask api_key), `PUT /api/admin/lidarr/config`, `POST /api/admin/lidarr/test`, `GET /api/admin/lidarr/quality-profiles`, `GET /api/admin/lidarr/root-folders`. + +PUT logic: if request `api_key` is empty string → preserve saved value; if non-empty → update. `enabled=true` requires `base_url` and `api_key` to be non-empty (validate at handler). + +Test endpoint: per-field fallback to saved values when absent or empty; always returns 200 with `{ok, version?, error?}`. + +**Tests:** +- GET config masks api_key when set. +- PUT empty api_key preserves saved value. +- PUT enabled=true with empty base_url → 400. +- POST test happy path returns `{ok:true, version}`. +- POST test with stubbed-unreachable client returns `{ok:false, error}`. +- Quality-profiles + root-folders proxy through to client. +- All endpoints return 403 for non-admin tokens. + +Commit: `feat(api): add /api/admin/lidarr/* config + profiles + folders + test`. + +--- + +### Task 10 — `/api/admin/requests/*` approval queue handlers + +**Files:** `internal/api/admin_requests.go`, `internal/api/admin_requests_test.go`. Modify: `internal/api/api.go` to register inside the `RequireAdmin` group. + +Three handlers: `GET /api/admin/requests?status=&limit=` (default `status=pending`), `POST /api/admin/requests/:id/approve` (body: optional override), `POST /api/admin/requests/:id/reject` (body: optional notes). + +Approve handler delegates to `Service.Approve`; surfaces `ErrLidarrDisabled` / `lidarr.ErrUnreachable` / `ErrNotPending` / `ErrNotFound` per error code table. + +**Tests:** +- List with status=pending returns pending rows only. +- Approve happy path: stubbed Client receives correct AddArtist/AddAlbum payload, row transitions to approved with snapshot fields, scan trigger called. +- Approve with override snapshots override values, not config defaults. +- Approve when Lidarr returns ErrUnreachable → 503; row stays pending. +- Reject with notes records notes; reject without notes works with NULL notes. +- All endpoints return 403 for non-admin tokens. + +Commit: `feat(api): add /api/admin/requests approval queue`. + +--- + +### Task 11 — Wire the Reconciler in `cmd/minstrel/main.go` + +**Files:** Modify `cmd/minstrel/main.go`. + +Mirror the existing scrobble/similarity worker spin-up. Construct `lidarrconfig.Service`, the `lidarr.Client` (BaseURL+APIKey loaded from the singleton on demand), `lidarrrequests.Reconciler`, and start its `Run(ctx)` in a goroutine alongside the others. + +Subtle: the Lidarr client's `BaseURL` and `APIKey` change at runtime when admin updates config. Two ways to handle — (a) construct a new Client per request inside the Service from the latest config, or (b) wrap a `*atomic.Pointer[lidarr.Client]` that the config-save handler swaps. Pick (a) — simpler, no atomic dance, the cost of constructing an `http.Client` per request is negligible. Refactor `Service` to hold a `func() *lidarr.Client` factory instead of a `*Client` so it always reads fresh config. + +(Update Task 4's `Service` shape to use the factory accordingly. This is a foreseeable refactor — better to absorb it now than fight stale clients in production.) + +Commit: `feat(cmd): start Lidarr reconciler worker alongside HTTP server`. + +--- + +### Task 12 — Frontend: FabledSword design tokens + fonts + +**Files:** Create `web/src/lib/styles/fabledsword-tokens.css`. Modify `web/src/app.css`. Modify `web/src/app.html` to load Google Fonts. Modify `web/tailwind.config.js` to alias semantic Tailwind utilities (e.g. `bg-surface`, `text-text-primary`, `border-border`) to FS tokens. + +Token file content: every variable from `project_design_system.md` `:root` block — surfaces, text, action, semantic, accent, font families, radii. Plus the per-app data-attribute hook that sets `--fs-accent` to forest-teal `#4A6B5C` for Minstrel. + +In Tailwind config, replace existing palette aliases: +- `surface`, `surface-hover` → Iron, Slate +- `background` → Obsidian +- `text-primary`, `text-secondary`, `text-muted` → Parchment, Vellum, Ash +- `border` → Pewter +- Add new utility classes for action (`bg-action-primary` → Moss, `bg-action-secondary` → Bronze, `bg-action-destructive` → Oxblood) and `accent` → forest teal. + +This is the slice that converts the rest of the app to the design system implicitly — by aliasing existing utility names. Verify by visiting the dev server and confirming the existing pages now read in the new palette without any per-page changes (a sign the alias mapping is correct). + +Commit: `feat(web): introduce FabledSword design system tokens + Tailwind aliases`. + +--- + +### Task 13 — Frontend: Lidarr + requests + admin API client modules + +**Files:** Create `web/src/lib/api/lidarr.ts`, `web/src/lib/api/requests.ts`, `web/src/lib/api/admin.ts`. + +Mirror existing client modules (e.g. `web/src/lib/api/likes.ts`). Each file exports typed async functions backed by the existing `api.get/post/put/delete` helper. + +Types match the API surface in spec §5. Vitest tests live alongside (e.g. `lidarr.test.ts`) using the existing fetch mocking setup — verify URL construction, query params, error mapping. + +Commit: `feat(web): add API client modules for Lidarr, requests, admin`. + +--- + +### Task 14 — Frontend: `` component + +**Files:** Create `web/src/lib/components/DiscoverResultCard.svelte`, `DiscoverResultCard.test.ts`. + +Component props: `{ kind: 'artist'|'album'|'track', title: string, subtitle?: string, imageUrl?: string, state: 'requestable'|'kept'|'requested', onRequest?: () => void }`. + +Layout discipline (per spec §6 + brainstorm): +- Outer `.card` is flex column with reserved `.text` block (`min-height` covers title + meta + badge row); `.actions` block uses `margin-top: auto`. +- Badge slot is always rendered as a 22px-min-height div; "Kept" pill (accent at 15% bg + accent text) appears only when `state==='kept'`. +- Three states render different actions: + - `requestable`: `bg-action-primary` button with plus icon, label "Request" + - `kept`: disabled ghost button "In library" + "Kept" pill in badge slot + - `requested`: disabled ghost button "Requested" +- Cover art: render `` when `imageUrl`; otherwise render Lucide fallback glyph (`Disc3` for artist, `Album` for album, `Music2` for track) inside the Slate-bg art square. + +**Tests:** +- Renders all three states with correct button text. +- Calls `onRequest` only in `requestable` state. +- Computes badge slot height with `min-height: 22px` even when no badge content (assert via `getComputedStyle`). +- Button is anchored to bottom of card body (assert `margin-top` === `auto` on `.actions`). + +Commit: `feat(web): add DiscoverResultCard with reserved badge slot + anchored button`. + +--- + +### Task 15 — Frontend: `` component + +**Files:** Create `web/src/lib/components/StatusPill.svelte`, `StatusPill.test.ts`. + +Single prop: `status: 'pending'|'approved'|'completed'|'rejected'|'failed'`. Renders a pill with semantic color (Warning / Info / Moss / Error / Error) per spec §6 and the design-system memory. Voice-rule labels: "Awaiting review" / "Approved · downloading" / "Kept" / "Set aside" / "Couldn't add." + +Tests: each status renders with the correct text and the correct semantic CSS class (use `bg-warning-tint`, etc., aliases). + +Commit: `feat(web): add StatusPill semantic-color status indicator`. + +--- + +### Task 16 — Frontend: `/discover` route + +**Files:** Create `web/src/routes/discover/+page.svelte`, `discover.test.ts`. Modify `web/src/lib/components/Shell.svelte` to add `/discover` to the main nav. + +Page elements: +- H2 "Add music to the library" (Fraunces 24/500), Vellum subtitle +- Search input (Obsidian inset, focus ring forest-teal) +- Tabs (Artists / Albums / Tracks) — active tab gets 2px forest-teal bottom border +- Card grid using `` +- Track-kind confirm modal: opens on Request click with a track-state result; "Requesting *Track X* will add the album *Album Y*. Continue?" — Confirm = Moss, Cancel = Bronze. Modal dismissed = no-op. + +Debounce search input by 250ms before querying. + +**Tests:** +- Debounced query fires correct API call with kind selector. +- Tab switch refetches with new `kind`. +- Track-kind result triggers modal; confirm triggers API call; cancel does not. +- Requestable card with `onRequest` flips to `requested` state on success. +- Empty results state shows "Nothing to add for that search yet." (voice-rule copy). + +Commit: `feat(web): add /discover route with search + request flow`. + +--- + +### Task 17 — Frontend: `/requests` user request history + +**Files:** Create `web/src/routes/requests/+page.svelte`, `requests.test.ts`. Modify `Shell.svelte` to add `/requests` link in the main nav (visible to all authed users). + +Page renders the caller's requests as rows (mirrors the mockup at `.superpowers/brainstorm/.../user-requests.html`). Each row: +- 56px album-art square (Slate fallback) +- Kind pill + StatusPill +- Title + meta line +- Per-status actions: Cancel button on pending; "Listen" link (forest-teal text) on completed (navigates to `/tracks/` if set, else fallthrough to album/artist) + +**Tests:** +- Renders one row per request from the API. +- Pending row exposes Cancel; Cancel calls API and removes row. +- Completed row renders "Listen" link with correct href. +- Rejected row renders admin notes if present, hides "Cancel" / "Listen." +- Empty list shows "Nothing requested yet." (voice-rule copy). + +Commit: `feat(web): add /requests user-facing request history`. + +--- + +### Task 18 — Frontend: `/admin/*` layout + role gate + +**Files:** Create `web/src/routes/admin/+layout.svelte`, `web/src/routes/admin/+layout.ts`, `web/src/routes/admin/+page.svelte` (Overview landing). + +`+layout.ts` exports a `load` function that checks `currentUser.is_admin`; if false, throws a SvelteKit `redirect(302, '/')`. Redirect happens before layout/child renders — exactly the hard route gate the operator specified. + +`+layout.svelte` renders the admin shell: +- Page header: "Admin" wordmark in Fraunces, the FabledSword small mark in Oxblood at top-left +- 220px sidebar `` component (separate file `web/src/lib/components/AdminSidebar.svelte`) with nav items: Overview / Integrations / **Requests** / Quarantine (placeholder, dimmed) / Users (placeholder, dimmed) / Library (placeholder, dimmed) +- Active nav item: 12% accent-tinted bg + 2px forest-teal left strip +- Main content area: `` for children + +`+page.svelte` (Overview): plain landing with two callout cards — "Pending requests: N" and "Lidarr: connected/unset" — each linking to its sub-page. Functional, not decorative. + +**Tests** (browser-mode, since SvelteKit `load` requires it): +- Non-admin user redirected to `/` before layout renders. +- Admin user lands on `/admin` and sees sidebar with Overview active. + +Commit: `feat(web): add /admin layout with role-gated load + sidebar`. + +--- + +### Task 19 — Frontend: `/admin/integrations` Lidarr panel + +**Files:** Create `web/src/routes/admin/integrations/+page.svelte`, `integrations.test.ts`. + +Page elements (matches mockup `admin-integrations.html`): +- Page header with status pill ("Lidarr · connected" Moss-tinted; "unset" Pewter ghost when not configured) +- Form section "Lidarr" with rows: + - Base URL — text input (Obsidian inset, JetBrains Mono for the URL value) + - API key — password input (masked) + - Default quality profile — `` populated from `GET /api/admin/lidarr/root-folders` +- Action row: Save changes (Moss + check icon), Test connection (Pewter ghost + refresh icon), Disconnect (Oxblood + trash icon, right-aligned) +- Disconnect requires a typed-confirm modal ("Type DISCONNECT to remove the Lidarr connection") because it sets `enabled=false` and clears `api_key`. + +Disabled section "MusicBrainz overrides" with `unset` foreshadows future integrations. Visually present, not implemented. + +**Tests:** +- Save changes calls PUT with form values. +- Empty api_key field on Save preserves saved value (sends empty string per spec). +- Test connection populates Lidarr's reported version on success. +- Disconnect requires modal confirmation; cancelling modal does not clear config. +- Quality-profile / root-folder dropdowns populated from API. + +Commit: `feat(web): add /admin/integrations Lidarr connection panel`. + +--- + +### Task 20 — Frontend: `/admin/requests` approval queue + override modal + +**Files:** Create `web/src/routes/admin/requests/+page.svelte`, `requests.test.ts`. Reuse ``. + +Page elements (matches mockup `admin-requests.html`): +- Tabs: Pending (default) / Approved / Completed / Rejected — each shows count from API as accent-tinted pill +- Request rows with action cluster: Override (Pewter ghost), Approve (Moss + check icon), Reject (Bronze + ✕ icon) +- Track-kind row's meta line spells out "Approving will add the album *X*" +- Override modal: collapsed-by-default form with Quality profile dropdown (populated via the admin endpoint) + Root folder dropdown; "Use defaults" leaves both empty (server uses snapshot defaults) + +**Tests:** +- Tab switch refetches with `?status=`. +- Approve fires POST with optional override values. +- Reject opens a notes input (textarea) above a Confirm button; Confirm sends notes. +- Approve with override modal returns chosen values to handler. +- Toast on Lidarr-unreachable error. + +Commit: `feat(web): add /admin/requests approval queue with override modal`. + +--- + +### Task 21 — Final verification + branch finish + +- [ ] **Step 21.1: Full Go test sweep** + +```bash +go test -short -race ./... +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race -p 1 ./... +``` + +Expected: short suite + integration suite both green. + +- [ ] **Step 21.2: Lint clean** + +```bash +golangci-lint run ./... +``` + +- [ ] **Step 21.3: Coverage check on new packages** + +```bash +go test -race -coverprofile=/tmp/cov.out ./internal/lidarr/... ./internal/lidarrconfig/... ./internal/lidarrrequests/... +go tool cover -func=/tmp/cov.out | tail -1 +``` + +Expected: combined ≥ 80% per spec §8. + +- [ ] **Step 21.4: Frontend full check** + +```bash +cd web && npm run check && npm test && npm run build +``` + +Expected: 0 errors, all vitest tests pass, build succeeds. + +- [ ] **Step 21.5: Manual smoke** + +- Set Lidarr config in `/admin/integrations` (use real Lidarr or stub). +- Search at `/discover`, request an artist. +- Approve from `/admin/requests`. +- Verify request shows up at `/requests` as Approved → wait for next library scan → status flips to Kept. +- Cancel a pending request from `/requests`. +- Verify non-admin is redirected when navigating to `/admin/*`. + +- [ ] **Step 21.6: Use `superpowers:finishing-a-development-branch`** + +Verify tests are still green, then run the skill to present finish options (merge / PR / keep / discard). Default for this slice is "create a PR to main" matching the established cadence. + +--- + +## Self-review checklist (run before declaring the plan ready) + +**Spec coverage** — every spec section maps to a task: +- §3 Architecture: Tasks 2 (client), 3 (config), 4 (service), 5 (reconciler), 6 (middleware), 11 (wiring) +- §4 Schema: Task 1 +- §5 API surface: Tasks 7 (search), 8 (requests CRUD), 9 (admin lidarr), 10 (admin requests) +- §6 UI surfaces: Tasks 12 (tokens), 13 (api), 14 (DiscoverResultCard), 15 (StatusPill), 16 (/discover), 17 (/requests), 18 (/admin layout), 19 (/admin/integrations), 20 (/admin/requests) +- §7 Error handling: distributed across Tasks 7-10 (each handler maps Service errors to API codes) +- §8 Testing: every Task includes tests; Task 21 verifies coverage targets +- §9 Decisions ledger: not directly implemented but referenced in commit messages +- §10 Out of scope: explicitly excluded — no quarantine, no suggested-additions, no webhook +- §11 Open questions: cover-art proxy + debounce/cache deferred to plan time → debounce at 250ms in Task 16; cover-art direct fetch (no proxy) for v1 + +**Placeholder scan:** the per-task detail level drops after Task 4 (each becomes one paragraph) — this is intentional for plan navigability, not a placeholder. When a subagent picks up Task 5+ they expand the paragraph into the same step-level TDD detail using Tasks 1-4 as templates, and reference the spec for any ambiguity. No "TBD" or "TODO" remains. + +**Type consistency:** +- Method names match across plan: `Service.Create/ListPending/ListByStatus/ListForUser/Approve/Reject/Cancel`, `Reconciler.Run/tickOnce`, `Client.LookupArtist/LookupAlbum/LookupTrack/AddArtist/AddAlbum/ListQualityProfiles/ListRootFolders/Ping` +- API paths match spec §5 +- Component names: ``, ``, `` — used consistently +- DB field names: `lidarr_artist_mbid`, `lidarr_album_mbid`, `lidarr_track_mbid`, `quality_profile_id`, `root_folder_path`, `matched_track_id`, etc. — consistent + +Plan is complete. diff --git a/docs/superpowers/plans/2026-04-30-m5b-quarantine.md b/docs/superpowers/plans/2026-04-30-m5b-quarantine.md new file mode 100644 index 00000000..358cf510 --- /dev/null +++ b/docs/superpowers/plans/2026-04-30-m5b-quarantine.md @@ -0,0 +1,2916 @@ +# M5b — Quarantine workflow + admin resolution UI — 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 per-user track-level quarantine into Minstrel — flag affordance via a kebab `` on every track row + the player, soft-hide enforcement on user-context `/api/*` reads, dedicated `/library/hidden` for the user, aggregated admin queue at `/admin/quarantine` with Resolve / Delete file / Delete via Lidarr actions, audit log for admin actions. + +**Architecture:** New `internal/lidarrquarantine` package (Service, no background worker) backed by two tables (`lidarr_quarantine` per-user complaints + `lidarr_quarantine_actions` audit log). Lidarr HTTP client gains `LookupArtistByMBID`, `LookupAlbumByMBID`, `DeleteAlbum` (always called with `deleteFiles=true` and `addImportListExclusion=true`). `internal/library` gains `DeleteTrackFile`. Existing read queries that return tracks in user-context get `*ForUser` variants that join against `lidarr_quarantine`; Subsonic queries are untouched. SPA gets a `` overflow component (mounted in `TrackRow` and `PlayerBar`) opening a ``, plus `/library/hidden` and `/admin/quarantine` routes. + +**Tech Stack:** Go 1.23 · chi router · pgx/v5 + sqlc · Postgres + golang-migrate · SvelteKit 2 / Svelte 5 (runes) · TanStack Query · Vitest · golangci-lint · FabledSword design tokens (existing M5a infrastructure). + +**Spec:** [`docs/superpowers/specs/2026-04-30-m5b-quarantine-design.md`](../specs/2026-04-30-m5b-quarantine-design.md). Read it before starting — every decision is explained there. + +**Memory dependencies:** `project_design_system.md` (FabledSword token palette + voice rules), `project_subsonic_legacy.md` (`/rest/*` does not honor quarantine), `project_no_github.md` (Forgejo MCP for PR ops, not gh CLI), `project_git_workflow.md` (commit on `dev`; PR to `main` separately). + +--- + +## File map + +### Backend — create + +- `internal/db/migrations/0011_lidarr_quarantine.up.sql` · `0011_lidarr_quarantine.down.sql` — schema +- `internal/db/queries/lidarr_quarantine.sql` — sqlc queries for both tables +- `internal/lidarrquarantine/service.go` — `Service` (Flag/Unflag/ListMine/ListAdminQueue/Resolve/DeleteFile/DeleteViaLidarr) +- `internal/lidarrquarantine/service_test.go` — integration tests +- `internal/lidarr/lookup_mbid.go` — `LookupArtistByMBID`, `LookupAlbumByMBID` (split from `client.go` to keep that file from growing) +- `internal/lidarr/delete.go` — `DeleteAlbum` HTTP method + `DELETE` helper +- `internal/lidarr/delete_test.go` — tests for the new methods +- `internal/lidarr/testdata/album_lookup_by_mbid.json`, `artist_lookup_by_mbid.json` — captured fixtures +- `internal/library/delete.go` — `DeleteTrackFile` +- `internal/library/delete_test.go` — tests +- `internal/api/quarantine.go` — `/api/quarantine/*` user-facing handlers +- `internal/api/quarantine_test.go` +- `internal/api/admin_quarantine.go` — `/api/admin/quarantine/*` admin handlers +- `internal/api/admin_quarantine_test.go` + +### Backend — modify + +- `internal/db/queries/tracks.sql` — add `ListTracksByAlbumForUser`, `SearchTracksForUser`, `CountTracksMatchingForUser` (filtered variants) +- `internal/db/queries/recommendation.sql` — extend `LoadRadioCandidates` and `LoadRadioCandidatesV2` to also exclude quarantined tracks +- `internal/api/api.go` — register routes, mount `/api/admin/quarantine` group, route the modified user-context endpoints to the `*ForUser` queries +- `internal/api/auth_test.go` — extend `testHandlers` to inject `lidarrquarantine.Service` +- `internal/api/albums.go` (or wherever album-detail composes its track list) — switch to `ListTracksByAlbumForUser` when user context is present +- `internal/api/search.go` — switch to `SearchTracksForUser` +- `internal/api/radio.go` (or wherever radio handlers live) — pass through the existing user_id parameter to the now-quarantine-aware query +- `cmd/minstrel/main.go` — construct `lidarrquarantine.Service` and inject +- `internal/db/dbq/*` — regenerated by `sqlc generate` + +### Frontend — create + +- `web/src/lib/api/quarantine.ts` — user-facing client (Flag/Unflag/ListMine) +- `web/src/lib/api/quarantine.test.ts` +- `web/src/lib/components/TrackMenu.svelte` — kebab overflow menu +- `web/src/lib/components/TrackMenu.test.ts` +- `web/src/lib/components/FlagPopover.svelte` — reason + notes form +- `web/src/lib/components/FlagPopover.test.ts` +- `web/src/lib/components/QuarantineRow.svelte` — shared row used by both `/library/hidden` and `/admin/quarantine` +- `web/src/lib/components/QuarantineRow.test.ts` +- `web/src/routes/library/hidden/+page.svelte` +- `web/src/routes/library/hidden/hidden.test.ts` +- `web/src/routes/admin/quarantine/+page.svelte` +- `web/src/routes/admin/quarantine/quarantine.test.ts` + +### Frontend — modify + +- `web/src/lib/api/admin.ts` — add `listAdminQuarantine`, `resolveQuarantine`, `deleteQuarantineFile`, `deleteQuarantineViaLidarr`, `listQuarantineActions` plus query factories +- `web/src/lib/api/queries.ts` — add `qk.myQuarantine`, `qk.adminQuarantine`, `qk.adminQuarantineActions` +- `web/src/lib/api/types.ts` — add `LidarrQuarantineReason`, `LidarrQuarantineRow`, `AdminQuarantineRow`, `LidarrQuarantineActionRow`, `LidarrQuarantineAction` enums +- `web/src/lib/components/Shell.svelte` — add `Hidden` to the main nav after `Liked` +- `web/src/lib/components/Shell.test.ts` — assert the new nav order +- `web/src/lib/components/AdminSidebar.svelte` — promote `Quarantine` from `placeholder: true` to a real link +- `web/src/lib/components/AdminSidebar.test.ts` — update tests; quarantine is now a link +- `web/src/lib/components/TrackRow.svelte` — mount `` next to `` +- `web/src/lib/components/TrackRow.test.ts` — extend to cover the menu +- `web/src/lib/components/PlayerBar.svelte` — mount `` in the right cluster +- `web/src/lib/components/PlayerBar.test.ts` — extend to cover the menu + +--- + +## Task list + +### Task 1 — Migration 0011 + sqlc queries + +**Files:** +- Create: `internal/db/migrations/0011_lidarr_quarantine.up.sql` +- Create: `internal/db/migrations/0011_lidarr_quarantine.down.sql` +- Create: `internal/db/queries/lidarr_quarantine.sql` +- Modify: `internal/db/dbq/*` (regenerated by `sqlc generate`) + +- [ ] **Step 1.1: Write the up migration** + +`internal/db/migrations/0011_lidarr_quarantine.up.sql`: + +```sql +-- M5b: per-user track quarantines + admin action audit log. +-- +-- lidarr_quarantine — one row per (user, track) complaint. PK matches +-- the general_likes pattern. Re-flagging the same track upserts. Deleted +-- on user resolution (un-hide), admin Resolve, or any of the deletes. +-- +-- lidarr_quarantine_actions — audit log of admin destructive actions. +-- Snapshot text columns let the log stay readable after the underlying +-- track/album rows are gone. + +CREATE TYPE lidarr_quarantine_reason AS ENUM ( + 'bad_rip', 'wrong_file', 'wrong_tags', 'duplicate', 'other' +); + +CREATE TABLE lidarr_quarantine ( + user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, + track_id uuid NOT NULL REFERENCES tracks(id) ON DELETE CASCADE, + reason lidarr_quarantine_reason NOT NULL, + notes text, + created_at timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY (user_id, track_id) +); + +CREATE INDEX lidarr_quarantine_track_idx ON lidarr_quarantine (track_id); +CREATE INDEX lidarr_quarantine_user_idx ON lidarr_quarantine (user_id, created_at DESC); + +CREATE TYPE lidarr_quarantine_action AS ENUM ( + 'resolved', 'deleted_file', 'deleted_via_lidarr' +); + +CREATE TABLE lidarr_quarantine_actions ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + track_id uuid NOT NULL, + track_title text NOT NULL, + artist_name text NOT NULL, + album_title text, + action lidarr_quarantine_action NOT NULL, + admin_id uuid REFERENCES users(id) ON DELETE SET NULL, + lidarr_album_mbid text, + affected_users int NOT NULL, + created_at timestamptz NOT NULL DEFAULT now() +); + +CREATE INDEX lidarr_quarantine_actions_track_idx ON lidarr_quarantine_actions (track_id); +CREATE INDEX lidarr_quarantine_actions_created_idx ON lidarr_quarantine_actions (created_at DESC); +``` + +- [ ] **Step 1.2: Write the down migration** + +`internal/db/migrations/0011_lidarr_quarantine.down.sql`: + +```sql +DROP INDEX IF EXISTS lidarr_quarantine_actions_created_idx; +DROP INDEX IF EXISTS lidarr_quarantine_actions_track_idx; +DROP TABLE IF EXISTS lidarr_quarantine_actions; +DROP TYPE IF EXISTS lidarr_quarantine_action; +DROP INDEX IF EXISTS lidarr_quarantine_user_idx; +DROP INDEX IF EXISTS lidarr_quarantine_track_idx; +DROP TABLE IF EXISTS lidarr_quarantine; +DROP TYPE IF EXISTS lidarr_quarantine_reason; +``` + +- [ ] **Step 1.3: Apply migration locally to confirm it runs** + +```bash +docker compose up -d postgres +docker compose exec -T postgres psql -U minstrel -d minstrel -c "DROP TABLE IF EXISTS lidarr_quarantine_actions; DROP TYPE IF EXISTS lidarr_quarantine_action; DROP TABLE IF EXISTS lidarr_quarantine; DROP TYPE IF EXISTS lidarr_quarantine_reason;" +go run ./cmd/minstrel up 2>/dev/null || true # apply via server start instead +docker compose exec -T postgres psql -U minstrel -d minstrel -c "\d lidarr_quarantine" +docker compose exec -T postgres psql -U minstrel -d minstrel -c "\d lidarr_quarantine_actions" +``` + +Expected: both `\d` commands print the table with columns and indexes. + +If the project doesn't have a standalone migrate command, the migration applies on server start via `db.Migrate(...)` — restart the minstrel container instead. + +- [ ] **Step 1.4: Write the queries** + +`internal/db/queries/lidarr_quarantine.sql`: + +```sql +-- name: UpsertQuarantine :one +-- Insert a new quarantine row, or update reason/notes if the user has +-- already flagged this track. +INSERT INTO lidarr_quarantine (user_id, track_id, reason, notes) +VALUES ($1, $2, $3, $4) +ON CONFLICT (user_id, track_id) DO UPDATE SET + reason = EXCLUDED.reason, + notes = EXCLUDED.notes, + created_at = now() +RETURNING user_id, track_id, reason, notes, created_at; + +-- name: DeleteQuarantine :one +-- Removes the caller's row. Returns the deleted row so the handler can +-- distinguish "no row existed" (zero rows -> ErrNoRows) from success. +DELETE FROM lidarr_quarantine + WHERE user_id = $1 AND track_id = $2 + RETURNING user_id, track_id, reason, notes, created_at; + +-- name: ListQuarantineForUser :many +-- Caller's own quarantines joined with track + album + artist for full +-- detail. Drives /library/hidden. +SELECT + sqlc.embed(q), + sqlc.embed(t), + sqlc.embed(al), + sqlc.embed(ar) +FROM lidarr_quarantine q +JOIN tracks t ON t.id = q.track_id +JOIN albums al ON al.id = t.album_id +JOIN artists ar ON ar.id = t.artist_id +WHERE q.user_id = $1 +ORDER BY q.created_at DESC; + +-- name: ListAdminQuarantineQueue :many +-- Aggregated admin queue. One row per track. The handler post-processes +-- the rows it gets from this query plus a per-track ListQuarantineReports +-- call to materialize reason_counts and the per-user reports list. +SELECT + t.id AS track_id, + t.title AS track_title, + ar.name AS artist_name, + al.title AS album_title, + al.id AS album_id, + al.mbid AS lidarr_album_mbid, + count(q.user_id)::int AS report_count, + max(q.created_at) AS latest_at +FROM lidarr_quarantine q +JOIN tracks t ON t.id = q.track_id +JOIN albums al ON al.id = t.album_id +JOIN artists ar ON ar.id = t.artist_id +GROUP BY t.id, ar.name, al.title, al.id, al.mbid +ORDER BY max(q.created_at) DESC; + +-- name: ListQuarantineReportsForTrack :many +-- Per-user reports for a single track. Returned by ListAdminQuarantineQueue +-- post-processing and exposed expandable in the SPA admin queue rows. +SELECT + q.user_id, + u.username, + q.reason, + q.notes, + q.created_at +FROM lidarr_quarantine q +JOIN users u ON u.id = q.user_id +WHERE q.track_id = $1 +ORDER BY q.created_at DESC; + +-- name: DeleteQuarantineForTrack :exec +-- Clears all per-user rows for a given track. Used by Resolve and the +-- two delete actions. Caller writes the audit row separately before +-- this fires (so we can capture the affected_users count). +DELETE FROM lidarr_quarantine WHERE track_id = $1; + +-- name: CountQuarantineForTrack :one +-- Reads affected_users for the audit row before the delete fires. +SELECT count(*)::int FROM lidarr_quarantine WHERE track_id = $1; + +-- name: WriteQuarantineAction :one +-- Audit row for an admin destructive action. +INSERT INTO lidarr_quarantine_actions ( + track_id, track_title, artist_name, album_title, + action, admin_id, lidarr_album_mbid, affected_users +) VALUES ($1, $2, $3, $4, $5, $6, $7, $8) +RETURNING *; + +-- name: ListQuarantineActions :many +SELECT * FROM lidarr_quarantine_actions +ORDER BY created_at DESC +LIMIT $1; +``` + +- [ ] **Step 1.5: Run sqlc generate** + +```bash +cd internal/db && sqlc generate && cd - +go build ./... +``` + +Expected: clean build. New types `LidarrQuarantine`, `LidarrQuarantineAction`, `ListAdminQuarantineQueueRow`, `ListQuarantineForUserRow`, `ListQuarantineReportsForTrackRow` etc. surface in `internal/db/dbq/`. + +- [ ] **Step 1.6: Commit** + +```bash +git add internal/db/migrations/0011_lidarr_quarantine.up.sql \ + internal/db/migrations/0011_lidarr_quarantine.down.sql \ + internal/db/queries/lidarr_quarantine.sql \ + internal/db/dbq/ +git commit -m "feat(db): add lidarr_quarantine + actions schema (migration 0011)" +``` + +--- + +### Task 2 — Lidarr HTTP client extensions + +**Files:** +- Create: `internal/lidarr/lookup_mbid.go` +- Create: `internal/lidarr/delete.go` +- Create: `internal/lidarr/delete_test.go` +- Create: `internal/lidarr/testdata/album_lookup_by_mbid.json` +- Create: `internal/lidarr/testdata/artist_lookup_by_mbid.json` +- Modify: `internal/lidarr/types.go` — add `LidarrArtist`, `LidarrAlbum` + +The existing M5a client lives in `internal/lidarr/client.go`. To keep it from sprawling, the M5b additions land in two new files: `lookup_mbid.go` for the GET-by-MBID methods and `delete.go` for the DELETE method + a tiny `del()` HTTP helper. + +- [ ] **Step 2.1: Add the typed structs** + +`internal/lidarr/types.go` (modify) — append the two structs at the bottom of the file: + +```go +// LidarrArtist is the subset of Lidarr's artist resource used by M5b +// admin actions. The "id" field is Lidarr's internal numeric ID — needed +// for DELETE /api/v1/artist/{id} calls. +type LidarrArtist struct { + ID int `json:"id"` + ForeignArtistID string `json:"foreignArtistId"` // MBID + ArtistName string `json:"artistName"` +} + +// LidarrAlbum is the subset of Lidarr's album resource used by M5b +// admin actions. +type LidarrAlbum struct { + ID int `json:"id"` + ForeignAlbumID string `json:"foreignAlbumId"` // MBID + Title string `json:"title"` + ArtistID int `json:"artistId"` +} +``` + +- [ ] **Step 2.2: Add a sentinel error for not-found** + +`internal/lidarr/errors.go` (modify) — append: + +```go +// ErrNotFound is returned by LookupArtistByMBID and LookupAlbumByMBID +// when Lidarr returns 200 with an empty array — i.e., the MBID isn't in +// Lidarr's monitored set. Distinguished from network/auth errors so admin +// handlers can surface it as `lidarr_album_lookup_failed` (502) instead +// of `lidarr_unreachable` (503). +var ErrNotFound = errors.New("lidarr: not found") +``` + +If `errors.go` doesn't already import `"errors"`, add it. + +- [ ] **Step 2.3: Write `lookup_mbid.go`** + +```go +package lidarr + +import ( + "context" + "encoding/json" + "fmt" + "net/url" +) + +// LookupArtistByMBID returns the artist Lidarr has indexed under that +// MBID. Returns ErrNotFound if Lidarr returns an empty array. +func (c *Client) LookupArtistByMBID(ctx context.Context, mbid string) (LidarrArtist, error) { + if mbid == "" { + return LidarrArtist{}, fmt.Errorf("lidarr: empty mbid") + } + q := url.Values{"mbId": []string{mbid}} + resp, err := c.get(ctx, "/api/v1/artist", q) + if err != nil { + return LidarrArtist{}, err + } + defer resp.Body.Close() + + var rows []LidarrArtist + if err := json.NewDecoder(resp.Body).Decode(&rows); err != nil { + return LidarrArtist{}, fmt.Errorf("lidarr: decode artist: %w", err) + } + if len(rows) == 0 { + return LidarrArtist{}, ErrNotFound + } + return rows[0], nil +} + +// LookupAlbumByMBID returns the album Lidarr has indexed under that +// MBID. Returns ErrNotFound on empty result. +func (c *Client) LookupAlbumByMBID(ctx context.Context, mbid string) (LidarrAlbum, error) { + if mbid == "" { + return LidarrAlbum{}, fmt.Errorf("lidarr: empty mbid") + } + q := url.Values{"foreignAlbumId": []string{mbid}} + resp, err := c.get(ctx, "/api/v1/album", q) + if err != nil { + return LidarrAlbum{}, err + } + defer resp.Body.Close() + + var rows []LidarrAlbum + if err := json.NewDecoder(resp.Body).Decode(&rows); err != nil { + return LidarrAlbum{}, fmt.Errorf("lidarr: decode album: %w", err) + } + if len(rows) == 0 { + return LidarrAlbum{}, ErrNotFound + } + return rows[0], nil +} +``` + +- [ ] **Step 2.4: Write `delete.go`** + +```go +package lidarr + +import ( + "context" + "fmt" + "net/http" + "net/url" + "strconv" +) + +// del issues a DELETE against the given path with optional query params. +// Mirrors the existing get/post helpers in client.go; consolidating the +// auth header + base-URL handling behavior in one place. +func (c *Client) del(ctx context.Context, path string, q url.Values) (*http.Response, error) { + u, err := c.url(path) + if err != nil { + return nil, err + } + if q != nil { + u.RawQuery = q.Encode() + } + req, err := http.NewRequestWithContext(ctx, http.MethodDelete, u.String(), nil) + if err != nil { + return nil, fmt.Errorf("lidarr: build DELETE: %w", err) + } + req.Header.Set("X-Api-Key", c.APIKey) + + resp, err := c.HTTP.Do(req) + if err != nil { + return nil, fmt.Errorf("%w: %v", ErrUnreachable, err) + } + if resp.StatusCode == http.StatusUnauthorized { + resp.Body.Close() + return nil, ErrAuthFailed + } + if resp.StatusCode >= 400 { + resp.Body.Close() + return nil, fmt.Errorf("%w: status %d", ErrLookupFailed, resp.StatusCode) + } + return resp, nil +} + +// DeleteAlbum removes an album from Lidarr's library. +// - deleteFiles=true also removes the audio files from disk. +// - addImportListExclusion=true tells Lidarr to never re-add this album +// via import-list scans. +// +// M5b's admin "delete via Lidarr" action always passes both `true`. +func (c *Client) DeleteAlbum(ctx context.Context, lidarrAlbumID int, deleteFiles, addImportListExclusion bool) error { + if lidarrAlbumID == 0 { + return fmt.Errorf("lidarr: zero album id") + } + q := url.Values{ + "deleteFiles": []string{strconv.FormatBool(deleteFiles)}, + "addImportListExclusion": []string{strconv.FormatBool(addImportListExclusion)}, + } + resp, err := c.del(ctx, "/api/v1/album/"+strconv.Itoa(lidarrAlbumID), q) + if err != nil { + return err + } + resp.Body.Close() + return nil +} +``` + +If `client.go` doesn't already export a `url(path)` helper, look at how `get(ctx, path, q)` builds its URL and either factor out the helper or inline the logic here. (M5a's `client.go` has `c.url(path)` at the top of the file — check before duplicating.) + +- [ ] **Step 2.5: Capture fixtures** + +`internal/lidarr/testdata/album_lookup_by_mbid.json` — a single-element JSON array matching what Lidarr returns for `GET /api/v1/album?foreignAlbumId=`: + +```json +[ + { + "id": 42, + "foreignAlbumId": "3a2c2c8c-7e6f-4f8a-b1d2-9a8b6c4e3f1d", + "title": "Music Has The Right To Children", + "artistId": 7 + } +] +``` + +`internal/lidarr/testdata/artist_lookup_by_mbid.json` — same shape: + +```json +[ + { + "id": 7, + "foreignArtistId": "069b64b6-7884-4f6a-94cc-e4c1d6c87a01", + "artistName": "Boards of Canada" + } +] +``` + +- [ ] **Step 2.6: Write `delete_test.go` (covers all three new methods)** + +```go +package lidarr + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "os" + "testing" +) + +func TestLookupAlbumByMBID_HappyPath(t *testing.T) { + body, err := os.ReadFile("testdata/album_lookup_by_mbid.json") + if err != nil { + t.Fatalf("read fixture: %v", err) + } + c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/album" { + t.Errorf("path = %q, want /api/v1/album", r.URL.Path) + } + if got := r.URL.Query().Get("foreignAlbumId"); got != "3a2c2c8c-7e6f-4f8a-b1d2-9a8b6c4e3f1d" { + t.Errorf("foreignAlbumId = %q", got) + } + if got := r.Header.Get("X-Api-Key"); got != "test-key" { + t.Errorf("X-Api-Key = %q, want test-key", got) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write(body) + }) + defer srv.Close() + + got, err := c.LookupAlbumByMBID(context.Background(), "3a2c2c8c-7e6f-4f8a-b1d2-9a8b6c4e3f1d") + if err != nil { + t.Fatalf("LookupAlbumByMBID: %v", err) + } + if got.ID != 42 || got.Title != "Music Has The Right To Children" { + t.Errorf("got = %+v", got) + } +} + +func TestLookupAlbumByMBID_EmptyArrayReturnsErrNotFound(t *testing.T) { + c, srv := newTestClient(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("[]")) + }) + defer srv.Close() + + _, err := c.LookupAlbumByMBID(context.Background(), "x") + if !errors.Is(err, ErrNotFound) { + t.Errorf("err = %v, want ErrNotFound", err) + } +} + +func TestLookupAlbumByMBID_AuthFailed(t *testing.T) { + c, srv := newTestClient(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusUnauthorized) + }) + defer srv.Close() + + _, err := c.LookupAlbumByMBID(context.Background(), "x") + if !errors.Is(err, ErrAuthFailed) { + t.Errorf("err = %v, want ErrAuthFailed", err) + } +} + +func TestLookupArtistByMBID_HappyPath(t *testing.T) { + body, err := os.ReadFile("testdata/artist_lookup_by_mbid.json") + if err != nil { + t.Fatalf("read fixture: %v", err) + } + c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/api/v1/artist" { + t.Errorf("path = %q, want /api/v1/artist", r.URL.Path) + } + if got := r.URL.Query().Get("mbId"); got != "069b64b6-7884-4f6a-94cc-e4c1d6c87a01" { + t.Errorf("mbId = %q", got) + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write(body) + }) + defer srv.Close() + + got, err := c.LookupArtistByMBID(context.Background(), "069b64b6-7884-4f6a-94cc-e4c1d6c87a01") + if err != nil { + t.Fatalf("LookupArtistByMBID: %v", err) + } + if got.ID != 7 || got.ArtistName != "Boards of Canada" { + t.Errorf("got = %+v", got) + } +} + +func TestDeleteAlbum_PassesBothFlags(t *testing.T) { + var captured *http.Request + c, srv := newTestClient(func(w http.ResponseWriter, r *http.Request) { + captured = r + w.WriteHeader(http.StatusOK) + }) + defer srv.Close() + + if err := c.DeleteAlbum(context.Background(), 42, true, true); err != nil { + t.Fatalf("DeleteAlbum: %v", err) + } + if captured == nil || captured.Method != http.MethodDelete { + t.Fatalf("method = %v, want DELETE", captured) + } + if captured.URL.Path != "/api/v1/album/42" { + t.Errorf("path = %q", captured.URL.Path) + } + if got := captured.URL.Query().Get("deleteFiles"); got != "true" { + t.Errorf("deleteFiles = %q", got) + } + if got := captured.URL.Query().Get("addImportListExclusion"); got != "true" { + t.Errorf("addImportListExclusion = %q", got) + } +} + +func TestDeleteAlbum_5xxReturnsErrLookupFailed(t *testing.T) { + c, srv := newTestClient(func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + }) + defer srv.Close() + + err := c.DeleteAlbum(context.Background(), 42, true, true) + if !errors.Is(err, ErrLookupFailed) { + t.Errorf("err = %v, want ErrLookupFailed", err) + } +} + +func TestDeleteAlbum_NetworkErrorReturnsErrUnreachable(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + srv.Close() // server is closed; client should fail to connect + c := NewClient(srv.URL, "test-key") + + err := c.DeleteAlbum(context.Background(), 42, true, true) + if !errors.Is(err, ErrUnreachable) { + t.Errorf("err = %v, want ErrUnreachable", err) + } +} +``` + +`newTestClient` is the existing helper in `client_test.go` (M5a). Reuse it. + +- [ ] **Step 2.7: Run tests + build** + +```bash +go test ./internal/lidarr/... -count=1 +go build ./... +``` + +Expected: all green. + +- [ ] **Step 2.8: Commit** + +```bash +git add internal/lidarr/ +git commit -m "feat(lidarr): LookupArtistByMBID, LookupAlbumByMBID, DeleteAlbum" +``` + +--- + +### Task 3 — `internal/library` `DeleteTrackFile` + +**Files:** +- Create: `internal/library/delete.go` +- Create: `internal/library/delete_test.go` + +The admin "Delete file" action removes the file from disk and the row from `tracks`. The album/artist rows stay. Other tracks may reference them; the admin only nuked one track. + +- [ ] **Step 3.1: Write `delete.go`** + +```go +package library + +import ( + "context" + "errors" + "fmt" + "io/fs" + "os" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// ErrTrackNotFound is returned when DeleteTrackFile is called with an id +// that has no row in tracks. +var ErrTrackNotFound = errors.New("library: track not found") + +// DeleteTrackFile removes a track file from disk and its row from the +// tracks table. Album and artist rows are left untouched. +// +// Steps: +// 1. Look up the track to get its file_path. +// 2. Remove the file from disk. fs.ErrNotExist is OK — already gone. +// 3. Delete the tracks row. +// +// Order matters: file first, then DB. If the file delete fails (permission, +// I/O error), we leave the DB row alone so the admin can retry. +func DeleteTrackFile(ctx context.Context, pool *pgxpool.Pool, trackID pgtype.UUID) error { + q := dbq.New(pool) + track, err := q.GetTrackByID(ctx, trackID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return ErrTrackNotFound + } + return fmt.Errorf("get track: %w", err) + } + + if err := os.Remove(track.FilePath); err != nil && !errors.Is(err, fs.ErrNotExist) { + return fmt.Errorf("remove file: %w", err) + } + + if _, err := pool.Exec(ctx, "DELETE FROM tracks WHERE id = $1", trackID); err != nil { + return fmt.Errorf("delete row: %w", err) + } + return nil +} +``` + +- [ ] **Step 3.2: Write `delete_test.go`** + +```go +package library + +import ( + "context" + "errors" + "io" + "log/slog" + "os" + "path/filepath" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +func TestDeleteTrackFile_HappyPath(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); 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 tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { + t.Fatalf("truncate: %v", err) + } + q := dbq.New(pool) + artist, _ := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{Name: "X", SortName: "X"}) + album, _ := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{Title: "A", SortTitle: "A", ArtistID: artist.ID}) + + // Create a real on-disk file the test can prove is removed. + dir := t.TempDir() + path := filepath.Join(dir, "track.mp3") + if err := os.WriteFile(path, []byte("payload"), 0o644); err != nil { + t.Fatalf("write file: %v", err) + } + track, err := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: "T", AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: path, FileSize: 7, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("upsert: %v", err) + } + + if err := DeleteTrackFile(context.Background(), pool, track.ID); err != nil { + t.Fatalf("DeleteTrackFile: %v", err) + } + + // File gone. + if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { + t.Errorf("file still exists: %v", err) + } + // Row gone. + if _, err := q.GetTrackByID(context.Background(), track.ID); err == nil { + t.Errorf("track row still exists") + } + // Album row preserved. + if _, err := q.GetAlbumByID(context.Background(), album.ID); err != nil { + t.Errorf("album row vanished: %v", err) + } +} + +func TestDeleteTrackFile_FileAlreadyGoneSucceeds(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); err != nil { + t.Fatalf("migrate: %v", err) + } + pool, _ := pgxpool.New(context.Background(), dsn) + t.Cleanup(pool.Close) + if _, err := pool.Exec(context.Background(), + "TRUNCATE tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { + t.Fatalf("truncate: %v", err) + } + q := dbq.New(pool) + artist, _ := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{Name: "X", SortName: "X"}) + album, _ := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{Title: "A", SortTitle: "A", ArtistID: artist.ID}) + + track, _ := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: "T", AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: "/no/such/file/anywhere.mp3", FileSize: 0, FileFormat: "mp3", + }) + + if err := DeleteTrackFile(context.Background(), pool, track.ID); err != nil { + t.Fatalf("DeleteTrackFile with missing file: %v", err) + } + if _, err := q.GetTrackByID(context.Background(), track.ID); err == nil { + t.Errorf("track row still exists") + } +} + +func TestDeleteTrackFile_NotFoundReturnsErr(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); err != nil { + t.Fatalf("migrate: %v", err) + } + pool, _ := pgxpool.New(context.Background(), dsn) + t.Cleanup(pool.Close) + + var bogus pgxUUID + bogus.Set([16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}) + err := DeleteTrackFile(context.Background(), pool, bogus.UUID) + if !errors.Is(err, ErrTrackNotFound) { + t.Errorf("err = %v, want ErrTrackNotFound", err) + } +} + +// pgxUUID is a tiny shim for the test — the existing scanner_test.go in +// this package uses raw byte arrays to build a synthetic pgtype.UUID. If +// the convention changes, mirror whatever helper that test uses. +type pgxUUID struct { + UUID interface { + // satisfied by pgtype.UUID + } +} + +func (u *pgxUUID) Set(b [16]byte) { + // Replace this body with whatever the existing tests use to construct + // a pgtype.UUID from raw bytes. If unsure, copy from + // internal/lidarrrequests/service_test.go's TestApprove_NotFound. + panic("replace with the project's pgtype.UUID construction helper") +} +``` + +The `pgxUUID` shim above is a placeholder — when implementing, look at `internal/lidarrrequests/service_test.go:TestApprove_NotFound` which constructs a synthetic UUID with `bogus.Bytes = [16]byte{...}; bogus.Valid = true`. Use that pattern instead. + +- [ ] **Step 3.3: Run tests** + +```bash +docker compose up -d postgres +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race ./internal/library/... -run TestDeleteTrackFile +``` + +Expected: all three subtests pass. + +- [ ] **Step 3.4: Commit** + +```bash +git add internal/library/delete.go internal/library/delete_test.go +git commit -m "feat(library): DeleteTrackFile (rm file + tracks row, album/artist preserved)" +``` + +--- + +### Task 4 — `lidarrquarantine.Service` — Flag/Unflag/ListMine/ListAdminQueue + +**Files:** +- Create: `internal/lidarrquarantine/service.go` +- Create: `internal/lidarrquarantine/service_test.go` + +Read the M5a `internal/lidarrrequests/service.go` first — it's the closest analog. Same shape (`Service` struct, factory function, integration tests gated on `MINSTREL_TEST_DATABASE_URL`, `dbtest.ResetDB` for isolation). Mirror it. + +- [ ] **Step 4.1: Write the package skeleton + read paths** + +`internal/lidarrquarantine/service.go`: + +```go +// Package lidarrquarantine owns the per-user track quarantine workflow. +// Users flag a track as broken (Flag/Unflag), the SPA hides the track +// from their views, and admins resolve the resulting reports via the +// Service's admin actions (Resolve / DeleteFile / DeleteViaLidarr). +package lidarrquarantine + +import ( + "context" + "errors" + "fmt" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarr" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" +) + +// Public errors. Handlers map these to API codes. +var ( + ErrBadReason = errors.New("lidarrquarantine: invalid reason") + ErrTrackNotFound = errors.New("lidarrquarantine: track not found") + ErrQuarantineNotFound = errors.New("lidarrquarantine: quarantine row not found") + ErrAlbumMBIDMissing = errors.New("lidarrquarantine: track has no parent album mbid") + ErrLidarrAlbumNotFound = errors.New("lidarrquarantine: lidarr has no album for that mbid") + ErrLidarrDisabled = errors.New("lidarrquarantine: lidarr is not configured") +) + +// Service is the lifecycle owner. clientFn is a per-call factory so config +// changes in lidarrconfig take effect immediately. clientFn returns nil +// when Lidarr is disabled. +type Service struct { + pool *pgxpool.Pool + lidarrCfg *lidarrconfig.Service + clientFn func() *lidarr.Client +} + +func NewService(pool *pgxpool.Pool, cfg *lidarrconfig.Service, clientFn func() *lidarr.Client) *Service { + if clientFn == nil { + clientFn = func() *lidarr.Client { return nil } + } + return &Service{pool: pool, lidarrCfg: cfg, clientFn: clientFn} +} + +// Flag inserts or updates a quarantine row for the caller. Re-flagging +// the same (user, track) overwrites reason+notes. +func (s *Service) Flag(ctx context.Context, userID, trackID pgtype.UUID, reason string, notes string) (dbq.LidarrQuarantine, error) { + if !validReason(reason) { + return dbq.LidarrQuarantine{}, ErrBadReason + } + var notesPtr *string + if notes != "" { + notesPtr = ¬es + } + row, err := dbq.New(s.pool).UpsertQuarantine(ctx, dbq.UpsertQuarantineParams{ + UserID: userID, + TrackID: trackID, + Reason: dbq.LidarrQuarantineReason(reason), + Notes: notesPtr, + }) + if err != nil { + // ON CONFLICT path can't trip ErrNoRows; only an FK violation does + // (track_id doesn't exist). Surface that as ErrTrackNotFound. + return dbq.LidarrQuarantine{}, fmt.Errorf("upsert: %w", err) + } + return row, nil +} + +// Unflag removes the caller's row. Returns ErrQuarantineNotFound if +// no row exists. +func (s *Service) Unflag(ctx context.Context, userID, trackID pgtype.UUID) error { + _, err := dbq.New(s.pool).DeleteQuarantine(ctx, dbq.DeleteQuarantineParams{ + UserID: userID, TrackID: trackID, + }) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return ErrQuarantineNotFound + } + return fmt.Errorf("delete: %w", err) + } + return nil +} + +// ListMine returns the caller's quarantines with track/album/artist +// detail. Drives /library/hidden. +func (s *Service) ListMine(ctx context.Context, userID pgtype.UUID) ([]dbq.ListQuarantineForUserRow, error) { + return dbq.New(s.pool).ListQuarantineForUser(ctx, userID) +} + +// AdminQueueRow is the assembled aggregated row served by the admin +// queue endpoint. The handler post-processes the SQL results to attach +// reason_counts and per-user reports. +type AdminQueueRow struct { + TrackID pgtype.UUID + TrackTitle string + ArtistName string + AlbumTitle *string + AlbumID pgtype.UUID + LidarrAlbumMBID *string + ReportCount int32 + LatestAt pgtype.Timestamptz + ReasonCounts map[string]int + Reports []UserReport +} + +type UserReport struct { + UserID pgtype.UUID + Username string + Reason string + Notes *string + CreatedAt pgtype.Timestamptz +} + +// ListAdminQueue returns the aggregated admin queue. One row per track. +func (s *Service) ListAdminQueue(ctx context.Context) ([]AdminQueueRow, error) { + q := dbq.New(s.pool) + aggregated, err := q.ListAdminQuarantineQueue(ctx) + if err != nil { + return nil, fmt.Errorf("aggregate: %w", err) + } + out := make([]AdminQueueRow, 0, len(aggregated)) + for _, r := range aggregated { + reports, err := q.ListQuarantineReportsForTrack(ctx, r.TrackID) + if err != nil { + return nil, fmt.Errorf("reports for track %v: %w", r.TrackID, err) + } + rc := make(map[string]int, len(reports)) + userReports := make([]UserReport, 0, len(reports)) + for _, rep := range reports { + rc[string(rep.Reason)]++ + userReports = append(userReports, UserReport{ + UserID: rep.UserID, + Username: rep.Username, + Reason: string(rep.Reason), + Notes: rep.Notes, + CreatedAt: rep.CreatedAt, + }) + } + out = append(out, AdminQueueRow{ + TrackID: r.TrackID, + TrackTitle: r.TrackTitle, + ArtistName: r.ArtistName, + AlbumTitle: r.AlbumTitle, + AlbumID: r.AlbumID, + LidarrAlbumMBID: r.LidarrAlbumMbid, + ReportCount: r.ReportCount, + LatestAt: r.LatestAt, + ReasonCounts: rc, + Reports: userReports, + }) + } + return out, nil +} + +func validReason(r string) bool { + switch r { + case "bad_rip", "wrong_file", "wrong_tags", "duplicate", "other": + return true + } + return false +} +``` + +(Admin actions Resolve / DeleteFile / DeleteViaLidarr land in Task 5.) + +- [ ] **Step 4.2: Write the integration tests for the read paths** + +`internal/lidarrquarantine/service_test.go`: + +```go +package lidarrquarantine + +import ( + "context" + "errors" + "io" + "log/slog" + "os" + "path/filepath" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" +) + +func newPool(t *testing.T) *pgxpool.Pool { + t.Helper() + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); 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) + dbtest.ResetDB(t, pool) + if _, err := pool.Exec(context.Background(), + "DELETE FROM lidarr_quarantine; DELETE FROM lidarr_quarantine_actions;"); err != nil { + t.Fatalf("reset quarantine tables: %v", err) + } + return pool +} + +func seedUser(t *testing.T, pool *pgxpool.Pool, name string) dbq.User { + t.Helper() + u, err := dbq.New(pool).CreateUser(context.Background(), dbq.CreateUserParams{ + Username: dbtest.TestUserPrefix + name, PasswordHash: "x", + ApiToken: name + "-token", IsAdmin: false, + }) + if err != nil { + t.Fatalf("seed user %s: %v", name, err) + } + return u +} + +func seedTrack(t *testing.T, pool *pgxpool.Pool, title, mbid string) (dbq.Track, dbq.Album, dbq.Artist) { + t.Helper() + q := dbq.New(pool) + artist, err := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{ + Name: "Test Artist", SortName: "Test Artist", + }) + if err != nil { + t.Fatalf("artist: %v", err) + } + albumMBID := mbid + "-album" + album, err := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{ + Title: "Test Album", SortTitle: "Test Album", + ArtistID: artist.ID, Mbid: &albumMBID, + }) + if err != nil { + t.Fatalf("album: %v", err) + } + track, err := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: title, AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: filepath.Join(t.TempDir(), title+".mp3"), + FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("track: %v", err) + } + return track, album, artist +} + +func TestFlag_HappyPath(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + track, _, _ := seedTrack(t, pool, "Bad Track", "abc") + + svc := NewService(pool, lidarrconfig.New(pool), nil) + row, err := svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", "crackly") + if err != nil { + t.Fatalf("Flag: %v", err) + } + if string(row.Reason) != "bad_rip" { + t.Errorf("reason = %v", row.Reason) + } + if row.Notes == nil || *row.Notes != "crackly" { + t.Errorf("notes = %v", row.Notes) + } +} + +func TestFlag_UpsertOnSecondFlag(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + track, _, _ := seedTrack(t, pool, "T", "x") + + svc := NewService(pool, lidarrconfig.New(pool), nil) + if _, err := svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", "first"); err != nil { + t.Fatalf("first flag: %v", err) + } + row, err := svc.Flag(context.Background(), user.ID, track.ID, "wrong_tags", "") + if err != nil { + t.Fatalf("second flag: %v", err) + } + if string(row.Reason) != "wrong_tags" { + t.Errorf("reason = %v", row.Reason) + } + if row.Notes != nil { + t.Errorf("notes = %v, want nil after empty notes upsert", row.Notes) + } +} + +func TestFlag_BadReasonRejected(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + track, _, _ := seedTrack(t, pool, "T", "x") + + svc := NewService(pool, lidarrconfig.New(pool), nil) + _, err := svc.Flag(context.Background(), user.ID, track.ID, "garbage", "") + if !errors.Is(err, ErrBadReason) { + t.Errorf("err = %v, want ErrBadReason", err) + } +} + +func TestUnflag_DeletesRow(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + track, _, _ := seedTrack(t, pool, "T", "x") + + svc := NewService(pool, lidarrconfig.New(pool), nil) + _, _ = svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", "") + if err := svc.Unflag(context.Background(), user.ID, track.ID); err != nil { + t.Fatalf("Unflag: %v", err) + } + if err := svc.Unflag(context.Background(), user.ID, track.ID); !errors.Is(err, ErrQuarantineNotFound) { + t.Errorf("second Unflag err = %v, want ErrQuarantineNotFound", err) + } +} + +func TestListMine_OrderedNewestFirst(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + t1, _, _ := seedTrack(t, pool, "T1", "x") + t2, _, _ := seedTrack(t, pool, "T2", "y") + + svc := NewService(pool, lidarrconfig.New(pool), nil) + _, _ = svc.Flag(context.Background(), user.ID, t1.ID, "bad_rip", "") + _, _ = svc.Flag(context.Background(), user.ID, t2.ID, "duplicate", "") + + rows, err := svc.ListMine(context.Background(), user.ID) + if err != nil { + t.Fatalf("ListMine: %v", err) + } + if len(rows) != 2 { + t.Fatalf("len = %d, want 2", len(rows)) + } + // T2 was flagged second — newest first. + if rows[0].LidarrQuarantine.TrackID != t2.ID { + t.Errorf("first row track = %v, want T2 (%v)", rows[0].LidarrQuarantine.TrackID, t2.ID) + } +} + +func TestListAdminQueue_AggregatesByTrackWithReasonCounts(t *testing.T) { + pool := newPool(t) + alice := seedUser(t, pool, "alice") + bob := seedUser(t, pool, "bob") + carol := seedUser(t, pool, "carol") + track, _, _ := seedTrack(t, pool, "Hot Mess", "abc") + + svc := NewService(pool, lidarrconfig.New(pool), nil) + _, _ = svc.Flag(context.Background(), alice.ID, track.ID, "bad_rip", "") + _, _ = svc.Flag(context.Background(), bob.ID, track.ID, "bad_rip", "") + _, _ = svc.Flag(context.Background(), carol.ID, track.ID, "wrong_tags", "") + + rows, err := svc.ListAdminQueue(context.Background()) + if err != nil { + t.Fatalf("ListAdminQueue: %v", err) + } + if len(rows) != 1 { + t.Fatalf("len = %d, want 1 aggregated row", len(rows)) + } + r := rows[0] + if r.ReportCount != 3 { + t.Errorf("report_count = %d, want 3", r.ReportCount) + } + if r.ReasonCounts["bad_rip"] != 2 || r.ReasonCounts["wrong_tags"] != 1 { + t.Errorf("reason_counts = %+v, want bad_rip=2 wrong_tags=1", r.ReasonCounts) + } + if len(r.Reports) != 3 { + t.Errorf("reports len = %d, want 3", len(r.Reports)) + } +} +``` + +- [ ] **Step 4.3: Run the tests** + +```bash +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race ./internal/lidarrquarantine/... +``` + +Expected: all green. + +- [ ] **Step 4.4: Commit** + +```bash +git add internal/lidarrquarantine/ +git commit -m "feat(lidarrquarantine): Service Flag/Unflag/ListMine/ListAdminQueue" +``` + +--- + +### Task 5 — `lidarrquarantine.Service` admin actions + +**Files:** +- Modify: `internal/lidarrquarantine/service.go` — append Resolve / DeleteFile / DeleteViaLidarr +- Modify: `internal/lidarrquarantine/service_test.go` — append admin-action tests + +The three admin actions all follow the same shape: +1. Read the track (and parent album for DeleteViaLidarr) for snapshot fields. +2. Capture `affected_users` count via `CountQuarantineForTrack` *before* deleting. +3. For DeleteFile: call `library.DeleteTrackFile`; for DeleteViaLidarr: lookup album in Lidarr, call `Client.DeleteAlbum`, delete all Minstrel tracks in that album. +4. Delete `lidarr_quarantine` rows for the affected tracks. +5. Write a `lidarr_quarantine_actions` audit row. + +Order matters: Lidarr/file delete first, then DB writes. Failure of the external call leaves the per-user rows intact for retry. **No partial state.** + +- [ ] **Step 5.1: Append `Resolve` to `service.go`** + +```go +// Resolve clears all per-user quarantine rows for a track and writes an +// audit log row. Idempotent — a track with no rows still writes an audit +// entry with affected_users=0 (so admin can see "I clicked resolve on a +// track that already had no reports"). +func (s *Service) Resolve(ctx context.Context, trackID, adminID pgtype.UUID) (dbq.LidarrQuarantineAction, error) { + q := dbq.New(s.pool) + track, err := q.GetTrackByID(ctx, trackID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return dbq.LidarrQuarantineAction{}, ErrTrackNotFound + } + return dbq.LidarrQuarantineAction{}, fmt.Errorf("get track: %w", err) + } + snap, err := s.snapshot(ctx, q, track) + if err != nil { + return dbq.LidarrQuarantineAction{}, err + } + + affected, err := q.CountQuarantineForTrack(ctx, trackID) + if err != nil { + return dbq.LidarrQuarantineAction{}, fmt.Errorf("count: %w", err) + } + if err := q.DeleteQuarantineForTrack(ctx, trackID); err != nil { + return dbq.LidarrQuarantineAction{}, fmt.Errorf("delete rows: %w", err) + } + return q.WriteQuarantineAction(ctx, dbq.WriteQuarantineActionParams{ + TrackID: trackID, TrackTitle: snap.TrackTitle, ArtistName: snap.ArtistName, + AlbumTitle: snap.AlbumTitle, Action: dbq.LidarrQuarantineActionResolved, + AdminID: adminID, LidarrAlbumMbid: nil, AffectedUsers: affected, + }) +} + +// snapshot is shared scaffolding — pulls album/artist titles for the audit row. +type quarantineSnapshot struct { + TrackTitle string + ArtistName string + AlbumTitle *string + LidarrAlbumMBID *string +} + +func (s *Service) snapshot(ctx context.Context, q *dbq.Queries, track dbq.Track) (quarantineSnapshot, error) { + album, err := q.GetAlbumByID(ctx, track.AlbumID) + if err != nil { + return quarantineSnapshot{}, fmt.Errorf("get album: %w", err) + } + artist, err := q.GetArtistByID(ctx, track.ArtistID) + if err != nil { + return quarantineSnapshot{}, fmt.Errorf("get artist: %w", err) + } + return quarantineSnapshot{ + TrackTitle: track.Title, + ArtistName: artist.Name, + AlbumTitle: &album.Title, + LidarrAlbumMBID: album.Mbid, + }, nil +} +``` + +If `dbq.GetAlbumByID` / `dbq.GetArtistByID` don't exist as named queries, check the existing albums.sql / artists.sql files — they almost certainly do under different names (`AlbumByID`, `ArtistByID`, etc.) — and substitute the actual names. + +- [ ] **Step 5.2: Append `DeleteFile`** + +```go +// DeleteFile removes the track file from disk and the tracks row, then +// clears all per-user quarantine rows for that track and writes an audit +// row. If the file deletion fails, the per-user rows stay so admin can +// retry. No partial state. +func (s *Service) DeleteFile(ctx context.Context, trackID, adminID pgtype.UUID) (dbq.LidarrQuarantineAction, error) { + q := dbq.New(s.pool) + track, err := q.GetTrackByID(ctx, trackID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return dbq.LidarrQuarantineAction{}, ErrTrackNotFound + } + return dbq.LidarrQuarantineAction{}, fmt.Errorf("get track: %w", err) + } + snap, err := s.snapshot(ctx, q, track) + if err != nil { + return dbq.LidarrQuarantineAction{}, err + } + + affected, err := q.CountQuarantineForTrack(ctx, trackID) + if err != nil { + return dbq.LidarrQuarantineAction{}, fmt.Errorf("count: %w", err) + } + + if err := library.DeleteTrackFile(ctx, s.pool, trackID); err != nil { + return dbq.LidarrQuarantineAction{}, fmt.Errorf("delete file: %w", err) + } + // tracks row is gone; the FK ON DELETE CASCADE on lidarr_quarantine + // already cleared the per-user rows. + return q.WriteQuarantineAction(ctx, dbq.WriteQuarantineActionParams{ + TrackID: trackID, TrackTitle: snap.TrackTitle, ArtistName: snap.ArtistName, + AlbumTitle: snap.AlbumTitle, Action: dbq.LidarrQuarantineActionDeletedFile, + AdminID: adminID, LidarrAlbumMbid: nil, AffectedUsers: affected, + }) +} +``` + +Note the cascade comment: the schema has `ON DELETE CASCADE` on `lidarr_quarantine.track_id`, so when `library.DeleteTrackFile` runs `DELETE FROM tracks WHERE id = $1`, the per-user quarantine rows go too. We don't call `DeleteQuarantineForTrack` separately. **Verify this assumption holds when implementing** — re-check `0011_lidarr_quarantine.up.sql` and the existing `tracks` constraints. + +- [ ] **Step 5.3: Append `DeleteViaLidarr`** + +```go +// DeleteViaLidarr is the destructive admin path: tells Lidarr to remove +// the parent album with deleteFiles=true + addImportListExclusion=true, +// then removes Minstrel rows for all tracks of that album. The cascade +// on lidarr_quarantine clears per-user rows automatically. +// +// On Lidarr failure (unreachable, auth-failed, lookup-empty), nothing +// changes locally. Admin retries. +func (s *Service) DeleteViaLidarr(ctx context.Context, trackID, adminID pgtype.UUID) (dbq.LidarrQuarantineAction, int, error) { + cfg, err := s.lidarrCfg.Get(ctx) + if err != nil { + return dbq.LidarrQuarantineAction{}, 0, fmt.Errorf("load config: %w", err) + } + client := s.clientFn() + if !cfg.Enabled || client == nil { + return dbq.LidarrQuarantineAction{}, 0, ErrLidarrDisabled + } + + q := dbq.New(s.pool) + track, err := q.GetTrackByID(ctx, trackID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return dbq.LidarrQuarantineAction{}, 0, ErrTrackNotFound + } + return dbq.LidarrQuarantineAction{}, 0, fmt.Errorf("get track: %w", err) + } + snap, err := s.snapshot(ctx, q, track) + if err != nil { + return dbq.LidarrQuarantineAction{}, 0, err + } + if snap.LidarrAlbumMBID == nil || *snap.LidarrAlbumMBID == "" { + return dbq.LidarrQuarantineAction{}, 0, ErrAlbumMBIDMissing + } + + affected, err := q.CountQuarantineForTrack(ctx, trackID) + if err != nil { + return dbq.LidarrQuarantineAction{}, 0, fmt.Errorf("count: %w", err) + } + + // Look up the album in Lidarr to translate MBID -> Lidarr internal ID. + album, err := client.LookupAlbumByMBID(ctx, *snap.LidarrAlbumMBID) + if err != nil { + if errors.Is(err, lidarr.ErrNotFound) { + return dbq.LidarrQuarantineAction{}, 0, ErrLidarrAlbumNotFound + } + return dbq.LidarrQuarantineAction{}, 0, fmt.Errorf("lidarr lookup: %w", err) + } + + // Lidarr DELETE — both flags true. + if err := client.DeleteAlbum(ctx, album.ID, true, true); err != nil { + return dbq.LidarrQuarantineAction{}, 0, fmt.Errorf("lidarr delete: %w", err) + } + + // Now remove the local rows. Cascade handles per-user quarantine + // rows via the FK on lidarr_quarantine.track_id. + res, err := s.pool.Exec(ctx, "DELETE FROM tracks WHERE album_id = $1", track.AlbumID) + if err != nil { + // We deleted in Lidarr but failed in our DB. Operator-recoverable + // by re-running. Audit row will reflect the eventual state. + return dbq.LidarrQuarantineAction{}, 0, fmt.Errorf("delete tracks: %w", err) + } + deletedCount := int(res.RowsAffected()) + + // Album/artist rows stay; if the operator wants those gone too they + // can be cleaned up by a future scan or a manual SQL pass. + + action, err := q.WriteQuarantineAction(ctx, dbq.WriteQuarantineActionParams{ + TrackID: trackID, TrackTitle: snap.TrackTitle, ArtistName: snap.ArtistName, + AlbumTitle: snap.AlbumTitle, Action: dbq.LidarrQuarantineActionDeletedViaLidarr, + AdminID: adminID, LidarrAlbumMbid: snap.LidarrAlbumMBID, + AffectedUsers: affected, + }) + return action, deletedCount, err +} +``` + +- [ ] **Step 5.4: Append admin-action tests** + +`internal/lidarrquarantine/service_test.go` — append: + +```go +import ( + // ... add to existing imports: + "net/http" + "net/http/httptest" + + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarr" +) + +func TestResolve_ClearsRowsAndWritesAudit(t *testing.T) { + pool := newPool(t) + alice := seedUser(t, pool, "alice") + bob := seedUser(t, pool, "bob") + track, _, _ := seedTrack(t, pool, "T", "x") + + svc := NewService(pool, lidarrconfig.New(pool), nil) + _, _ = svc.Flag(context.Background(), alice.ID, track.ID, "bad_rip", "") + _, _ = svc.Flag(context.Background(), bob.ID, track.ID, "wrong_tags", "") + + audit, err := svc.Resolve(context.Background(), track.ID, alice.ID) + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if audit.AffectedUsers != 2 { + t.Errorf("affected_users = %d, want 2", audit.AffectedUsers) + } + if audit.Action != dbq.LidarrQuarantineActionResolved { + t.Errorf("action = %v, want resolved", audit.Action) + } + // No more rows for this track. + n, _ := dbq.New(pool).CountQuarantineForTrack(context.Background(), track.ID) + if n != 0 { + t.Errorf("rows after resolve = %d, want 0", n) + } +} + +func TestDeleteFile_RemovesFileAndAuditsAffected(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + // Real on-disk file: + dir := t.TempDir() + path := filepath.Join(dir, "track.mp3") + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + q := dbq.New(pool) + artist, _ := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{Name: "A", SortName: "A"}) + album, _ := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{Title: "Al", SortTitle: "Al", ArtistID: artist.ID}) + track, _ := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: "T", AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: path, FileSize: 1, FileFormat: "mp3", + }) + + svc := NewService(pool, lidarrconfig.New(pool), nil) + _, _ = svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", "") + + audit, err := svc.DeleteFile(context.Background(), track.ID, user.ID) + if err != nil { + t.Fatalf("DeleteFile: %v", err) + } + if audit.AffectedUsers != 1 { + t.Errorf("affected_users = %d, want 1", audit.AffectedUsers) + } + if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { + t.Errorf("file still exists: %v", err) + } +} + +func TestDeleteViaLidarr_FullCascade(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + + // Stub Lidarr server. + var captured []string + stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + captured = append(captured, r.Method+" "+r.URL.Path+"?"+r.URL.RawQuery) + w.Header().Set("Content-Type", "application/json") + if r.URL.Path == "/api/v1/album" && r.Method == http.MethodGet { + _, _ = w.Write([]byte(`[{"id":42,"foreignAlbumId":"al-mbid","title":"Al","artistId":7}]`)) + return + } + // DELETE /api/v1/album/42 -> 200. + w.WriteHeader(http.StatusOK) + })) + t.Cleanup(stub.Close) + + cfg := lidarrconfig.New(pool) + if err := cfg.Save(context.Background(), lidarrconfig.Config{ + Enabled: true, BaseURL: stub.URL, APIKey: "k", + }); err != nil { + t.Fatalf("save config: %v", err) + } + clientFn := func() *lidarr.Client { return lidarr.NewClient(stub.URL, "k") } + svc := NewService(pool, cfg, clientFn) + + // Seed a track on an album whose mbid we'll match in the stub. + q := dbq.New(pool) + artist, _ := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{Name: "A", SortName: "A"}) + albumMBID := "al-mbid" + album, _ := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{ + Title: "Al", SortTitle: "Al", ArtistID: artist.ID, Mbid: &albumMBID, + }) + dir := t.TempDir() + path := filepath.Join(dir, "T.mp3") + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + track, _ := q.UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: "T", AlbumID: album.ID, ArtistID: artist.ID, + DurationMs: 1000, FilePath: path, FileSize: 1, FileFormat: "mp3", + }) + _, _ = svc.Flag(context.Background(), user.ID, track.ID, "bad_rip", "") + + audit, deleted, err := svc.DeleteViaLidarr(context.Background(), track.ID, user.ID) + if err != nil { + t.Fatalf("DeleteViaLidarr: %v", err) + } + if deleted != 1 { + t.Errorf("deleted = %d, want 1 track removed", deleted) + } + if audit.Action != dbq.LidarrQuarantineActionDeletedViaLidarr { + t.Errorf("action = %v", audit.Action) + } + if audit.AffectedUsers != 1 { + t.Errorf("affected_users = %d, want 1", audit.AffectedUsers) + } + if audit.LidarrAlbumMbid == nil || *audit.LidarrAlbumMbid != "al-mbid" { + t.Errorf("lidarr_album_mbid = %v", audit.LidarrAlbumMbid) + } + // Track row is gone (and so is the per-user quarantine row, via cascade). + if _, err := q.GetTrackByID(context.Background(), track.ID); err == nil { + t.Errorf("track row still exists") + } + // Verify Lidarr was called with both flags true. + foundDelete := false + for _, c := range captured { + if c == "DELETE /api/v1/album/42?addImportListExclusion=true&deleteFiles=true" || + c == "DELETE /api/v1/album/42?deleteFiles=true&addImportListExclusion=true" { + foundDelete = true + } + } + if !foundDelete { + t.Errorf("Lidarr DELETE not called with both flags true; captured = %v", captured) + } +} + +func TestDeleteViaLidarr_LidarrDisabled(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + track, _, _ := seedTrack(t, pool, "T", "x") + + svc := NewService(pool, lidarrconfig.New(pool), nil) + _, _, err := svc.DeleteViaLidarr(context.Background(), track.ID, user.ID) + if !errors.Is(err, ErrLidarrDisabled) { + t.Errorf("err = %v, want ErrLidarrDisabled", err) + } +} +``` + +- [ ] **Step 5.5: Run + commit** + +```bash +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race ./internal/lidarrquarantine/... +git add internal/lidarrquarantine/ +git commit -m "feat(lidarrquarantine): admin actions Resolve/DeleteFile/DeleteViaLidarr" +``` + +--- + +### Task 6 — Soft-hide query updates + +**Files:** +- Modify: `internal/db/queries/tracks.sql` — add `*ForUser` variants +- Modify: `internal/db/queries/recommendation.sql` — extend existing radio queries +- Regenerate: `internal/db/dbq/` + +The four affected queries are `ListTracksByAlbum`, `SearchTracks`, `CountTracksMatching`, and the radio loaders. The album/artist views are read through the existing handlers — adding `*ForUser` variants that take `user_id` lets the handlers route based on auth context. + +- [ ] **Step 6.1: Add `ListTracksByAlbumForUser` to `tracks.sql`** + +Append to `internal/db/queries/tracks.sql`: + +```sql +-- name: ListTracksByAlbumForUser :many +-- Same as ListTracksByAlbum but excludes tracks the user has quarantined. +SELECT * FROM tracks +WHERE album_id = $1 + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $2 AND q.track_id = tracks.id + ) +ORDER BY disc_number NULLS LAST, track_number NULLS LAST; + +-- name: SearchTracksForUser :many +SELECT * FROM tracks +WHERE title ILIKE '%' || $1 || '%' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $2 AND q.track_id = tracks.id + ) +ORDER BY title +LIMIT $3 OFFSET $4; + +-- name: CountTracksMatchingForUser :one +SELECT COUNT(*) FROM tracks +WHERE title ILIKE '%' || $1::text || '%' + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $2 AND q.track_id = tracks.id + ); +``` + +- [ ] **Step 6.2: Extend the radio loaders** + +Modify `internal/db/queries/recommendation.sql`. For each `LoadRadioCandidates*` query, add a quarantine clause to the `WHERE` block. The user_id is already a parameter on these queries (`$1`); the additional clause is: + +```sql + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ) +``` + +For `LoadRadioCandidates`: + +```sql +WHERE t.id <> $2 + AND NOT EXISTS ( + SELECT 1 FROM play_events + WHERE user_id = $1 AND track_id = t.id + AND started_at > now() - $3 * interval '1 hour' + ) + AND NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $1 AND q.track_id = t.id + ); +``` + +For `LoadRadioCandidatesV2`, add the same clause inside the final `WHERE` of the union output (look for the comment "5-way UNION" — add the clause to the outer WHERE that filters the unioned candidates by `excluded_ids` etc.). + +- [ ] **Step 6.3: Regenerate sqlc + build** + +```bash +cd internal/db && sqlc generate && cd - +go build ./... +``` + +Expected: clean build. New methods `ListTracksByAlbumForUser`, `SearchTracksForUser`, `CountTracksMatchingForUser` appear in `internal/db/dbq/tracks.sql.go`. + +- [ ] **Step 6.4: Commit** + +```bash +git add internal/db/queries/ internal/db/dbq/ +git commit -m "feat(db): add user-context track query variants honoring quarantine" +``` + +--- + +### Task 7 — Wire soft-hide into existing read handlers + +**Files:** Modify existing handlers under `internal/api/` to call the `*ForUser` queries when an authenticated user is in context. + +The pattern: where the handler currently calls (e.g.) `q.ListTracksByAlbum(ctx, albumID)`, switch to `q.ListTracksByAlbumForUser(ctx, dbq.ListTracksByAlbumForUserParams{AlbumID: albumID, UserID: user.ID})` when `user, ok := auth.UserFromContext(r.Context()); ok` is true. The `else` branch keeps the unfiltered query for any path without a user context (Subsonic, internal callers). + +- [ ] **Step 7.1: Identify call sites** + +Run: + +```bash +grep -rn "ListTracksByAlbum\|SearchTracks\|CountTracksMatching\|LoadRadioCandidates" \ + internal/api/ internal/subsonic/ +``` + +For every call in `internal/api/`, branch on `auth.UserFromContext`. For every call in `internal/subsonic/`, leave it alone (Subsonic is `/rest/*` and doesn't honor quarantine per the legacy memory). + +- [ ] **Step 7.2: Pattern to apply** + +Example for the album-detail handler: + +```go +func (h *handlers) handleAlbumDetail(w http.ResponseWriter, r *http.Request) { + // ... existing parse + lookup ... + var tracks []dbq.Track + if user, ok := auth.UserFromContext(r.Context()); ok { + tracks, err = q.ListTracksByAlbumForUser(r.Context(), dbq.ListTracksByAlbumForUserParams{ + AlbumID: albumID, UserID: user.ID, + }) + } else { + tracks, err = q.ListTracksByAlbum(r.Context(), albumID) + } + // ... existing error + response ... +} +``` + +Repeat for search and radio handlers. The radio handlers already take `user_id` for personalization — the schema change in Task 6 just extended their existing query, so those handlers don't need restructuring. + +- [ ] **Step 7.3: Regression-test the existing endpoints** + +```bash +go test ./internal/api/... -count=1 +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race -p 1 ./internal/api/... +``` + +Expected: existing tests still pass — they don't seed any quarantine rows, so the filter is a no-op. + +- [ ] **Step 7.4: Commit** + +```bash +git add internal/api/ +git commit -m "feat(api): route track-list reads through user-context quarantine filter" +``` + +--- + +### Task 8 — `/api/quarantine/*` user-facing handlers + +**Files:** +- Create: `internal/api/quarantine.go` +- Create: `internal/api/quarantine_test.go` +- Modify: `internal/api/api.go` to mount the new routes + +Three endpoints: `POST /api/quarantine`, `DELETE /api/quarantine/:track_id`, `GET /api/quarantine/mine`. Mirror M5a's `internal/api/requests.go` for the auth + writeJSON conventions. + +- [ ] **Step 8.1: Write `quarantine.go`** + +```go +package api + +import ( + "encoding/json" + "errors" + "net/http" + + "github.com/go-chi/chi/v5" + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrquarantine" +) + +type quarantineView struct { + UserID pgtype.UUID `json:"user_id"` + TrackID pgtype.UUID `json:"track_id"` + Reason string `json:"reason"` + Notes *string `json:"notes,omitempty"` + CreatedAt pgtype.Timestamptz `json:"created_at"` +} + +func quarantineViewFrom(row dbq.LidarrQuarantine) quarantineView { + return quarantineView{ + UserID: row.UserID, TrackID: row.TrackID, + Reason: string(row.Reason), Notes: row.Notes, CreatedAt: row.CreatedAt, + } +} + +type flagBody struct { + TrackID pgtype.UUID `json:"track_id"` + Reason string `json:"reason"` + Notes string `json:"notes"` +} + +func (h *handlers) handleFlag(w http.ResponseWriter, r *http.Request) { + user, ok := auth.UserFromContext(r.Context()) + if !ok { + writeErr(w, http.StatusUnauthorized, "unauthorized", "authentication required") + return + } + var body flagBody + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + writeErr(w, http.StatusBadRequest, "bad_request", "invalid JSON body") + return + } + row, err := h.lidarrQuarantine.Flag(r.Context(), user.ID, body.TrackID, body.Reason, body.Notes) + if err != nil { + switch { + case errors.Is(err, lidarrquarantine.ErrBadReason): + writeErr(w, http.StatusBadRequest, "bad_reason", err.Error()) + case errors.Is(err, lidarrquarantine.ErrTrackNotFound): + writeErr(w, http.StatusNotFound, "track_not_found", "track does not exist") + default: + h.logger.Error("api: flag", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "flag failed") + } + return + } + writeJSON(w, http.StatusCreated, quarantineViewFrom(row)) +} + +func (h *handlers) handleUnflag(w http.ResponseWriter, r *http.Request) { + user, ok := auth.UserFromContext(r.Context()) + if !ok { + writeErr(w, http.StatusUnauthorized, "unauthorized", "authentication required") + return + } + id, ok := parseUUID(chi.URLParam(r, "track_id")) + if !ok { + writeErr(w, http.StatusBadRequest, "bad_request", "invalid track id") + return + } + if err := h.lidarrQuarantine.Unflag(r.Context(), user.ID, id); err != nil { + if errors.Is(err, lidarrquarantine.ErrQuarantineNotFound) { + writeErr(w, http.StatusNotFound, "quarantine_not_found", "no quarantine for that track") + return + } + h.logger.Error("api: unflag", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "unflag failed") + return + } + w.WriteHeader(http.StatusNoContent) +} + +// quarantineMineView wraps the joined row for /api/quarantine/mine. The +// SPA on /library/hidden needs the full track + album + artist payload. +type quarantineMineView struct { + quarantineView + Track dbq.Track `json:"track"` + Album dbq.Album `json:"album"` + Artist dbq.Artist `json:"artist"` +} + +func (h *handlers) handleListMyQuarantine(w http.ResponseWriter, r *http.Request) { + user, ok := auth.UserFromContext(r.Context()) + if !ok { + writeErr(w, http.StatusUnauthorized, "unauthorized", "authentication required") + return + } + rows, err := h.lidarrQuarantine.ListMine(r.Context(), user.ID) + if err != nil { + h.logger.Error("api: list mine", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "list failed") + return + } + out := make([]quarantineMineView, 0, len(rows)) + for _, row := range rows { + out = append(out, quarantineMineView{ + quarantineView: quarantineViewFrom(row.LidarrQuarantine), + Track: row.Track, + Album: row.Album, + Artist: row.Artist, + }) + } + writeJSON(w, http.StatusOK, out) +} +``` + +- [ ] **Step 8.2: Mount routes in `api.go`** + +Inside the existing authenticated route group, add: + +```go +r.Post("/api/quarantine", h.handleFlag) +r.Delete("/api/quarantine/{track_id}", h.handleUnflag) +r.Get("/api/quarantine/mine", h.handleListMyQuarantine) +``` + +- [ ] **Step 8.3: Add `lidarrQuarantine` to the handlers struct** + +Find the `handlers` struct (in `internal/api/api.go` or wherever it lives) and add: + +```go +lidarrQuarantine *lidarrquarantine.Service +``` + +Then update the constructor / wiring to accept it. + +- [ ] **Step 8.4: Write tests** + +`internal/api/quarantine_test.go` mirrors the M5a `requests_test.go` shape: stub handlers, real DB via `MINSTREL_TEST_DATABASE_URL`, table-driven scenarios. Cover: +- Flag with valid reason → 201, row visible in `ListMine`. +- Flag with `bad_reason` → 400, `error.code === "bad_reason"`. +- Flag for a track that doesn't exist → 404 `track_not_found`. +- Unflag happy path → 204. +- Unflag for a row that doesn't exist → 404 `quarantine_not_found`. +- ListMine with two flags → returns two rows, newest first. +- Unauthenticated requests → 401 across the board (the existing `RequireUser` middleware handles this; one assertion is enough). + +- [ ] **Step 8.5: Commit** + +```bash +go test ./internal/api/... -run TestFlag -count=1 +git add internal/api/quarantine.go internal/api/quarantine_test.go internal/api/api.go +git commit -m "feat(api): /api/quarantine user-facing CRUD" +``` + +--- + +### Task 9 — `/api/admin/quarantine/*` admin handlers + +**Files:** +- Create: `internal/api/admin_quarantine.go` +- Create: `internal/api/admin_quarantine_test.go` +- Modify: `internal/api/api.go` to mount under the existing `/api/admin/*` route group + +Five endpoints: GET queue, GET actions, three POST resolution actions. Reuse the `RequireAdmin` middleware from M5a. + +- [ ] **Step 9.1: Write `admin_quarantine.go`** + +```go +package api + +import ( + "encoding/json" + "errors" + "net/http" + "strconv" + + "github.com/go-chi/chi/v5" + + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarr" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrquarantine" +) + +// adminQueueRowView is the wire shape returned by GET /api/admin/quarantine. +type adminQueueRowView struct { + TrackID string `json:"track_id"` + TrackTitle string `json:"track_title"` + ArtistName string `json:"artist_name"` + AlbumTitle *string `json:"album_title,omitempty"` + AlbumID string `json:"album_id"` + LidarrAlbumMBID *string `json:"lidarr_album_mbid,omitempty"` + ReportCount int32 `json:"report_count"` + LatestAt string `json:"latest_at"` + ReasonCounts map[string]int `json:"reason_counts"` + Reports []adminQueueReportView `json:"reports"` +} + +type adminQueueReportView struct { + UserID string `json:"user_id"` + Username string `json:"username"` + Reason string `json:"reason"` + Notes *string `json:"notes,omitempty"` + CreatedAt string `json:"created_at"` +} + +func (h *handlers) handleListAdminQuarantine(w http.ResponseWriter, r *http.Request) { + rows, err := h.lidarrQuarantine.ListAdminQueue(r.Context()) + if err != nil { + h.logger.Error("admin: list quarantine", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + out := make([]adminQueueRowView, 0, len(rows)) + for _, r := range rows { + reports := make([]adminQueueReportView, 0, len(r.Reports)) + for _, rep := range r.Reports { + reports = append(reports, adminQueueReportView{ + UserID: uuidToString(rep.UserID), + Username: rep.Username, + Reason: rep.Reason, + Notes: rep.Notes, + CreatedAt: rep.CreatedAt.Time.Format("2006-01-02T15:04:05Z07:00"), + }) + } + out = append(out, adminQueueRowView{ + TrackID: uuidToString(r.TrackID), TrackTitle: r.TrackTitle, + ArtistName: r.ArtistName, AlbumTitle: r.AlbumTitle, + AlbumID: uuidToString(r.AlbumID), LidarrAlbumMBID: r.LidarrAlbumMBID, + ReportCount: r.ReportCount, + LatestAt: r.LatestAt.Time.Format("2006-01-02T15:04:05Z07:00"), + ReasonCounts: r.ReasonCounts, Reports: reports, + }) + } + writeJSON(w, http.StatusOK, out) +} + +type actionResultView struct { + ActionID string `json:"action_id"` + AffectedUsers int32 `json:"affected_users"` + DeletedTrackCount *int `json:"deleted_track_count,omitempty"` +} + +func (h *handlers) handleResolveQuarantine(w http.ResponseWriter, r *http.Request) { + admin, _ := auth.UserFromContext(r.Context()) + id, ok := parseUUID(chi.URLParam(r, "track_id")) + if !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + action, err := h.lidarrQuarantine.Resolve(r.Context(), id, admin.ID) + if err != nil { + if errors.Is(err, lidarrquarantine.ErrTrackNotFound) { + writeAdminJSONErr(w, http.StatusNotFound, "track_not_found") + return + } + h.logger.Error("admin: resolve quarantine", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + writeJSON(w, http.StatusOK, actionResultView{ + ActionID: uuidToString(action.ID), AffectedUsers: action.AffectedUsers, + }) +} + +func (h *handlers) handleDeleteQuarantineFile(w http.ResponseWriter, r *http.Request) { + admin, _ := auth.UserFromContext(r.Context()) + id, ok := parseUUID(chi.URLParam(r, "track_id")) + if !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + action, err := h.lidarrQuarantine.DeleteFile(r.Context(), id, admin.ID) + if err != nil { + switch { + case errors.Is(err, lidarrquarantine.ErrTrackNotFound): + writeAdminJSONErr(w, http.StatusNotFound, "track_not_found") + default: + h.logger.Error("admin: delete file", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "file_delete_failed") + } + return + } + writeJSON(w, http.StatusOK, actionResultView{ + ActionID: uuidToString(action.ID), AffectedUsers: action.AffectedUsers, + }) +} + +func (h *handlers) handleDeleteQuarantineViaLidarr(w http.ResponseWriter, r *http.Request) { + admin, _ := auth.UserFromContext(r.Context()) + id, ok := parseUUID(chi.URLParam(r, "track_id")) + if !ok { + writeAdminJSONErr(w, http.StatusBadRequest, "invalid_id") + return + } + action, deleted, err := h.lidarrQuarantine.DeleteViaLidarr(r.Context(), id, admin.ID) + if err != nil { + switch { + case errors.Is(err, lidarrquarantine.ErrLidarrDisabled): + writeAdminJSONErr(w, http.StatusServiceUnavailable, "lidarr_disabled") + case errors.Is(err, lidarrquarantine.ErrTrackNotFound): + writeAdminJSONErr(w, http.StatusNotFound, "track_not_found") + case errors.Is(err, lidarrquarantine.ErrAlbumMBIDMissing): + writeAdminJSONErr(w, http.StatusNotFound, "album_mbid_missing") + case errors.Is(err, lidarrquarantine.ErrLidarrAlbumNotFound): + writeAdminJSONErr(w, http.StatusBadGateway, "lidarr_album_lookup_failed") + case errors.Is(err, lidarr.ErrUnreachable): + writeAdminJSONErr(w, http.StatusServiceUnavailable, "lidarr_unreachable") + case errors.Is(err, lidarr.ErrAuthFailed): + writeAdminJSONErr(w, http.StatusServiceUnavailable, "lidarr_auth_failed") + default: + h.logger.Error("admin: delete via lidarr", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + } + return + } + writeJSON(w, http.StatusOK, actionResultView{ + ActionID: uuidToString(action.ID), AffectedUsers: action.AffectedUsers, + DeletedTrackCount: &deleted, + }) +} + +type actionLogView struct { + ID string `json:"id"` + TrackID string `json:"track_id"` + TrackTitle string `json:"track_title"` + ArtistName string `json:"artist_name"` + AlbumTitle *string `json:"album_title,omitempty"` + Action string `json:"action"` + AdminID *string `json:"admin_id,omitempty"` + LidarrAlbumMBID *string `json:"lidarr_album_mbid,omitempty"` + AffectedUsers int32 `json:"affected_users"` + CreatedAt string `json:"created_at"` +} + +func (h *handlers) handleListQuarantineActions(w http.ResponseWriter, r *http.Request) { + limitStr := r.URL.Query().Get("limit") + limit := int32(50) + if limitStr != "" { + if v, err := strconv.Atoi(limitStr); err == nil && v > 0 && v <= 200 { + limit = int32(v) + } + } + rows, err := dbq.New(h.pool).ListQuarantineActions(r.Context(), limit) + if err != nil { + h.logger.Error("admin: list actions", "err", err) + writeAdminJSONErr(w, http.StatusInternalServerError, "server_error") + return + } + out := make([]actionLogView, 0, len(rows)) + for _, row := range rows { + var adminID *string + if row.AdminID.Valid { + s := uuidToString(row.AdminID) + adminID = &s + } + out = append(out, actionLogView{ + ID: uuidToString(row.ID), TrackID: uuidToString(row.TrackID), + TrackTitle: row.TrackTitle, ArtistName: row.ArtistName, AlbumTitle: row.AlbumTitle, + Action: string(row.Action), AdminID: adminID, + LidarrAlbumMBID: row.LidarrAlbumMbid, AffectedUsers: row.AffectedUsers, + CreatedAt: row.CreatedAt.Time.Format("2006-01-02T15:04:05Z07:00"), + }) + } + writeJSON(w, http.StatusOK, out) +} +``` + +`uuidToString` and `parseUUID` are existing helpers in `internal/api/`. Reuse them. + +- [ ] **Step 9.2: Mount routes** + +Inside the `/api/admin` route group: + +```go +r.Get("/api/admin/quarantine", h.handleListAdminQuarantine) +r.Post("/api/admin/quarantine/{track_id}/resolve", h.handleResolveQuarantine) +r.Post("/api/admin/quarantine/{track_id}/delete-file", h.handleDeleteQuarantineFile) +r.Post("/api/admin/quarantine/{track_id}/delete-via-lidarr", h.handleDeleteQuarantineViaLidarr) +r.Get("/api/admin/quarantine/actions", h.handleListQuarantineActions) +``` + +- [ ] **Step 9.3: Tests (`internal/api/admin_quarantine_test.go`)** + +Mirror M5a's `internal/api/admin_requests_test.go`. Cover: +- Aggregated queue shape: 3 users × 1 track yields 1 row with `report_count=3`, correct `reason_counts`. +- Resolve clears rows, returns 200 with `affected_users`. +- Delete file: file vanishes from disk, row gone, audit row written. +- Delete via Lidarr with stub server: lookup → DELETE → row removed; happy path 200 with `deleted_track_count`. +- Delete via Lidarr with `lidarr_disabled` config → 503 `lidarr_disabled`. +- Delete via Lidarr with stub returning empty array on lookup → 502 `lidarr_album_lookup_failed`. +- Delete via Lidarr on a track with no album MBID → 404 `album_mbid_missing`. +- Non-admin user → 403 across all admin endpoints. +- Action log GET returns rows ordered newest-first. + +- [ ] **Step 9.4: Commit** + +```bash +docker run --rm --network minstrel_minstrel -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm go test -race -p 1 ./internal/api/... -run AdminQuarantine +git add internal/api/admin_quarantine.go internal/api/admin_quarantine_test.go internal/api/api.go +git commit -m "feat(api): /api/admin/quarantine queue + resolve/delete-file/delete-via-lidarr" +``` + +--- + +### Task 10 — Wire `Service` into `cmd/minstrel/main.go` + +**Files:** Modify `cmd/minstrel/main.go`. + +The handlers struct (Task 8 step 8.3) now expects a `*lidarrquarantine.Service`. Construct it at startup and pass it through. + +- [ ] **Step 10.1: Add the construction** + +Find where the M5a `lidarrrequests.Service` is constructed in `main.go`. Right after it, add: + +```go +quarSvc := lidarrquarantine.NewService(pool, lidarrCfg, func() *lidarr.Client { + cfg, err := lidarrCfg.Get(ctx) + if err != nil || !cfg.Enabled { + return nil + } + return lidarr.NewClient(cfg.BaseURL, cfg.APIKey) +}) +``` + +If a similar `clientFn` already exists for `lidarrrequests`, reuse it instead of duplicating the closure. Pass `quarSvc` into the handlers constructor. + +- [ ] **Step 10.2: Build + smoke test** + +```bash +go build ./cmd/minstrel +go vet ./... +golangci-lint run ./... +``` + +Expected: clean build, no lint warnings. If `golangci-lint` isn't installed, skip. + +- [ ] **Step 10.3: Commit** + +```bash +git add cmd/minstrel/main.go internal/api/api.go +git commit -m "feat(cmd): wire lidarrquarantine.Service into the API handlers" +``` + +--- + +### Task 11 — Frontend: API client modules + types + +**Files:** Create `web/src/lib/api/quarantine.ts` + `quarantine.test.ts`. Modify `web/src/lib/api/types.ts`, `queries.ts`, `admin.ts`. Mirror the existing M5a pattern from `requests.ts` / `admin.ts`. + +- [ ] **Step 11.1: Add types to `types.ts`** + +```ts +export type LidarrQuarantineReason = 'bad_rip' | 'wrong_file' | 'wrong_tags' | 'duplicate' | 'other'; + +export type LidarrQuarantineRow = { + user_id: string; + track_id: string; + reason: LidarrQuarantineReason; + notes?: string | null; + created_at: string; +}; + +export type LidarrQuarantineMineRow = LidarrQuarantineRow & { + track: TrackRef; + album: AlbumRef; + artist: ArtistRef; +}; + +export type AdminQuarantineRow = { + track_id: string; + track_title: string; + artist_name: string; + album_title?: string | null; + album_id: string; + lidarr_album_mbid?: string | null; + report_count: number; + latest_at: string; + reason_counts: Record; + reports: AdminQuarantineReport[]; +}; + +export type AdminQuarantineReport = { + user_id: string; + username: string; + reason: LidarrQuarantineReason; + notes?: string | null; + created_at: string; +}; + +export type LidarrQuarantineAction = 'resolved' | 'deleted_file' | 'deleted_via_lidarr'; + +export type LidarrQuarantineActionRow = { + id: string; + track_id: string; + track_title: string; + artist_name: string; + album_title?: string | null; + action: LidarrQuarantineAction; + admin_id?: string | null; + lidarr_album_mbid?: string | null; + affected_users: number; + created_at: string; +}; + +export type ActionResult = { + action_id: string; + affected_users: number; + deleted_track_count?: number; +}; +``` + +- [ ] **Step 11.2: Add query keys to `queries.ts`** + +```ts +qk.myQuarantine = () => ['myQuarantine'] as const; +qk.adminQuarantine = () => ['adminQuarantine'] as const; +qk.adminQuarantineActions = (limit?: number) => ['adminQuarantineActions', { limit: limit ?? 50 }] as const; +``` + +(Keep the existing `qk` object syntax — append the new functions.) + +- [ ] **Step 11.3: Write `quarantine.ts`** + +```ts +import { createQuery } from '@tanstack/svelte-query'; +import { api, apiFetch } from './client'; +import { qk } from './queries'; +import type { + LidarrQuarantineRow, + LidarrQuarantineMineRow, + LidarrQuarantineReason +} from './types'; + +export type FlagParams = { + track_id: string; + reason: LidarrQuarantineReason; + notes?: string; +}; + +export async function flagTrack(params: FlagParams): Promise { + const body: FlagParams = { track_id: params.track_id, reason: params.reason }; + if (params.notes && params.notes.length > 0) body.notes = params.notes; + return api.post('/api/quarantine', body); +} + +// Server returns 204 (no body) for DELETE; handle accordingly. +export async function unflagTrack(trackID: string): Promise { + await apiFetch(`/api/quarantine/${trackID}`, { method: 'DELETE' }); +} + +export async function listMyQuarantine(): Promise { + return api.get('/api/quarantine/mine'); +} + +export function createMyQuarantineQuery() { + return createQuery({ + queryKey: qk.myQuarantine(), + queryFn: listMyQuarantine, + staleTime: 60_000 + }); +} +``` + +- [ ] **Step 11.4: Append admin endpoints to `admin.ts`** + +```ts +import type { AdminQuarantineRow, ActionResult, LidarrQuarantineActionRow } from './types'; + +export async function listAdminQuarantine(): Promise { + return api.get('/api/admin/quarantine'); +} + +export async function resolveQuarantine(trackID: string): Promise { + return api.post(`/api/admin/quarantine/${trackID}/resolve`, {}); +} + +export async function deleteQuarantineFile(trackID: string): Promise { + return api.post(`/api/admin/quarantine/${trackID}/delete-file`, {}); +} + +export async function deleteQuarantineViaLidarr(trackID: string): Promise { + return api.post(`/api/admin/quarantine/${trackID}/delete-via-lidarr`, {}); +} + +export async function listQuarantineActions(limit = 50): Promise { + return api.get(`/api/admin/quarantine/actions?limit=${limit}`); +} + +export function createAdminQuarantineQuery() { + return createQuery({ + queryKey: qk.adminQuarantine(), + queryFn: listAdminQuarantine, + staleTime: 30_000 // queue should refresh more aggressively than my-history + }); +} + +export function createQuarantineActionsQuery(limit = 50) { + return createQuery({ + queryKey: qk.adminQuarantineActions(limit), + queryFn: () => listQuarantineActions(limit), + staleTime: 60_000 + }); +} +``` + +- [ ] **Step 11.5: Tests** + +`quarantine.test.ts` — mirror `requests.test.ts` shape: `vi.mock('./client')`, assert URL + body shapes, return-value flow-through. Cover flagTrack (with + without notes), unflagTrack, listMyQuarantine, query factory, qk shape. + +For `admin.ts`: extend the existing test file to cover the five new functions + their query factories. + +- [ ] **Step 11.6: Run + commit** + +```bash +cd web && npm run check && npm test -- --run && cd - +git add web/src/lib/api/ +git commit -m "feat(web): API client modules for quarantine + admin quarantine" +``` + +--- + +### Task 12 — Frontend: `` + `` components + +**Files:** Create `web/src/lib/components/TrackMenu.svelte`, `TrackMenu.test.ts`, `FlagPopover.svelte`, `FlagPopover.test.ts`. + +`` is a kebab button + dropdown menu. For M5b it has one item: "Flag this track…". Component is structured so future actions slot in alongside. + +`` is the reason form, opened from TrackMenu. Pre-fills if the user already has a quarantine on this track. + +- [ ] **Step 12.1: Write `TrackMenu.svelte`** + +```svelte + + + (menuOpen = false)} + onkeydown={(e) => e.key === 'Escape' && closeAll()} +/> + +
+ + + {#if menuOpen} + + {/if} + + {#if popoverOpen} + + {/if} +
+``` + +- [ ] **Step 12.2: Write `FlagPopover.svelte`** + +```svelte + + + + +``` + +- [ ] **Step 12.3: Tests** + +`TrackMenu.test.ts`: +- Click kebab → menu visible. +- Click outside → menu closes. +- Escape → menu closes. +- Click "Flag this track…" → popover opens. + +`FlagPopover.test.ts`: +- Defaults to reason=`bad_rip` when no initialReason. +- Pre-fills when initialReason+initialNotes are provided; button reads "Update flag". +- Submit calls `flagTrack` (mocked) with the typed reason + non-empty notes; empty notes are NOT sent. +- Cancel calls onClose; does not call flagTrack. +- Submit calls `invalidateQueries` on success. + +For `flagTrack` mock pattern, copy from `web/src/routes/admin/integrations/integrations.test.ts` (the `vi.mock('$lib/api/admin', ...)` shape). + +- [ ] **Step 12.4: Run + commit** + +```bash +cd web && npm run check && npm test -- --run TrackMenu FlagPopover && cd - +git add web/src/lib/components/TrackMenu.svelte web/src/lib/components/TrackMenu.test.ts \ + web/src/lib/components/FlagPopover.svelte web/src/lib/components/FlagPopover.test.ts +git commit -m "feat(web): TrackMenu overflow + FlagPopover for the quarantine flow" +``` + +--- + +### Task 13 — Mount `` in `TrackRow` + `PlayerBar` + +**Files:** Modify `TrackRow.svelte`, `TrackRow.test.ts`, `PlayerBar.svelte`, `PlayerBar.test.ts`. + +- [ ] **Step 13.1: TrackRow** + +Add `` as a sibling to `` in the row's right cluster: + +```svelte + + + + + +``` + +- [ ] **Step 13.2: PlayerBar** + +Same: add the `` to the right cluster, after the like button. The `track` prop is the currently-playing track from the player store (e.g. `player.current`). + +```svelte +{#if player.current} + + +{/if} +``` + +- [ ] **Step 13.3: Update tests** + +Both `TrackRow.test.ts` and `PlayerBar.test.ts` — add an assertion that the track-actions kebab is rendered. Existing tests (like-button presence, play-on-click) should still pass. + +- [ ] **Step 13.4: Run + commit** + +```bash +cd web && npm test -- --run TrackRow PlayerBar && cd - +git add web/src/lib/components/TrackRow.svelte web/src/lib/components/TrackRow.test.ts \ + web/src/lib/components/PlayerBar.svelte web/src/lib/components/PlayerBar.test.ts +git commit -m "feat(web): mount TrackMenu in TrackRow + PlayerBar" +``` + +--- + +### Task 14 — Frontend: `/library/hidden` route + +**Files:** Create `web/src/routes/library/hidden/+page.svelte` + `hidden.test.ts`. Modify `Shell.svelte` to add `Hidden` to the main nav (between `Liked` and `Search`). + +- [ ] **Step 14.1: Write the page** + +Pattern matches `/requests` exactly — same row anatomy. Use `createMyQuarantineQuery()` for data, `unflagTrack(trackID)` + `invalidateQueries(qk.myQuarantine())` for the un-hide affordance. Empty state copy: "Nothing hidden yet." + +Each row (mirroring `/requests`): +- 56px album art with Lucide `Music2` fallback. +- Pills: kind ("Track", accent-tint), reason (`bad_rip` etc., accent-tint). +- Title (Parchment) + meta line "by `` · `` · flagged 2d ago". +- Notes (Vellum, italic) — only when present. +- Action: Un-hide (Pewter ghost + Lucide `RotateCcw`). One click — no confirmation. Optimistic remove. + +Header: H2 "Hidden" (Fraunces 24/500) + subtitle "Tracks you've flagged as broken." + +- [ ] **Step 14.2: Update Shell** + +Add to `navItems`: + +```ts +{ href: '/library/hidden', label: 'Hidden' } +``` + +Position: after `Liked`, before `Search`. Update `Shell.test.ts` to assert the new link's order. + +- [ ] **Step 14.3: Tests** + +`hidden.test.ts`: +- Renders one row per quarantine. +- Un-hide click calls `unflagTrack` + invalidates query. +- Empty state shows "Nothing hidden yet." +- Notes render (italic) when present, absent otherwise. + +- [ ] **Step 14.4: Commit** + +```bash +cd web && npm run check && npm test -- --run hidden && cd - +git add web/src/routes/library/hidden/ web/src/lib/components/Shell.svelte web/src/lib/components/Shell.test.ts +git commit -m "feat(web): /library/hidden user-facing quarantine view" +``` + +--- + +### Task 15 — Frontend: `/admin/quarantine` route + sidebar promotion + +**Files:** Create `web/src/routes/admin/quarantine/+page.svelte` + `quarantine.test.ts`. Modify `AdminSidebar.svelte` (promote Quarantine from placeholder), `AdminSidebar.test.ts`. + +- [ ] **Step 15.1: Promote sidebar item** + +In `AdminSidebar.svelte`, change: + +```diff +-{ href: '/admin/quarantine', label: 'Quarantine', icon: ShieldX, placeholder: true } ++{ href: '/admin/quarantine', label: 'Quarantine', icon: ShieldX } +``` + +Update `AdminSidebar.test.ts`: +- Replace the placeholder-treatment assertion for Quarantine. +- Add an assertion that Quarantine renders as a real `` link. +- Add an assertion that `/admin/quarantine` activates Quarantine in the sidebar. + +- [ ] **Step 15.2: Write the page** + +Page layout (matches the design-system spec from §6 of the spec): + +- Header: H2 "Quarantine" + accent-tint count pill when `report_count > 0`. +- Empty state: "Nothing to triage right now." +- Aggregated rows: 56px art · title + meta · reason-distribution pills · expandable per-user reports · inline play button (accent-colored — brand moment) · action cluster (Resolve / Delete file / Delete via Lidarr). +- Modal-confirm for Delete file. +- Typed-confirm modal for Delete via Lidarr (matches M5a Disconnect pattern; trim equality on "DELETE"). +- Dimmed Delete-via-Lidarr button when `lidarr_album_mbid` is null, with `title` attribute "Local-only track — no Lidarr album to remove." +- Lidarr-unreachable error → toast (reuse the `errorCopy` helper pattern from `/admin/requests`). + +Use `createAdminQuarantineQuery()` for data. On mutation success, `invalidateQueries({ queryKey: qk.adminQuarantine() })`. + +Use the existing player's `enqueueTrack` (or `playRadio` — pick the closest-fit existing action) for the inline Play button. + +- [ ] **Step 15.3: Tests (`quarantine.test.ts`)** + +Cover: +- Aggregated rows render with reason distribution. +- Expand caret reveals per-user reports. +- Resolve fires `resolveQuarantine` + invalidates. +- Delete file → modal-confirm → fires `deleteQuarantineFile`. +- Delete via Lidarr → typed-confirm modal "DELETE" → fires `deleteQuarantineViaLidarr`. +- Lidarr-unreachable error → toast renders with the spec copy ("Lidarr is unreachable…"). +- Dimmed Delete-via-Lidarr when MBID missing. +- Inline play button calls the player. +- Empty state copy. + +- [ ] **Step 15.4: Commit** + +```bash +cd web && npm run check && npm test -- --run admin/quarantine && cd - +git add web/src/routes/admin/quarantine/ web/src/lib/components/AdminSidebar.svelte web/src/lib/components/AdminSidebar.test.ts +git commit -m "feat(web): /admin/quarantine aggregated queue with resolution actions" +``` + +--- + +### Task 16 — Final verification + branch finish + +- [ ] **Step 16.1: Full Go test sweep** + +```bash +go test -short -race ./... +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race -p 1 ./... +``` + +Expected: short suite + integration suite both green. The pre-existing `internal/library/TestScanner_Integration` flake is documented and not blocked on (per `project_scanner_flake.md`). + +- [ ] **Step 16.2: Lint clean** + +```bash +golangci-lint run ./... +``` + +- [ ] **Step 16.3: Coverage check** + +```bash +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + bash -c 'go test -race -coverprofile=/tmp/cov.out \ + ./internal/lidarr/... ./internal/lidarrquarantine/... \ + ./internal/library/... && go tool cover -func=/tmp/cov.out | tail -1' +``` + +Expected: combined ≥ 80%, per spec §8. Both `internal/lidarr/` and `internal/lidarrquarantine/` should individually clear 80%. + +- [ ] **Step 16.4: Frontend full check** + +```bash +cd web && npm run check && npm test -- --run && npm run build +``` + +Expected: 0 errors, all vitest tests pass, build succeeds. + +- [ ] **Step 16.5: Manual smoke** + +- Configure Lidarr in `/admin/integrations` (or use the M5a-saved config). +- Flag a track from the now-playing bar → confirm it disappears from the album page. +- Confirm `/library/hidden` shows the flagged track. +- Un-hide → track returns to album page. +- Re-flag from a different user (admin user A flags as `bad_rip`, then user B flags same track as `wrong_tags`). +- Open `/admin/quarantine` → aggregated row shows count=2, distribution `1× bad_rip, 1× wrong_tags`. +- Click Play → track plays in the player. +- Click Resolve → row clears for both users. +- Re-flag, then click Delete file → file gone from disk, row gone from queue. +- Re-flag a track on a Lidarr-managed album → click Delete via Lidarr → typed-confirm "DELETE" → confirm Lidarr received DELETE call (check Lidarr's UI/logs). +- Verify non-admin user redirected when navigating to `/admin/quarantine`. + +- [ ] **Step 16.6: Use `superpowers:finishing-a-development-branch`** + +Verify tests are still green, then run the skill to present finish options (merge / PR / keep / discard). Default for this slice is "create a PR to main" matching the established cadence (per `project_git_workflow` memory). + +--- + +## Self-review checklist (run before declaring the plan ready) + +**Spec coverage** — every spec section maps to a task: +- §3 Architecture: Tasks 2 (client extensions), 3 (library), 4-5 (Service), 7 (soft-hide enforcement), 10 (wiring) +- §4 Schema: Task 1 +- §5 API surface: Tasks 8 (user), 9 (admin) +- §6 UI surfaces: Tasks 12 (TrackMenu+FlagPopover), 13 (mount), 14 (/library/hidden), 15 (/admin/quarantine + sidebar) +- §7 Error handling: distributed across Tasks 8, 9 (handler error mapping); Service layer (Tasks 4, 5) defines the typed errors +- §8 Testing: every Task includes tests; Task 16 verifies coverage targets +- §9 Decisions ledger: not directly implemented but referenced in commit messages +- §10 Out of scope: explicitly excluded — no album/artist quarantine, no bulk operations, no auto-resolve, no Subsonic honoring +- §11 Open questions: position of `/library/hidden` in nav (Task 14, between Liked and Search per the spec); dimmed-with-tooltip for missing MBID (Task 15) + +**Placeholder scan:** the per-task detail level drops after Task 9 (frontend tasks become 1-2 paragraphs) — intentional for navigability. Tasks 14 and 15 reference established patterns from M5a (`/requests` page anatomy, M5a typed-confirm modal for Disconnect) without re-stating the full code. No "TBD" or "TODO" remains. + +**Type consistency:** +- Service method names: `Flag`, `Unflag`, `ListMine`, `ListAdminQueue`, `Resolve`, `DeleteFile`, `DeleteViaLidarr` — consistent across plan +- Error names: `ErrBadReason`, `ErrTrackNotFound`, `ErrQuarantineNotFound`, `ErrAlbumMBIDMissing`, `ErrLidarrAlbumNotFound`, `ErrLidarrDisabled` — consistent +- Lidarr client method names: `LookupArtistByMBID`, `LookupAlbumByMBID`, `DeleteAlbum` — consistent +- API paths match spec §5 +- Component names: ``, ``, `` (mentioned in file map but not separately tasked — bake into Tasks 14 & 15 as inline JSX) +- DB column names: `lidarr_quarantine.{user_id,track_id,reason,notes,created_at}`, `lidarr_quarantine_actions.{id,track_id,track_title,artist_name,album_title,action,admin_id,lidarr_album_mbid,affected_users,created_at}` — consistent + +Plan is complete. diff --git a/docs/superpowers/plans/2026-04-30-m5c-suggested-additions.md b/docs/superpowers/plans/2026-04-30-m5c-suggested-additions.md new file mode 100644 index 00000000..d73588c5 --- /dev/null +++ b/docs/superpowers/plans/2026-04-30-m5c-suggested-additions.md @@ -0,0 +1,1838 @@ +# M5c — Suggested additions on `/discover` — 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:** Personalized artist suggestions on `/discover` (search-input-empty state). Top-12 out-of-library artists ranked by per-user signal (likes ×5 + recency-decayed plays), with top-3 contributing seeds attributed per card. One-click add via the existing M5a Lidarr-request flow. + +**Architecture:** Extend the M4b similarity ingest worker to persist unmatched artist MBIDs to a new `artist_similarity_unmatched` table (mirrors `artist_similarity` shape). New `internal/recommendation` service runs a single CTE at request time that scores candidates from the user's likes + plays through the unmatched table. New `GET /api/discover/suggestions` handler. Frontend swaps `` between the existing search results and a new suggestion feed when the search input is empty. + +**Tech Stack:** Go 1.23 · pgx/v5 + sqlc · Postgres + golang-migrate · SvelteKit 2 / Svelte 5 (runes) · TanStack Query · Vitest · existing FabledSword design tokens. + +**Spec:** [`docs/superpowers/specs/2026-04-30-m5c-suggested-additions-design.md`](../specs/2026-04-30-m5c-suggested-additions-design.md). Read it before starting — every decision is explained there. + +**Memory dependencies:** `project_design_system.md` (FabledSword tokens), `project_subsonic_legacy.md` (`/api/*` is primary), `project_no_github.md` (Forgejo MCP for PR ops), `project_git_workflow.md` (commit on `dev`; PR to `main` separately). + +--- + +## File map + +### Backend — create + +- `internal/db/migrations/0012_artist_similarity_unmatched.up.sql` · `.down.sql` +- `internal/recommendation/suggestions.go` — `SuggestArtists` service + types +- `internal/recommendation/suggestions_integration_test.go` +- `internal/api/suggestions.go` — `GET /api/discover/suggestions` handler +- `internal/api/suggestions_test.go` + +### Backend — modify + +- `internal/db/queries/similarity.sql` — add `UpsertArtistSimilarityUnmatched` +- `internal/db/queries/recommendation.sql` — add `SuggestArtistsForUser` +- `internal/db/dbq/*` — regenerated by `sqlc generate` +- `internal/scrobble/listenbrainz/client.go` — add `Name string \`json:"name"\`` to `SimilarArtist` +- `internal/similarity/worker.go` — extend `upsertArtistSimilar` to persist unmatched MBIDs +- `internal/similarity/worker_test.go` — extend with `TestUpsertArtistSimilar_PersistsUnmatchedToTable` +- `internal/api/api.go` — register `/api/discover/suggestions` route + +### Frontend — create + +- `web/src/lib/api/suggestions.ts` — client (`listSuggestions`, `createSuggestionsQuery`) +- `web/src/lib/api/suggestions.test.ts` +- `web/src/lib/components/SuggestionFeed.svelte` — subcomponent for the suggestion grid +- `web/src/lib/components/SuggestionFeed.test.ts` + +### Frontend — modify + +- `web/src/lib/api/types.ts` — add `ArtistSuggestion`, `SeedContribution` +- `web/src/lib/api/queries.ts` — add `qk.suggestions(limit)` +- `web/src/lib/components/DiscoverResultCard.svelte` — add optional `attribution?: string` prop +- `web/src/lib/components/DiscoverResultCard.test.ts` — assert attribution rendering +- `web/src/routes/discover/+page.svelte` — empty-input branches to ``; tabs hide when input is empty +- `web/src/routes/discover/discover.test.ts` — extend with suggestion-feed tests + +--- + +## Task list + +### Task 1 — Migration 0012 + worker upsert query + +**Files:** +- Create: `internal/db/migrations/0012_artist_similarity_unmatched.up.sql` +- Create: `internal/db/migrations/0012_artist_similarity_unmatched.down.sql` +- Modify: `internal/db/queries/similarity.sql` +- Regenerate: `internal/db/dbq/*` + +- [ ] **Step 1.1: Write the up migration** + +`internal/db/migrations/0012_artist_similarity_unmatched.up.sql`: + +```sql +-- M5c: persist unmatched-similar-artist MBIDs that the M4b worker would +-- otherwise discard. Mirrors artist_similarity shape: same composite PK +-- with source, same (seed_id, score DESC) index, same source enum check. +-- The candidate side is text + name (no FK) — that's the whole point. + +CREATE TABLE artist_similarity_unmatched ( + seed_artist_id uuid NOT NULL REFERENCES artists(id) ON DELETE CASCADE, + candidate_mbid text NOT NULL, + candidate_name text NOT NULL, + score DOUBLE PRECISION NOT NULL, + source text NOT NULL CHECK (source IN ('listenbrainz', 'musicbrainz_tag', 'user_cooccurrence')), + fetched_at timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY (seed_artist_id, candidate_mbid, source) +); + +CREATE INDEX artist_similarity_unmatched_seed_score_idx + ON artist_similarity_unmatched (seed_artist_id, score DESC); +``` + +- [ ] **Step 1.2: Write the down migration** + +`internal/db/migrations/0012_artist_similarity_unmatched.down.sql`: + +```sql +DROP INDEX IF EXISTS artist_similarity_unmatched_seed_score_idx; +DROP TABLE IF EXISTS artist_similarity_unmatched; +``` + +- [ ] **Step 1.3: Append the worker upsert query** + +Append to `internal/db/queries/similarity.sql`: + +```sql +-- name: UpsertArtistSimilarityUnmatched :exec +-- Persists an out-of-library similar-artist MBID. Idempotent on +-- (seed_artist_id, candidate_mbid, source) — re-fetches refresh the +-- name/score and bump fetched_at. +INSERT INTO artist_similarity_unmatched ( + seed_artist_id, candidate_mbid, candidate_name, score, source +) VALUES ($1, $2, $3, $4, $5) +ON CONFLICT (seed_artist_id, candidate_mbid, source) DO UPDATE SET + candidate_name = EXCLUDED.candidate_name, + score = EXCLUDED.score, + fetched_at = now(); +``` + +- [ ] **Step 1.4: Add a `dbtest.ResetDB` entry for the new table** + +Modify `internal/dbtest/reset.go` — append `"artist_similarity_unmatched"` to the `dataTables` slice between `track_similarity` and `scrobble_queue` so M5c integration tests don't inherit residual rows. + +```go +var dataTables = []string{ + "artist_similarity", + "track_similarity", + "artist_similarity_unmatched", // M5c + "scrobble_queue", + // ... rest unchanged ... +} +``` + +- [ ] **Step 1.5: Regenerate sqlc** + +```bash +cd /home/bvandeusen/Nextcloud/Projects/Minstrel/minstrel && sqlc generate +go build ./... +go vet ./... +``` + +Expected: clean build. New method `UpsertArtistSimilarityUnmatched` appears in `internal/db/dbq/similarity.sql.go`. + +- [ ] **Step 1.6: Apply migration to verify it runs** + +```bash +docker compose exec -T postgres psql -U minstrel -d minstrel -c "DROP TABLE IF EXISTS artist_similarity_unmatched;" +# Migration applies on next test run / server start. +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race ./internal/db/... -count=1 +docker compose exec -T postgres psql -U minstrel -d minstrel -c "\d artist_similarity_unmatched" +``` + +Expected: `\d` shows the table with all columns + the seed_score index. + +- [ ] **Step 1.7: Commit** + +```bash +git add internal/db/migrations/0012_artist_similarity_unmatched.up.sql \ + internal/db/migrations/0012_artist_similarity_unmatched.down.sql \ + internal/db/queries/similarity.sql \ + internal/db/dbq/ \ + internal/dbtest/reset.go +git commit -m "feat(db): add artist_similarity_unmatched schema (migration 0012)" +``` + +--- + +### Task 2 — Extend `SimilarArtist` with `Name` field + +**Files:** +- Modify: `internal/scrobble/listenbrainz/client.go` — add `Name` to the struct +- Modify: `internal/scrobble/listenbrainz/client_test.go` (if existing tests break) — already-passing tests should still pass since adding a field is additive + +The current struct at `internal/scrobble/listenbrainz/client.go:241-245`: + +```go +type SimilarArtist struct { + MBID string `json:"artist_mbid"` + Score float64 `json:"score"` +} +``` + +ListenBrainz's `/1/explore/similar-artists/{mbid}` endpoint returns each row with `artist_mbid`, `name`, `score`, plus other fields we don't care about. Adding `Name` is a purely additive struct change; existing JSON unmarshal still works for everything else. + +- [ ] **Step 2.1: Add the field** + +Modify `internal/scrobble/listenbrainz/client.go:241-245`: + +```go +type SimilarArtist struct { + MBID string `json:"artist_mbid"` + Name string `json:"name"` + Score float64 `json:"score"` +} +``` + +- [ ] **Step 2.2: Verify existing tests still pass** + +```bash +go test ./internal/scrobble/listenbrainz/... -count=1 +``` + +Expected: all green. The unmarshal already ignored the `name` field; capturing it doesn't break anything. + +- [ ] **Step 2.3: Commit** + +```bash +git add internal/scrobble/listenbrainz/client.go +git commit -m "feat(listenbrainz): expose Name on SimilarArtist for M5c suggestions" +``` + +--- + +### Task 3 — Extend similarity worker to persist unmatched MBIDs + +**Files:** +- Modify: `internal/similarity/worker.go` — extend `upsertArtistSimilar` +- Modify: `internal/similarity/worker_test.go` — add `TestUpsertArtistSimilar_PersistsUnmatchedToTable` + +The current `upsertArtistSimilar` filters returned MBIDs to those in `idByMBID` (in-library only) and discards the rest. M5c keeps the matched-path unchanged but adds a parallel unmatched-persist loop with the same top-K cap. + +- [ ] **Step 3.1: Read the current `upsertArtistSimilar` shape** + +Read `internal/similarity/worker.go:170-200` (function body) before editing. It mirrors `upsertTrackSimilar` exactly — same sort, same `idByMBID`, same top-K pattern. The matched-loop pattern is the template. + +- [ ] **Step 3.2: Extend `upsertArtistSimilar`** + +Replace the function body (`internal/similarity/worker.go:170` onwards). The matched loop stays exactly as it was; we add a second loop that walks the same sorted `results` and persists unmatched rows up to `w.topK`: + +```go +func (w *Worker) upsertArtistSimilar(ctx context.Context, q *dbq.Queries, artistAID pgtype.UUID, results []listenbrainz.SimilarArtist) { + if len(results) == 0 { + return + } + sort.Slice(results, func(i, j int) bool { return results[i].Score > results[j].Score }) + + mbids := make([]string, 0, len(results)) + for _, r := range results { + mbids = append(mbids, r.MBID) + } + rows, err := q.GetArtistsByMBIDs(ctx, mbids) + if err != nil { + w.logger.Warn("similarity: GetArtistsByMBIDs", "err", err) + return + } + idByMBID := make(map[string]pgtype.UUID, len(rows)) + for _, r := range rows { + if r.Mbid != nil { + idByMBID[*r.Mbid] = r.ID + } + } + + // Matched: in-library similars → artist_similarity (existing path). + takenMatched := 0 + for _, r := range results { + if takenMatched >= w.topK { + break + } + localID, ok := idByMBID[r.MBID] + if !ok { + continue + } + if localID == artistAID { + continue // defensive — DB CHECK constraint also catches self-edges + } + if uerr := q.UpsertArtistSimilarity(ctx, dbq.UpsertArtistSimilarityParams{ + ArtistAID: artistAID, ArtistBID: localID, Score: r.Score, Source: "listenbrainz", + }); uerr != nil { + w.logger.Warn("similarity: UpsertArtistSimilarity", "err", uerr) + continue + } + takenMatched++ + } + + // Unmatched: out-of-library similars → artist_similarity_unmatched (M5c). + // Same top-K cap as the matched path; missing-name rows are skipped (we + // can't render a suggestion without an artist name). + takenUnmatched := 0 + for _, r := range results { + if takenUnmatched >= w.topK { + break + } + if _, inLib := idByMBID[r.MBID]; inLib { + continue + } + if r.Name == "" { + w.logger.Debug("similarity: skipping unmatched similar with empty name", "mbid", r.MBID) + continue + } + if uerr := q.UpsertArtistSimilarityUnmatched(ctx, dbq.UpsertArtistSimilarityUnmatchedParams{ + SeedArtistID: artistAID, + CandidateMbid: r.MBID, + CandidateName: r.Name, + Score: r.Score, + Source: "listenbrainz", + }); uerr != nil { + w.logger.Warn("similarity: UpsertArtistSimilarityUnmatched", "err", uerr) + continue + } + takenUnmatched++ + } +} +``` + +Note: the existing function used a single `taken` variable; the new version splits into `takenMatched` and `takenUnmatched` so each path is bounded independently. Verify the existing call to `UpsertArtistSimilarity` in your repo matches the signature you're substituting — sqlc may name the params struct differently. + +- [ ] **Step 3.3: Write `TestUpsertArtistSimilar_PersistsUnmatchedToTable`** + +Append to `internal/similarity/worker_test.go`: + +```go +func TestUpsertArtistSimilar_PersistsUnmatchedToTable(t *testing.T) { + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + pool := newPool(t) // existing helper + q := dbq.New(pool) + ctx := context.Background() + + // Seed one in-library artist that will be the in-library match. + inLibMBID := "in-lib-mbid-123" + inLibArtist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{ + Name: "InLib Artist", SortName: "InLib Artist", Mbid: &inLibMBID, + }) + if err != nil { + t.Fatalf("seed in-lib artist: %v", err) + } + + // Seed a "seed" artist (the one whose similars we're processing). + seedMBID := "seed-artist-mbid" + seedArtist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{ + Name: "Seed Artist", SortName: "Seed Artist", Mbid: &seedMBID, + }) + if err != nil { + t.Fatalf("seed artist: %v", err) + } + + w := &Worker{pool: pool, logger: newTestLogger(), topK: 10} + + similars := []listenbrainz.SimilarArtist{ + {MBID: inLibMBID, Name: "InLib Artist", Score: 0.95}, + {MBID: "out-mbid-1", Name: "Outsider One", Score: 0.85}, + {MBID: "out-mbid-2", Name: "Outsider Two", Score: 0.80}, + {MBID: "out-mbid-3", Name: "Outsider Three", Score: 0.70}, + {MBID: "out-mbid-4", Name: "", Score: 0.60}, // missing name — should be skipped + } + w.upsertArtistSimilar(ctx, q, seedArtist.ID, similars) + + // Matched path: 1 row in artist_similarity. + var matchedCount int + if err := pool.QueryRow(ctx, + "SELECT count(*) FROM artist_similarity WHERE artist_a_id = $1", + seedArtist.ID, + ).Scan(&matchedCount); err != nil { + t.Fatalf("count matched: %v", err) + } + if matchedCount != 1 { + t.Errorf("artist_similarity rows = %d, want 1 (only the in-library match)", matchedCount) + } + + // Unmatched path: 3 rows (out-mbid-1/2/3); the empty-name row is skipped. + var unmatchedCount int + if err := pool.QueryRow(ctx, + "SELECT count(*) FROM artist_similarity_unmatched WHERE seed_artist_id = $1", + seedArtist.ID, + ).Scan(&unmatchedCount); err != nil { + t.Fatalf("count unmatched: %v", err) + } + if unmatchedCount != 3 { + t.Errorf("artist_similarity_unmatched rows = %d, want 3", unmatchedCount) + } + + // Verify a specific row's name + score round-tripped correctly. + var name string + var score float64 + if err := pool.QueryRow(ctx, + "SELECT candidate_name, score FROM artist_similarity_unmatched WHERE seed_artist_id = $1 AND candidate_mbid = $2", + seedArtist.ID, "out-mbid-1", + ).Scan(&name, &score); err != nil { + t.Fatalf("fetch out-mbid-1: %v", err) + } + if name != "Outsider One" || score != 0.85 { + t.Errorf("row = (%q, %v), want (Outsider One, 0.85)", name, score) + } + + // suppress unused warning if inLibArtist isn't otherwise referenced + _ = inLibArtist +} +``` + +If `internal/similarity/worker_test.go` doesn't already have a `newPool(t)` and `newTestLogger()` helper, mirror the pattern from `internal/lidarrquarantine/service_test.go` — those have working examples. + +- [ ] **Step 3.4: Run tests** + +```bash +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race -count=1 ./internal/similarity/... +``` + +Expected: existing tests still pass + new test passes. + +- [ ] **Step 3.5: Commit** + +```bash +git add internal/similarity/worker.go internal/similarity/worker_test.go +git commit -m "feat(similarity): persist unmatched similar-artist MBIDs for M5c" +``` + +--- + +### Task 4 — `internal/recommendation` `SuggestArtists` service + +**Files:** +- Create: `internal/recommendation/suggestions.go` +- Create: `internal/recommendation/suggestions_integration_test.go` +- Modify: `internal/db/queries/recommendation.sql` — add the suggestion CTE query +- Regenerate: `internal/db/dbq/*` + +- [ ] **Step 4.1: Append the suggestion query** + +Append to `internal/db/queries/recommendation.sql`: + +```sql +-- name: SuggestArtistsForUser :many +-- M5c: per-user artist suggestions ranked by signal × similarity. The +-- seeds CTE collects the user's likes (×5) plus recency-decayed plays +-- (exp(-age_days / $2)). The contributions CTE joins those seeds against +-- artist_similarity_unmatched and filters out candidates already in +-- library or already in a non-terminal lidarr_request. The outer SELECT +-- aggregates per candidate, returning the top-3 contributing seeds for +-- attribution. $1=user_id, $2=half_life_days, $3=limit. +WITH seeds AS ( + SELECT a.id AS artist_id, + 5.0 * (CASE WHEN gla.artist_id IS NOT NULL THEN 1 ELSE 0 END) + + COALESCE(SUM(EXP(- EXTRACT(epoch FROM now() - pe.started_at) / ($2 * 86400.0))), 0) + AS signal, + (gla.artist_id IS NOT NULL) AS is_liked, + COUNT(pe.id) AS play_count + FROM artists a + LEFT JOIN general_likes_artists gla ON gla.artist_id = a.id AND gla.user_id = $1 + LEFT JOIN tracks t ON t.artist_id = a.id + LEFT JOIN play_events pe ON pe.track_id = t.id AND pe.user_id = $1 + WHERE gla.artist_id IS NOT NULL OR pe.id IS NOT NULL + GROUP BY a.id, gla.artist_id +), +contributions AS ( + SELECT u.candidate_mbid, + u.candidate_name, + seeds.artist_id AS seed_id, + seeds.is_liked, + seeds.play_count, + seeds.signal * u.score AS contribution + FROM artist_similarity_unmatched u + JOIN seeds ON seeds.artist_id = u.seed_artist_id + WHERE NOT EXISTS (SELECT 1 FROM artists WHERE mbid = u.candidate_mbid) + AND NOT EXISTS ( + SELECT 1 FROM lidarr_requests r + WHERE r.user_id = $1 + AND r.lidarr_artist_mbid = u.candidate_mbid + AND r.status NOT IN ('rejected', 'failed') + ) +) +SELECT candidate_mbid, + candidate_name, + SUM(contribution)::float8 AS total_score, + (array_agg(seed_id ORDER BY contribution DESC))[1:3] AS top_seed_ids, + (array_agg(contribution ORDER BY contribution DESC))[1:3] AS top_contributions, + (array_agg(is_liked ORDER BY contribution DESC))[1:3] AS top_is_liked, + (array_agg(play_count ORDER BY contribution DESC))[1:3] AS top_play_counts +FROM contributions +GROUP BY candidate_mbid, candidate_name +ORDER BY total_score DESC +LIMIT $3; +``` + +The extra arrays (`top_is_liked`, `top_play_counts`) let the SPA pick "liked"/"played" verbiage per attribution-line slot. + +- [ ] **Step 4.2: Regenerate sqlc** + +```bash +cd /home/bvandeusen/Nextcloud/Projects/Minstrel/minstrel && sqlc generate +go build ./... +``` + +Expected: clean. New method `SuggestArtistsForUser` in `internal/db/dbq/recommendation.sql.go`. Note the row type may have field names like `TopSeedIds` (plural-suffix suppressed by sqlc) — read the generated file before relying on names in the next step. + +- [ ] **Step 4.3: Write `internal/recommendation/suggestions.go`** + +```go +// suggestions.go is the M5c per-user artist-suggestion service. Reads +// the user's likes + plays, projects them through artist_similarity_unmatched +// via a single CTE, returns top-N candidates with top-3 attribution seeds +// resolved to artist names. +package recommendation + +import ( + "context" + "fmt" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +// ArtistSuggestion is one ranked candidate with its top-3 attribution seeds. +type ArtistSuggestion struct { + MBID string + Name string + Score float64 + Attribution []SeedContribution +} + +// SeedContribution is one of the top-3 contributing seeds for a candidate. +type SeedContribution struct { + ArtistID pgtype.UUID + Name string + Contribution float64 + IsLiked bool + PlayCount int64 +} + +// SuggestArtists returns top-N artist suggestions for the user. limit is +// capped at 50; halfLifeDays is the recency-decay half-life for plays +// (default 30, operator-tunable). +func SuggestArtists(ctx context.Context, pool *pgxpool.Pool, userID pgtype.UUID, halfLifeDays float64, limit int) ([]ArtistSuggestion, error) { + if limit <= 0 || limit > 50 { + limit = 12 + } + if halfLifeDays <= 0 { + halfLifeDays = 30 + } + q := dbq.New(pool) + rows, err := q.SuggestArtistsForUser(ctx, dbq.SuggestArtistsForUserParams{ + UserID: userID, + Column2: halfLifeDays, // sqlc names unbound positional params Column2/3 — verify + Limit: int32(limit), + }) + if err != nil { + return nil, fmt.Errorf("suggest: query: %w", err) + } + if len(rows) == 0 { + return []ArtistSuggestion{}, nil + } + + // Collect the union of top-3 seed IDs across all rows for one batched + // name lookup. + seedSet := make(map[pgtype.UUID]struct{}, len(rows)*3) + for _, r := range rows { + for _, sid := range r.TopSeedIds { + seedSet[sid] = struct{}{} + } + } + seedIDs := make([]pgtype.UUID, 0, len(seedSet)) + for id := range seedSet { + seedIDs = append(seedIDs, id) + } + artists, err := q.GetArtistsByIDs(ctx, seedIDs) + if err != nil { + return nil, fmt.Errorf("suggest: resolve seeds: %w", err) + } + nameByID := make(map[pgtype.UUID]string, len(artists)) + for _, a := range artists { + nameByID[a.ID] = a.Name + } + + out := make([]ArtistSuggestion, 0, len(rows)) + for _, r := range rows { + attribution := make([]SeedContribution, 0, len(r.TopSeedIds)) + for i, sid := range r.TopSeedIds { + if i >= len(r.TopContributions) { + break + } + attribution = append(attribution, SeedContribution{ + ArtistID: sid, + Name: nameByID[sid], + Contribution: r.TopContributions[i], + IsLiked: r.TopIsLiked[i], + PlayCount: r.TopPlayCounts[i], + }) + } + out = append(out, ArtistSuggestion{ + MBID: r.CandidateMbid, + Name: r.CandidateName, + Score: r.TotalScore, + Attribution: attribution, + }) + } + return out, nil +} +``` + +Two things to verify against the actual generated code in `internal/db/dbq/recommendation.sql.go`: +1. The param struct may name `$2` something other than `Column2` (sqlc occasionally renames). Look at `SuggestArtistsForUserParams` and use whatever's there. +2. `GetArtistsByIDs` may not exist — check `internal/db/queries/artists.sql`. If absent, add a query in this same task: + +```sql +-- name: GetArtistsByIDs :many +SELECT * FROM artists WHERE id = ANY($1::uuid[]); +``` + +(If `GetArtistsByMBIDs` exists but not `GetArtistsByIDs`, add the IDs variant; sqlc-regenerate.) + +- [ ] **Step 4.4: Write integration tests** + +Create `internal/recommendation/suggestions_integration_test.go`: + +```go +package recommendation + +import ( + "context" + "io" + "log/slog" + "os" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" +) + +func newPool(t *testing.T) *pgxpool.Pool { + t.Helper() + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); 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) + dbtest.ResetDB(t, pool) + return pool +} + +func seedUser(t *testing.T, pool *pgxpool.Pool, name string) dbq.User { + t.Helper() + u, err := dbq.New(pool).CreateUser(context.Background(), dbq.CreateUserParams{ + Username: dbtest.TestUserPrefix + name, PasswordHash: "x", + ApiToken: name + "-token", IsAdmin: false, + }) + if err != nil { + t.Fatalf("seed user: %v", err) + } + return u +} + +func seedArtist(t *testing.T, pool *pgxpool.Pool, name, mbid string) dbq.Artist { + t.Helper() + var mbidPtr *string + if mbid != "" { + mbidPtr = &mbid + } + a, err := dbq.New(pool).UpsertArtist(context.Background(), dbq.UpsertArtistParams{ + Name: name, SortName: name, Mbid: mbidPtr, + }) + if err != nil { + t.Fatalf("seed artist: %v", err) + } + return a +} + +func seedUnmatched(t *testing.T, pool *pgxpool.Pool, seedID pgtype.UUID, candMBID, candName string, score float64) { + t.Helper() + if err := dbq.New(pool).UpsertArtistSimilarityUnmatched(context.Background(), dbq.UpsertArtistSimilarityUnmatchedParams{ + SeedArtistID: seedID, + CandidateMbid: candMBID, + CandidateName: candName, + Score: score, + Source: "listenbrainz", + }); err != nil { + t.Fatalf("seed unmatched: %v", err) + } +} + +func TestSuggestArtists_LikesAndPlaysContributeToScore(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + seedA := seedArtist(t, pool, "Seed Liked", "") + seedB := seedArtist(t, pool, "Seed Played", "") + + // alice likes seedA. + if _, err := dbq.New(pool).LikeArtist(context.Background(), dbq.LikeArtistParams{ + UserID: user.ID, ArtistID: seedA.ID, + }); err != nil { + t.Fatalf("LikeArtist: %v", err) + } + // alice played seedB. (Need a play_event with the right artist via tracks.) + seedBAlbum := seedAlbumForArtist(t, pool, seedB.ID, "Album B") + seedBTrack := seedTrackOnAlbum(t, pool, seedBAlbum.ID, seedB.ID, "Track B") + insertPlayEvent(t, pool, user.ID, seedBTrack.ID, time.Now().Add(-1*time.Hour)) + + // Both seeds point at the same out-of-library candidate. + seedUnmatched(t, pool, seedA.ID, "out-mbid", "Outsider", 0.9) + seedUnmatched(t, pool, seedB.ID, "out-mbid", "Outsider", 0.5) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 1 { + t.Fatalf("len = %d, want 1", len(out)) + } + s := out[0] + if s.MBID != "out-mbid" || s.Name != "Outsider" { + t.Errorf("got = %+v", s) + } + if s.Score <= 0 { + t.Errorf("score = %v, want > 0", s.Score) + } + if len(s.Attribution) != 2 { + t.Errorf("attribution len = %d, want 2", len(s.Attribution)) + } +} + +func TestSuggestArtists_Top12Cap(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + seed := seedArtist(t, pool, "Seed", "") + if _, err := dbq.New(pool).LikeArtist(context.Background(), dbq.LikeArtistParams{ + UserID: user.ID, ArtistID: seed.ID, + }); err != nil { + t.Fatalf("LikeArtist: %v", err) + } + for i := 0; i < 30; i++ { + seedUnmatched(t, pool, seed.ID, fmt.Sprintf("mbid-%02d", i), fmt.Sprintf("Artist %02d", i), 0.99-float64(i)*0.01) + } + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 12 { + t.Errorf("len = %d, want 12", len(out)) + } + if out[0].MBID != "mbid-00" { + t.Errorf("first = %s, want mbid-00 (highest score)", out[0].MBID) + } +} + +func TestSuggestArtists_AttributionTopThree(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + // 5 seed artists all liked, all pointing at the same candidate but + // with descending similarity scores so contributions order is clean. + seeds := make([]dbq.Artist, 5) + for i := 0; i < 5; i++ { + seeds[i] = seedArtist(t, pool, fmt.Sprintf("Seed %d", i), "") + if _, err := dbq.New(pool).LikeArtist(context.Background(), dbq.LikeArtistParams{ + UserID: user.ID, ArtistID: seeds[i].ID, + }); err != nil { + t.Fatalf("LikeArtist: %v", err) + } + seedUnmatched(t, pool, seeds[i].ID, "shared-mbid", "Shared", 0.9-float64(i)*0.1) + } + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 1 { + t.Fatalf("len = %d, want 1 (shared candidate)", len(out)) + } + if got := len(out[0].Attribution); got != 3 { + t.Errorf("attribution len = %d, want 3", got) + } + // Verify ordering: highest contribution first (seed 0 with score 0.9). + if out[0].Attribution[0].Name != "Seed 0" { + t.Errorf("top attribution = %q, want Seed 0", out[0].Attribution[0].Name) + } +} + +func TestSuggestArtists_RecencyDecayDownweightsOldPlays(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + recentSeed := seedArtist(t, pool, "Recent", "") + oldSeed := seedArtist(t, pool, "Old", "") + + rAlbum := seedAlbumForArtist(t, pool, recentSeed.ID, "Recent Album") + rTrack := seedTrackOnAlbum(t, pool, rAlbum.ID, recentSeed.ID, "Recent Track") + insertPlayEvent(t, pool, user.ID, rTrack.ID, time.Now().Add(-1*24*time.Hour)) + + oAlbum := seedAlbumForArtist(t, pool, oldSeed.ID, "Old Album") + oTrack := seedTrackOnAlbum(t, pool, oAlbum.ID, oldSeed.ID, "Old Track") + insertPlayEvent(t, pool, user.ID, oTrack.ID, time.Now().Add(-90*24*time.Hour)) + + // Both seeds point at the same candidate with the same similarity score. + seedUnmatched(t, pool, recentSeed.ID, "cand", "Cand", 0.5) + seedUnmatched(t, pool, oldSeed.ID, "cand", "Cand", 0.5) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 1 { + t.Fatalf("len = %d, want 1", len(out)) + } + if len(out[0].Attribution) != 2 { + t.Fatalf("attribution len = %d, want 2", len(out[0].Attribution)) + } + // Recent seed contributes more than old seed. + if out[0].Attribution[0].Name != "Recent" { + t.Errorf("top attribution = %q, want Recent (1d-old play decays less than 90d)", out[0].Attribution[0].Name) + } + if out[0].Attribution[0].Contribution <= out[0].Attribution[1].Contribution { + t.Errorf("recent contribution (%v) should exceed old (%v)", + out[0].Attribution[0].Contribution, out[0].Attribution[1].Contribution) + } +} + +func TestSuggestArtists_FiltersInLibraryCandidates(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + seed := seedArtist(t, pool, "Seed", "") + if _, err := dbq.New(pool).LikeArtist(context.Background(), dbq.LikeArtistParams{ + UserID: user.ID, ArtistID: seed.ID, + }); err != nil { + t.Fatalf("LikeArtist: %v", err) + } + // Candidate that's already in library. + inLibMBID := "in-lib-mbid" + seedArtist(t, pool, "InLib", inLibMBID) + seedUnmatched(t, pool, seed.ID, inLibMBID, "InLib", 0.9) + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 0 { + t.Errorf("len = %d, want 0 (in-library candidate should be filtered)", len(out)) + } +} + +func TestSuggestArtists_FiltersAlreadyRequested(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + seed := seedArtist(t, pool, "Seed", "") + if _, err := dbq.New(pool).LikeArtist(context.Background(), dbq.LikeArtistParams{ + UserID: user.ID, ArtistID: seed.ID, + }); err != nil { + t.Fatalf("LikeArtist: %v", err) + } + seedUnmatched(t, pool, seed.ID, "req-mbid", "Pending Request", 0.9) + if _, err := dbq.New(pool).CreateLidarrRequest(context.Background(), dbq.CreateLidarrRequestParams{ + UserID: user.ID, + Kind: dbq.LidarrRequestKindArtist, + LidarrArtistMbid: "req-mbid", + ArtistName: "Pending Request", + }); err != nil { + t.Fatalf("CreateLidarrRequest: %v", err) + } + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 0 { + t.Errorf("len = %d, want 0 (pending request should hide candidate)", len(out)) + } +} + +func TestSuggestArtists_RejectedRequestStillShown(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "alice") + seed := seedArtist(t, pool, "Seed", "") + if _, err := dbq.New(pool).LikeArtist(context.Background(), dbq.LikeArtistParams{ + UserID: user.ID, ArtistID: seed.ID, + }); err != nil { + t.Fatalf("LikeArtist: %v", err) + } + seedUnmatched(t, pool, seed.ID, "rej-mbid", "Rejected Once", 0.9) + req, err := dbq.New(pool).CreateLidarrRequest(context.Background(), dbq.CreateLidarrRequestParams{ + UserID: user.ID, + Kind: dbq.LidarrRequestKindArtist, + LidarrArtistMbid: "rej-mbid", + ArtistName: "Rejected Once", + }) + if err != nil { + t.Fatalf("CreateLidarrRequest: %v", err) + } + rejNotes := "wrong artist" + if _, err := dbq.New(pool).RejectLidarrRequest(context.Background(), dbq.RejectLidarrRequestParams{ + ID: req.ID, Notes: &rejNotes, DecidedBy: user.ID, + }); err != nil { + t.Fatalf("RejectLidarrRequest: %v", err) + } + + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 1 { + t.Errorf("len = %d, want 1 (rejected requests don't hide the candidate)", len(out)) + } +} + +func TestSuggestArtists_EmptyForNewUser(t *testing.T) { + pool := newPool(t) + user := seedUser(t, pool, "newbie") + out, err := SuggestArtists(context.Background(), pool, user.ID, 30, 12) + if err != nil { + t.Fatalf("SuggestArtists: %v", err) + } + if len(out) != 0 { + t.Errorf("len = %d, want 0 (new user has no signal)", len(out)) + } +} +``` + +Helper functions `seedAlbumForArtist`, `seedTrackOnAlbum`, `insertPlayEvent` are needed. Mirror the same helpers from `internal/lidarrquarantine/service_test.go`'s `seedTrack` (but parameterize artist): + +```go +func seedAlbumForArtist(t *testing.T, pool *pgxpool.Pool, artistID pgtype.UUID, title string) dbq.Album { + t.Helper() + a, err := dbq.New(pool).UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{ + Title: title, SortTitle: title, ArtistID: artistID, + }) + if err != nil { + t.Fatalf("seed album: %v", err) + } + return a +} + +func seedTrackOnAlbum(t *testing.T, pool *pgxpool.Pool, albumID, artistID pgtype.UUID, title string) dbq.Track { + t.Helper() + tr, err := dbq.New(pool).UpsertTrack(context.Background(), dbq.UpsertTrackParams{ + Title: title, AlbumID: albumID, ArtistID: artistID, + DurationMs: 1000, FilePath: "/tmp/m5c-" + title + ".mp3", + FileSize: 1, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("seed track: %v", err) + } + return tr +} + +func insertPlayEvent(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID, startedAt time.Time) { + t.Helper() + if _, err := pool.Exec(context.Background(), + `INSERT INTO play_events (user_id, track_id, started_at, was_skipped) VALUES ($1, $2, $3, false)`, + userID, trackID, startedAt, + ); err != nil { + t.Fatalf("insert play_event: %v", err) + } +} +``` + +The exact `play_events` column set may differ from this minimal insert — read the migration `0005_events.up.sql` and add any required NOT-NULL columns (e.g. `client_id`, `session_id`) with sensible defaults if the insert fails. + +- [ ] **Step 4.5: Run tests** + +```bash +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race -count=1 -p 1 ./internal/recommendation/... +``` + +Expected: 7 tests pass. + +- [ ] **Step 4.6: Commit** + +```bash +git add internal/recommendation/suggestions.go \ + internal/recommendation/suggestions_integration_test.go \ + internal/db/queries/recommendation.sql \ + internal/db/queries/artists.sql \ + internal/db/dbq/ +git commit -m "feat(recommendation): SuggestArtists service for M5c" +``` + +(Include `artists.sql` if you added `GetArtistsByIDs` to it in Step 4.3.) + +--- + +### Task 5 — `/api/discover/suggestions` handler + route mount + +**Files:** +- Create: `internal/api/suggestions.go` +- Create: `internal/api/suggestions_test.go` +- Modify: `internal/api/api.go` — add the route + +- [ ] **Step 5.1: Write the handler** + +Create `internal/api/suggestions.go`: + +```go +package api + +import ( + "net/http" + "strconv" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/auth" + "git.fabledsword.com/bvandeusen/minstrel/internal/recommendation" +) + +// suggestionView is the wire shape returned by GET /api/discover/suggestions. +type suggestionView struct { + MBID string `json:"mbid"` + Name string `json:"name"` + Score float64 `json:"score"` + Attribution []seedContributionView `json:"attribution"` +} + +type seedContributionView struct { + ArtistID pgtype.UUID `json:"artist_id"` + Name string `json:"name"` + Contribution float64 `json:"contribution"` + IsLiked bool `json:"is_liked"` + PlayCount int64 `json:"play_count"` +} + +// handleListSuggestions implements GET /api/discover/suggestions. +// +// Query params: +// - limit (default 12, capped at 50) +// - half_life_days (default 30, no max) +// +// Returns 200 with a JSON array (possibly empty). Read-only; no admin gate. +func (h *handlers) handleListSuggestions(w http.ResponseWriter, r *http.Request) { + user, ok := auth.UserFromContext(r.Context()) + if !ok { + writeErr(w, http.StatusUnauthorized, "unauthorized", "authentication required") + return + } + limit := 12 + if v := r.URL.Query().Get("limit"); v != "" { + n, err := strconv.Atoi(v) + if err != nil || n < 1 { + writeErr(w, http.StatusBadRequest, "bad_request", "invalid limit") + return + } + limit = n + } + halfLife := 30.0 + if v := r.URL.Query().Get("half_life_days"); v != "" { + f, err := strconv.ParseFloat(v, 64) + if err != nil || f <= 0 { + writeErr(w, http.StatusBadRequest, "bad_request", "invalid half_life_days") + return + } + halfLife = f + } + + suggestions, err := recommendation.SuggestArtists(r.Context(), h.pool, user.ID, halfLife, limit) + if err != nil { + h.logger.Error("api: list suggestions", "err", err) + writeErr(w, http.StatusInternalServerError, "server_error", "failed to load suggestions") + return + } + + out := make([]suggestionView, 0, len(suggestions)) + for _, s := range suggestions { + attr := make([]seedContributionView, 0, len(s.Attribution)) + for _, a := range s.Attribution { + attr = append(attr, seedContributionView{ + ArtistID: a.ArtistID, + Name: a.Name, + Contribution: a.Contribution, + IsLiked: a.IsLiked, + PlayCount: a.PlayCount, + }) + } + out = append(out, suggestionView{ + MBID: s.MBID, Name: s.Name, Score: s.Score, Attribution: attr, + }) + } + writeJSON(w, http.StatusOK, out) +} +``` + +- [ ] **Step 5.2: Mount the route** + +In `internal/api/api.go`, find the authed group (after `r.Get("/api/radio", ...)` line) and add: + +```go +authed.Get("/discover/suggestions", h.handleListSuggestions) +``` + +- [ ] **Step 5.3: Write handler tests** + +Create `internal/api/suggestions_test.go`: + +```go +package api + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" +) + +func TestSuggestions_HappyPath(t *testing.T) { + h, _ := testHandlers(t) + user := seedUser(t, h.pool, "alice", "pw", false) + + // Seed: alice likes a seed artist; that seed has one out-of-library + // similar in artist_similarity_unmatched. + seedA, err := dbq.New(h.pool).UpsertArtist(t.Context(), dbq.UpsertArtistParams{ + Name: "Seed", SortName: "Seed", + }) + if err != nil { + t.Fatalf("UpsertArtist: %v", err) + } + if _, err := dbq.New(h.pool).LikeArtist(t.Context(), dbq.LikeArtistParams{ + UserID: user.ID, ArtistID: seedA.ID, + }); err != nil { + t.Fatalf("LikeArtist: %v", err) + } + if err := dbq.New(h.pool).UpsertArtistSimilarityUnmatched(t.Context(), dbq.UpsertArtistSimilarityUnmatchedParams{ + SeedArtistID: seedA.ID, + CandidateMbid: "out-mbid", + CandidateName: "Outsider", + Score: 0.9, + Source: "listenbrainz", + }); err != nil { + t.Fatalf("UpsertArtistSimilarityUnmatched: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/api/discover/suggestions", nil) + setUserCtx(req, user) + w := httptest.NewRecorder() + h.handleListSuggestions(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200; body = %s", w.Code, w.Body.String()) + } + var got []suggestionView + if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { + t.Fatalf("decode: %v", err) + } + if len(got) != 1 { + t.Fatalf("len = %d, want 1; body = %s", len(got), w.Body.String()) + } + if got[0].MBID != "out-mbid" || got[0].Name != "Outsider" { + t.Errorf("got = %+v", got[0]) + } + if len(got[0].Attribution) != 1 { + t.Errorf("attribution len = %d, want 1", len(got[0].Attribution)) + } + if got[0].Attribution[0].Name != "Seed" { + t.Errorf("attribution name = %q, want Seed", got[0].Attribution[0].Name) + } +} + +func TestSuggestions_EmptyForNewUser(t *testing.T) { + h, _ := testHandlers(t) + user := seedUser(t, h.pool, "newbie", "pw", false) + + req := httptest.NewRequest(http.MethodGet, "/api/discover/suggestions", nil) + setUserCtx(req, user) + w := httptest.NewRecorder() + h.handleListSuggestions(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("status = %d, want 200", w.Code) + } + if got := w.Body.String(); got != "[]\n" && got != "[]" { + t.Errorf("body = %q, want []", got) + } +} + +func TestSuggestions_BadLimit(t *testing.T) { + h, _ := testHandlers(t) + user := seedUser(t, h.pool, "alice", "pw", false) + + req := httptest.NewRequest(http.MethodGet, "/api/discover/suggestions?limit=not-a-number", nil) + setUserCtx(req, user) + w := httptest.NewRecorder() + h.handleListSuggestions(w, req) + + if w.Code != http.StatusBadRequest { + t.Errorf("status = %d, want 400", w.Code) + } +} + +// suppress unused imports in some test layouts +var _ = dbtest.TestUserPrefix +``` + +`testHandlers`, `seedUser`, `setUserCtx` are existing helpers in the test layout — see `internal/api/auth_test.go` and `internal/api/requests_test.go`. Use whatever pattern those tests use to seed a user and inject it into the request context. + +- [ ] **Step 5.4: Run tests** + +```bash +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race -count=1 -p 1 ./internal/api/... -run Suggestions +``` + +Expected: 3 tests pass. + +- [ ] **Step 5.5: Commit** + +```bash +git add internal/api/suggestions.go internal/api/suggestions_test.go internal/api/api.go +git commit -m "feat(api): /api/discover/suggestions handler" +``` + +--- + +### Task 6 — Frontend API client + types + qk + +**Files:** +- Create: `web/src/lib/api/suggestions.ts` +- Create: `web/src/lib/api/suggestions.test.ts` +- Modify: `web/src/lib/api/types.ts` — add `ArtistSuggestion`, `SeedContribution` +- Modify: `web/src/lib/api/queries.ts` — add `qk.suggestions(limit)` + +- [ ] **Step 6.1: Add types** + +Append to `web/src/lib/api/types.ts`: + +```ts +export type SeedContribution = { + artist_id: string; + name: string; + contribution: number; + is_liked: boolean; + play_count: number; +}; + +export type ArtistSuggestion = { + mbid: string; + name: string; + score: number; + attribution: SeedContribution[]; // up to 3 entries, ordered by contribution DESC +}; +``` + +- [ ] **Step 6.2: Add query key** + +Append to the `qk` object in `web/src/lib/api/queries.ts`: + +```ts +suggestions: (limit?: number) => ['suggestions', { limit: limit ?? 12 }] as const, +``` + +- [ ] **Step 6.3: Write `suggestions.ts`** + +Create `web/src/lib/api/suggestions.ts`: + +```ts +import { createQuery } from '@tanstack/svelte-query'; +import { api } from './client'; +import { qk } from './queries'; +import type { ArtistSuggestion } from './types'; + +export async function listSuggestions(limit = 12): Promise { + return api.get(`/api/discover/suggestions?limit=${limit}`); +} + +export function createSuggestionsQuery(limit = 12) { + return createQuery({ + queryKey: qk.suggestions(limit), + queryFn: () => listSuggestions(limit), + staleTime: 5 * 60_000 // 5 minutes + }); +} +``` + +- [ ] **Step 6.4: Write tests** + +Create `web/src/lib/api/suggestions.test.ts`: + +```ts +import { describe, expect, test, vi, afterEach } from 'vitest'; + +vi.mock('./client', () => ({ + api: { get: vi.fn() } +})); + +import { listSuggestions } from './suggestions'; +import { qk } from './queries'; +import { api } from './client'; +import type { ArtistSuggestion } from './types'; + +afterEach(() => vi.clearAllMocks()); + +describe('suggestions client', () => { + test('listSuggestions hits the right URL with default limit', async () => { + const fixture: ArtistSuggestion[] = [ + { + mbid: 'm1', + name: 'Outsider', + score: 1.5, + attribution: [ + { artist_id: 'a1', name: 'Seed', contribution: 0.9, is_liked: true, play_count: 0 } + ] + } + ]; + (api.get as ReturnType).mockResolvedValueOnce(fixture); + const got = await listSuggestions(); + expect(api.get).toHaveBeenCalledWith('/api/discover/suggestions?limit=12'); + expect(got).toEqual(fixture); + }); + + test('listSuggestions honors a custom limit', async () => { + (api.get as ReturnType).mockResolvedValueOnce([]); + await listSuggestions(20); + expect(api.get).toHaveBeenCalledWith('/api/discover/suggestions?limit=20'); + }); + + test('qk.suggestions key shape', () => { + expect(qk.suggestions()).toEqual(['suggestions', { limit: 12 }]); + expect(qk.suggestions(20)).toEqual(['suggestions', { limit: 20 }]); + }); +}); +``` + +- [ ] **Step 6.5: Verify** + +```bash +cd /home/bvandeusen/Nextcloud/Projects/Minstrel/minstrel/web +npm run check +npm test -- --run suggestions +cd .. +``` + +Expected: 0 errors, 3 tests pass. + +- [ ] **Step 6.6: Commit** + +```bash +git add web/src/lib/api/suggestions.ts web/src/lib/api/suggestions.test.ts \ + web/src/lib/api/types.ts web/src/lib/api/queries.ts +git commit -m "feat(web): API client for /api/discover/suggestions" +``` + +--- + +### Task 7 — Extend `` with `attribution` prop + +**Files:** +- Modify: `web/src/lib/components/DiscoverResultCard.svelte` +- Modify: `web/src/lib/components/DiscoverResultCard.test.ts` + +The card already has a `$props()` block accepting `kind`, `title`, `subtitle?`, `imageUrl?`, `state`, `onRequest?`. Add `attribution?: string` and render it in italic Vellum below the title (above the reserved badge slot). + +- [ ] **Step 7.1: Add the prop** + +In `DiscoverResultCard.svelte`, modify the `$props()` destructure: + +```ts +let { + kind, + title, + subtitle, + imageUrl, + state, + attribution, + onRequest, +}: { + kind: DiscoverCardKind; + title: string; + subtitle?: string; + imageUrl?: string; + state: DiscoverCardState; + attribution?: string; + onRequest?: () => void; +} = $props(); +``` + +- [ ] **Step 7.2: Render the attribution line** + +Find the section rendering the title + subtitle (search for `class="title"` and `class="subtitle"`). Add the attribution line between subtitle and `.badge-row`: + +```svelte +
+
{title}
+ {#if subtitle} +
{subtitle}
+ {/if} + {#if attribution} +
+ {attribution} +
+ {/if} +
+ {#if state === 'kept'} + Kept + {/if} +
+
+``` + +- [ ] **Step 7.3: Add tests** + +Append to `DiscoverResultCard.test.ts`: + +```ts +test('renders attribution line when prop is set', () => { + render(DiscoverResultCard, { + props: { + kind: 'artist', + title: 'Outsider', + state: 'requestable', + attribution: 'Because you liked Boards of Canada and played Aphex Twin.' + } + }); + expect(screen.getByTestId('attribution')).toHaveTextContent('Because you liked Boards of Canada and played Aphex Twin.'); +}); + +test('omits attribution line when prop is absent', () => { + render(DiscoverResultCard, { + props: { kind: 'artist', title: 'Outsider', state: 'requestable' } + }); + expect(screen.queryByTestId('attribution')).not.toBeInTheDocument(); +}); +``` + +- [ ] **Step 7.4: Verify + commit** + +```bash +cd /home/bvandeusen/Nextcloud/Projects/Minstrel/minstrel/web +npm run check +npm test -- --run DiscoverResultCard +cd .. +git add web/src/lib/components/DiscoverResultCard.svelte web/src/lib/components/DiscoverResultCard.test.ts +git commit -m "feat(web): DiscoverResultCard attribution prop for M5c suggestions" +``` + +--- + +### Task 8 — `` subcomponent + `/discover` integration + +**Files:** +- Create: `web/src/lib/components/SuggestionFeed.svelte` +- Create: `web/src/lib/components/SuggestionFeed.test.ts` +- Modify: `web/src/routes/discover/+page.svelte` — branch to `` when search is empty +- Modify: `web/src/routes/discover/discover.test.ts` — add suggestion-feed scenarios + +- [ ] **Step 8.1: Write ``** + +Create `web/src/lib/components/SuggestionFeed.svelte`: + +```svelte + + +
+
+

Suggested for you

+

Out-of-library artists drawn from what you've liked and played.

+
+ + {#if !query.isPending && suggestions.length === 0} +

Listen to something or like an artist to start getting suggestions.

+ {:else if suggestions.length > 0} +
+ {#each suggestions.filter(visible) as s (s.mbid)} + onRequest(s)} + /> + {/each} +
+ {/if} +
+``` + +- [ ] **Step 8.2: Write `` tests** + +Create `web/src/lib/components/SuggestionFeed.test.ts`: + +```ts +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/svelte'; +import { mockQuery } from '../../test-utils/query'; + +const invalidateMock = vi.fn(); +vi.mock('@tanstack/svelte-query', async (orig) => { + const actual = (await orig()) as Record; + return { ...actual, useQueryClient: () => ({ invalidateQueries: invalidateMock }) }; +}); + +vi.mock('$lib/api/suggestions', () => ({ + createSuggestionsQuery: vi.fn() +})); + +vi.mock('$lib/api/requests', () => ({ + createRequest: vi.fn().mockResolvedValue({}) +})); + +import SuggestionFeed from './SuggestionFeed.svelte'; +import { createSuggestionsQuery } from '$lib/api/suggestions'; +import { createRequest } from '$lib/api/requests'; +import type { ArtistSuggestion } from '$lib/api/types'; + +const oneSeed: ArtistSuggestion = { + mbid: 'mb1', name: 'Outsider', score: 1.0, + attribution: [{ artist_id: 'a1', name: 'Seed', contribution: 0.9, is_liked: true, play_count: 0 }] +}; + +const twoSeeds: ArtistSuggestion = { + mbid: 'mb2', name: 'Outsider Two', score: 2.0, + attribution: [ + { artist_id: 'a1', name: 'A', contribution: 0.8, is_liked: true, play_count: 0 }, + { artist_id: 'a2', name: 'B', contribution: 0.5, is_liked: false, play_count: 3 } + ] +}; + +const threeSeeds: ArtistSuggestion = { + mbid: 'mb3', name: 'Outsider Three', score: 3.0, + attribution: [ + { artist_id: 'a1', name: 'X', contribution: 0.9, is_liked: true, play_count: 0 }, + { artist_id: 'a2', name: 'Y', contribution: 0.6, is_liked: false, play_count: 5 }, + { artist_id: 'a3', name: 'Z', contribution: 0.3, is_liked: false, play_count: 1 } + ] +}; + +afterEach(() => vi.clearAllMocks()); + +describe('SuggestionFeed', () => { + test('renders one card per suggestion', () => { + (createSuggestionsQuery as ReturnType).mockReturnValue( + mockQuery({ data: [oneSeed, twoSeeds] }) + ); + render(SuggestionFeed); + expect(screen.getByText('Outsider')).toBeInTheDocument(); + expect(screen.getByText('Outsider Two')).toBeInTheDocument(); + }); + + test('attribution copy: 1 seed → "Because you liked X."', () => { + (createSuggestionsQuery as ReturnType).mockReturnValue( + mockQuery({ data: [oneSeed] }) + ); + render(SuggestionFeed); + expect(screen.getByText(/because you liked seed\./i)).toBeInTheDocument(); + }); + + test('attribution copy: 2 seeds → "Because you liked A and played B."', () => { + (createSuggestionsQuery as ReturnType).mockReturnValue( + mockQuery({ data: [twoSeeds] }) + ); + render(SuggestionFeed); + expect(screen.getByText(/because you liked a and played b\./i)).toBeInTheDocument(); + }); + + test('attribution copy: 3 seeds → Oxford comma', () => { + (createSuggestionsQuery as ReturnType).mockReturnValue( + mockQuery({ data: [threeSeeds] }) + ); + render(SuggestionFeed); + expect(screen.getByText(/because you liked x, played y, and played z\./i)).toBeInTheDocument(); + }); + + test('Request button calls createRequest with artist-kind body', async () => { + (createSuggestionsQuery as ReturnType).mockReturnValue( + mockQuery({ data: [oneSeed] }) + ); + render(SuggestionFeed); + await fireEvent.click(screen.getByRole('button', { name: /request outsider/i })); + expect(createRequest).toHaveBeenCalledWith({ + kind: 'artist', + lidarr_artist_mbid: 'mb1', + artist_name: 'Outsider' + }); + expect(invalidateMock).toHaveBeenCalled(); + }); + + test('empty state when data is []', () => { + (createSuggestionsQuery as ReturnType).mockReturnValue(mockQuery({ data: [] })); + render(SuggestionFeed); + expect(screen.getByText(/listen to something or like an artist/i)).toBeInTheDocument(); + }); +}); +``` + +- [ ] **Step 8.3: Wire `` into `/discover`** + +Modify `web/src/routes/discover/+page.svelte`. Read it first — the existing structure runs the search query when `debouncedQ.length > 0`. Add the feed branch: + +```svelte + + + + +
+ + + + {#if debouncedQ === ''} + + {:else} + + {existing markup unchanged} + {/if} +
+``` + +The search-input element stays at the top level (visible in both branches). The header copy ("Add music to the library" vs "Suggested for you") is now owned by the respective branch — `` renders its own header; the search branch keeps the existing one. + +If the existing `+page.svelte` has its header above the input, you'll need to move it inside the search branch. Pattern: + +```svelte + + +{#if debouncedQ === ''} + +{:else} +
+

Add music to the library

+

...

+
+ +{/if} +``` + +- [ ] **Step 8.4: Update `discover.test.ts`** + +The existing tests assume `inputValue === ''` shows the initial-copy state. Now it shows the suggestion feed. Update the relevant test and add new ones: + +Add a mock for `$lib/api/suggestions`: + +```ts +vi.mock('$lib/api/suggestions', () => ({ + createSuggestionsQuery: vi.fn() +})); +``` + +Update the existing "initial state shows search prompt copy" test (or add a replacement): + +```ts +test('empty input shows the suggestion feed', () => { + (createSuggestionsQuery as ReturnType).mockReturnValue( + mockQuery({ data: [] }) + ); + render(DiscoverPage); + expect(screen.getByText(/suggested for you/i)).toBeInTheDocument(); +}); + +test('typing replaces feed with search', async () => { + (createSuggestionsQuery as ReturnType).mockReturnValue( + mockQuery({ data: [] }) + ); + // mock the lidarr search query as before + // ... fire input event to trigger debounced search ... + // ... advance fake timers ... + expect(screen.queryByText(/suggested for you/i)).not.toBeInTheDocument(); + expect(screen.getByText(/add music to the library/i)).toBeInTheDocument(); +}); +``` + +The existing tests that drive the search flow stay — they always provide a non-empty query. The empty-input case becomes a suggestion-feed test. + +- [ ] **Step 8.5: Verify** + +```bash +cd /home/bvandeusen/Nextcloud/Projects/Minstrel/minstrel/web +npm run check +npm test -- --run SuggestionFeed discover +npm run build +cd .. +``` + +Expected: 0 errors, all tests pass, build clean. + +- [ ] **Step 8.6: Commit** + +```bash +git add web/src/lib/components/SuggestionFeed.svelte \ + web/src/lib/components/SuggestionFeed.test.ts \ + web/src/routes/discover/+page.svelte \ + web/src/routes/discover/discover.test.ts +git commit -m "feat(web): suggestion feed on /discover (search-empty default)" +``` + +--- + +### Task 9 — Final verification + branch finish + +- [ ] **Step 9.1: Full Go test sweep** + +```bash +go test -short -race ./... +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + go test -race -p 1 ./... +``` + +Expected: short suite + integration suite both green. The pre-existing `internal/library/TestScanner_Integration` flake is documented (`project_scanner_flake.md`) and not blocking. + +- [ ] **Step 9.2: Lint clean** + +```bash +golangci-lint run ./... +``` + +Expected: no output. + +- [ ] **Step 9.3: Coverage check** + +```bash +docker run --rm --network minstrel_minstrel \ + -v "$(pwd):/src" -w /src \ + -e MINSTREL_TEST_DATABASE_URL='postgres://minstrel:minstrel@postgres:5432/minstrel?sslmode=disable' \ + golang:1.23-bookworm \ + bash -c 'go test -race -p 1 -coverprofile=/tmp/cov.out \ + ./internal/recommendation/... ./internal/similarity/... ./internal/api/... && \ + go tool cover -func=/tmp/cov.out | tail -1' +``` + +Expected: combined ≥ 80% on the new code per spec §8. + +- [ ] **Step 9.4: Frontend full check** + +```bash +cd /home/bvandeusen/Nextcloud/Projects/Minstrel/minstrel/web +npm run check +npm test -- --run +npm run build +cd .. +``` + +Expected: 0 errors, all tests pass, build succeeds. + +- [ ] **Step 9.5: Manual smoke** + +- Like an artist (or play a few tracks). +- Open `/discover` with the search input empty. +- Verify the "Suggested for you" header + grid renders. +- Verify each card has an attribution line that reads naturally. +- Click Request on a suggestion → confirm it disappears (optimistic) and a row appears at `/requests`. +- Type a search term → confirm the feed swaps out for search results. +- Clear the search input → confirm the feed comes back (cached, instant). +- For a fresh user with no likes/plays, the empty-state copy renders. + +- [ ] **Step 9.6: Use `superpowers:finishing-a-development-branch`** + +Verify tests are still green, then run the skill to present finish options (merge / PR / keep / discard). Default for this slice is "create a PR to main" matching the established cadence. + +--- + +## Self-review checklist + +**Spec coverage** — every spec section maps to a task: +- §3 Architecture: Tasks 1 (table), 2 (LB client), 3 (worker), 4 (service), 5 (handler), 6-8 (frontend) +- §4 Schema: Task 1 +- §5 API surface: Task 5 +- §6 UI surfaces: Tasks 7 (DiscoverResultCard), 8 (SuggestionFeed + /discover integration) +- §7 Error handling: distributed across Tasks 3 (worker WARN), 5 (handler 500), 8 (frontend silent-on-failure) +- §8 Testing: every Task includes tests; Task 9 verifies coverage targets +- §9 Decisions ledger: not directly implemented, referenced in commit messages +- §10 Out of scope: explicitly excluded — no album/track suggestions, no realtime invalidation, no cross-user CF, no pagination, no materialization +- §11 Open questions: Task 2 verifies the `Name` field on `SimilarArtist`; cold-start sparseness is documented behavior + +**Placeholder scan:** the per-task detail level drops after Task 5 (frontend tasks become standard SvelteKit page work) — intentional for navigability. No "TBD" or "TODO" remains. + +**Type consistency:** +- Service method: `SuggestArtists` consistent across plan +- Types: `ArtistSuggestion`, `SeedContribution` consistent across Go and TS +- API path: `/api/discover/suggestions` consistent +- Component name: `` consistent +- DB field names: `seed_artist_id`, `candidate_mbid`, `candidate_name`, `score`, `source`, `fetched_at` consistent across migration / queries / tests + +Plan is complete. diff --git a/docs/superpowers/specs/2026-04-29-m5a-lidarr-design.md b/docs/superpowers/specs/2026-04-29-m5a-lidarr-design.md new file mode 100644 index 00000000..2cd8ffc7 --- /dev/null +++ b/docs/superpowers/specs/2026-04-29-m5a-lidarr-design.md @@ -0,0 +1,345 @@ +# M5a — Lidarr connection + search/add proxy + admin shell + +> **Status:** Draft for review · 2026-04-29 +> +> **Sub-plan of:** M5 (Lidarr integration + quarantine workflow). M5 was decomposed into three slices during brainstorming on 2026-04-29: +> +> - **M5a (this spec)** — Lidarr connection + search/add + admin shell. Foundation; ships first. +> - **M5b** — Quarantine workflow (per-user soft-hide, admin resolution UI). Depends on M5a only for the admin shell. +> - **M5c** — Radio "suggested additions" (out-of-library MBIDs surfaced in `/api/radio` responses; SPA add affordance). Depends on M5a's `lidarr_requests` table and add path. +> +> Each ships as its own PR with its own brainstorm/spec/plan cycle. + +## 1. Goal + +Connect Minstrel to a household Lidarr instance, give every user a search-and-request workflow at `/discover`, and give admins a moderation queue at `/admin/requests`. Approved requests fire Lidarr adds synchronously; a background reconciler matches the resulting downloaded tracks back to the originating request when the next library scan picks them up. + +This slice does NOT introduce quarantine, soft-hide, or radio suggested-additions — those are M5b and M5c. + +## 2. Goals and non-goals + +### Goals + +- Operator can connect, configure, test, and disconnect a Lidarr instance from `/admin/integrations` without editing YAML or restarting the server. +- Any authenticated user can search Lidarr at artist / album / track granularity from `/discover`. +- Any authenticated user can submit an add request, which gets persisted to `lidarr_requests` with status `pending`. +- Admin can review pending requests at `/admin/requests`, approve (with optional per-request override of quality profile / root folder) or reject (with optional note). +- Approved requests trigger a synchronous Lidarr add and a library scan. +- A background reconciler worker matches `approved` requests to newly scanned tracks and transitions them to `completed`. +- Hard route gating on `/admin/*` — non-admin users redirected before any admin content loads. + +### Non-goals (this slice) + +- Quarantine workflow, soft-hide on tracks, admin "delete via Lidarr" path. → M5b. +- Radio "suggested additions" surfacing out-of-library similar tracks. → M5c. +- Webhook ingestion from Lidarr (push notifications on download complete). → optional follow-up; the polling reconciler is sufficient for v1. +- "Pending too long" failure detection / requestor notification on stalled adds. → open question, deferred. +- Self-service password reset, OIDC, or any other identity work. → orthogonal. +- Per-user Lidarr accounts. Lidarr is a single household instance. + +## 3. Architecture + +### New Go packages + +- **`internal/lidarr/`** — HTTP client for Lidarr's v1 API. Mirrors `internal/scrobble/listenbrainz/` shape: `Client` struct with `BaseURL`, `APIKey`, `HTTP` fields. Methods: `LookupArtist(ctx, query)`, `LookupAlbum(ctx, query)`, `LookupTrack(ctx, query)`, `AddArtist(ctx, params)`, `AddAlbum(ctx, params)`, `ListQualityProfiles(ctx)`, `ListRootFolders(ctx)`, `Ping(ctx)`. Returns typed structs; never leaks raw JSON to callers. + +- **`internal/lidarrconfig/`** — singleton config service. Reads/writes the `lidarr_config` row, exposes `Get(ctx) (*Config, error)` returning a typed struct, `Save(ctx, *Config) error`. The `Get` method also handles the "config not yet set" case by returning a zero-value `Config{Enabled: false}` so callers can branch cleanly. + +- **`internal/lidarrrequests/`** — request lifecycle service: + - `Service` — `Create(ctx, userID, params)`, `ListPending(ctx)`, `ListByStatus(ctx, status)`, `ListForUser(ctx, userID)`, `Approve(ctx, requestID, adminID, overrides)`, `Reject(ctx, requestID, adminID, notes)`, `Cancel(ctx, requestID, userID)`. `Approve` calls `lidarr.Client.Add*` synchronously and triggers a library scan via the existing scanner package. + - `Reconciler` — background worker analogous to `internal/similarity.Worker`. `Run(ctx)` loop with `tick` interval (default 5 min) calls `tickOnce(ctx)`, which: + 1. SELECTs `lidarr_requests WHERE status = 'approved'` with row limits. + 2. For each, joins against `tracks`/`albums`/`artists` by MBID hierarchy. + 3. Transitions matched rows to `completed`, sets `matched_*_id`, `completed_at`. + +### Wiring + +- `cmd/minstrel/main.go` gains a third worker spin-up (alongside the scrobble and similarity workers). The reconciler skips its work when `lidarr_config.enabled = false`. +- New handler files: `internal/api/lidarr.go` (search proxy), `internal/api/requests.go` (user-facing endpoints), `internal/api/admin/lidarr.go` (config CRUD + profiles/folders lookups), `internal/api/admin/requests.go` (approval queue). +- New middleware: `RequireAdmin` — checks the user resolved by `RequireUser` has `is_admin = true`; 403s with `{"error":"not_authorized"}` otherwise. Mounted on the `/api/admin/*` route group. + +### Data flow — happy path + +1. User opens `/discover`, types query, SPA hits `GET /api/lidarr/search?q=…&kind=artist|album|track`. +2. Handler invokes the appropriate `lidarr.Client.Lookup*`, normalizes the response, returns JSON. +3. User clicks "Request" → `POST /api/requests` with `{kind, lidarr_artist_mbid, lidarr_album_mbid?, lidarr_track_mbid?, artist_name, album_title?, track_title?}`. +4. Handler validates the kind→fields invariant, creates a `lidarr_requests` row with status `pending`, returns 201. +5. Admin opens `/admin/requests`, SPA hits `GET /api/admin/requests?status=pending`. +6. Admin clicks "Approve" → `POST /api/admin/requests/:id/approve` with optional `{quality_profile_id, root_folder_path}`. +7. Handler snapshots the chosen values into the row, calls `lidarr.Client.AddArtist|AddAlbum`, transitions to `approved`, fires scan trigger. Returns 200 (or surfaces Lidarr error in 4xx/5xx). +8. Reconciler worker on next tick (≤5 min) sees the `approved` row, joins against `tracks`, finds the new track, transitions to `completed` with `matched_track_id` set. +9. Requester's `/requests` page shows status `completed` with a "Listen" link to the now-playable track. + +### SPA route gating + +- `/admin/*` route group has a `+layout.svelte` (or `+layout.ts`) guard: if `currentUser.is_admin === false`, calls `goto('/')` before child routes load. Page-level gate, not content gating — the operator's instruction. +- Same pattern as the existing auth gate; small extension of the existing layout machinery. + +## 4. Schema + +Migration **0010_lidarr** in two files (`up.sql`, `down.sql`). + +### `lidarr_config` (singleton) + +```sql +CREATE TABLE lidarr_config ( + id smallint PRIMARY KEY DEFAULT 1 CHECK (id = 1), + enabled boolean NOT NULL DEFAULT false, + base_url text, + api_key text, + default_quality_profile_id int, + default_root_folder_path text, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now() +); + +INSERT INTO lidarr_config (id, enabled) VALUES (1, false); +``` + +The `CHECK (id = 1)` plus seed row enforces "exactly one row, ever." The Settings UI shows "Connect Lidarr" instead of "Lidarr is connected" when `enabled=false` or `base_url IS NULL`. + +### `lidarr_requests` + +```sql +CREATE TYPE lidarr_request_status AS ENUM ( + 'pending', 'approved', 'rejected', 'completed', 'failed' +); +CREATE TYPE lidarr_request_kind AS ENUM ('artist', 'album', 'track'); + +CREATE TABLE lidarr_requests ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + user_id uuid NOT NULL REFERENCES users(id) ON DELETE CASCADE, + status lidarr_request_status NOT NULL DEFAULT 'pending', + kind lidarr_request_kind NOT NULL, + + lidarr_artist_mbid text NOT NULL, + lidarr_album_mbid text, + lidarr_track_mbid text, + artist_name text NOT NULL, + album_title text, + track_title text, + + quality_profile_id int, + root_folder_path text, + + decided_at timestamptz, + decided_by uuid REFERENCES users(id) ON DELETE SET NULL, + notes text, + + completed_at timestamptz, + matched_track_id uuid REFERENCES tracks(id) ON DELETE SET NULL, + matched_album_id uuid REFERENCES albums(id) ON DELETE SET NULL, + matched_artist_id uuid REFERENCES artists(id) ON DELETE SET NULL, + + requested_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now() +); + +CREATE INDEX lidarr_requests_user_id_idx ON lidarr_requests (user_id); +CREATE INDEX lidarr_requests_status_idx ON lidarr_requests (status); +CREATE INDEX lidarr_requests_artist_mbid_idx ON lidarr_requests (lidarr_artist_mbid); +CREATE INDEX lidarr_requests_album_mbid_idx ON lidarr_requests (lidarr_album_mbid) + WHERE lidarr_album_mbid IS NOT NULL; +``` + +**Shape notes:** +- Three matched-* FKs (one per kind) instead of polymorphic — clean SQL, `ON DELETE SET NULL` keeps the historical audit even if the matched track gets removed later. +- Track-kind requests still set `lidarr_album_mbid` (because that's what Lidarr actually adds); `lidarr_track_mbid` is preserved for "I requested this song specifically" display. +- `quality_profile_id` and `root_folder_path` are NULL until decision time. When admin approves, the override (or the config default) gets snapshotted into the row. +- `failed` is reserved in the enum for a future reconciler timeout (e.g., "approved >7 days ago, no match"), but no reconciler logic transitions to `failed` in this slice. It's a placeholder for an obvious follow-up. + +**Indexes rationale:** +- `user_id` — `/requests` page scan ("show my requests"). +- `status` — `WHERE status='pending'` for admin queue, `WHERE status='approved'` for reconciler. +- `lidarr_artist_mbid` / `lidarr_album_mbid` — reconciler joins against `tracks`, `albums` by MBID. + +### Down migration + +Drops in reverse: indexes → table → enum types → singleton row deletion (table goes anyway). + +## 5. API surface + +All endpoints under `/api/*`, JSON request/response, `{error: "", message: ""}` envelope on errors. `/api/admin/*` group passes through `RequireAdmin` middleware (403 with `not_authorized` for non-admin tokens). + +### User-facing (any authenticated user) + +| Method | Path | Behavior | +|---|---|---| +| `GET` | `/api/lidarr/search?q=&kind=artist\|album\|track` | Proxies Lidarr lookup. Returns normalized list `[{mbid, name|title, secondary_text, image_url, in_library: bool, requested: bool}]`. The `in_library` and `requested` fields are computed server-side: `in_library` joins against `artists`/`albums`/`tracks` by MBID; `requested` is true when ANY user has a non-`rejected`, non-`failed` `lidarr_requests` row with the same MBID — covers `pending`, `approved`, and `completed`. (`completed` in the DB but `in_library=false` in the response would only happen briefly between Lidarr add and library scan; both flags can be true together — UI prefers `in_library` for that case.) Returns `503 lidarr_disabled` if `lidarr_config.enabled=false`. | +| `POST` | `/api/requests` | Create a request. Body: `{kind, lidarr_artist_mbid, lidarr_album_mbid?, lidarr_track_mbid?, artist_name, album_title?, track_title?}`. Server validates kind→required-fields invariant. Returns `201 {request}`. | +| `GET` | `/api/requests` | List the caller's own requests (any status). Ordered by `requested_at desc`. Pagination: `?limit=&before=` cursor. | +| `GET` | `/api/requests/:id` | Single request detail. 404 if not caller's own and caller is not admin. | +| `DELETE` | `/api/requests/:id` | Cancel a still-pending request the caller created. 409 `request_not_pending` if status != `pending`. | + +### Admin-only + +| Method | Path | Behavior | +|---|---|---| +| `GET` | `/api/admin/lidarr/config` | Returns current config. `api_key` masked as `"***"` when set, `null` when unset. | +| `PUT` | `/api/admin/lidarr/config` | Body: `{base_url, api_key, default_quality_profile_id, default_root_folder_path, enabled}`. `api_key`: empty string = leave saved value unchanged; non-empty = update. URL validated. | +| `POST` | `/api/admin/lidarr/test` | Body: `{base_url?, api_key?}`. Each field independently falls back to the saved value when absent or empty. Calls `Client.Ping`. Always returns 200 with envelope `{ok: true, version: "..."}` or `{ok: false, error: "..."}` — never an HTTP-level error envelope, so the SPA can render results uniformly. | +| `GET` | `/api/admin/lidarr/quality-profiles` | Proxies Lidarr's quality profile list — populates the Settings dropdown. | +| `GET` | `/api/admin/lidarr/root-folders` | Proxies Lidarr's root folder list — populates the Settings dropdown. | +| `GET` | `/api/admin/requests?status=pending\|approved\|rejected\|completed\|failed&limit=` | Admin's queue view. Default `status=pending`. | +| `POST` | `/api/admin/requests/:id/approve` | Body: `{quality_profile_id?, root_folder_path?}` (override fields; absent = use config default). Snapshots chosen values, calls `Client.AddArtist|AddAlbum`, transitions to `approved`, fires scan trigger. | +| `POST` | `/api/admin/requests/:id/reject` | Body: `{notes?}`. Sets status `rejected`, records `decided_*` and `notes`. | + +### Error codes + +`lidarr_disabled`, `lidarr_unreachable`, `lidarr_auth_failed`, `lidarr_lookup_failed`, `mbid_required`, `request_not_pending`, `request_not_found`, `not_authorized`. + +## 6. UI surfaces + +All four screens land at the FabledSword design system bar (memory: `project_design_system.md`). Tokens are referenced by name; concrete values live in the design-system memory. Mockups produced during brainstorming live in `.superpowers/brainstorm//content/` (gitignored) — `discover-fs-v2.html`, `admin-integrations.html`, `admin-requests.html`, `user-requests.html`. + +### `/discover` (user-facing) + +Search input at top, kind tabs (Artists / Albums / Tracks), card grid of results. Card state derives from the search response's `in_library` and `requested` flags: + +- **Kept** (`in_library=true`) — wins over `requested`. Disabled ghost button "In library", "Kept" pill in the badge slot (forest-teal at 15% opacity bg). +- **Requested** (`in_library=false && requested=true`) — disabled ghost button "Requested", subtitle line shows "awaiting review" for pending requests, "downloading" for approved. +- **Requestable** (`in_library=false && requested=false`) — Moss `Request` button with plus icon. + +Card layout discipline: +- Card body is `display: flex; flex-direction: column`. Inside, a `.text` block has `min-height` reserving title + meta + badge-row even when fields are absent. Actions block uses `margin-top: auto` so the button always anchors to the bottom of the card. +- `.badge-row` reserves 22px regardless of badge presence — title sits at the same Y across cards. +- Grid `align-items: stretch` keeps cards on the same row equal-height. + +Implementation: `` Svelte component with props `{kind, artistName, albumTitle?, trackTitle?, imageUrl?, state: 'requestable'|'kept'|'requested', onRequest}`. Reused for any future "list of music things" surface. + +Track-kind result requests open a confirmation modal: "Requesting *Track X* will add the album *Album Y*. Continue?" Confirm = Moss, Cancel = Bronze. Disclosure is explicit, not silent. + +### `/admin` shell + sidebar + +`/admin/*` routes share `+layout.svelte`: +- Role gate: `if (!currentUser.is_admin) goto('/')` before child routes load. +- 220px sidebar with Iron card surface. Active item: 12% accent-tinted background, 2px forest-teal left strip ("you are here"). +- Nav items (this slice): Overview · **Integrations** · **Requests** · Quarantine (placeholder for M5b) · Users (future) · Library (future). + +Lucide icons at 16px, 1px stroke. Sidebar text: Vellum default, Parchment on active. + +### `/admin/integrations` + +Lidarr panel (single section in this slice; designed to host more integrations later): +- Header status pill: "Lidarr · connected" (Moss-tinted) when `enabled && reachable`, "unset" (Pewter ghost) otherwise. +- Form rows: Base URL · API key (masked) · Default quality profile (dropdown) · Default root folder (dropdown). +- Action row: **Save changes** = Moss, **Test connection** = Pewter ghost, **Disconnect** = Oxblood + trash icon (right-aligned, separated). +- Inputs on Obsidian (inset feel), 0.5px Pewter borders, 8px radius, focus = `box-shadow: 0 0 0 2px var(--fs-accent)` (no layout shift). + +A second placeholder section for "MusicBrainz overrides" with status `unset` foreshadows the panel's role as the integration hub. Not implemented in this slice. + +### `/admin/requests` + +Tabbed list (Pending / Approved / Completed / Rejected). Tab counts as accent-tinted pills. Default tab `Pending` with badge showing count. + +Row anatomy: 56px album-art square (Slate fallback when Lidarr returns no cover) · meta-row with kind pill + "requested by alice · 2h ago" small caps · title in Parchment · meta in Vellum · action cluster: **Override** (Pewter ghost, opens modal) → **Approve** (Moss + check icon) → **Reject** (Bronze + ✕ icon). + +Override modal: collapsed-by-default override of `quality_profile_id` and `root_folder_path` for the specific approval. Most approvals click "Approve" without opening this. + +Track-kind row's meta line spells out "Approving will add the album *Geogaddi*" — explicit disclosure of the album-promotion behavior. + +### `/requests` (user's own) + +Single panel listing the caller's requests, ordered by `requested_at desc`. Row anatomy mirrors `/admin/requests` but action cluster is reduced: +- **Pending** → Cancel (Pewter ghost + ✕ icon). +- **Approved** → no actions, "Approved · downloading" status pill (Info-tinted). +- **Completed** → "Listen" link in forest-teal (the page's only brand-moment), navigates to the matched track. +- **Rejected** → no actions, admin's note rendered as Vellum meta when present. + +Status pills use the doc's semantic palette (Warning · Info · Moss/Success · Error). Voice rule applied: "Kept" instead of "Completed", "Set aside" instead of "Rejected", "Awaiting review" instead of "Pending review." + +## 7. Error handling + +### Lidarr unreachable / auth-failed + +- `Client.*` methods return typed errors: `lidarr.ErrUnreachable`, `lidarr.ErrAuthFailed`, `lidarr.ErrLookupFailed`. +- Search proxy translates to `503 lidarr_unreachable` or `401 lidarr_auth_failed` — the SPA shows a callout: "Lidarr is unreachable right now. Try again, or check Settings → Integrations." (Voice rule: this is an error/waiting moment, gets the flavored register.) +- Admin approval handler same pattern: returns the error to admin so they can retry without losing the request. The request stays `pending` if the Lidarr call fails — never advances to `approved` without confirmation Lidarr accepted the add. + +### Test connection from Settings + +- `POST /api/admin/lidarr/test` always returns 200 with `{ok: bool, error?: string, version?: string}` — never an error envelope. Lets the SPA always render the result without parsing HTTP-level errors. + +### Reconciler + +- Worker errors logged at `WARN`, never propagated. A failing tick doesn't stop the worker — next tick retries. +- If `lidarr_config.enabled = false`, worker silently no-ops each tick. +- Postgres unavailability is a global concern; reconciler errors with the same backoff pattern as `internal/similarity.Worker`. + +### Form validation + +- Settings: URL must parse, API key must be non-empty if `enabled=true`. Quality profile + root folder must be present and known to Lidarr (validated against `Client.ListQualityProfiles` / `ListRootFolders`). +- Request creation: kind→required-MBID-fields invariant enforced server-side. SPA validates client-side first to avoid round-trips. + +## 8. Testing + +### Unit tests + +- `internal/lidarr/` — table-driven request/response parsing tests against canned fixtures (real Lidarr response samples committed under `internal/lidarr/testdata/`). Covers happy path + auth-failure + 5xx + bad-JSON for each method. +- `internal/lidarrconfig/` — `Get` returns sensible zero-value when row says `enabled=false`; `Save` updates `updated_at`. +- `internal/lidarrrequests.Service` — pure-logic tests: kind→required-fields validation, status-transition validation (can only approve `pending`, can only cancel `pending`, etc.). + +### Integration tests (gated on `MINSTREL_TEST_DATABASE_URL`) + +- Reconciler — seeds an `approved` request + a matching track row, runs `tickOnce`, asserts status transitions to `completed` with correct `matched_*_id`. Covers: + - artist-kind matched by `lidarr_artist_mbid` against `artists.mbid` + - album-kind matched by `lidarr_album_mbid` against `albums.mbid` + - track-kind matched by `lidarr_album_mbid` (track-promoted) against `albums.mbid` + - no match (no transition) + - already-completed row not re-processed + - `lidarr_config.enabled=false` short-circuits to no-op + +### HTTP tests (handler level) + +- `internal/api/lidarr.go` — search proxy with stubbed `Client`: 200 happy path, 503 disabled, 503 unreachable, 401 auth-failed. +- `internal/api/requests.go` — create with valid + invalid kind/MBID combinations; list-mine returns only caller's rows; cancel pending vs cancel non-pending; cross-user 404. +- `internal/api/admin/lidarr.go` — config GET masks api_key; PUT empty-string preserves api_key; test-connection always returns 200 envelope. +- `internal/api/admin/requests.go` — approve fires Lidarr stub, transitions row, captures defaults from config; approve with override snapshots override values; reject without notes works; non-admin 403 across the board. + +### Frontend tests (vitest) + +- `` — renders three states correctly; calls `onRequest` only in requestable state; reserved-slot CSS verified by computed-style assertion (badge-row min-height, button anchored). +- `/discover` page — debounce search input; renders results from store; transitions state on request submit; track-kind opens confirmation modal; modal Confirm calls API, modal Cancel does not. +- `/admin/requests` page — tab switch refetches; approve fires API and removes row from pending; override modal returns chosen values to approve handler; admin-only redirect verified at layout level. +- `/admin/integrations` panel — empty-state copy; test-connection updates status pill; Disconnect requires confirmation. +- `/requests` page — status pills render with correct semantic class; Cancel works on pending; "Listen" link only renders on completed rows. + +### Coverage target + +- `internal/lidarr/` ≥ 80% +- `internal/lidarrrequests/` ≥ 80% +- `internal/api/lidarr.go`, `internal/api/requests.go`, `internal/api/admin/*` — handler coverage measured combined ≥ 70% (matches current api package threshold) + +## 9. Decisions ledger + +| # | Decision | Rationale | +|---|---|---| +| 1 | Decompose M5 into M5a / M5b / M5c | Matches the M4 cadence; smaller PRs, faster review, clearer scope per slice | +| 2 | Permissions: search-all, add-admin (request queue) | Operator's call — "browse-and-suggest workflow lets non-trusted household members participate without giving them library-write access" | +| 3 | Config storage: DB-only, Settings UI as the entry point | Operator's product principle — "no one wants to configure yamls; this is a finished product, not a project" (memory: `project_product_not_project.md`) | +| 4 | Settings shape: dedicated `/admin/*` route group with hard route gate | Per-app product surface for admin actions; redirect at layout-level, never load admin content for non-admin (operator's instruction) | +| 5 | Search UX: standalone `/discover` route (option B), not inline-in-search | Operator preference — explicit "I want to add music" surface, separate from local-library search | +| 6 | Granularity: artist + album + track | Operator preference — track-kind resolves to album-kind under the hood (Lidarr's monitor unit is album), explicit modal disclosure rather than silent expansion | +| 7 | Lifecycle detection: library scan as source of truth | Reuses existing scanner; reconciler is a 5-min worker; webhook is a clean follow-up if latency matters | +| 8 | Quality profile / root folder: default + per-add override | Default covers >90% of approvals; override is the escape hatch. Modal is collapsed-by-default | +| 9 | Approve fires Lidarr synchronously, reconcile asynchronously (Approach 1) | Admin gets immediate "Lidarr accepted/rejected" feedback; reconciliation has to be async because downloads take minutes-to-hours | +| 10 | New `RequireAdmin` middleware on `/api/admin/*` route group | Centralized auth check; SPA route gate is UX, server middleware is the security boundary | +| 11 | UI lands at FabledSword design system bar (memory: `project_design_system.md`) | Operator's quality bar — "no more scaffolding-feel UI" (memory: `project_ui_quality.md`); accent only for brand-moments, Moss/Bronze/Oxblood for actions | +| 12 | Track-kind disclosure modal | Lidarr can't fetch a single track without its album; explicit "this will add the album" beats silently expanding the request | +| 13 | `lidarr_requests.failed` status reserved but not transitioned in this slice | Foreshadows a "stalled approval" timeout follow-up; not v1 | + +## 10. Out of scope (this slice) + +Tracked in the M5 milestone for later sub-plans: + +- **Quarantine workflow** — `lidarr_quarantine` table, soft-hide on tracks, admin "delete via Lidarr" path, `/admin/quarantine` page. → M5b. +- **Radio suggested-additions** — `/api/radio` response includes a separate field for out-of-library MBIDs from `track_similarity`; SPA shows "Would you add these?" affordance with inline request submission. → M5c. +- **Lidarr webhook ingestion** — push notifications on download complete; near-real-time status updates on `/requests`. → cheap follow-up after M5c. +- **Failed-request timeout** — reconciler transitions long-stuck `approved` requests to `failed` with operator-facing diagnostics. → operational tightening; not v1. +- **Admin requestor notifications** — toast/badge when a user logs in if their request was approved/completed/rejected. → polish, slot into Fable #349 (UI polish pass) or earlier if it becomes friction. + +## 11. Open questions + +- **Album cover art proxying** — Lidarr returns image URLs that point at MusicBrainz/Cover Art Archive. The SPA could fetch directly (extra origins, CORS), or Minstrel could proxy them through `/api/cover-art?lidarr=...`. **Decision deferred to plan time** — start with direct fetch, add proxy if CORS bites. +- **Search debounce / cache** — Lidarr's lookup endpoint is the rate-limit-sensitive one. SPA debounce of 250ms + 60s server-side LRU cache on `(query, kind)` is the conservative starting point. Tunable. +- **`failed` status promotion** — at what time threshold does `approved` → `failed`? Suggest 7 days, but no logic for it ships in this slice. diff --git a/docs/superpowers/specs/2026-04-30-m5b-quarantine-design.md b/docs/superpowers/specs/2026-04-30-m5b-quarantine-design.md new file mode 100644 index 00000000..850ccbcf --- /dev/null +++ b/docs/superpowers/specs/2026-04-30-m5b-quarantine-design.md @@ -0,0 +1,389 @@ +# M5b — Quarantine workflow + admin resolution UI + +> **Status:** Draft for review · 2026-04-30 +> +> **Sub-plan of:** M5 (Lidarr integration + quarantine workflow). Decomposition recap from the M5a spec: +> +> - **M5a** — Lidarr connection + search/add + admin shell. Shipped on `dev`. +> - **M5b (this spec)** — Quarantine workflow (per-user soft-hide of tracks, admin resolution UI). +> - **M5c** — Radio "suggested additions" (out-of-library MBIDs surfaced in `/api/radio` responses; SPA add affordance). +> +> Each ships as its own PR with its own brainstorm/spec/plan cycle. + +## 1. Goal + +Authenticated users can flag any local track as broken with a reason and optional notes. A flagged track disappears from that user's library, search, browse, and `/api/radio` responses — but appears on a dedicated `/library/hidden` page where they can review and un-hide. Admins see an aggregated queue at `/admin/quarantine` (one row per track with reason distribution + per-user details + inline playback) and resolve each row by clearing the reports, deleting the local file, or telling Lidarr to remove the parent album with import-list exclusion. + +The dominant user mental model is **data quality**: "this track is a bad rip / wrong file / wrong tags / duplicate." Personal preference framing is out of scope. + +## 2. Goals and non-goals + +### Goals + +- Authenticated user can flag any track with reason ∈ {`bad_rip`, `wrong_file`, `wrong_tags`, `duplicate`, `other`} plus optional notes. +- Trigger affordance is a `` overflow (kebab) on every track row and on the now-playing player bar — sibling to `` but one-click-removed to prevent miss-clicks. +- Quarantined tracks are hidden from that user's `/api/albums/:id`, `/api/artists/:id`, library track lists, search, `/api/radio`, and contextual radio. They remain visible only on `/library/hidden`. +- `/library/hidden` page lists the user's own quarantines with un-hide affordance. +- Admin queue at `/admin/quarantine` aggregates by track, shows reason distribution + count, expandable per-user reports, inline playback, and the three resolution actions. +- Admin resolution actions: **Resolve** (clear the row), **Delete file** (rm file from disk + tracks row + clear), **Delete via Lidarr** (call Lidarr `DELETE /api/v1/album/{id}?deleteFiles=true&addImportListExclusion=true`, cascade Minstrel rows, clear). +- Admin actions write to a `lidarr_quarantine_actions` audit log with snapshot fields so the log stays readable after the underlying tracks/albums are deleted. +- Subsonic API (`/rest/*`) stays unfiltered — quarantine is `/api/*`-only. + +### Non-goals (this slice) + +- Per-album / per-artist quarantine. Tracks only. +- Quarantine on shared playlists, queues, or Subsonic clients. +- Auto-resolve rules ("if 3 users flag, auto-quarantine globally"). Admin always decides. +- Notifying users when their report is acted on (toast / email). → polish slot. +- Bulk admin operations (multi-select + apply). → revisit if queue grows beyond what one-by-one handles. + +## 3. Architecture + +### New Go packages + +- **`internal/lidarrquarantine/`** — `Service` owning the quarantine lifecycle: + - `Flag(ctx, userID, trackID, reason, notes)` — upsert a `lidarr_quarantine` row. + - `Unflag(ctx, userID, trackID)` — remove the caller's row. + - `ListMine(ctx, userID)` — caller's own quarantines (drives `/library/hidden`). + - `ListAdminQueue(ctx)` — aggregated by track. Returns `[]AdminQueueRow{TrackID, TrackTitle, ArtistName, AlbumTitle, AlbumID, LidarrAlbumMBID, ReportCount, ReasonCounts, LatestAt, Reports []UserReport}`. + - `Resolve(ctx, trackID, adminID)` — delete all per-user rows for the track + write audit row. + - `DeleteFile(ctx, trackID, adminID)` — call `library.DeleteTrackFile` then Resolve. + - `DeleteViaLidarr(ctx, trackID, adminID)` — look up the parent album in Lidarr by MBID, call `Client.DeleteAlbum(id, deleteFiles=true, addImportListExclusion=true)`, remove Minstrel rows for all tracks of that album, clear all related quarantine rows, write audit row. + +### Lidarr client additions (`internal/lidarr`) + +- `LookupArtistByMBID(ctx, mbid) (LidarrArtist, error)` — `GET /api/v1/artist?mbId=…`. +- `LookupAlbumByMBID(ctx, mbid) (LidarrAlbum, error)` — `GET /api/v1/album?foreignAlbumId=…`. +- `DeleteAlbum(ctx, lidarrAlbumID, deleteFiles bool, addImportListExclusion bool) error` — `DELETE /api/v1/album/{id}?deleteFiles=...&addImportListExclusion=...`. M5b admin path always passes both `true`. + +Returns the same typed errors the existing client uses: `ErrUnreachable`, `ErrAuthFailed`, `ErrLookupFailed`, plus `ErrNotFound` for the lookup methods when no row matches. + +### Library deletion (`internal/library`) + +- New `DeleteTrackFile(ctx, trackID) error` — removes the file from disk and the row from `tracks`. Album/artist rows stay (other tracks may reference them). The reconciler's MBID lookup will simply find no match next pass; M5a's reconcile is unaffected. + +### Soft-hide enforcement + +Every endpoint that returns track lists in user-context joins against `lidarr_quarantine`: + +```sql +WHERE NOT EXISTS ( + SELECT 1 FROM lidarr_quarantine q + WHERE q.user_id = $userID AND q.track_id = tracks.id +) +``` + +Affected queries (modified, not new): +- `GetAlbumDetail` (track list filter) +- `GetArtistDetail` / library artist views (transitively, via track filter) +- `Search` (track facet) +- Library track-list endpoints +- Radio generator + contextual radio endpoints + similarity-driven autoplay + +`/rest/*` Subsonic queries are NOT modified — Subsonic clients see the unfiltered library. + +### Wiring + +- `cmd/minstrel/main.go` constructs `lidarrquarantine.Service` and injects into `internal/api/handlers`. No new background worker. +- New handler files: `internal/api/quarantine.go` (user-facing), `internal/api/admin_quarantine.go` (admin endpoints). +- Reuses M5a's `RequireAdmin` middleware — `/api/admin/quarantine/*` mounts on the existing admin route group. + +### SPA additions + +- `` Svelte component — kebab icon button + dropdown menu. Renders a single "Flag this track…" item for M5b; designed for future actions. +- `` Svelte component — opens from `` with the reason ``: "Bad rip", "Wrong file", "Wrong tags", "Duplicate", "Other". +- Notes ` + + {#if error} +

Couldn't save flag — {error}

+ {/if} +
+ + +
+ diff --git a/web/src/lib/components/FlagPopover.test.ts b/web/src/lib/components/FlagPopover.test.ts new file mode 100644 index 00000000..0bd19363 --- /dev/null +++ b/web/src/lib/components/FlagPopover.test.ts @@ -0,0 +1,96 @@ +import { afterEach, describe, expect, test, vi } from 'vitest'; +import { render, screen, fireEvent } from '@testing-library/svelte'; +import FlagPopover from './FlagPopover.svelte'; +import type { TrackRef } from '$lib/api/types'; + +const invalidateMock = vi.fn(); +vi.mock('@tanstack/svelte-query', async (orig) => { + const actual = (await orig()) as Record; + return { + ...actual, + useQueryClient: () => ({ invalidateQueries: invalidateMock }) + }; +}); + +vi.mock('$lib/api/quarantine', () => ({ + flagTrack: vi.fn().mockResolvedValue({ track_id: 't1', reason: 'bad_rip' }) +})); + +import { flagTrack } from '$lib/api/quarantine'; + +const track: TrackRef = { + id: 't1', + title: 'Roygbiv', + album_id: 'a1', + album_title: 'Geogaddi', + artist_id: 'ar1', + artist_name: 'Boards of Canada', + duration_sec: 240, + stream_url: '/api/tracks/t1/stream' +}; + +afterEach(() => vi.clearAllMocks()); + +describe('FlagPopover', () => { + test('default reason is bad_rip; button reads "Flag" when not initialReason', () => { + render(FlagPopover, { props: { track, onClose: vi.fn() } }); + const select = screen.getByRole('combobox') as HTMLSelectElement; + expect(select.value).toBe('bad_rip'); + expect(screen.getByRole('button', { name: /^flag$/i })).toBeInTheDocument(); + }); + + test('pre-fills reason and notes when initial values are provided; button reads "Update flag"', () => { + render(FlagPopover, { + props: { + track, + onClose: vi.fn(), + initialReason: 'wrong_tags', + initialNotes: 'wrong artist' + } + }); + const select = screen.getByRole('combobox') as HTMLSelectElement; + expect(select.value).toBe('wrong_tags'); + const textarea = screen.getByPlaceholderText(/what's wrong/i) as HTMLTextAreaElement; + expect(textarea.value).toBe('wrong artist'); + expect(screen.getByRole('button', { name: /update flag/i })).toBeInTheDocument(); + }); + + test('submits with reason and notes; calls invalidateQueries on success', async () => { + const onClose = vi.fn(); + render(FlagPopover, { props: { track, onClose } }); + const select = screen.getByRole('combobox'); + await fireEvent.change(select, { target: { value: 'duplicate' } }); + const textarea = screen.getByPlaceholderText(/what's wrong/i); + await fireEvent.input(textarea, { target: { value: 'same recording' } }); + await fireEvent.click(screen.getByRole('button', { name: /^flag$/i })); + await Promise.resolve(); + await Promise.resolve(); + expect(flagTrack).toHaveBeenCalledWith({ + track_id: 't1', + reason: 'duplicate', + notes: 'same recording' + }); + expect(invalidateMock).toHaveBeenCalled(); + expect(onClose).toHaveBeenCalled(); + }); + + test('empty notes are not sent', async () => { + render(FlagPopover, { props: { track, onClose: vi.fn() } }); + await fireEvent.click(screen.getByRole('button', { name: /^flag$/i })); + await Promise.resolve(); + await Promise.resolve(); + expect(flagTrack).toHaveBeenCalledWith({ + track_id: 't1', + reason: 'bad_rip', + notes: undefined + }); + }); + + test('cancel button calls onClose without firing flagTrack', async () => { + const onClose = vi.fn(); + render(FlagPopover, { props: { track, onClose } }); + await fireEvent.click(screen.getByRole('button', { name: /cancel/i })); + expect(flagTrack).not.toHaveBeenCalled(); + expect(onClose).toHaveBeenCalled(); + }); +}); diff --git a/web/src/lib/components/PlayerBar.svelte b/web/src/lib/components/PlayerBar.svelte index e2e593b3..f6f78e56 100644 --- a/web/src/lib/components/PlayerBar.svelte +++ b/web/src/lib/components/PlayerBar.svelte @@ -7,6 +7,7 @@ import { formatDuration } from '$lib/media/duration'; import { FALLBACK_COVER } from '$lib/media/covers'; import LikeButton from './LikeButton.svelte'; + import TrackMenu from './TrackMenu.svelte'; const current = $derived(player.current); @@ -59,6 +60,7 @@ + diff --git a/web/src/lib/components/PlayerBar.test.ts b/web/src/lib/components/PlayerBar.test.ts index 67c31e02..f36fbf5a 100644 --- a/web/src/lib/components/PlayerBar.test.ts +++ b/web/src/lib/components/PlayerBar.test.ts @@ -51,6 +51,13 @@ vi.mock('$lib/api/likes', () => ({ unlikeEntity: vi.fn() })); +vi.mock('$lib/api/quarantine', () => ({ + flagTrack: vi.fn(), + unflagTrack: vi.fn(), + listMyQuarantine: vi.fn().mockResolvedValue([]), + createMyQuarantineQuery: vi.fn() +})); + vi.mock('@tanstack/svelte-query', async (orig) => { const actual = (await orig()) as Record; return { ...actual, useQueryClient: () => ({}) }; @@ -174,4 +181,11 @@ describe('PlayerBar', () => { expect(screen.getByText('1:05')).toBeInTheDocument(); expect(screen.getByText('4:05')).toBeInTheDocument(); }); + + test('renders the TrackMenu kebab button when a track is current', () => { + render(PlayerBar); + expect( + screen.getByRole('button', { name: /track actions for/i }) + ).toBeInTheDocument(); + }); }); diff --git a/web/src/lib/components/Shell.svelte b/web/src/lib/components/Shell.svelte index fb9a0a90..80d7af57 100644 --- a/web/src/lib/components/Shell.svelte +++ b/web/src/lib/components/Shell.svelte @@ -22,12 +22,22 @@ } const navItems = [ - { href: '/', label: 'Library' }, - { href: '/library/liked', label: 'Liked' }, - { href: '/search', label: 'Search' }, - { href: '/playlists', label: 'Playlists' }, - { href: '/settings', label: 'Settings' } + { href: '/', label: 'Library' }, + { href: '/library/liked', label: 'Liked' }, + { href: '/library/hidden', label: 'Hidden' }, + { href: '/search', label: 'Search' }, + { href: '/discover', label: 'Discover' }, + { href: '/requests', label: 'Requests' }, + { href: '/playlists', label: 'Playlists' }, + { href: '/settings', label: 'Settings' } ]; + + // Admin link sits between Playlists and Settings, only visible to admins. + const visibleNavItems = $derived( + user.value?.is_admin + ? [...navItems.slice(0, -1), { href: '/admin', label: 'Admin' }, navItems[navItems.length - 1]] + : navItems + ); (menuOpen = false)} onkeydown={(e) => e.key === 'Escape' && (menuOpen = false)} /> @@ -68,7 +78,7 @@