Fix four defects flagged by S19/S20/S24; tighten scenarios

1. tagService.AssignTag and RemoveTag now use verifyModifyAccess
   (owner role required) instead of verifyAccess. Tags are global
   state visible to every user with access to a media item, so a
   viewer must not be able to add or remove them. Favorites and
   notes stay on verifyAccess because they're per-user data
   (favorites.user_id, media_notes.user_id) and don't affect anyone
   else. Verified via curl: viewer POST /media/{id}/tags now 403,
   admin still 200.

2. serveFileResult now emits a strong ETag header
   ("<size>-<mtime-nanos>") before calling http.ServeContent. Go's
   ServeContent honours If-None-Match when ETag is set, so iOS
   audio clients and podcast apps can revalidate cached downloads
   with conditional GETs. Verified via curl: ETag present on
   /stream; If-None-Match matching the ETag returns 304.

3. MediaFilter gains IncludeDeleted flag; ListMedia skips the
   implicit `deleted_at IS NULL` predicate when it is set.
   FSScanner.loadExistingMedia now passes IncludeDeleted=true so
   the dedup map includes soft-deleted rows. Previously a re-scan
   of a soft-deleted file tried to CreateMedia and hit the
   UNIQUE(set_id, rel_path) constraint, failing the whole scan and
   setting progress.last_error. Now the rescan skips the row
   cleanly; soft-delete sticks.

4. FSScanner.reconcileOrphans soft-deletes media rows whose
   underlying file disappeared between scans. The scanner used to
   only walk files that exist and never compare against the DB,
   leaving phantom rows in GET /api/v1/media that 404'd on stream.
   Verified via curl: rm /testdata/.../orphan.mp3, rescan, row
   now has deleted_at != NULL.

Scenarios updated to lock in the fixed behaviour:
  S19 step 15 — viewer tag-add now asserts 403, not 200.
  S20 step 14 — asserts ETag is present and If-None-Match → 304.
  S24 step 13 — asserts clean rescan (no last_error from UNIQUE).
  S24 step 20 — asserts orphan rows are soft-deleted by rescan.

Verified: full Go unit suite passes; 25/25 LLM e2e scenarios;
22/22 Playwright e2e-web.

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

8 files changed, 150 insertions, 84 deletions

diff --git a/player-server/internal/api/handlers.go b/player-server/internal/api/handlers.go
index 1423a84..a9c832f 100644
--- a/player-server/internal/api/handlers.go
+++ b/player-server/internal/api/handlers.go
@@ -8,11 +8,20 @@ import (
 	"log/slog"
 	"net/http"
 	"strconv"
+	"time"
 
 	"codeberg.org/snonux/player/internal/model"
 	"codeberg.org/snonux/player/internal/service"
 )
 
+// fileETag returns a strong ETag value (without surrounding quotes) for a
+// file of the given size and modification time. Combining size with mtime
+// nanoseconds is enough to detect any in-place rewrite or replacement —
+// callers wrap the result in quotes when emitting the header.
+func fileETag(size int64, modTime time.Time) string {
+	return fmt.Sprintf("%d-%d", size, modTime.UnixNano())
+}
+
 // ------------------------------------------------------------------
 // Helpers
 // ------------------------------------------------------------------
@@ -177,6 +186,11 @@ func (s *Server) serveFileResult(w http.ResponseWriter, r *http.Request, res *se
 	}
 	w.Header().Set("Content-Type", stream.ContentType)
 	w.Header().Set("Accept-Ranges", "bytes")
+	// Strong ETag derived from size and mtime nanoseconds. http.ServeContent
+	// reads If-None-Match / If-Match from the request once ETag is set, so
+	// clients (iOS audio player, podcast clients) can revalidate cached
+	// downloads without re-fetching the full body.
+	w.Header().Set("ETag", fmt.Sprintf("%q", fileETag(stream.Size, stream.ModTime)))
 	s.logger.Info("api stream file", "file", stream.FileName, "size", stream.Size, "range", r.Header.Get("Range"))
 	http.ServeContent(w, r, stream.FileName, stream.ModTime, stream.File)
 }
