fix(api): #316 review — POST-ify graphics-elements refresh, LockedError→409, no-store machine-key

- GET /api/graphics-elements no longer side-effects; refresh moved to
  POST /api/graphics-elements/refresh (204), closing a CSRF vector on a GET.
- PrepareTroubleshootingPlaybackHandler now returns a typed LockedError from
  both atomic lock-acquire failures; ApiResults.ToErrorResult maps it to 409
  instead of falling through to 422, so a lock lost in the race between the
  controller's pre-check and the handler's atomic acquire still reports 409.
- AuthController.MachineKey sets Cache-Control: no-store + Pragma: no-cache
  on the 200 response carrying the master API key.
- Reworded the stale "subtitleId query parameter" endpoint description now
  that playback/start takes a JSON body.
- Regenerated openapi/v1.json + docs/endpoint-index.md; docs/api-conventions.md
  updated with the LockedError pattern (§3a) and the ToErrorResult table row.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

ErsatzTV.Application/Troubleshooting/Commands/PrepareTroubleshootingPlaybackHandler.cs

@@ -3,6 +3,7 @@ using ErsatzTV.Application.Streaming;
 using ErsatzTV.Core;
 using ErsatzTV.Core.Domain;
 using ErsatzTV.Core.Domain.Filler;
+using ErsatzTV.Core.Errors;
 using ErsatzTV.Core.Extensions;
 using ErsatzTV.Core.FFmpeg;
 using ErsatzTV.Core.Interfaces.Emby;
@@ -72,7 +73,7 @@ public class PrepareTroubleshootingPlaybackHandler(
                 // already holds it (no check-then-set race, no double-owner)
                 if (!entityLocker.LockTroubleshootingPlayback())
                 {
-                    return BaseError.New("Troubleshooting playback is locked");
+                    return new LockedError("Troubleshooting playback is locked");
                 }
 
                 lockAcquired = true;
@@ -143,7 +144,7 @@ public class PrepareTroubleshootingPlaybackHandler(
                     // report the conflict. Ownership is tracked so the catch never cross-releases.
                     if (!entityLocker.LockTroubleshootingPlayback())
                     {
-                        return BaseError.New("Troubleshooting playback is locked");
+                        return new LockedError("Troubleshooting playback is locked");
                     }
 
                     lockAcquired = true;

ErsatzTV.Core/Errors/LockedError.cs

@@ -0,0 +1,13 @@
+namespace ErsatzTV.Core.Errors;
+
+/// <summary>
+///     A <see cref="BaseError" /> raised when a mutation races another operation that already holds an
+///     exclusive lock (e.g. a troubleshooting playback session in flight). REST endpoints map this to
+///     HTTP 409 Conflict, distinguishing it from the generic 422 other <see cref="BaseError" /> values get.
+/// </summary>
+public class LockedError : BaseError
+{
+    public LockedError(string value) : base(value)
+    {
+    }
+}

ErsatzTV.Tests/Application/Troubleshooting/PrepareTroubleshootingPlaybackHandlerTests.cs

@@ -2,6 +2,7 @@ using System.IO.Abstractions;
 using ErsatzTV.Application.Troubleshooting;
 using ErsatzTV.Core;
 using ErsatzTV.Core.Domain;
+using ErsatzTV.Core.Errors;
 using ErsatzTV.Core.Interfaces.Emby;
 using ErsatzTV.Core.Interfaces.FFmpeg;
 using ErsatzTV.Core.Interfaces.Jellyfin;
@@ -84,6 +85,43 @@ public class PrepareTroubleshootingPlaybackHandlerTests
         _entityLocker.Received(1).UnlockTroubleshootingPlayback();
     }
 
+    [Test]
+    public async Task Handle_Should_Return_LockedError_When_Lock_Already_Held()
+    {
+        await SeedFFmpegConfig();
+        int profileId = await SeedFFmpegProfile();
+        int movieId = await SeedMovieWithMissingFile();
+
+        // another session already holds the troubleshooting lock
+        _entityLocker.LockTroubleshootingPlayback().Returns(false);
+
+        PrepareTroubleshootingPlaybackHandler handler = CreateHandler();
+
+        var request = new PrepareTroubleshootingPlayback(
+            Guid.NewGuid(),
+            StreamingMode.HttpLiveStreamingSegmenter,
+            movieId,
+            ChannelId: 0,
+            profileId,
+            StreamSelector: string.Empty,
+            WatermarkIds: [],
+            GraphicsElementIds: [],
+            SubtitleId: null,
+            SeekSeconds: Option<int>.None,
+            Start: Option<DateTimeOffset>.None);
+
+        Either<BaseError, PlayoutItemResult> result = await handler.Handle(request, CancellationToken.None);
+
+        result.IsLeft.ShouldBeTrue();
+        foreach (BaseError error in result.LeftToSeq())
+        {
+            error.ShouldBeOfType<LockedError>();
+        }
+
+        // never acquired, so never releases
+        _entityLocker.DidNotReceive().UnlockTroubleshootingPlayback();
+    }
+
     [Test]
     public async Task Handle_Should_Not_Release_Lock_It_Never_Acquired()
     {

ErsatzTV.Tests/Controllers/AuthControllerTests.cs

@@ -114,4 +114,24 @@ public class AuthControllerTests
         var ok = result.ShouldBeOfType<OkObjectResult>();
         ok.Value.ShouldBeOfType<MachineKeyResponse>().ApiKey.ShouldBe("the-machine-key");
     }
+
+    [Test]
+    public void MachineKey_Sets_CacheControl_NoStore_For_An_Authenticated_Session()
+    {
+        var mediator = Substitute.For<IMediator>();
+        var httpContext = new DefaultHttpContext
+        {
+            User = new ClaimsPrincipal(
+                new ClaimsIdentity([new Claim(ClaimTypes.Name, "admin")], AuthConstants.CookieScheme))
+        };
+        var controller = new AuthController(mediator, Config(envSeed: false), ApiKeyProvider("the-machine-key"))
+        {
+            ControllerContext = new ControllerContext { HttpContext = httpContext }
+        };
+
+        controller.MachineKey();
+
+        httpContext.Response.Headers.CacheControl.ToString().ShouldBe("no-store");
+        httpContext.Response.Headers.Pragma.ToString().ShouldBe("no-cache");
+    }
 }

