fix(topics): stop resolved titles being overwritten on scheduler check (#94)

* docs(90): spec placeholder-only self-heal gate for display name

Design for issue #90. The scheduler self-heal overwrites a topic's
stored title with check.DisplayName on any difference, so a noisier
Check title clobbers a good add-time title every poll. Kinozal also
fetches its Check title from the raw mirror host while the rest of the
plugin pins kinozal.tv, producing the divergent title.

Spec: a display_name_is_placeholder provenance flag (migration 0010)
gates self-heal to placeholder-only and locks on first resolve; plus
canonicalize kinozal's Check title host. Lock all existing rows.

* docs(#90): add implementation plan for self-heal gate

* feat(#90): add topics.display_name_is_placeholder column

* feat(#90): flag placeholder vs resolved title at topic creation

* fix(#90): gate display-name self-heal to placeholder titles only

* feat(#90): lock display-name flag on resolve and user rename

* fix(#90): canonicalize kinozal Check title host to p.domain

* docs(#90): document display-name placeholder flag and gated self-heal

Also apply gofmt to domain/domain.go, topics/create.go, and
kinozal/kinozal_test.go (whitespace-only alignment fixes left by
earlier tasks in the series).

Author: artyomsv

CLAUDE.md

@@ -49,7 +49,7 @@ techdebt/       Debt-tracking files (one per issue, see global rule)
 | `cfsolver` | in-process client to the standalone `cfsolver/` service |
 | `config` | env-driven config struct (caarlos0/env) — **add new env vars here** |
 | `crypto` | AES-256-GCM for tracker credentials and client config blobs |
-| `db` / `db/repo` | pgxpool wrapper + repository structs (`Topics`, `Clients`, `Notifiers`, `Users`, `TrackerCredentials`, `Deliveries`, `Audit`, `Settings`). `Settings` (repo added for the Sonarr integration) reads/writes the singleton `settings` row — `GetSonarr`/`UpsertSonarr` (API key encrypted at rest like `oidc_client_secret_enc`, migration `0009`) + `UpdateSonarrCursor` (history-poll cursor). `Topics` adds `GetByURL(user,url)` for the poller's dedup pre-check; `Users` adds `GetInitialAdmin`. `TrackerCredentials` carries encrypted `secret_enc` (password) **and** `session_enc`/`session_nonce` (cookie-session blob; migration `0002`), plus a nullable `session_expired_at` marker (migration `0003`) the scheduler uses to dedupe expiry notifications (atomic `MarkSessionExpired`, cleared by `SetSession` on re-auth). `Deliveries` (migration `0006`, `topic_deliveries`) records every torrent pushed to a client — `{topic_id, infohash, label, client_id, delivered_at}`, unique on `(topic_id, infohash)` so `Record` is idempotent (`ON CONFLICT DO NOTHING`). `Notifiers` gains `is_default bool` (migration `0008`, per-type unique partial index) + `Update(ctx, id, userID, name, displayName string, events []string, isDefault bool, configEnc, configNonce []byte) error` |
+| `db` / `db/repo` | pgxpool wrapper + repository structs (`Topics`, `Clients`, `Notifiers`, `Users`, `TrackerCredentials`, `Deliveries`, `Audit`, `Settings`). `Settings` (repo added for the Sonarr integration) reads/writes the singleton `settings` row — `GetSonarr`/`UpsertSonarr` (API key encrypted at rest like `oidc_client_secret_enc`, migration `0009`) + `UpdateSonarrCursor` (history-poll cursor). `Topics` adds `GetByURL(user,url)` for the poller's dedup pre-check; `Users` adds `GetInitialAdmin`. `Topics` also carries `display_name_is_placeholder` (migration `0010`): true while the title is a tracker-generated placeholder, set false once resolved (metadata, first self-heal, or a user rename). `UpdateDisplayName` clears it; `Update` clears it only when the submitted name changes. `TrackerCredentials` carries encrypted `secret_enc` (password) **and** `session_enc`/`session_nonce` (cookie-session blob; migration `0002`), plus a nullable `session_expired_at` marker (migration `0003`) the scheduler uses to dedupe expiry notifications (atomic `MarkSessionExpired`, cleared by `SetSession` on re-auth). `Deliveries` (migration `0006`, `topic_deliveries`) records every torrent pushed to a client — `{topic_id, infohash, label, client_id, delivered_at}`, unique on `(topic_id, infohash)` so `Record` is idempotent (`ON CONFLICT DO NOTHING`). `Notifiers` gains `is_default bool` (migration `0008`, per-type unique partial index) + `Update(ctx, id, userID, name, displayName string, events []string, isDefault bool, configEnc, configNonce []byte) error` |
 | **`notify`** | reusable notification dispatcher — `Send(userID, domain.Message)` fans out to all of a user's configured notifiers (best-effort, metered); `SendVia(userID, notifierID, …)` scopes the same fan-out to one notifier when a topic overrides it (nil ⇒ the user's **default** notifiers only (strict; none set ⇒ no send)). Consumers: scheduler new-release alerts + topic-error alerts (both per-topic via `SendVia`; error events deduped to fire once per error episode, on the first failure) + credential session-expiry alerts (global). The single event→notifier fan-out point |
 | `domain` | core types: `Topic` (incl. per-topic `ClientID`, **`NotifierID`** (per-topic notifier override, migration `0007`), `DownloadDir`, `Category`, `ImageURL`), `Check`, `Payload`, `TrackerCredential`, `TopicDelivery`, `AddOptions` (`DownloadDir` + `Category`) |
 | **`topics`** | shared `BuildAndCreate(store, CreateInput)` — the one tracker-match → `Parse` → fail-open `ResolveMetadata` → build → persist sequence, used by BOTH the `POST /topics` handler and the Sonarr poller (no duplication). Idempotent: a `(user_id,url)` unique-violation returns `Result{Created:false}` instead of erroring. Sentinels `ErrNoTracker`/`ErrParse`/`ErrQualityUnsupported` |
@@ -400,6 +400,8 @@ time so a new topic shows a real name + image immediately instead of a
 for the AddTopic form's preview card. The scheduler additionally self-heals the
 title on each check (`displayNamePersister` → `Topics.UpdateDisplayName`) when a
 plugin's `Check` reports a `DisplayName` that differs from what's stored. The
+self-heal is gated to placeholder titles only (`display_name_is_placeholder`)
+so a noisier `Check` title can never downgrade an already-resolved name (issue #90). The
 image is stored in `topics.image_url` (migration `0005`) and rendered as a
 graceful `<img>` (hidden on load error — no fake fallback).
 

backend/internal/db/migrations/0010_topic_display_name_placeholder.sql

@@ -0,0 +1,10 @@
+-- +goose Up
+-- +goose StatementBegin
+ALTER TABLE topics
+    ADD COLUMN display_name_is_placeholder BOOLEAN NOT NULL DEFAULT false;
+-- +goose StatementEnd
+-- +goose Down
+-- +goose StatementBegin
+ALTER TABLE topics
+    DROP COLUMN display_name_is_placeholder;
+-- +goose StatementEnd

backend/internal/db/repo/topics.go

@@ -40,7 +40,7 @@ const topicColumns = `id, user_id, tracker_name, url, display_name,
 		COALESCE(download_dir,''), COALESCE(category,''), extra, COALESCE(last_hash,''),
 		last_checked_at, last_updated_at, next_check_at,
 		check_interval_sec, consecutive_errors, status,
-		COALESCE(last_error,''), created_at, updated_at`
+		COALESCE(last_error,''), created_at, updated_at, display_name_is_placeholder`
 
 func scanTopic(row pgx.Row) (*domain.Topic, error) {
 	var t domain.Topic
@@ -53,7 +53,7 @@ func scanTopic(row pgx.Row) (*domain.Topic, error) {
 		&t.ImageURL, &clientID, &notifierID, &t.DownloadDir, &t.Category, &extraRaw, &t.LastHash,
 		&lastChecked, &lastUpdated, &t.NextCheckAt,
 		&t.CheckIntervalSec, &t.ConsecutiveErrors, &status,
-		&t.LastError, &t.CreatedAt, &t.UpdatedAt,
+		&t.LastError, &t.CreatedAt, &t.UpdatedAt, &t.DisplayNameIsPlaceholder,
 	)
 	if err != nil {
 		return nil, err
@@ -83,12 +83,14 @@ func (r *Topics) Create(ctx context.Context, t *domain.Topic) (*domain.Topic, er
 	extra, _ := json.Marshal(t.Extra)
 	q := `
 INSERT INTO topics (user_id, tracker_name, url, display_name, image_url, client_id, notifier_id,
-                    download_dir, category, extra, check_interval_sec, next_check_at, status)
-VALUES ($1,$2,$3,$4,NULLIF($5,''),$6,$7,NULLIF($8,''),NULLIF($9,''),$10,$11,$12,$13)
+                    download_dir, category, extra, check_interval_sec, next_check_at, status,
+                    display_name_is_placeholder)
+VALUES ($1,$2,$3,$4,NULLIF($5,''),$6,$7,NULLIF($8,''),NULLIF($9,''),$10,$11,$12,$13,$14)
 RETURNING ` + topicColumns
 	row := r.pool.QueryRow(ctx, q,
 		t.UserID, t.TrackerName, t.URL, t.DisplayName, t.ImageURL, t.ClientID, t.NotifierID,
 		t.DownloadDir, t.Category, extra, t.CheckIntervalSec, t.NextCheckAt, string(t.Status),
+		t.DisplayNameIsPlaceholder,
 	)
 	return scanTopic(row)
 }
@@ -266,7 +268,9 @@ func (r *Topics) Update(ctx context.Context, id, userID uuid.UUID, displayName s
 	}
 	row := r.pool.QueryRow(ctx, `UPDATE topics SET
 		display_name = $3, client_id = $4, notifier_id = $5, download_dir = $6, category = $7,
-		extra = $8, updated_at = now()
+		extra = $8,
+		display_name_is_placeholder = CASE WHEN display_name <> $3 THEN false ELSE display_name_is_placeholder END,
+		updated_at = now()
 	WHERE id = $1 AND user_id = $2
 	RETURNING `+topicColumns, id, userID, displayName, clientID, notifierID, downloadDir, category, raw)
 	t, err := scanTopic(row)
@@ -287,7 +291,7 @@ func (r *Topics) UpdateDisplayName(ctx context.Context, id uuid.UUID, displayNam
 		return nil
 	}
 	_, err := r.pool.Exec(ctx,
-		`UPDATE topics SET display_name = $2, updated_at = now() WHERE id = $1`,
+		`UPDATE topics SET display_name = $2, display_name_is_placeholder = false, updated_at = now() WHERE id = $1`,
 		id, displayName,
 	)
 	return err

backend/internal/db/repo/topics_test.go

@@ -197,9 +197,9 @@ func TestTopics_MarkEpisodeDownloaded_DBError(t *testing.T) {
 // ---------- scanTopic malformed extra ----------
 
 // topicRow returns a pgxmock row slice that matches topicColumns exactly
-// (21 columns as of migration 0007, which added notifier_id after
-// client_id). Callers override individual fields as needed. The helper
-// centralises column-order so tests don't drift.
+// (22 columns as of migration 0010, which added display_name_is_placeholder).
+// Callers override individual fields as needed. The helper centralises
+// column-order so tests don't drift.
 func topicRow(id, userID uuid.UUID, now time.Time) []any {
 	return []any{
 		id, userID, "faketracker", "https://example.invalid/t/1",
@@ -213,16 +213,17 @@ func topicRow(id, userID uuid.UUID, now time.Time) []any {
 		(*time.Time)(nil), (*time.Time)(nil), now,
 		3600, 0, "active",
 		"", now, now,
+		false, // display_name_is_placeholder
 	}
 }
 
-// topicColumnsAll mirrors the header slice for pgxmock.NewRows (21 cols).
+// topicColumnsAll mirrors the header slice for pgxmock.NewRows (22 cols).
 var topicColumnsAll = []string{
 	"id", "user_id", "tracker_name", "url", "display_name", "image_url", "client_id", "notifier_id",
 	"download_dir", "category", "extra", "last_hash",
 	"last_checked_at", "last_updated_at", "next_check_at",
 	"check_interval_sec", "consecutive_errors", "status",
-	"last_error", "created_at", "updated_at",
+	"last_error", "created_at", "updated_at", "display_name_is_placeholder",
 }
 
 // TestTopics_ScanTopic_MalformedExtra drives GetByID through a mocked
@@ -237,7 +238,7 @@ func TestTopics_ScanTopic_MalformedExtra(t *testing.T) {
 	userID := uuid.New()
 	now := time.Now().UTC()
 
-	// Build a row that matches topicColumns exactly (21 columns).
+	// Build a row that matches topicColumns exactly (22 columns).
 	rows := pgxmock.NewRows(topicColumnsAll).AddRow(
 		id, userID, "faketracker", "https://example.invalid/t/1",
 		"My Topic", "", // display_name, image_url
@@ -248,6 +249,7 @@ func TestTopics_ScanTopic_MalformedExtra(t *testing.T) {
 		(*time.Time)(nil), (*time.Time)(nil), now,
 		3600, 0, "active",
 		"", now, now,
+		false, // display_name_is_placeholder
 	)
 
 	mock.ExpectQuery(`SELECT .* FROM topics WHERE id = \$1`).
@@ -352,6 +354,7 @@ func TestTopics_Create_RoundTripsCategory(t *testing.T) {
 			"movies",          // category
 			pgxmock.AnyArg(),  // extra (JSON)
 			3600, pgxmock.AnyArg(), "active",
+			false, // display_name_is_placeholder
 		).
 		WillReturnRows(rows)
 
@@ -490,6 +493,7 @@ func TestTopics_Create_RoundTripsNotifierID(t *testing.T) {
 			"", "",
 			pgxmock.AnyArg(),
 			3600, pgxmock.AnyArg(), "active",
+			false, // display_name_is_placeholder
 		).
 		WillReturnRows(rows)
 

backend/internal/domain/domain.go

@@ -65,27 +65,32 @@ const (
 
 // Topic represents a URL that Marauder is monitoring.
 type Topic struct {
-	ID                uuid.UUID
-	UserID            uuid.UUID
-	TrackerName       string
-	URL               string
-	DisplayName       string
-	ImageURL          string
-	ClientID          *uuid.UUID
-	NotifierID        *uuid.UUID
-	DownloadDir       string
-	Category          string
-	Extra             map[string]any
-	LastHash          string
-	LastCheckedAt     *time.Time
-	LastUpdatedAt     *time.Time
-	NextCheckAt       time.Time
-	CheckIntervalSec  int
-	ConsecutiveErrors int
-	Status            TopicStatus
-	LastError         string
-	CreatedAt         time.Time
-	UpdatedAt         time.Time
+	ID          uuid.UUID
+	UserID      uuid.UUID
+	TrackerName string
+	URL         string
+	DisplayName string
+	ImageURL    string
+	// DisplayNameIsPlaceholder is true while DisplayName is a tracker-generated
+	// placeholder (e.g. "Kinozal topic 123") eligible for scheduler self-heal.
+	// Set false once a real title is resolved (metadata, first self-heal, or a
+	// user rename) so self-heal can never downgrade a good title. See issue #90.
+	DisplayNameIsPlaceholder bool
+	ClientID                 *uuid.UUID
+	NotifierID               *uuid.UUID
+	DownloadDir              string
+	Category                 string
+	Extra                    map[string]any
+	LastHash                 string
+	LastCheckedAt            *time.Time
+	LastUpdatedAt            *time.Time
+	NextCheckAt              time.Time
+	CheckIntervalSec         int
+	ConsecutiveErrors        int
+	Status                   TopicStatus
+	LastError                string
+	CreatedAt                time.Time
+	UpdatedAt                time.Time
 }
 
 // TrackerCredential holds a user's login details for a tracker plugin.

backend/internal/plugins/trackers/kinozal/kinozal.go

@@ -178,6 +178,22 @@ func extractImageURL(body []byte, host string) string {
 	}
 }
 
+// canonicalDetailsURL rebuilds the details URL from the trusted host (p.domain)
+// + the numeric id parsed from rawURL — never the raw user URL. Avoids request
+// forgery (CodeQL go/request-forgery) and pins the request to p.domain so
+// Check's title matches ResolveMetadata's (issue #90).
+func (p *plugin) canonicalDetailsURL(rawURL string) (string, error) {
+	u, err := url.Parse(strings.TrimSpace(rawURL))
+	if err != nil {
+		return "", fmt.Errorf("kinozal: parse url: %w", err)
+	}
+	id, err := strconv.Atoi(u.Query().Get("id"))
+	if err != nil {
+		return "", fmt.Errorf("kinozal: topic id: %w", err)
+	}
+	return fmt.Sprintf("https://%s/details.php?id=%d", p.domain, id), nil
+}
+
 // Check makes TWO requests per tick: the details page for the display title,
 // then get_srv_details.php for the infohash (Kinozal exposes the hash only
 // there, not on the details page). Both run under the scheduler's single
@@ -186,7 +202,11 @@ func extractImageURL(body []byte, host string) string {
 // already-fetched title is intentionally discarded — title self-heal simply
 // retries next tick rather than persisting a title with no matching hash.
 func (p *plugin) Check(ctx context.Context, topic *domain.Topic, creds *domain.TrackerCredential) (*domain.Check, error) {
-	body, err := p.fetch(ctx, topic.URL, creds)
+	canonical, err := p.canonicalDetailsURL(topic.URL)
+	if err != nil {
+		return nil, err
+	}
+	body, err := p.fetch(ctx, canonical, creds)
 	if err != nil {
 		return nil, err
 	}
@@ -240,18 +260,10 @@ var _ registry.WithMetadata = (*plugin)(nil)
 // is publicly viewable, so a session isn't required. Image URL is "" when the
 // page exposes no poster; the caller treats errors as fail-open.
 func (p *plugin) ResolveMetadata(ctx context.Context, rawURL string, creds *domain.TrackerCredential) (*registry.Metadata, error) {
-	m := urlPattern.FindStringSubmatch(strings.TrimSpace(rawURL))
-	if m == nil {
-		return nil, errors.New("resolve metadata: not a kinozal details URL")
-	}
-	id, err := strconv.Atoi(m[1])
+	canonical, err := p.canonicalDetailsURL(rawURL)
 	if err != nil {
-		return nil, fmt.Errorf("resolve metadata: topic id: %w", err)
+		return nil, fmt.Errorf("resolve metadata: %w", err)
 	}
-	// Rebuild the URL from the trusted host (p.domain) + the numeric id, never
-	// the raw user-supplied URL, so a crafted URL cannot redirect the request
-	// to an arbitrary host (CodeQL go/request-forgery). Mirrors Download.
-	canonical := fmt.Sprintf("https://%s/details.php?id=%d", p.domain, id)
 	body, err := p.fetch(ctx, canonical, creds)
 	if err != nil {
 		return nil, fmt.Errorf("resolve metadata: %w", err)

backend/internal/plugins/trackers/kinozal/kinozal_test.go

@@ -320,3 +320,52 @@ func TestDownload_ValidTopic_ReturnsTorrentBytes(t *testing.T) {
 		t.Error("expected torrent bytes")
 	}
 }
+
+func TestCanonicalDetailsURL_RebuildsFromDomain(t *testing.T) {
+	p := &plugin{domain: "kinozal.tv"}
+	got, err := p.canonicalDetailsURL("https://kinozal.guru/details.php?id=2072973")
+	if err != nil {
+		t.Fatalf("canonicalDetailsURL: %v", err)
+	}
+	if want := "https://kinozal.tv/details.php?id=2072973"; got != want {
+		t.Errorf("canonicalDetailsURL = %q, want %q", got, want)
+	}
+}
+
+// hostRecordingRewrite records the Host of every outgoing request, then
+// redirects it to the test server (target) over http so no real network is hit.
+type hostRecordingRewrite struct {
+	target string // test server host:port (p.domain)
+	hosts  []string
+}
+
+func (h *hostRecordingRewrite) RoundTrip(req *http.Request) (*http.Response, error) {
+	h.hosts = append(h.hosts, req.URL.Host)
+	req.URL.Scheme = "http"
+	req.URL.Host = h.target
+	return http.DefaultTransport.RoundTrip(req)
+}
+
+func TestCheck_CanonicalizesMirrorHost(t *testing.T) {
+	p := newTestPlugin(t) // p.domain == test server host
+	rec := &hostRecordingRewrite{target: p.domain}
+	p.transport = rec // applied per-session inside fetch()
+
+	// Topic URL points at a different mirror than p.domain.
+	topic := &domain.Topic{URL: "https://kinozal.guru/details.php?id=99999"}
+	check, err := p.Check(context.Background(), topic, nil)
+	if err != nil {
+		t.Fatalf("Check: %v", err)
+	}
+	if !strings.Contains(check.DisplayName, "The Movie") {
+		t.Errorf("display name = %q, want it to contain The Movie", check.DisplayName)
+	}
+	if len(rec.hosts) == 0 {
+		t.Fatal("no requests recorded")
+	}
+	// The FIRST request is the title/details fetch — it must target p.domain,
+	// not the raw kinozal.guru mirror.
+	if rec.hosts[0] != p.domain {
+		t.Errorf("title fetch host = %q, want canonical %q", rec.hosts[0], p.domain)
+	}
+}

backend/internal/scheduler/scheduler.go

@@ -398,10 +398,11 @@ func (s *Scheduler) runCheck(ctx context.Context, log zerolog.Logger, t *domain.
 		}
 	}
 
-	// Self-heal the display name: a tracker's Check often resolves the real
-	// title (e.g. RuTracker's <title>) that wasn't available at add time.
-	// Persist it when it changed so placeholder names upgrade themselves.
-	if check.DisplayName != "" && check.DisplayName != t.DisplayName {
+	// Self-heal the display name only while it's still a generated placeholder.
+	// Once a real title is resolved (add-time metadata, a prior self-heal, or a
+	// user rename) the topic is locked, so a noisier Check title can't downgrade
+	// it (issue #90).
+	if check.DisplayName != "" && check.DisplayName != t.DisplayName && t.DisplayNameIsPlaceholder {
 		if p, ok := s.topics.(displayNamePersister); ok {
 			if err := p.UpdateDisplayName(ctx, t.ID, check.DisplayName); err != nil {
 				log.Warn().Err(err).Msg("UpdateDisplayName failed")