Map service errors to correct HTTP status codes (was 500)

Three more handlers had the same pattern as the RevokeShare bug fixed
in 0848488: the service returned a plain errors.New(...) for an
expected condition, so handleError fell through to its default 500
branch instead of mapping to 404/400.

  - browse.GetThumbnail → ErrNotFound (404 instead of 500) when the
    media row has no thumbnail path.
  - tag.RemoveTag → ErrNotFound (404 instead of 500) when the tag name
    does not exist. DELETE /media/{id}/tags/{unknown} now returns 404.
  - write.RegenerateSetCover → new sentinel ErrEmptySetForCover, mapped
    to 400 in handleError, when the target set contains no media files
    eligible to be promoted to a cover.

S15 (auth-boundary negatives) is tightened: the share-revoke step used
to accept {404, 500} as a workaround for the bug; it now requires 404.
A new section C2 locks in the three regressions above so they cannot
silently return 500 again.

Verified end-to-end via curl: DELETE /media/{id}/tags/unknown → 404,
DELETE /shares/unknown-token → 404. Full LLM e2e suite 15/15 still
passes against the rebuilt server.

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

6 files changed, 45 insertions, 15 deletions

diff --git a/player-server/internal/api/handlers.go b/player-server/internal/api/handlers.go
index b276b10..1423a84 100644
--- a/player-server/internal/api/handlers.go
+++ b/player-server/internal/api/handlers.go
@@ -57,7 +57,8 @@ func handleError(w http.ResponseWriter, err error) {
 		writeJSON(w, http.StatusUnauthorized, map[string]string{"error": "invalid credentials"})
 	case errors.Is(err, service.ErrUnsupportedExtension),
 		errors.Is(err, service.ErrInvalidFeed),
-		errors.Is(err, service.ErrCannotDeleteSelf):
+		errors.Is(err, service.ErrCannotDeleteSelf),
+		errors.Is(err, service.ErrEmptySetForCover):
 		badRequest(w, err.Error())
 	default:
 		writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
diff --git a/player-server/internal/service/browse.go b/player-server/internal/service/browse.go
index 6308e1a..0294a91 100644
--- a/player-server/internal/service/browse.go
+++ b/player-server/internal/service/browse.go
@@ -2,7 +2,6 @@ package service
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	mrand "math/rand"
 	"os"
@@ -231,7 +230,9 @@ func (s *browseService) GetThumbnail(ctx context.Context, mediaID, userID int64)
 		return nil, err
 	}
 	if media.ThumbnailPath == "" {
-		return nil, errors.New("thumbnail not found")
+		// Use the sentinel so handleError maps this to HTTP 404 instead of
+		// falling through to the default 500 branch.
+		return nil, ErrNotFound
 	}
 	info, err := os.Stat(media.ThumbnailPath)
 	if err == nil {
diff --git a/player-server/internal/service/service.go b/player-server/internal/service/service.go
index 84525e5..b4ae127 100644
--- a/player-server/internal/service/service.go
+++ b/player-server/internal/service/service.go
@@ -18,6 +18,7 @@ var (
 	ErrShareExpired         = errors.New("share expired")
 	ErrMediaNotFound        = errors.New("media not found")
 	ErrUnsupportedExtension = errors.New("unsupported file extension")
+	ErrEmptySetForCover     = errors.New("no media files available for cover")
 	ErrAlreadyBootstrapped  = errors.New("already bootstrapped")
 	ErrInvalidCredentials   = errors.New("invalid credentials")
 	ErrInvalidFeed          = errors.New("invalid feed")
diff --git a/player-server/internal/service/tag.go b/player-server/internal/service/tag.go
index 88e19b6..c2a9a7a 100644
--- a/player-server/internal/service/tag.go
+++ b/player-server/internal/service/tag.go
@@ -2,7 +2,6 @@ package service
 
 import (
 	"context"
-	"errors"
 	"fmt"
 
 	"codeberg.org/snonux/player/internal/model"
@@ -58,7 +57,9 @@ func (s *tagService) RemoveTag(ctx context.Context, mediaID, userID int64, tagNa
 		return fmt.Errorf("get tag: %w", err)
 	}
 	if tag == nil {
-		return errors.New("tag not found")
+		// Use the sentinel so handleError maps this to HTTP 404 instead of
+		// falling through to the default 500 branch.
+		return ErrNotFound
 	}
 	return s.store.RemoveTag(ctx, mediaID, tag.ID)
 }
diff --git a/player-server/internal/service/write.go b/player-server/internal/service/write.go
index f2f5080..23fd130 100644
--- a/player-server/internal/service/write.go
+++ b/player-server/internal/service/write.go
@@ -211,7 +211,10 @@ func (s *writeService) RegenerateSetCover(ctx context.Context, setID int64, fold
 			return fmt.Errorf("copy thumbnail cover: %w", err)
 		}
 	default:
-		return errors.New("no media files available for cover")
+		// Use the sentinel so handleError maps this to HTTP 400 (bad request)
+		// instead of falling through to the default 500 branch — the request
+		// is well-formed, the set just has nothing usable as a cover.
+		return ErrEmptySetForCover
 	}
 
 	return nil
diff --git a/player-server/test/e2e-llm/scenarios/S15-auth-boundaries.md b/player-server/test/e2e-llm/scenarios/S15-auth-boundaries.md
index ace7be9..5f1f5e6 100644
--- a/player-server/test/e2e-llm/scenarios/S15-auth-boundaries.md
+++ b/player-server/test/e2e-llm/scenarios/S15-auth-boundaries.md
@@ -96,26 +96,49 @@ as a real auth-boundary defect and file a task.
     handler maps to 404.)
 
 14. Call `DELETE /api/v1/shares/does-not-exist-token-xyz` with `ADMIN_COOKIE`.