ErsatzTV.Tests/Controllers/GraphicsElementControllerTests.cs

@@ -3,6 +3,7 @@ using ErsatzTV.Application.Graphics;
 using ErsatzTV.Controllers.Api;
 using ErsatzTV.Core.Api.Graphics;
 using MediatR;
+using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.Routing;
 using NSubstitute;
 using NUnit.Framework;
@@ -24,7 +25,7 @@ public class GraphicsElementControllerTests
     }
 
     [Test]
-    public void Controller_Should_Expose_Idiomatic_Rest_Route()
+    public void Controller_Should_Expose_Idiomatic_Rest_Route_For_GetAll()
     {
         MethodInfo action = typeof(GraphicsElementController).GetMethod(nameof(GraphicsElementController.GetAll))
             ?? throw new AssertionException("Missing action GetAll");
@@ -34,6 +35,17 @@ public class GraphicsElementControllerTests
         attribute.Template.ShouldBe("/api/graphics-elements");
     }
 
+    [Test]
+    public void Controller_Should_Expose_Idiomatic_Rest_Route_For_Refresh()
+    {
+        MethodInfo action = typeof(GraphicsElementController).GetMethod(nameof(GraphicsElementController.Refresh))
+            ?? throw new AssertionException("Missing action Refresh");
+
+        HttpMethodAttribute attribute = action.GetCustomAttributes<HttpMethodAttribute>(inherit: true).Single();
+        attribute.HttpMethods.ShouldContain("POST");
+        attribute.Template.ShouldBe("/api/graphics-elements/refresh");
+    }
+
     [Test]
     public async Task GetAll_Should_Return_GraphicsElements()
     {
@@ -45,7 +57,7 @@ public class GraphicsElementControllerTests
         _mediator.Send(Arg.Any<GetAllGraphicsElementsForApi>(), Arg.Any<CancellationToken>())
             .Returns(models);
 
-        List<GraphicsElementResponseModel> result = await _controller.GetAll(refresh: false, CancellationToken.None);
+        List<GraphicsElementResponseModel> result = await _controller.GetAll(CancellationToken.None);
 
         result.ShouldBe(models);
     }
@@ -56,34 +68,29 @@ public class GraphicsElementControllerTests
         _mediator.Send(Arg.Any<GetAllGraphicsElementsForApi>(), Arg.Any<CancellationToken>())
             .Returns([]);
 
-        List<GraphicsElementResponseModel> result = await _controller.GetAll(refresh: false, CancellationToken.None);
+        List<GraphicsElementResponseModel> result = await _controller.GetAll(CancellationToken.None);
 
         result.ShouldBeEmpty();
     }
 
     [Test]
