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.