-    Confirm the response status code is one of `{404, 500}` and record which
-    one was actually returned. NOTE: At the time of writing,
-    `shareService.RevokeShare` returns a plain `errors.New("share not found")`
-    instead of the sentinel `ErrShareNotFound`, so `handleError` falls into
-    its default branch and returns HTTP 500. The "correct" status is 404; if
-    you see 500 here, file a task — this is a real auth-boundary / error-
-    mapping defect in `internal/service/share.go`.
+    Confirm the response is HTTP 404. (`shareService.RevokeShare` returns
+    the sentinel `ErrShareNotFound` for missing tokens; `handleError`
+    maps that to 404. A 500 here would be a regression — this exact path
+    used to return 500 because the service returned a plain
+    `errors.New("share not found")` instead of the sentinel.)
 
 15. Call `GET {PLAYER_URL}/s/does-not-exist-token-xyz/thumbnail` with no
     cookie (this is a public share route). Confirm the response is HTTP 404.
 
+## C2) Error-code mapping regressions (must not return 500)
+
+These three paths used to fall through to HTTP 500 because the service
+layer returned plain `errors.New(...)` values instead of the sentinels
+that `handleError` knows how to map. They were fixed by switching to
+`ErrNotFound` / `ErrShareNotFound` / `ErrEmptySetForCover`; the steps
+below lock that behaviour in.
+
+16. Pick any existing media id (e.g. from `GET /api/v1/media?limit=1`).
+    Call `DELETE /api/v1/media/{media_id}/tags/does-not-exist-tag-xyz`
+    with `ADMIN_COOKIE`. Confirm the response is HTTP 404 — not 500.
+    (`tagService.RemoveTag` returns `ErrNotFound` for unknown tag names.)
+
+17. Find a set whose media items have no generated thumbnail. If every
+    media item in `testmedia/` happens to have one, skip the rest of this
+    step and continue — the regression case is unreachable in this DB.
+    Otherwise, call `GET /api/v1/media/{id}/thumbnail` for one of those
+    items and confirm the response is HTTP 404 — not 500.
+
+18. The empty-set cover regen path: `POST /api/v1/sets/{id}/cover` for a
+    set with no eligible media files. If the seeded `testmedia/` library
+    contains no empty set, skip this step. Otherwise, confirm the response
+    is HTTP 400 — not 500. (`writeService.RegenerateSetCover` returns
+    `ErrEmptySetForCover` which maps to 400.)
+
 ## D) Cleanup
 
-16. Delete the temporary non-admin user: call
+19. Delete the temporary non-admin user: call
     `DELETE /api/v1/admin/users/{temp_user_id}` with `ADMIN_COOKIE`. Confirm
     the response is HTTP 200 and the body is `{"status": "ok"}`. Do NOT skip
     this cleanup — leftover users will pollute subsequent runs of S12, S13,
     and S15.
 
-17. Confirm cleanup succeeded: call `GET /api/v1/admin/users` with
+20. Confirm cleanup succeeded: call `GET /api/v1/admin/users` with
     `ADMIN_COOKIE`. Confirm the response is HTTP 200 and the array does NOT
     contain any entry whose `id` matches `temp_user_id` or whose `username`
     is `e2e-auth-boundaries`.