-    public async Task GetAll_Should_Not_Refresh_When_Refresh_Is_False()
+    public async Task GetAll_Should_Never_Trigger_A_Refresh()
     {
+        // the GET must be a pure read (CSRF vector when it wrote to the DB — ersatztv#316 review)
         _mediator.Send(Arg.Any<GetAllGraphicsElementsForApi>(), Arg.Any<CancellationToken>())
             .Returns([]);
 
-        await _controller.GetAll(refresh: false, CancellationToken.None);
+        await _controller.GetAll(CancellationToken.None);
 
         await _mediator.DidNotReceive().Send(Arg.Any<RefreshGraphicsElements>(), Arg.Any<CancellationToken>());
     }
 
     [Test]
-    public async Task GetAll_Should_Refresh_Before_Listing_When_Refresh_Is_True()
+    public async Task Refresh_Should_Send_RefreshGraphicsElements_And_Return_204()
     {
-        _mediator.Send(Arg.Any<GetAllGraphicsElementsForApi>(), Arg.Any<CancellationToken>())
-            .Returns([]);
+        IActionResult result = await _controller.Refresh(CancellationToken.None);
 
-        await _controller.GetAll(refresh: true, CancellationToken.None);
-
-        Received.InOrder(() =>
-        {
-            _mediator.Send(Arg.Any<RefreshGraphicsElements>(), Arg.Any<CancellationToken>());
-            _mediator.Send(Arg.Any<GetAllGraphicsElementsForApi>(), Arg.Any<CancellationToken>());
-        });
+        result.ShouldBeOfType<NoContentResult>();
+        await _mediator.Received(1).Send(Arg.Any<RefreshGraphicsElements>(), Arg.Any<CancellationToken>());
     }
 }

ErsatzTV.Tests/Controllers/TroubleshootControllerTests.cs

@@ -318,6 +318,24 @@ public class TroubleshootControllerTests
             .Send(Arg.Any<PrepareTroubleshootingPlayback>(), Arg.Any<CancellationToken>());
     }
 
+    [Test]
+    public async Task TroubleshootPlayback_Should_Return_409_ProblemDetails_When_Prepare_Loses_Lock_Race()
+    {
+        // pre-check passes, but the handler's atomic acquire loses the race to another session
+        // that grabbed the lock in between -> still a 409, not a 422 (ersatztv#316 review)
+        _entityLocker.IsTroubleshootingPlaybackLocked().Returns(false);
+        _mediator.Send(Arg.Any<PrepareTroubleshootingPlayback>(), Arg.Any<CancellationToken>())
+            .Returns(Either<BaseError, PlayoutItemResult>.Left(
+                new ErsatzTV.Core.Errors.LockedError("Troubleshooting playback is locked")));
+
+        IActionResult result = await _controller.TroubleshootPlayback(
+            DefaultPlaybackRequest(),
+            CancellationToken.None);
+
+        var conflict = result.ShouldBeOfType<ConflictObjectResult>();
+        conflict.Value.ShouldBeOfType<ProblemDetails>().Status.ShouldBe(409);
+    }
+
     [Test]
     public async Task TroubleshootPlayback_Should_Return_404_ProblemDetails_When_Prepare_Not_Found()
     {

ErsatzTV.Tests/Extensions/ApiResultsTests.cs

@@ -38,6 +38,24 @@ public class ApiResultsTests
         problemDetails.Detail.ShouldBe("missing");
     }
 
+    [Test]
+    public void ToErrorResult_Should_Map_LockedError_To_409()
+    {
+        IActionResult result = new LockedError("locked").ToErrorResult();
+        result.ShouldBeOfType<ConflictObjectResult>().StatusCode.ShouldBe(409);
+    }
+
+    [Test]
+    public void ToErrorResult_Should_Return_ProblemDetails_For_LockedError()
+    {
+        IActionResult result = new LockedError("locked").ToErrorResult();
+
+        var conflict = result.ShouldBeOfType<ConflictObjectResult>();
+        var problemDetails = conflict.Value.ShouldBeOfType<ProblemDetails>();
+        problemDetails.Status.ShouldBe(409);
+        problemDetails.Detail.ShouldBe("locked");
+    }
+
     [Test]
     public void ToErrorResult_Should_Map_Other_Error_To_422()
     {

ErsatzTV/Controllers/Api/AuthController.cs

@@ -69,6 +69,8 @@ public class AuthController(IMediator mediator, IConfiguration configuration, IA
             });
         }
 
+        Response.Headers.CacheControl = "no-store";
+        Response.Headers.Pragma = "no-cache";
         return Ok(new MachineKeyResponse(apiKeyProvider.ApiKey));
     }
 

ErsatzTV/Controllers/Api/GraphicsElementController.cs

