fix(568): the raw-count cap is the request's bound, and a recovery path re-asks the whole validator set

Two holes the review round found in the previous fix, both of the same shape: a
guard that names its own fields instead of deriving them.

The deco validators short-circuited the entire Validators.IdsMustExist call when
the DecoMode does not consume the ids, which took the 512-item raw-count cap with
it -- an arbitrarily large array under Inherit/Disable parsed and materialized
with nothing bounding it. Only the EXISTENCE half is the apply path's business,
so the mode predicate is now a required argument of the shared validator and gates
that half alone; the cap runs under every mode.

The channel recovery path rechecked GraphicsElementIdsMustExist alone, so a
watermark deleted between validation and SaveChangesAsync still surfaced as the
unhandled 500 the fix exists to remove -- WatermarkId, FFmpegProfileId,
FallbackFillerId and MirrorSourceChannelId are all written by the same save and
lose the same race. Both handlers now re-ask the whole of Validate on
DbUpdateException, so a validator added later is covered without editing the
recovery path.

The API-site outside-folder discriminator test seeded an Image row, so the Kind
conjunct rejected it whatever the path comparison did: a composite revert to
Path.GetFileName(path) == filename && kind == Text passed every API test. It now
carries the seeded Kind, mirroring the seeder-site twin, so only the path half can
reject it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015QqCpYFsKgnAnx6jVwrKiV

ErsatzTV.Application/Channels/Commands/UpdateChannelHandler.cs

