fix: database migration, config validation and provider client defects

Completes the correctness pass.

Download database migration no longer destroys dedupe state
  The pre-source-column migration carried every legacy row over with
  source='' and then dropped downloads_legacy, while IsDownloaded matched
  source exactly. The result was that every previously recorded download
  looked new, the user silently re-downloaded their whole library, and the
  original rows were gone. Now the file is copied to <path>.bak-<unixtime>
  first, downloads_legacy is kept, and IsDownloaded falls back to the
  sourceless rows (over-matching across sources costs a skipped re-download,
  which is the cheaper error). Schema state is tracked in PRAGMA user_version
  so later migrations can be ordered explicitly. Databases already migrated by
  the old code are detected on open.

  Also enables WAL and a 10s busy_timeout and caps the pool at one connection,
  so a concurrent reader no longer gets SQLITE_BUSY with no retry, and builds
  the DSN through url.URL so a path containing '?' is not parsed as query
  parameters.

--quality now applies to every source
  applyGlobalConfigOverrides set only qobuz, tidal, deezer and soundcloud, so
  `rip -q 4 url <beatport-url>` was a silent no-op even though
  qualityForSource reads Yandex and Beatport. All six are set, with the
  per-source floor applied and reported rather than silently ignored.

config: validation and a fixed reset
  Load checked nothing but misc.version, so a negative max_connections or a
  bogus conversion codec was accepted and only surfaced mid-rip. Adds
  Validate() covering per-source quality ranges, max_connections,
  requests_per_minute and truncate_to, and normalizes the conversion codec
  from the file through the same path as --codec. `config reset` assigned one
  value to both Config.File and Config.Session, so their Secrets slices shared
  a backing array; it now takes separate copies.

  Tests cover the shipped defaults and config.toml.example, which is how the
  over-strict artwork max-width bound in the first draft was caught: -1 is the
  documented "do not downscale" sentinel.

soundcloud: use the shared HTTP client
  It was the only provider constructing its own http.Client, so it silently
  ignored downloads.verify_ssl and max_connections and bypassed the transport
  that redacts credentials from -vv logs. It also had no rate limiter at all.

qobuz: mobile fallback could not download a long track
  http.Client.Timeout bounds the whole request including the body read, so the
  30s API client aborted any track that took longer than 30s to stream. The
  body copy now uses a no-timeout client, as internal/download already does.

qobuz: HKDF no longer fails silently
  hexDecodeOrNil discarded its error, so a non-hex app secret keyed HKDF on
  nil, derived a wrong KEK and produced silently corrupt audio instead of an
  error.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

cmd/rip/args.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"os"
 	"strconv"
 	"strings"
 
@@ -124,16 +125,10 @@ func parseGlobalArgs(args []string) (globalOptions, error) {
 	return opts, nil
 }
 