@@ -12,20 +12,23 @@ public class GraphicsElementController(IMediator mediator) : ControllerBase
     [HttpGet("/api/graphics-elements", Name = "GetGraphicsElements")]
     [Tags("Graphics Elements")]
     [EndpointSummary("Get all graphics elements")]
-    [EndpointDescription(
-        "Returns all graphics elements. Pass refresh=true to first re-sync the on-disk graphics element " +
-        "definitions into the database (matching the legacy Blazor behavior) so newly added files appear.")]
+    [EndpointDescription("Returns all graphics elements.")]
     [EndpointGroupName("general")]
     [ProducesResponseType(typeof(List<GraphicsElementResponseModel>), StatusCodes.Status200OK)]
-    public async Task<List<GraphicsElementResponseModel>> GetAll(
-        [FromQuery] bool refresh,
-        CancellationToken cancellationToken)
-    {
-        if (refresh)
-        {
-            await mediator.Send(new RefreshGraphicsElements(), cancellationToken);
-        }
+    public async Task<List<GraphicsElementResponseModel>> GetAll(CancellationToken cancellationToken) =>
+        await mediator.Send(new GetAllGraphicsElementsForApi(), cancellationToken);
 
-        return await mediator.Send(new GetAllGraphicsElementsForApi(), cancellationToken);
+    [HttpPost("/api/graphics-elements/refresh", Name = "RefreshGraphicsElements")]
+    [Tags("Graphics Elements")]
+    [EndpointSummary("Re-sync graphics elements from disk")]
+    [EndpointDescription(
+        "Re-syncs the on-disk graphics element definitions into the database (matching the legacy Blazor " +
+        "behavior) so newly added files appear.")]
+    [EndpointGroupName("general")]
+    [ProducesResponseType(StatusCodes.Status204NoContent)]
+    public async Task<IActionResult> Refresh(CancellationToken cancellationToken)
+    {
+        await mediator.Send(new RefreshGraphicsElements(), cancellationToken);
+        return NoContent();
     }
 }

ErsatzTV/Controllers/Api/TroubleshootController.cs

@@ -350,7 +350,7 @@ public class TroubleshootController(
     [EndpointSummary("List selectable subtitle streams for a media item")]
     [EndpointDescription(
         "Returns the subtitle streams that can be burned in for a troubleshooting playback. Each item's id is the " +
-        "value to pass back as the playback.m3u8 endpoint's subtitleId query parameter.")]
+        "value to pass back as the POST /api/troubleshoot/playback/start request body's subtitleId field.")]
     [EndpointGroupName("general")]
     [ProducesResponseType(typeof(List<TroubleshootingSubtitleResponseModel>), StatusCodes.Status200OK)]
     [ProducesResponseType(typeof(ProblemDetails), StatusCodes.Status404NotFound)]

ErsatzTV/Extensions/ApiResults.cs

@@ -18,7 +18,8 @@ public static class ApiResults
     /// <summary>
     ///     Maps a failure to 404 when it is a <see cref="NotFoundError" />, 412 when it is a
     ///     <see cref="PreconditionFailedError" /> (optimistic-concurrency mismatch, issue #253),
-    ///     otherwise 422.
+    ///     409 when it is a <see cref="LockedError" /> (a mutation raced another operation's exclusive
+    ///     lock), otherwise 422.
     /// </summary>
     public static IActionResult ToErrorResult(this BaseError error) =>
         error switch
@@ -30,6 +31,8 @@ public static class ApiResults
                 {
                     StatusCode = StatusCodes.Status412PreconditionFailed
                 },
+            LockedError =>
+                ConflictProblem("Troubleshooting playback is locked", error.Value),
             _ =>
                 new UnprocessableEntityObjectResult(CreateProblemDetails(422, "Validation failed", error.Value))
         };

ErsatzTV/wwwroot/openapi/v1.json

@@ -7532,17 +7532,8 @@
           "Graphics Elements"
         ],
         "summary": "Get all graphics elements",
-        "description": "Returns all graphics elements. Pass refresh=true to first re-sync the on-disk graphics element definitions into the database (matching the legacy Blazor behavior) so newly added files appear.",
+        "description": "Returns all graphics elements.",
         "operationId": "GetGraphicsElements",
-        "parameters": [
-          {
-            "name": "refresh",
-            "in": "query",
-            "schema": {
-              "type": "boolean"
-            }
-          }
-        ],
         "responses": {
           "200": {
             "description": "OK",
@@ -7582,13 +7573,31 @@
                 }
               }
             }