@@ -89,6 +89,13 @@ public class UpdateChannelHandler(
     // again on the failure path instead, and return the same 422 the validator would have returned;
     // a DbUpdateException from any other cause keeps its own exception rather than being reported as
     // a client error.
+    //
+    // What is re-asked is the WHOLE of Validate, not the graphics-element half: every FK on this
+    // full-replace DTO -- FFmpegProfileId, WatermarkId, FallbackFillerId, MirrorSourceChannelId and
+    // the graphics element ids -- is written by ApplyUpdateRequest and can lose the same race, and a
+    // recovery path that names its fields one by one silently omits the next FK the DTO gains.
+    // Re-running the validator set is what keeps the two paths from drifting: a check added to
+    // Validate is covered here by construction.
     private async Task<Either<BaseError, ChannelViewModel>> ApplyUpdateRequestTranslatingLostRace(
         TvContext dbContext,
         Channel channel,
@@ -104,10 +111,12 @@ public class UpdateChannelHandler(
         catch (DbUpdateException)
         {
             // a fresh context: the failed save left the original one tracking the changes that
-            // could not be written, so the same query there could be answered from those.
+            // could not be written, so the same query there could be answered from those. The
+            // channel entity is still the tracked one from the failed context, which Validate reads
+            // only in memory (MirrorSourceMustBeValid's own-playout count) and never re-queries.
             await using TvContext recheckContext = await dbContextFactory.CreateDbContextAsync(cancellationToken);
-            Validation<BaseError, Unit> recheck =
-                await GraphicsElementIdsMustExist(recheckContext, request, cancellationToken);
+            Validation<BaseError, Channel> recheck =
+                await Validate(recheckContext, request, channel, cancellationToken);
 
             Option<BaseError> maybeError = recheck.Match(
                 Succ: _ => Option<BaseError>.None,
@@ -357,6 +366,7 @@ public class UpdateChannelHandler(
             request,
             r => r.GraphicsElementIds,
             "Graphics element",
+            idsAreConsumed: true,
             (ids, token) => dbContext.GraphicsElements
                 .Where(e => ids.Contains(e.Id))
                 .Select(e => e.Id)

ErsatzTV.Application/Scheduling/Commands/UpdateDecoHandler.cs

@@ -31,6 +31,10 @@ public class UpdateDecoHandler(
     // does not close that window either (neither provider locks the rows the validator merely READ),
     // so ask the existence questions again on the failure path and return the same 422; a
     // DbUpdateException from any other cause keeps its own exception.
+    //
+    // The whole of Validate is re-asked rather than a named pair of fields, for the same reason as
+    // the channel twin: a recovery path that enumerates its own fields omits the next one the DTO
+    // gains, while re-running the validator set covers a check added to Validate by construction.
     private async Task<Either<BaseError, Unit>> ApplyUpdateRequestTranslatingLostRace(
         TvContext dbContext,
         Deco existing,
@@ -46,10 +50,7 @@ public class UpdateDecoHandler(
             // a fresh context: the failed save left the original one tracking the changes that
             // could not be written, so the same query there could be answered out of those.
             await using TvContext recheckContext = await dbContextFactory.CreateDbContextAsync(cancellationToken);
-            Validation<BaseError, Unit> recheck = (
-                    await WatermarkIdsMustExist(recheckContext, request, cancellationToken),
-                    await GraphicsElementIdsMustExist(recheckContext, request, cancellationToken))
-                .Apply((_, _) => Unit.Default);
+            Validation<BaseError, Deco> recheck = await Validate(recheckContext, request, cancellationToken);
 
             Option<BaseError> maybeError = recheck.Match(
                 Succ: _ => Option<BaseError>.None,
@@ -271,7 +272,10 @@ public class UpdateDecoHandler(
     // validator can never reject an id the apply path was going to discard (#568). The SPA sends both
     // id lists regardless of the mode selector, so that shape arrives from the real editor: a draft
     // holding an element that has since been deleted must still be able to save the deco back to
-    // Inherit.
+    // Inherit. The predicate is handed to Validators.IdsMustExist rather than short-circuiting the
+    // call, because only the EXISTENCE half belongs to the apply path: a discarded list was still
+    // parsed and materialized out of the request body, so the raw-count cap has to apply under
+    // every mode.
     private static bool ConsumesWatermarkIds(UpdateDeco request) =>
         request.WatermarkMode is (DecoMode.Override or DecoMode.Merge);
 
@@ -290,33 +294,31 @@ public class UpdateDecoHandler(
         TvContext dbContext,
         UpdateDeco request,
         CancellationToken cancellationToken) =>
-        !ConsumesWatermarkIds(request)
-            ? Task.FromResult<Validation<BaseError, Unit>>(Unit.Default)
-            : Validators.IdsMustExist(
-                request,
-                r => r.WatermarkIds,
-                "Watermark",
-                (ids, token) => dbContext.ChannelWatermarks
-                    .Where(w => ids.Contains(w.Id))
-                    .Select(w => w.Id)
-                    .ToListAsync(token),
-                cancellationToken);
+        Validators.IdsMustExist(
+            request,
+            r => r.WatermarkIds,
+            "Watermark",
+            idsAreConsumed: ConsumesWatermarkIds(request),
+            (ids, token) => dbContext.ChannelWatermarks
+                .Where(w => ids.Contains(w.Id))
+                .Select(w => w.Id)
+                .ToListAsync(token),
+            cancellationToken);
 
     private static Task<Validation<BaseError, Unit>> GraphicsElementIdsMustExist(
         TvContext dbContext,
         UpdateDeco request,
         CancellationToken cancellationToken) =>
-        !ConsumesGraphicsElementIds(request)
-            ? Task.FromResult<Validation<BaseError, Unit>>(Unit.Default)
-            : Validators.IdsMustExist(
-                request,
-                r => r.GraphicsElementIds,
-                "Graphics element",
-                (ids, token) => dbContext.GraphicsElements
-                    .Where(e => ids.Contains(e.Id))
-                    .Select(e => e.Id)
-                    .ToListAsync(token),
-                cancellationToken);
+        Validators.IdsMustExist(
+            request,
+            r => r.GraphicsElementIds,
+            "Graphics element",
+            idsAreConsumed: ConsumesGraphicsElementIds(request),
+            (ids, token) => dbContext.GraphicsElements
+                .Where(e => ids.Contains(e.Id))
+                .Select(e => e.Id)
+                .ToListAsync(token),
+            cancellationToken);
 
     private static Task<Validation<BaseError, Deco>> DecoMustExist(
         TvContext dbContext,

ErsatzTV.Application/Validators/IdListValidation.cs

@@ -23,6 +23,14 @@ public static partial class Validators
     ///     bound the list, resolve which of its ids exist through <paramref name="findExisting" />,
     ///     and reject the rest with a 422 that names the request field it came from.
     /// </summary>
+    /// <param name="idsAreConsumed">
+    ///     Whether the apply path will actually read this list — false where another field of the
+    ///     same request (a deco's <c>DecoMode</c>) makes the reconcile discard it. It gates the
+    ///     EXISTENCE half only, never the count: a validator may not reject an id the apply path
+    ///     was going to throw away, but the raw list was still parsed and materialized out of the
+    ///     request body whatever is done with it afterwards, so the cap is the request's bound and
+    ///     not the apply path's (#568).
+    /// </param>
     /// <remarks>
     ///     The count is taken from the RAW list, before <c>Distinct</c> and before any database
     ///     work: deduplication is not what the request costs. A million-entry list of one repeated
@@ -34,6 +42,7 @@ public static partial class Validators
         T input,
         Expression<Func<T, List<int>>> expression,
         string noun,
+        bool idsAreConsumed,
         Func<List<int>, CancellationToken, Task<List<int>>> findExisting,
         CancellationToken cancellationToken)
     {
@@ -48,6 +57,11 @@ public static partial class Validators
                 "row already attached, so a longer list turns a single request into unbounded work.");
         }
 
+        if (!idsAreConsumed)
+        {
+            return Unit.Default;
+        }
+
         List<int> requested = submitted.Distinct().ToList();
         if (requested.Count == 0)
         {

ErsatzTV.Tests/Application/Channels/UpdateChannelGraphicsElementsTests.cs

@@ -31,6 +31,15 @@ public class UpdateChannelGraphicsElementsTests : ChannelHandlerTestBase
         return (elementA.Id, elementB.Id);
     }
 
+    private async Task<int> SeedWatermark()
+    {
+        await using TvContext context = Db.CreateContext();
+        var watermark = new ChannelWatermark { Name = "W" };
+        context.ChannelWatermarks.Add(watermark);
+        await context.SaveChangesAsync();
+        return watermark.Id;
+    }
+
     private async Task<List<int>> SeedGraphicsElements(int count)
     {
         await using TvContext context = Db.CreateContext();
@@ -252,6 +261,32 @@ public class UpdateChannelGraphicsElementsTests : ChannelHandlerTestBase
         error.Value.ShouldContain(elementAId.ToString(CultureInfo.InvariantCulture));
     }
 
+    // graphicsElementIds is not the only FK this DTO writes, and the recovery path re-asks the whole
+    // of Validate rather than the graphics-element half precisely so the other FKs are covered:
+    // WatermarkId is written by the same SaveChangesAsync and loses the same race. Reddens if the
+    // recheck is narrowed back to GraphicsElementIdsMustExist -- row 45 of the mutation table in
+    // docs/graphics-elements.md.
+    [Test]
+    public async Task Should_Translate_A_Watermark_Deleted_Between_Validation_And_Save_Into_The_Same_422()
+    {
+        ArmedSaveFailureInterceptor interceptor = await UseFailingSaveHarness();
+        await SeedFFmpegProfile();
+        Channel channel = await SeedChannel(1, "5");
+        int watermarkId = await SeedWatermark();
+
+        interceptor.SqlBeforeFailing =
+            $"DELETE FROM ChannelWatermark WHERE Id = {watermarkId.ToString(CultureInfo.InvariantCulture)}";
+        interceptor.Armed = true;
+
+        Either<BaseError, ChannelViewModel> result = await MakeHandler().Handle(
+            MakeUpdate(channel.Id, number: "5", watermarkId: watermarkId),
+            CancellationToken.None);
+
+        BaseError error = LeftOf(result);
+        error.Value.ShouldContain("Watermark");
+        error.Value.ShouldContain(watermarkId.ToString(CultureInfo.InvariantCulture));
+    }
+
     // The other half of that catch: a DbUpdateException whose cause is NOT a missing graphics
     // element is a real fault and must keep its own exception rather than be reported to the client
     // as a validation error about ids that are all still present.

ErsatzTV.Tests/Application/Graphics/GraphicsElementHandlerTests.cs

@@ -66,13 +66,18 @@ public class GraphicsElementHandlerTests
 
     // #568: the discriminator used to be Path.GetFileName(e.Path) == OnNowNextFileName, which is
     // folder-agnostic -- a user element named exactly "on-now-next.yml" outside the seeded text
-    // template folder would also report builtIn:true. Reddens if the fix (compare the full seeded
-    // path) is reverted to a filename-only comparison.
+    // template folder would also report builtIn:true. The row carries the SAME Kind as the real
+    // seeded element, deliberately: a wrong-kind row here would be rejected by the Kind conjunct
+    // whatever the path comparison is, so the composite revert (filename AND Kind == Text) would
+    // pass. Only the PATH half can reject a Text row in another folder, which is what this pins --
+    // it mirrors the seeder-site Ignores_A_Same_Named_Same_Kind_Element_Outside_The_Seeded_Folder.
     [Test]
     public async Task GetAllGraphicsElementsForApi_Should_Not_Mark_Same_Filename_Outside_Seeded_Folder_As_BuiltIn()
     {
-        string userElementPath = System.IO.Path.Combine("/config/graphics-elements/image", GraphicsElementDefaults.OnNowNextFileName);
-        await SeedElement(1, userElementPath, GraphicsElementKind.Image, string.Empty);
+        string userElementPath = System.IO.Path.Combine(
+            "/config/graphics-elements/text/some-subfolder",
+            GraphicsElementDefaults.OnNowNextFileName);
+        await SeedElement(1, userElementPath, GraphicsElementKind.Text, string.Empty);
 
         var handler = new GetAllGraphicsElementsForApiHandler(_db.Factory);
 

ErsatzTV.Tests/Application/Scheduling/UpdateDecoGraphicsElementsTests.cs

@@ -97,6 +97,28 @@ public class UpdateDecoGraphicsElementsTests
         return element.Id;
     }
 
+    private async Task<List<int>> SeedGraphicsElements(int count)
+    {
+        await using TvContext context = _db.CreateContext();
+        List<GraphicsElement> elements = Enumerable.Range(0, count)
+            .Select(i => new GraphicsElement { Path = $"element-{i}.yml" })
+            .ToList();
+        context.GraphicsElements.AddRange(elements);
+        await context.SaveChangesAsync();
+        return elements.Select(e => e.Id).ToList();
+    }
+
+    private async Task<List<int>> SeedWatermarks(int count)
+    {
+        await using TvContext context = _db.CreateContext();
+        List<ChannelWatermark> watermarks = Enumerable.Range(0, count)
+            .Select(i => new ChannelWatermark { Name = $"W{i}" })
+            .ToList();
+        context.ChannelWatermarks.AddRange(watermarks);
+        await context.SaveChangesAsync();
+        return watermarks.Select(w => w.Id).ToList();
+    }
+
     private async Task<int> SeedWatermark()
     {
         await using TvContext context = _db.CreateContext();
@@ -294,6 +316,46 @@ public class UpdateDecoGraphicsElementsTests
         error.Value.ShouldContain(Validators.MaximumIdListCount.ToString(CultureInfo.InvariantCulture));
     }
 
+    // The mode gate is handed to Validators.IdsMustExist rather than short-circuiting the call,
+    // because only the EXISTENCE half is the apply path's business: a list the reconcile discards
+    // was still parsed and materialized out of the request body. These two pin that the cap holds
+    // under a mode that consumes nothing -- row 47 of the mutation table in
+    // docs/graphics-elements.md. Note the ids all EXIST here, so nothing but the cap can reject
+    // them: a rejection is the cap's, not a smuggled existence check.
+    [Test]
+    public async Task Should_Reject_Too_Many_GraphicsElementIds_Even_Under_A_Mode_That_Does_Not_Consume_Them()
+    {
+        await SeedDeco();
+        List<int> ids = await SeedGraphicsElements(Validators.MaximumIdListCount + 1);
+
+        var handler = new UpdateDecoHandler(_db.Factory, _channel);
+        Either<BaseError, Unit> result = await handler.Handle(
+            MakeUpdate(graphicsElementsMode: DecoMode.Inherit, graphicsElementIds: ids),
+            CancellationToken.None);
+
+        IsLeft(result).ShouldBeTrue();
+        BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
+        error.Value.ShouldContain("[GraphicsElementIds]");
+        error.Value.ShouldContain(Validators.MaximumIdListCount.ToString(CultureInfo.InvariantCulture));
+    }
+
+    [Test]
+    public async Task Should_Reject_Too_Many_WatermarkIds_Even_Under_A_Mode_That_Does_Not_Consume_Them()
+    {
+        await SeedDeco();
+        List<int> ids = await SeedWatermarks(Validators.MaximumIdListCount + 1);
+
+        var handler = new UpdateDecoHandler(_db.Factory, _channel);
+        Either<BaseError, Unit> result = await handler.Handle(
+            MakeUpdate(watermarkMode: DecoMode.Disable, watermarkIds: ids),
+            CancellationToken.None);
+
+        IsLeft(result).ShouldBeTrue();
+        BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
+        error.Value.ShouldContain("[WatermarkIds]");
+        error.Value.ShouldContain(Validators.MaximumIdListCount.ToString(CultureInfo.InvariantCulture));
+    }
+
     // The deco twin of the channel handler's lost-race translation: an element deleted between
     // Validate and SaveChangesAsync must come back as the validator's own 422, not the FK
     // exception. Removing the DbUpdateException catch from
@@ -323,4 +385,32 @@ public class UpdateDecoGraphicsElementsTests
         error.Value.ShouldContain("[GraphicsElementIds]");
         error.Value.ShouldContain(elementId.ToString(CultureInfo.InvariantCulture));
     }
+
+    // The watermark half of the same recovery. Without it, removing the watermark question from the
+    // recheck would redden nothing -- and the recheck re-asks the whole of Validate exactly so that
+    // neither id list is the only one covered. Row 46 of the mutation table.
+    [Test]
+    public async Task Should_Translate_A_Deco_Watermark_Deleted_Between_Validation_And_Save_Into_The_Same_422()
+    {
+        var interceptor = new ArmedSaveFailureInterceptor();
+        await _db.DisposeAsync();
+        _db = await InMemoryTvContext.CreateAsync(interceptor);
+
+        await SeedDeco();
+        int watermarkId = await SeedWatermark();
+
+        interceptor.SqlBeforeFailing =
+            $"DELETE FROM ChannelWatermark WHERE Id = {watermarkId.ToString(CultureInfo.InvariantCulture)}";
+        interceptor.Armed = true;
+
+        var handler = new UpdateDecoHandler(_db.Factory, _channel);
+        Either<BaseError, Unit> result = await handler.Handle(
+            MakeUpdate(watermarkMode: DecoMode.Override, watermarkIds: [watermarkId]),
+            CancellationToken.None);
+
+        IsLeft(result).ShouldBeTrue();
+        BaseError error = result.Match(Left: e => e, Right: _ => throw new AssertionException("expected Left"));
+        error.Value.ShouldContain("[WatermarkIds]");
+        error.Value.ShouldContain(watermarkId.ToString(CultureInfo.InvariantCulture));
+    }
 }

ErsatzTV.Tests/Support/ChannelHandlerTestBase.cs

@@ -128,7 +128,8 @@ public abstract class ChannelHandlerTestBase
         string logoPath = "",
         string name = "Test",
         string group = "ErsatzTV",
-        List<int> graphicsElementIds = null) =>
+        List<int> graphicsElementIds = null,
+        int? watermarkId = null) =>
         new(
             channelId,
             name,
@@ -147,7 +148,7 @@ public abstract class ChannelHandlerTestBase
             null,
             null,
             StreamingMode.TransportStreamHybrid,
-            null,
+            watermarkId,
             null,
             string.Empty,
             ChannelSubtitleMode.None,