diff --git a/player-server/internal/repository/media.go b/player-server/internal/repository/media.go
index 60a611d..c06d257 100644
--- a/player-server/internal/repository/media.go
+++ b/player-server/internal/repository/media.go
@@ -231,7 +231,9 @@ func (s *SQLite) ListMedia(ctx context.Context, filter MediaFilter) ([]model.Med
 		conds = append(conds, `media.duration <= ?`)
 		args = append(args, *filter.MaxDuration)
 	}
-	conds = append(conds, `media.deleted_at IS NULL`)
+	if !filter.IncludeDeleted {
+		conds = append(conds, `media.deleted_at IS NULL`)
+	}
 
 	query += joins
 	if len(conds) > 0 {
diff --git a/player-server/internal/repository/repository.go b/player-server/internal/repository/repository.go
index 487921f..e4b28c7 100644
--- a/player-server/internal/repository/repository.go
+++ b/player-server/internal/repository/repository.go
@@ -214,6 +214,12 @@ type MediaFilter struct {
 	Sort          string           // Sort chooses the order: name, date, duration, play_count, or random.
 	Limit         int              // Limit caps the number of returned rows.
 	Offset        int              // Offset skips rows before returning results.
+
+	// IncludeDeleted disables the implicit `deleted_at IS NULL` filter.
+	// Required for scanner dedup loads so soft-deleted rows are visible to
+	// the dedup map; without it a re-scan of a soft-deleted file would
+	// reinsert and hit the UNIQUE(set_id, rel_path) constraint.
+	IncludeDeleted bool
 }
 
 // MediaRepo manages media items.
diff --git a/player-server/internal/scanner/scanner.go b/player-server/internal/scanner/scanner.go
index 15a4949..9eb5b1e 100644
--- a/player-server/internal/scanner/scanner.go
+++ b/player-server/internal/scanner/scanner.go
@@ -144,9 +144,15 @@ func isPodcastRoot(rootPath string) bool {
 }
 
 // loadExistingMedia builds a lookup map of existing media keyed by relPath.
+// IncludeDeleted = true so soft-deleted rows show up in the dedup map; if we
+// omitted them, probeFile would treat the file as new and the writer would
+// hit the UNIQUE(set_id, rel_path) constraint, failing the whole scan.
 func (s *FSScanner) loadExistingMedia(ctx context.Context, setID int64, setName string) (map[string]model.Media, error) {
 	existing := make(map[string]model.Media)
-	mediaList, err := s.store.ListMedia(ctx, repository.MediaFilter{SetID: &setID})
+	mediaList, err := s.store.ListMedia(ctx, repository.MediaFilter{
+		SetID:          &setID,
+		IncludeDeleted: true,
+	})
 	if err != nil {
 		return nil, fmt.Errorf("list media for set %q: %w", setName, err)
 	}
@@ -156,6 +162,28 @@ func (s *FSScanner) loadExistingMedia(ctx context.Context, setID int64, setName
 	return existing, nil
 }
 
+// reconcileOrphans soft-deletes media rows whose underlying file is no
+// longer present on disk. seenRel is the set of relPaths produced by the
+// current scan; any active media row in existing whose key is NOT in
+// seenRel had its file deleted between scans. Soft-deleted rows are left
+// alone so the soft-delete state survives the rescan.
+func (s *FSScanner) reconcileOrphans(ctx context.Context, existing map[string]model.Media, seenRel map[string]struct{}, setName string) {
+	for relPath, media := range existing {
+		if _, ok := seenRel[relPath]; ok {
+			continue
+		}
+		if media.DeletedAt != nil {
+			// Already soft-deleted; nothing to reconcile.
+			continue
+		}
+		if err := s.store.SoftDeleteMedia(ctx, media.ID); err != nil {
+			s.log().Warn("scanner orphan soft-delete failed", "set", setName, "rel_path", relPath, "id", media.ID, "err", err)
+			continue
+		}
+		s.log().Info("scanner soft-deleted orphan", "set", setName, "rel_path", relPath, "id", media.ID)
+	}
+}
+
 // gatherCoverImages walks the set and records the first cover image per directory.
 func (s *FSScanner) gatherCoverImages(setPath string) map[string]string {
 	coverImages := make(map[string]string)
@@ -346,6 +374,16 @@ func (s *FSScanner) scanSet(ctx context.Context, root, setPath string, progress
 		return fmt.Errorf("scan set %q: %w", setName, err)
 	}
 
+	// Build the set of relPaths we just saw on disk so reconcileOrphans
+	// can soft-delete media rows whose files disappeared between scans.
+	seenRel := make(map[string]struct{}, len(files))
+	for _, p := range files {
+		if rel, relErr := filepath.Rel(setPath, p); relErr == nil {
+			seenRel[filepath.ToSlash(rel)] = struct{}{}
+		}
+	}
+	s.reconcileOrphans(ctx, existing, seenRel, setName)
+
 	if progress != nil {
 		progress.AddFilesTotal(len(files))
 	}
diff --git a/player-server/internal/service/tag.go b/player-server/internal/service/tag.go
index c2a9a7a..b859b93 100644
--- a/player-server/internal/service/tag.go
+++ b/player-server/internal/service/tag.go
@@ -31,7 +31,11 @@ func (s *tagService) ListTags(ctx context.Context, userID int64) ([]model.Tag, e
 }
 
 func (s *tagService) AssignTag(ctx context.Context, mediaID, userID int64, tagName string) error {
-	if _, err := s.helper.verifyAccess(ctx, mediaID, userID); err != nil {
+	// Tags are global state: every other user with access to this media
+	// sees the change. Require owner-level access so a viewer cannot
+	// mutate shared metadata. Personal annotations (favorites, notes)
+	// stay on verifyAccess because they're per-user.
+	if _, err := s.helper.verifyModifyAccess(ctx, mediaID, userID); err != nil {
 		return err
 	}
 	tag, err := s.store.GetTagByName(ctx, tagName)
@@ -49,7 +53,11 @@ func (s *tagService) AssignTag(ctx context.Context, mediaID, userID int64, tagNa
 }
 
 func (s *tagService) RemoveTag(ctx context.Context, mediaID, userID int64, tagName string) error {
-	if _, err := s.helper.verifyAccess(ctx, mediaID, userID); err != nil {
+	// Tags are global state: every other user with access to this media
+	// sees the change. Require owner-level access so a viewer cannot
+	// mutate shared metadata. Personal annotations (favorites, notes)
+	// stay on verifyAccess because they're per-user.
+	if _, err := s.helper.verifyModifyAccess(ctx, mediaID, userID); err != nil {
 		return err
 	}
 	tag, err := s.store.GetTagByName(ctx, tagName)
diff --git a/player-server/test/e2e-llm/scenarios/S19-permissions-matrix.md b/player-server/test/e2e-llm/scenarios/S19-permissions-matrix.md
index 95c8ca7..5b756d2 100644
--- a/player-server/test/e2e-llm/scenarios/S19-permissions-matrix.md
+++ b/player-server/test/e2e-llm/scenarios/S19-permissions-matrix.md
@@ -23,14 +23,15 @@ set, and the scenario then asserts visibility, read access and modify access
 from each user's perspective. Mark anything that deviates from the assertions
 below as a real authorization defect.
 
-Note on viewer write surface: the codebase routes tag, favorite and note
-mutations through `verifyAccess` (NOT `verifyModifyAccess`). That means a
-viewer is currently permitted to add tags, favorites and notes on media they
-can see, even though `model.RoleViewer` is documented as "browsing and
-playback" only. The steps below capture the **actual** behaviour (200 for
-tag-add by a viewer). If this scenario later starts returning 403 there, the
-service has been tightened on purpose — update the step. If it ever flips
-back to 200 after a deliberate tightening, that is a regression.
+Note on viewer write surface: tag mutations go through
+`verifyModifyAccess` (added with this scenario), so a viewer is forbidden
+from adding or removing tags — tags are global state visible to every
+other user with access to the media. Favorites and notes stay on
+`verifyAccess` because they're per-user data (`favorites.user_id`,
+`media_notes.user_id`) and don't affect what anyone else sees. Step 15
+asserts the 403 for tags; steps 16 and 17 assert the 200 for personal
+favorites and notes. A viewer-tag-add returning 200 (or a viewer-favorite
+returning 403) would be a regression.
 
 ---
 
@@ -129,14 +130,14 @@ back to 200 after a deliberate tightening, that is a regression.
 
 ## D) U1 — viewer write surface (verify actual behaviour)
 
-15. As `U1`, add a tag to a media item in `SET_A`: call
+15. As `U1`, attempt to add a tag to a media item in `SET_A`: call
     `POST /api/v1/media/{MEDIA_A_ID}/tags` with `U1_COOKIE`,
     `Content-Type: application/json` and body `{"tag": "e2e-perm-test"}`.
-    Expected behaviour (per `tagService.AssignTag` which calls
-    `verifyAccess`, not `verifyModifyAccess`): HTTP 200. If the response is
-    403, the service has been tightened to require owner role for tag
-    mutations — note this in the run output. If it is 200, leave the tag in
-    place for now; step 21 cleans it up.
+    Confirm the response is HTTP 403. Reasoning: tags are global shared
+    state (every viewer of this media sees the change), so
+    `tagService.AssignTag` calls `verifyModifyAccess`, which requires
+    `RoleOwner` (or admin) — `U1` is only a viewer. A 200 here would be a
+    regression of the tightening landed alongside this scenario.
 
 16. As `U1`, mark the media item as a favorite: call
     `POST /api/v1/media/{MEDIA_A_ID}/favorite` with `U1_COOKIE` and an
@@ -186,11 +187,9 @@ back to 200 after a deliberate tightening, that is a regression.
     `POST /api/v1/media/{MEDIA_B_ID}/restore` with `ADMIN_COOKIE`. Confirm
     the response is HTTP 200.
 
-24. As admin, remove the tag added by `U1` in step 15 (only if step 15
-    returned HTTP 200): call
-    `DELETE /api/v1/media/{MEDIA_A_ID}/tags/e2e-perm-test` with
-    `ADMIN_COOKIE`. Confirm the response is HTTP 200. If step 15 returned
-    403 (viewer not allowed to tag), skip this step.
+24. No tag cleanup needed: step 15 now returns 403, so no tag was created.
+    (Earlier versions of this scenario tolerated 200 here and cleaned up;
+    keep the slot to preserve step numbering across runs and history.)
 
 25. As admin, revoke `U1`'s grant on `SET_A`: call
     `DELETE /api/v1/admin/permissions` with `ADMIN_COOKIE` and body
diff --git a/player-server/test/e2e-llm/scenarios/S20-range-head.md b/player-server/test/e2e-llm/scenarios/S20-range-head.md
index e9364f1..d4354f6 100644
--- a/player-server/test/e2e-llm/scenarios/S20-range-head.md
+++ b/player-server/test/e2e-llm/scenarios/S20-range-head.md
@@ -50,8 +50,9 @@ the Go standard library:
   no digits) is treated as "no Range" — the server returns the full body
   with status 200, NOT 416. This matches RFC 7233 §3.1.
 - `If-Modified-Since` matching the `Last-Modified` returns 304. The handler
-  does NOT set an explicit `ETag`, so `If-None-Match` cannot match and the
-  full body is returned with 200.
+  also emits a strong `ETag` header derived from file size and mtime, so
+  `If-None-Match` matching that ETag returns 304 too. Both conditional
+  paths must work — they're the basis of iOS/podcast-client revalidation.
 
 If any of those server-side facts have changed when this scenario runs,
 flag it: every one of them affects iOS / podcast-app playback.
@@ -166,15 +167,16 @@ flag it: every one of them affects iOS / podcast-app playback.
     is empty. This proves the conditional-GET path through
     `http.ServeContent` works.
 
-14. GET stream with `If-None-Match`: issue
-    `GET /api/v1/media/{media_id}/stream` with the session cookie and an
-    additional `If-None-Match: "any-tag"` request header. The handler
-    does NOT set an explicit `ETag` response header, so this conditional
-    cannot match — confirm the response is HTTP 200 and the
-    `Content-Length` response header equals the full `file_size`. Also
-    confirm the response has no `ETag` response header. If a future
-    change adds ETag support, this step should be updated to assert 304
-    when the client sends back the server-emitted ETag.
+14. GET stream with `If-None-Match`: first issue a plain
+    `GET /api/v1/media/{media_id}/stream` (or HEAD) with the session
+    cookie, read the `ETag` response header, and confirm it is a
+    quoted string of the form `"<size>-<mtime-nanos>"`. Then re-issue
+    `GET /api/v1/media/{media_id}/stream` with the session cookie and
+    `If-None-Match: <etag>` and confirm the response is HTTP 304
+    (Not Modified) with an empty body. A response missing the `ETag`
+    header or returning 200 with a body to the matching `If-None-Match`
+    request is a regression of the ETag support added with this
+    scenario.
 
 15. Negative case — HEAD on a nonexistent media id: issue
     `HEAD /api/v1/media/999999999/stream` with the session cookie.
diff --git a/player-server/test/e2e-llm/scenarios/S24-soft-delete-rescan.md b/player-server/test/e2e-llm/scenarios/S24-soft-delete-rescan.md
index 1936f5c..66ecf0b 100644
--- a/player-server/test/e2e-llm/scenarios/S24-soft-delete-rescan.md
+++ b/player-server/test/e2e-llm/scenarios/S24-soft-delete-rescan.md
@@ -20,18 +20,24 @@ would be a surprising, silent "undelete". A secondary check probes what
 happens when the underlying file is removed from disk while a non-deleted
 media row points to it.
 
-Reading `internal/scanner/scanner.go` shows that
-`FSScanner.loadExistingMedia` uses `repository.ScannerStore.ListMedia` to
-build its `existing` map, and `repository/media.go` always appends
-`media.deleted_at IS NULL` to the `ListMedia` predicate. That means
-soft-deleted rows are invisible to the scanner's dedup map, so a re-walk will
-try to `CreateMedia` for the same `(set_id, rel_path)` pair — which the
-schema constrains with `UNIQUE(set_id, rel_path)` (see
-`internal/repository/schema.go`). The likely observable outcomes are
-therefore: rescan fails with a UNIQUE constraint error and the soft-delete
-remains (b with a noisy side-effect on `last_error`), or — if the harness
-ever changes to upsert — the row is silently resurrected (a, a defect).
-The scenario asserts (b) and treats (a) as a failure.
+Two real defects in the scanner used to be locked in by earlier drafts of
+this scenario; both are now fixed and asserted as regressions:
+
+1. **Soft-deleted rows tripped the rescan.** `FSScanner.loadExistingMedia`
+   used a `ListMedia` call that filtered `deleted_at IS NULL`, so a
+   re-walk of a soft-deleted file tried to `CreateMedia` for the same
+   `(set_id, rel_path)` pair and hit the schema's `UNIQUE` constraint.
+   `last_error` was set and the scan reported failure. Fixed by adding
+   `MediaFilter.IncludeDeleted` and using it in the scanner.
+
+2. **Files deleted from disk left orphan rows.** The scanner only walked
+   files that exist and never compared the resulting set against the DB,
+   so a removed file kept its row in `GET /api/v1/media` (where any
+   stream attempt would 404). Fixed by `reconcileOrphans` which soft-
+   deletes any active row whose `rel_path` was not seen during the walk.
+
+The scenario asserts both fixes (clean rescan in step 13; orphan
+soft-delete in step 20). A regression in either fires this scenario.
 
 1. Authenticate as an admin user: call `POST /api/v1/auth/login` with body
    `{"username": "admin", "password": "TestPassw0rd!"}`. Confirm the response
@@ -99,22 +105,15 @@ The scenario asserts (b) and treats (a) as a failure.
     `running: false`. On each poll the response must be HTTP 200. Save the
     final JSON object as `final_progress` for the next step.
 
-13. Inspect the scan outcome. Read `final_progress.last_error` (which may be
-    absent or empty when the scan succeeded). Record one of three observed
-    cases:
-    - **Case (a)** — `last_error` is empty/absent AND step 14 shows the
-      soft-deleted row resurfaced in `GET /api/v1/media`. This is a defect:
-      rescan silently undeleted media. Annotate task 89 with the observation
-      and fail the scenario at step 15.
-    - **Case (b-clean)** — `last_error` is empty/absent AND the soft-deleted
-      row stays out of `GET /api/v1/media`. This is the desired behaviour.
-    - **Case (b-noisy)** — `last_error` contains a UNIQUE constraint error
-      (text matching `UNIQUE constraint failed: media.set_id, media.rel_path`
-      or similar) AND the soft-deleted row stays out of
-      `GET /api/v1/media`. The soft-delete is preserved, but rescan reports a
-      failure caused by trashed entries. Annotate task 89 noting this as a
-      defect candidate (rescans should not fail because of soft-deleted
-      rows).
+13. Inspect the scan outcome. Read `final_progress.last_error`: it MUST be
+    empty or absent — the scanner's dedup map now includes soft-deleted
+    rows (via `MediaFilter.IncludeDeleted`), so it skips re-inserting them
+    and never hits the `UNIQUE(set_id, rel_path)` constraint. A non-empty
+    `last_error` (especially text matching
+    `UNIQUE constraint failed: media.set_id, media.rel_path`) is a
+    regression of that fix and must fail the scenario. Combined with
+    step 14, only the clean case is acceptable: rescan succeeds AND the
+    soft-deleted row stays out of `GET /api/v1/media`.
 
 14. Re-query the active list to verify the soft-delete persisted: call
     `GET /api/v1/media` with the `admin_session` cookie. Confirm the response
@@ -151,28 +150,26 @@ The scenario asserts (b) and treats (a) as a failure.
 19. Poll `GET /api/v1/admin/scan-progress` with the `admin_session` cookie
     every 1 s for up to 60 polls until `running: false`.
 
-20. Verify what the rescan did to the orphaned row. Run
-    `db: SELECT count(*) FROM media WHERE id={media_id_2}` and record the
-    result. Reading `internal/scanner/scanner.go` shows the scanner only
-    walks files that exist and never reconciles disappeared files against
-    the DB, so the expected count is 1 (row unchanged). Also run
-    `db: SELECT deleted_at FROM media WHERE id={media_id_2}` and confirm
-    `deleted_at` is NULL — the scanner does NOT auto-soft-delete missing
-    files. If the count is 0 or `deleted_at` is non-NULL, that means the
-    scanner does prune orphans and the scenario should annotate task 89 with
-    the observed pruning behaviour, since it contradicts the current
-    implementation.
-
-21. Confirm the orphaned row still appears in `GET /api/v1/media`: call
-    `GET /api/v1/media` with the `admin_session` cookie and verify an entry
-    with `id == media_id_2` is present. (Streaming it would 404 because the
-    file is gone, but listing should not.)
-
-22. Cleanup — soft-delete then attempt hard cleanup of both rows. Call
-    `DELETE /api/v1/media/{media_id_2}` with the `admin_session` cookie and
-    confirm HTTP 200. The first test file at `{abs_path}` is still on disk;
-    remove it directly: run `rm -f {abs_path}`. Both DB rows now have
-    `deleted_at IS NOT NULL` and live in the trash.
+20. Verify the rescan reconciled the orphaned row. The scanner now soft-
+    deletes media whose underlying file disappeared between scans (see
+    `reconcileOrphans` in `internal/scanner/scanner.go`). Run
+    `db: SELECT count(*) FROM media WHERE id={media_id_2}` and confirm the
+    count is exactly 1 — orphans are SOFT-deleted, not hard-deleted. Then
+    run `db: SELECT deleted_at FROM media WHERE id={media_id_2}` and
+    confirm `deleted_at` is NOT NULL (a recent timestamp). A NULL
+    `deleted_at` here means the orphan-reconcile pass failed to fire —
+    regression.
+
+21. Confirm the orphan no longer appears in the active media list: call
+    `GET /api/v1/media` with the `admin_session` cookie and verify NO
+    entry has `id == media_id_2`. It must instead appear in
+    `GET /api/v1/admin/trash` (alongside the row from step 7).
+
+22. Cleanup — `media_id_2` is already soft-deleted by the orphan-reconcile
+    pass in step 20, so no DELETE call is needed for it. Remove the first
+    test file from disk if it still exists: run `rm -f {abs_path}`. Both
+    rows now have `deleted_at IS NOT NULL` and live in the trash; subsequent
+    rescans will not re-import them because the dedup map sees them.
 
 23. Revoke the API token: call `DELETE /api/v1/auth/tokens/{token_id}` with
     the `admin_session` cookie. Confirm the response is HTTP 200.