+// normalizeCodec delegates to config so the --codec flag and a codec set in
+// the config file are validated by the same rules.
 func normalizeCodec(raw string) (string, error) {
-	codec := strings.ToUpper(strings.TrimSpace(raw))
+	return config.NormalizeCodec(raw)
-	switch codec {
-	case "ALAC", "FLAC", "MP3", "AAC", "VORBIS":
-		return codec, nil
-	case "OGG":
-		return "VORBIS", nil
-	default:
-		return "", fmt.Errorf("unsupported codec %q (expected ALAC, FLAC, OGG, MP3, AAC)", raw)
-	}
 }
 
 func applyGlobalConfigOverrides(cfg *config.Config, opts globalOptions) {
@@ -144,10 +139,7 @@ func applyGlobalConfigOverrides(cfg *config.Config, opts globalOptions) {
 		cfg.Session.Database.DownloadsEnabled = false
 	}
 	if opts.qualitySet {
-		cfg.Session.Qobuz.Quality = opts.quality
+		applyQualityOverride(cfg, opts.quality)
-		cfg.Session.Tidal.Quality = opts.quality
-		cfg.Session.Deezer.Quality = opts.quality
-		cfg.Session.Soundcloud.Quality = opts.quality
 	}
 	if opts.codecSet {
 		cfg.Session.Conversion.Enabled = true
@@ -161,6 +153,50 @@ func applyGlobalConfigOverrides(cfg *config.Config, opts globalOptions) {
 	}
 }
 
+// minQualityForSource is the lowest quality id each source accepts. Qobuz and
+// Beatport have no equivalent of the "lowest" tier that 0 means elsewhere.
+var minQualityForSource = map[string]int{
+	"qobuz":    1,
+	"beatport": 1,
+}
+
+// applyQualityOverride applies a global -q/--quality to every source.
+//
+// It previously set only qobuz, tidal, deezer and soundcloud, so
+// `rip -q 4 url <beatport-url>` was a silent no-op even though
+// Main.qualityForSource reads Yandex and Beatport too. Sources with a higher
+// floor are clamped rather than erroring, because one global flag has to work
+// across a mixed queue.
+func applyQualityOverride(cfg *config.Config, quality int) {
+	clamped := map[string]int{}
+	for _, source := range []string{"qobuz", "tidal", "deezer", "soundcloud", "yandex", "beatport"} {
+		q := quality
+		if min, ok := minQualityForSource[source]; ok && q < min {
+			q = min
+			clamped[source] = q
+		}
+		switch source {
+		case "qobuz":
+			cfg.Session.Qobuz.Quality = q
+		case "tidal":
+			cfg.Session.Tidal.Quality = q
+		case "deezer":
+			cfg.Session.Deezer.Quality = q
+		case "soundcloud":
+			cfg.Session.Soundcloud.Quality = q
+		case "yandex":
+			cfg.Session.Yandex.Quality = q
+		case "beatport":
+			cfg.Session.Beatport.Quality = q
+		}
+	}
+	for _, source := range []string{"qobuz", "beatport"} {
+		if q, ok := clamped[source]; ok {
+			fmt.Fprintf(os.Stderr, "note: %s has no quality %d; using %d for %s\n", source, quality, q, source)
+		}
+	}
+}
+
 func errorWithActionableHint(err error, opts globalOptions) string {
 	if err == nil {
 		return ""

cmd/rip/main.go

@@ -261,8 +261,11 @@ func run() int {
 				}
 			}
 			def := config.DefaultConfigData()
-			cfg.File = def
+			// Separate copies: assigning the same value to both halves made
-			cfg.Session = def
+			// File.Qobuz.Secrets and Session.Qobuz.Secrets share one backing
+			// array.
+			cfg.File = config.CloneConfigData(def)
+			cfg.Session = config.CloneConfigData(def)
 			if err = cfg.SaveFile(); err != nil {
 				fmt.Fprintf(os.Stderr, "reset config error: %v\n", err)
 				return 1

internal/config/config.go

@@ -213,9 +213,92 @@ func Load(path string) (*Config, error) {
 		return nil, fmt.Errorf("%w: need to update from %q to %q", ErrOutdatedConfig, data.Misc.Version, CurrentConfigVersion)
 	}
 
+	if err = Validate(&data); err != nil {
+		return nil, err
+	}
+
 	return &Config{Path: resolvedPath, File: data, Session: cloneConfigData(data)}, nil
 }
 
+// qualityRange is the accepted quality id range per source. Qobuz and Beatport
+// have no "lowest" tier, so their floor is 1.
+var qualityRange = map[string][2]int{
+	"qobuz":      {1, 4},
+	"tidal":      {0, 4},
+	"deezer":     {0, 2},
+	"soundcloud": {0, 1},
+	"yandex":     {0, 4},
+	"beatport":   {1, 4},
+}
+
+// Validate rejects config values that would otherwise fail much later, deep in
+// a rip, or be silently patched over at individual call sites. Load previously
+// checked nothing but misc.version, so a negative max_connections or a bogus
+// conversion codec was accepted and only noticed mid-download.
+//
+// It normalizes what can be normalized (the conversion codec, which until now
+// was only canonicalized when it arrived via --codec) and errors on the rest.
+func Validate(data *ConfigData) error {
+	for source, bounds := range qualityRange {
+		var q int
+		switch source {
+		case "qobuz":
+			q = data.Qobuz.Quality
+		case "tidal":
+			q = data.Tidal.Quality
+		case "deezer":
+			q = data.Deezer.Quality
+		case "soundcloud":
+			q = data.Soundcloud.Quality
+		case "yandex":
+			q = data.Yandex.Quality
+		case "beatport":
+			q = data.Beatport.Quality
+		}
+		if q < bounds[0] || q > bounds[1] {
+			return fmt.Errorf("config: %s.quality is %d, must be %d-%d", source, q, bounds[0], bounds[1])
+		}
+	}
+
+	if data.Downloads.MaxConnections < 1 {
+		return fmt.Errorf("config: downloads.max_connections is %d, must be >= 1", data.Downloads.MaxConnections)
+	}
+	if data.Downloads.RequestsPerMinute == 0 {
+		return errors.New("config: downloads.requests_per_minute is 0; use a positive value, or a negative value to disable throttling")
+	}
+	if data.Filepaths.TruncateTo < 1 {
+		return fmt.Errorf("config: filepaths.truncate_to is %d, must be >= 1", data.Filepaths.TruncateTo)
+	}
+
+	if data.Conversion.Enabled {
+		codec, err := NormalizeCodec(data.Conversion.Codec)
+		if err != nil {
+			return fmt.Errorf("config: conversion.codec: %w", err)
+		}
+		data.Conversion.Codec = codec
+	}
+
+	// artwork.*_max_width uses -1 (and any non-positive value) to mean "do not
+	// downscale", so there is no lower bound to enforce here.
+
+	return nil
+}
+
+// NormalizeCodec canonicalizes a conversion codec name. It lives here, rather
+// than only in the CLI flag parser, so a codec set in the config file goes
+// through the same check.
+func NormalizeCodec(raw string) (string, error) {
+	codec := strings.ToUpper(strings.TrimSpace(raw))
+	switch codec {
+	case "ALAC", "FLAC", "MP3", "AAC", "VORBIS":
+		return codec, nil
+	case "OGG":
+		return "VORBIS", nil
+	default:
+		return "", fmt.Errorf("unsupported codec %q (expected ALAC, FLAC, OGG, MP3, AAC)", raw)
+	}
+}
+
 func UpgradeOutdated(path string) (string, error) {
 	resolvedPath, err := resolvePath(path)
 	if err != nil {
@@ -420,6 +503,13 @@ func applyRuntimeDefaults(data *ConfigData) {
 	}
 }
 
+// CloneConfigData returns a deep copy of in. The slice fields must not be
+// shared between Config.File and Config.Session, or writing one silently
+// mutates the other.
+func CloneConfigData(in ConfigData) ConfigData {
+	return cloneConfigData(in)
+}
+
 func cloneConfigData(in ConfigData) ConfigData {
 	out := in
 	out.Qobuz.Secrets = append([]string(nil), in.Qobuz.Secrets...)

internal/config/config_test.go

@@ -5,6 +5,8 @@ import (
 	"os"
 	"path/filepath"
 	"testing"
+
+	"github.com/pelletier/go-toml/v2"
 )
 
 func TestDefaultConfigData(t *testing.T) {
@@ -110,3 +112,30 @@ func TestSessionCloneDoesNotAliasSlices(t *testing.T) {
 		t.Fatalf("file qobuz secrets unexpectedly mutated")
 	}
 }
+
+func TestDefaultConfigPassesValidation(t *testing.T) {
+	d := DefaultConfigData()
+	applyRuntimeDefaults(&d)
+	if err := Validate(&d); err != nil {
+		t.Fatalf("shipped defaults fail validation: %v", err)
+	}
+}
+
+// TestExampleConfigIsValid guards the file users are told to copy.
+func TestExampleConfigIsValid(t *testing.T) {
+	raw, err := os.ReadFile(filepath.Join("..", "..", "config.toml.example"))
+	if err != nil {
+		t.Skipf("example config not found: %v", err)
+	}
+	data := DefaultConfigData()
+	if err := toml.Unmarshal(raw, &data); err != nil {
+		t.Fatalf("config.toml.example does not parse: %v", err)
+	}
+	applyRuntimeDefaults(&data)
+	if err := Validate(&data); err != nil {
+		t.Fatalf("config.toml.example fails validation: %v", err)
+	}
+	if data.Misc.Version != CurrentConfigVersion {
+		t.Errorf("config.toml.example version = %q, want %q", data.Misc.Version, CurrentConfigVersion)
+	}
+}

internal/download/downloader_test.go

@@ -4,8 +4,8 @@ import (
 	"context"
 	"crypto/aes"
 	"crypto/cipher"
-	"errors"
 	"encoding/hex"
+	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"

internal/provider/qobuz/client.go

@@ -61,6 +61,7 @@ var (
 type Client struct {
 	cfg        *config.Config
 	http       *http.Client
+	streamHTTP *http.Client
 	limiter    *ratelimit.Limiter
 	baseURL    string
 	fetchCfg   func(ctx context.Context) (string, []string, error)
@@ -93,6 +94,11 @@ func New(cfg *config.Config) *Client {
 	return &Client{
 		cfg:  cfg,
 		http: netutil.NewHTTPClient(30*time.Second, cfg.Session.Downloads.VerifySSL, cfg.Session.Downloads.MaxConnections),
+		// Separate client with no Timeout for streaming a whole track body.
+		// http.Client.Timeout bounds the entire request including the body
+		// read, so the 30s API client aborted any track that took longer than
+		// 30s to stream. internal/download does the same for this reason.
+		streamHTTP: netutil.NewHTTPClient(0, cfg.Session.Downloads.VerifySSL, cfg.Session.Downloads.MaxConnections),
 		limiter:    ratelimit.New(cfg.Session.Downloads.RequestsPerMinute),
 		baseURL:    baseURL,
 		fetchCfg:   nil,
@@ -1171,7 +1177,11 @@ func (c *Client) mobileCopyURLToWriter(ctx context.Context, sourceURL string, ou
 		return err
 	}
 	c.setMobileHeaders(req)
-	resp, err := c.http.Do(req)
+	client := c.streamHTTP
+	if client == nil {
+		client = c.http
+	}
+	resp, err := client.Do(req)
 	if err != nil {
 		return err
 	}
@@ -1316,7 +1326,17 @@ func (c *Client) mobileDeriveTrackKey(encryptedKey string) ([]byte, error) {
 	if err != nil {
 		return nil, fmt.Errorf("decode mobile info: %w", err)
 	}
-	reader := hkdf.New(func() hash.Hash { return sha256.New() }, hexDecodeOrNil(mobileAppSecret), salt, info)
+	// A non-hex secret used to be swallowed and HKDF keyed on nil, which
+	// derives a wrong KEK and yields silently corrupt audio rather than an
+	// error.
+	secretKey, err := hex.DecodeString(mobileAppSecret)
+	if err != nil {
+		return nil, fmt.Errorf("decode mobile app secret: %w", err)
+	}
+	if len(secretKey) == 0 {
+		return nil, errors.New("mobile app secret is empty")
+	}
+	reader := hkdf.New(func() hash.Hash { return sha256.New() }, secretKey, salt, info)
 	kek := make([]byte, 16)
 	if _, err = io.ReadFull(reader, kek); err != nil {
 		return nil, fmt.Errorf("mobile hkdf derive failed: %w", err)
@@ -1325,11 +1345,6 @@ func (c *Client) mobileDeriveTrackKey(encryptedKey string) ([]byte, error) {
 	return unwrapQobuzTrackKey(encryptedKey, kek)
 }
 
-func hexDecodeOrNil(s string) []byte {
-	b, _ := hex.DecodeString(s)
-	return b
-}
-
 func unwrapQobuzTrackKey(encryptedKey string, kek []byte) ([]byte, error) {
 	parts := strings.SplitN(encryptedKey, ".", 3)
 	if len(parts) != 3 || parts[0] != mobileSessionProf {

internal/provider/soundcloud/client.go

@@ -16,7 +16,9 @@ import (
 
 	"streamrip-go/internal/config"
 	"streamrip-go/internal/jsonutil"
+	"streamrip-go/internal/netutil"
 	"streamrip-go/internal/provider"
+	"streamrip-go/internal/ratelimit"
 )
 
 var errUnsupportedMediaType = errors.New("unsupported soundcloud media type")
@@ -30,6 +32,7 @@ type Client struct {
 	bin     string
 	run     commandRunner
 	http    *http.Client
+	limiter *ratelimit.Limiter
 	mu      sync.Mutex
 	cache   map[string]map[string]any
 
@@ -44,7 +47,12 @@ func New(cfg *config.Config) *Client {
 		cfg: cfg,
 		bin: "yt-dlp",
 		run: runCommand,
-		http:  &http.Client{Timeout: 20 * time.Second},
+		// Use the shared constructor like every other provider: rolling our own
+		// client here meant soundcloud silently ignored downloads.verify_ssl and
+		// downloads.max_connections, and bypassed the transport that logs
+		// requests at -vv with credentials redacted.
+		http:    netutil.NewHTTPClient(20*time.Second, cfg.Session.Downloads.VerifySSL, cfg.Session.Downloads.MaxConnections),
+		limiter: ratelimit.New(cfg.Session.Downloads.RequestsPerMinute),
 		cache:   map[string]map[string]any{},
 	}
 }
@@ -141,6 +149,11 @@ func (c *Client) searchPlaylists(ctx context.Context, query string, limit int) (
 	}
 	req.Header.Set("User-Agent", "Mozilla/5.0")
 
+	if c.limiter != nil {
+		if err = c.limiter.Wait(ctx); err != nil {
+			return nil, err
+		}
+	}
 	resp, err := c.http.Do(req)
 	if err != nil {
 		return nil, err

internal/store/sqlite.go

@@ -3,23 +3,40 @@ package store
 import (
 	"context"
 	"database/sql"
+	"fmt"
+	"io"
+	"net/url"
+	"os"
+	"strings"
 	"sync"
+	"time"
 
 	_ "modernc.org/sqlite"
 )
 
 type SQLite struct {
 	db   *sql.DB
+	path string
 	mu   sync.Mutex
+
+	// hasLegacyRows is set when the database was migrated from the pre-source
+	// schema. Those rows have no source, so IsDownloaded has to consider them
+	// separately or the entire pre-migration library looks undownloaded.
+	hasLegacyRows bool
+	legacyWarned  bool
 }
 
 func NewSQLite(path string) (*SQLite, error) {
-	db, err := sql.Open("sqlite", path)
+	db, err := sql.Open("sqlite", sqliteDSN(path))
 	if err != nil {
 		return nil, err
 	}
+	// One writer at a time. SQLite allows a single writer anyway, and the
+	// mutex below already serializes this process; capping the pool keeps the
+	// driver from opening connections that can only contend with each other.
+	db.SetMaxOpenConns(1)
 
-	s := &SQLite{db: db}
+	s := &SQLite{db: db, path: path}
 	if err = s.init(); err != nil {
 		_ = db.Close()
 		return nil, err
@@ -27,6 +44,70 @@ func NewSQLite(path string) (*SQLite, error) {
 	return s, nil
 }
 
+// sqliteDSN builds a file: URI with the pragmas this app needs.
+//
+// WAL lets a concurrent reader (for example `rip db browse`) run while a rip
+// is writing, and busy_timeout makes a contended write wait instead of failing
+// immediately with SQLITE_BUSY. The path is passed through url.URL so that a
+// config path containing '?' or '#' is not parsed as query parameters.
+func sqliteDSN(path string) string {
+	u := url.URL{Scheme: "file", Opaque: (&url.URL{Path: path}).EscapedPath()}
+	q := url.Values{}
+	q.Add("_pragma", "busy_timeout(10000)")
+	q.Add("_pragma", "journal_mode(WAL)")
+	q.Add("_pragma", "foreign_keys(on)")
+	return u.String() + "?" + q.Encode()
+}
+
+// schemaVersion is stored in PRAGMA user_version so later migrations can be
+// ordered explicitly instead of being inferred from PRAGMA table_info.
+const schemaVersion = 1
+
+func (s *SQLite) userVersion() (int, error) {
+	var v int
+	if err := s.db.QueryRow(`PRAGMA user_version`).Scan(&v); err != nil {
+		return 0, err
+	}
+	return v, nil
+}
+
+func (s *SQLite) setUserVersion(v int) error {
+	_, err := s.db.Exec(fmt.Sprintf(`PRAGMA user_version = %d`, v))
+	return err
+}
+
+// backupFile copies the database aside before a destructive migration. A
+// migration that loses the download history silently costs the user a full
+// re-download, so keep a copy they can fall back to.
+func (s *SQLite) backupFile() (string, error) {
+	if strings.TrimSpace(s.path) == "" {
+		return "", nil
+	}
+	src, err := os.Open(s.path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return "", nil
+		}
+		return "", err
+	}
+	defer func() { _ = src.Close() }()
+
+	dstPath := fmt.Sprintf("%s.bak-%d", s.path, time.Now().Unix())
+	dst, err := os.OpenFile(dstPath, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600)
+	if err != nil {
+		return "", err
+	}
+	if _, err = io.Copy(dst, src); err != nil {
+		_ = dst.Close()
+		_ = os.Remove(dstPath)
+		return "", err
+	}
+	if err = dst.Close(); err != nil {
+		return "", err
+	}
+	return dstPath, nil
+}
+
 func (s *SQLite) init() error {
 	if err := s.ensureDownloadsSchema(); err != nil {
 		return err
@@ -47,6 +128,20 @@ func (s *SQLite) init() error {
 		}
 	}
 
+	// A database migrated by an earlier version has sourceless rows but no
+	// in-memory flag, so detect them here rather than only at migration time.
+	if !s.hasLegacyRows {
+		if n, err := s.legacyRowCount(context.Background()); err == nil && n > 0 {
+			s.hasLegacyRows = true
+		}
+	}
+
+	if v, err := s.userVersion(); err == nil && v < schemaVersion {
+		if err = s.setUserVersion(schemaVersion); err != nil {
+			return err
+		}
+	}
+
 	return nil
 }
 
@@ -94,6 +189,14 @@ func (s *SQLite) ensureDownloadsSchema() error {
 		return err
 	}
 
+	// Legacy schema: a downloads table with no source column. Back the file up
+	// first, because the rows below can only be carried over without a source
+	// and that is not fully reversible.
+	backupPath, backupErr := s.backupFile()
+	if backupErr != nil {
+		return fmt.Errorf("could not back up %s before migrating: %w", s.path, backupErr)
+	}
+
 	tx, err := s.db.Begin()
 	if err != nil {
 		return err
@@ -120,12 +223,27 @@ func (s *SQLite) ensureDownloadsSchema() error {
 	if _, err = tx.Exec(`INSERT OR IGNORE INTO downloads(source, id) SELECT '', id FROM downloads_legacy`); err != nil {
 		return err
 	}
-	if _, err = tx.Exec(`DROP TABLE downloads_legacy`); err != nil {
+	// downloads_legacy is deliberately kept. Dropping it used to make the
+	// migration unrecoverable, and IsDownloaded needs the sourceless rows to
+	// avoid re-downloading everything the user already has.
+
+	if err = tx.Commit(); err != nil {
 		return err
 	}
 
-	err = tx.Commit()
+	s.hasLegacyRows = true
-	return err
+	if backupPath != "" {
+		fmt.Fprintf(os.Stderr, "download database migrated to the source-aware schema; previous copy kept at %s\n", backupPath)
+	}
+	return nil
+}
+
+// legacyRowCount reports how many pre-migration rows exist. Rows carried over
+// from the old schema have an empty source.
+func (s *SQLite) legacyRowCount(ctx context.Context) (int, error) {
+	var n int
+	err := s.db.QueryRowContext(ctx, `SELECT COUNT(1) FROM downloads WHERE source = ''`).Scan(&n)
+	return n, err
 }
 
 func (s *SQLite) IsDownloaded(ctx context.Context, source, id string) (bool, error) {
@@ -137,6 +255,26 @@ func (s *SQLite) IsDownloaded(ctx context.Context, source, id string) (bool, err
 	if err != nil {
 		return false, err
 	}
+	if count > 0 {
+		return true, nil
+	}
+
+	// Rows migrated from the pre-source schema have an empty source, so an
+	// exact match on (source, id) misses them and the whole pre-migration
+	// library would look undownloaded. Matching on id alone can only ever
+	// over-match across sources, which costs a skipped re-download rather than
+	// a duplicated library.
+	if !s.hasLegacyRows {
+		return false, nil
+	}
+	err = s.db.QueryRowContext(ctx, `SELECT COUNT(1) FROM downloads WHERE source = '' AND id = ?`, id).Scan(&count)
+	if err != nil {
+		return false, err
+	}
+	if count > 0 && !s.legacyWarned {
+		s.legacyWarned = true
+		fmt.Fprintf(os.Stderr, "note: matching downloads recorded before sources were tracked; pass --force to re-download them\n")
+	}
 	return count > 0, nil
 }
 

internal/store/sqlite_test.go

@@ -2,7 +2,12 @@ package store
 
 import (
 	"context"
+	"database/sql"
+	"fmt"
+	"os"
 	"path/filepath"
+	"strings"
+	"sync"
 	"testing"
 )
 
@@ -48,3 +53,193 @@ func TestSQLiteStore(t *testing.T) {
 		t.Fatalf("MarkFailed() error = %v", err)
 	}
 }
+
+// makeLegacyDB creates a database with the pre-source-column schema and the
+// given ids, as a pre-migration install would have on disk.
+func makeLegacyDB(t *testing.T, path string, ids ...string) {
+	t.Helper()
+	db, err := sql.Open("sqlite", path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = db.Close() }()
+
+	if _, err = db.Exec(`CREATE TABLE downloads (id TEXT NOT NULL PRIMARY KEY)`); err != nil {
+		t.Fatal(err)
+	}
+	for _, id := range ids {
+		if _, err = db.Exec(`INSERT INTO downloads(id) VALUES (?)`, id); err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
+// TestLegacyMigrationPreservesHistory is the regression test for the migration
+// that silently wiped the dedupe state: rows were carried over with source=empty
+// while IsDownloaded matched source exactly, so every previously downloaded
+// track looked new and the whole library was re-downloaded.
+func TestLegacyMigrationPreservesHistory(t *testing.T) {
+	ctx := context.Background()
+	dir := t.TempDir()
+	path := filepath.Join(dir, "downloads.db")
+	makeLegacyDB(t, path, "old-1", "old-2")
+
+	s, err := NewSQLite(path)
+	if err != nil {
+		t.Fatalf("NewSQLite() error = %v", err)
+	}
+	defer func() { _ = s.Close() }()
+
+	for _, id := range []string{"old-1", "old-2"} {
+		ok, err := s.IsDownloaded(ctx, "qobuz", id)
+		if err != nil {
+			t.Fatalf("IsDownloaded(%q) error = %v", id, err)
+		}
+		if !ok {
+			t.Errorf("pre-migration download %q reported as not downloaded", id)
+		}
+	}
+
+	ok, err := s.IsDownloaded(ctx, "qobuz", "never-seen")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if ok {
+		t.Error("unknown id reported as downloaded")
+	}
+}
+
+// TestLegacyMigrationKeepsBackupAndLegacyTable asserts the migration is
+// recoverable: the original file is copied aside and downloads_legacy survives.
+func TestLegacyMigrationKeepsBackupAndLegacyTable(t *testing.T) {
+	dir := t.TempDir()
+	path := filepath.Join(dir, "downloads.db")
+	makeLegacyDB(t, path, "old-1")
+
+	s, err := NewSQLite(path)
+	if err != nil {
+		t.Fatalf("NewSQLite() error = %v", err)
+	}
+	defer func() { _ = s.Close() }()
+
+	entries, err := os.ReadDir(dir)
+	if err != nil {
+		t.Fatal(err)
+	}
+	backups := 0
+	for _, e := range entries {
+		if strings.Contains(e.Name(), ".bak-") {
+			backups++
+		}
+	}
+	if backups != 1 {
+		t.Errorf("expected exactly 1 backup file, found %d in %v", backups, entries)
+	}
+
+	var n int
+	if err = s.db.QueryRow(`SELECT COUNT(1) FROM downloads_legacy`).Scan(&n); err != nil {
+		t.Fatalf("downloads_legacy was dropped: %v", err)
+	}
+	if n != 1 {
+		t.Errorf("downloads_legacy row count = %d, want 1", n)
+	}
+
+	if v, err := s.userVersion(); err != nil || v != schemaVersion {
+		t.Errorf("user_version = %d (err %v), want %d", v, err, schemaVersion)
+	}
+}
+
+// TestLegacyRowsDetectedOnReopen covers a database already migrated by the
+// earlier, lossy code path: the sourceless rows are on disk but nothing in
+// this process migrated them.
+func TestLegacyRowsDetectedOnReopen(t *testing.T) {
+	ctx := context.Background()
+	path := filepath.Join(t.TempDir(), "downloads.db")
+	makeLegacyDB(t, path, "old-1")
+
+	s, err := NewSQLite(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_ = s.Close()
+
+	reopened, err := NewSQLite(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = reopened.Close() }()
+
+	if !reopened.hasLegacyRows {
+		t.Fatal("legacy rows not detected on reopen")
+	}
+	ok, err := reopened.IsDownloaded(ctx, "tidal", "old-1")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !ok {
+		t.Error("legacy row not honored after reopen")
+	}
+}
+
+// TestConcurrentAccess exercises the mutex and connection handling under load.
+func TestConcurrentAccess(t *testing.T) {
+	ctx := context.Background()
+	path := filepath.Join(t.TempDir(), "downloads.db")
+	s, err := NewSQLite(path)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func() { _ = s.Close() }()
+
+	var wg sync.WaitGroup
+	for i := 0; i < 32; i++ {
+		wg.Add(1)
+		go func(n int) {
+			defer wg.Done()
+			id := fmt.Sprintf("t%d", n)
+			if err := s.MarkDownloaded(ctx, "qobuz", id); err != nil {
+				t.Errorf("MarkDownloaded: %v", err)
+				return
+			}
+			if _, err := s.IsDownloaded(ctx, "qobuz", id); err != nil {
+				t.Errorf("IsDownloaded: %v", err)
+			}
+			if err := s.MarkFailed(ctx, "qobuz", "track", id); err != nil {
+				t.Errorf("MarkFailed: %v", err)
+			}
+		}(i)
+	}
+	wg.Wait()
+}
+
+func TestSQLiteDSNPragmasApplied(t *testing.T) {
+	// A path containing '?' must not be parsed as query parameters.
+	dir := t.TempDir()
+	path := filepath.Join(dir, "weird?name#x.db")
+
+	s, err := NewSQLite(path)
+	if err != nil {
+		t.Fatalf("NewSQLite(%q) = %v", path, err)
+	}
+	defer func() { _ = s.Close() }()
+
+	var mode string
+	if err := s.db.QueryRow(`PRAGMA journal_mode`).Scan(&mode); err != nil {
+		t.Fatal(err)
+	}
+	if mode != "wal" {
+		t.Errorf("journal_mode = %q, want wal", mode)
+	}
+
+	var timeout int
+	if err := s.db.QueryRow(`PRAGMA busy_timeout`).Scan(&timeout); err != nil {
+		t.Fatal(err)
+	}
+	if timeout != 10000 {
+		t.Errorf("busy_timeout = %d, want 10000", timeout)
+	}
+
+	if err := s.MarkDownloaded(context.Background(), "qobuz", "a"); err != nil {
+		t.Fatalf("write to odd path failed: %v", err)
+	}
+}