+          }
+        },
+        "security": [
+          { }
+        ]
+      }
+    },
+    "/api/graphics-elements/refresh": {
+      "post": {
+        "tags": [
+          "Graphics Elements"
+        ],
+        "summary": "Re-sync graphics elements from disk",
+        "description": "Re-syncs the on-disk graphics element definitions into the database (matching the legacy Blazor behavior) so newly added files appear.",
+        "operationId": "RefreshGraphicsElements",
+        "responses": {
+          "204": {
+            "description": "No Content"
           },
-          "400": {
-            "description": "Request validation failed (model binding or FluentValidation).",
+          "401": {
+            "description": "API key missing or invalid.",
             "content": {
               "application/json": {
                 "schema": {
-                  "$ref": "#/components/schemas/ValidationProblemDetails"
+                  "$ref": "#/components/schemas/ProblemDetails"
                 }
               }
             }
@@ -21386,7 +21395,7 @@
           "Troubleshooting"
         ],
         "summary": "List selectable subtitle streams for a media item",
-        "description": "Returns the subtitle streams that can be burned in for a troubleshooting playback. Each item's id is the value to pass back as the playback.m3u8 endpoint's subtitleId query parameter.",
+        "description": "Returns the subtitle streams that can be burned in for a troubleshooting playback. Each item's id is the value to pass back as the POST /api/troubleshoot/playback/start request body's subtitleId field.",
         "operationId": "GetTroubleshootingSubtitles",
         "parameters": [
           {

docs/api-conventions.md

@@ -94,7 +94,7 @@ hand-rolling `IActionResult` status codes:
 
 | Method | Input | Output |
 |---|---|---|
-| `ToErrorResult()` | `BaseError` | 404 if `NotFoundError`, **412 if `PreconditionFailedError`** (optimistic-concurrency mismatch, §7a), else 422 (`ProblemDetails`) |
+| `ToErrorResult()` | `BaseError` | 404 if `NotFoundError`, **412 if `PreconditionFailedError`** (optimistic-concurrency mismatch, §7a), **409 if `LockedError`** (a handler's own lock-acquire lost the race, §3a), else 422 (`ProblemDetails`) |
 | `ToCreatedResult(location, body)` | `Either<BaseError, T>` | `Left` → `ToErrorResult()`; `Right` → 201 + `Location` header |
 | `ToUpdatedResult()` | `Either<BaseError, T>` | `Left` → `ToErrorResult()`; `Right` → 200 + body |
 | `ToDeletedResult()` | `Either<BaseError, Unit>` | `Left` → `ToErrorResult()`; `Right` → 204 |
@@ -142,6 +142,15 @@ action. Precedent for the 409 shape: `TraktController` (its private `ConflictPro
   contract keys on `Id`, never the user-mutable `Number` (re-keyed in #197 Bundle C; see
   `docs/decisions.md` 2026-07-12). The broadcast-side lookup (`GetPlayoutIdByChannelNumber`, used by
   `HlsSessionWorker`) stays number-keyed — a separate contract. Reserve 200 for a synchronous durable result.
+- **Handler-side atomic lock loss also maps to 409, via a typed error, not 422** (issue #316 review):
+  when the *handler itself* is the one that atomically acquires an `IEntityLocker` lock (not just a
+  controller pre-check) and loses the race, return `new LockedError(...)` (`ErsatzTV.Core/Errors/LockedError.cs`,
+  sibling of `NotFoundError`/`PreconditionFailedError`) from the handler — `ToErrorResult()` maps it to
+  409 automatically. Exemplar: `PrepareTroubleshootingPlaybackHandler` — `TroubleshootController` does a
+  cheap `IsTroubleshootingPlaybackLocked()` pre-check (advisory, §3a's check-then-act caveat applies),
+  but the handler's own `LockTroubleshootingPlayback()` is the atomic acquire; if *that* loses the race
+  it returns `LockedError`, so the 409 survives even when the pre-check passed a moment too early. Don't
+  let a handler-side lock loss fall through to the generic 422 `BaseError.New(...)`.
 
 ### 3b. Map a "queue a background job" outcome to status codes with an enum, not a `bool`
 

docs/endpoint-index.md

@@ -2,7 +2,7 @@
 
 *Generated by `scripts/generate-endpoint-index.py` from `ErsatzTV/wwwroot/openapi/v1.json`. Do not edit by hand -- regenerated by `scripts/update-openapi.sh`.*
 
-159 endpoints, 241 operations.
+160 endpoints, 242 operations.
 
 ## Artists
 
@@ -150,6 +150,7 @@
 | Method | Path | Operation | Summary |
 |---|---|---|---|
 | GET | `/api/graphics-elements` | GetGraphicsElements | Get all graphics elements |
+| POST | `/api/graphics-elements/refresh` | RefreshGraphicsElements | Re-sync graphics elements from disk |
 
 ## Health