fix(754,757): declare graphicsElementIds + padToNearestMinute, and pin every MCP tool to its OpenAPI contract (#760)

Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>

ErsatzTV.Mcp.Tests/ErsatzTV.Mcp.Tests.csproj

@@ -21,4 +21,16 @@
         <ProjectReference Include="..\ErsatzTV.Mcp\ErsatzTV.Mcp.csproj" />
     </ItemGroup>
 
+    <!--
+        The generated OpenAPI document is the wire contract the MCP catalog wraps. Copying it into the
+        test output lets ToolCatalogTests assert that every write tool declares exactly the request-body
+        fields its endpoint accepts, so a new DTO property cannot drift out of a tool schema unnoticed
+        (issue #754). Regenerated by scripts/update-openapi.sh.
+    -->
+    <ItemGroup>
+        <Content Include="..\ErsatzTV\wwwroot\openapi\v1.json"
+                 Link="openapi\v1.json"
+                 CopyToOutputDirectory="PreserveNewest" />
+    </ItemGroup>
+
 </Project>

ErsatzTV.Mcp.Tests/ToolCatalogTests.cs

@@ -77,9 +77,22 @@ public class ToolCatalogTests
         // Enums must NOT be forced required (they have server-side defaults).
         createRequired.ShouldNotContain("streamingMode");
 
-        // Update carries the same body fields plus the route id.
-        update.InputSchema.RootElement.GetProperty("properties").TryGetProperty("id", out _).ShouldBeTrue();
-        update.InputSchema.RootElement.GetProperty("properties").TryGetProperty("showInEpg", out _).ShouldBeTrue();
+        // Update carries the create body fields plus the route id...
+        JsonElement updateProps = update.InputSchema.RootElement.GetProperty("properties");
+        updateProps.TryGetProperty("id", out _).ShouldBeTrue();
+        updateProps.TryGetProperty("showInEpg", out _).ShouldBeTrue();
+
+        // ...plus graphicsElementIds, which is on UpdateChannelRequest only. PUT is a full replace, so
+        // while the tool could not express this field an agent following the tool's own "send the full
+        // desired state" instruction silently detached every graphics element (issue #754).
+        updateProps.TryGetProperty("graphicsElementIds", out JsonElement graphicsElementIds).ShouldBeTrue();
+        graphicsElementIds.GetProperty("type").GetString().ShouldBe("array");
+        graphicsElementIds.GetProperty("items").GetProperty("type").GetString().ShouldBe("integer");
+
+        // Create must NOT send it: CreateChannelRequest has no such property, and the tool schema is
+        // additionalProperties:false. This is why it is declared on the update tool rather than in the
+        // shared ChannelFields().
+        createProps.TryGetProperty("graphicsElementIds", out _).ShouldBeFalse();
     }
 
     [Test]
@@ -256,4 +269,248 @@ public class ToolCatalogTests
         tool.QueryParameters.ShouldNotBeNull();
         tool.QueryParameters!.ShouldContain("deep");
     }
+
+    // #754: ToolCatalog declared 27 of UpdateChannelRequest's 28 properties. The missing one was
+    // graphicsElementIds, and because PUT /api/v1/channels/{id} is a FULL REPLACE the omission was not
+    // merely "one field you cannot set" — an agent that GET-edit-PUT the channel, exactly as the tool's
+    // description tells it to, detached every graphics element (including the On Now/Next overlay) with
+    // a 200 and no error. The same shape was live on ersatztv_update_schedule, which omitted
+    // padToNearestMinute and silently cleared a configured pad.
+    //
+    // Neither is fixable by counting fields once: the defect is that nothing tied the tool schema to the
+    // contract it wraps. So this test asserts the tie for EVERY write tool against the generated OpenAPI
+    // document (the actual wire contract, linked into the test output by the csproj). A new property on
+    // any request DTO now fails here until the catalog declares it.
+    [Test]
+    public void Every_Write_Tool_Should_Declare_Exactly_Its_OpenApi_Request_Body_Fields()
+    {
+        using JsonDocument spec = LoadOpenApiDocument();
+        JsonElement paths = spec.RootElement.GetProperty("paths");
+
+        ToolDefinition[] writeTools = ToolCatalog.All
+            .Where(t => t.HttpMethod == HttpMethod.Post
+                || t.HttpMethod == HttpMethod.Put
+                || t.HttpMethod == HttpMethod.Patch)
+            .ToArray();
+
+        // Pin the covered set rather than trusting the filter. A tool that stopped being a write verb,
+        // or a new write tool, must show up as a change here — a bare loop over a filtered set passes
+        // just as happily when the set silently shrinks to nothing.
+        string[] expectedWriteTools =
+        [
+            "ersatztv_add_collection_items",
+            "ersatztv_create_channel",
+            "ersatztv_create_collection",
+            "ersatztv_create_playout",
+            "ersatztv_create_schedule",
+            "ersatztv_create_smart_collection",
+            "ersatztv_enable_jellyfin_library_sync",
+            "ersatztv_refresh_jellyfin_libraries",
+            "ersatztv_reset_channel_playout",
+            "ersatztv_scan_jellyfin_collections",
+            "ersatztv_scan_library",
+            "ersatztv_update_channel",
+            "ersatztv_update_collection",
+            "ersatztv_update_collection_custom_order",
+            "ersatztv_update_playout",
+            "ersatztv_update_schedule",
+            "ersatztv_update_smart_collection"
+        ];
+
+        writeTools.Select(t => t.Name).OrderBy(n => n, StringComparer.Ordinal)
+            .ShouldBe(expectedWriteTools.OrderBy(n => n, StringComparer.Ordinal));
+
+        foreach (ToolDefinition tool in writeTools)
+        {
+            paths.TryGetProperty(tool.PathTemplate, out JsonElement pathItem)
+                .ShouldBeTrue($"{tool.Name}: {tool.PathTemplate} is not in the OpenAPI document");
+
+            string verb = tool.HttpMethod.Method.ToLowerInvariant();
+            pathItem.TryGetProperty(verb, out JsonElement operation)
+                .ShouldBeTrue($"{tool.Name}: {verb.ToUpperInvariant()} {tool.PathTemplate} is not in the OpenAPI document");
+
+            Dictionary<string, string> declared = DeclaredBodyArguments(tool);
+            Dictionary<string, string> accepted = RequestBodyProperties(spec, operation, tool.Name);
+
+            // Compare name AND type. Names alone would let a field drift to the wrong JSON type: the
+            // tool would advertise "string" for an int?, the agent would send "30", and the API would
+            // 400 — green test, broken tool.
+            declared.Select(p => $"{p.Key}: {p.Value}").OrderBy(s => s, StringComparer.Ordinal)
+                .ShouldBe(
+                    accepted.Select(p => $"{p.Key}: {p.Value}").OrderBy(s => s, StringComparer.Ordinal),
+                    customMessage:
+                    $"{tool.Name} declares body fields that do not match {verb.ToUpperInvariant()} {tool.PathTemplate}. "
+                    + "A field the endpoint accepts but the tool omits is silently dropped on a full-replace "
+                    + "write (#754); a field the tool sends but the endpoint does not accept is rejected; "
+                    + "a field declared with the wrong type is rejected at the API.");
+        }
+    }
+
+    // #757, the sibling of the body guard above. Query parameters drift the same way and are WORSE for
+    // reads: ToolArgumentValidator rejects undeclared arguments, so a parameter the tool omits is not
+    // merely undocumented, it is unreachable — the caller cannot pass it at all. That is how #616's
+    // paging omission hard-capped two tools at the first page. This covers EVERY tool, not just the
+    // write verbs, because the drift that existed when this was written was entirely on reads.
+    [Test]
+    public void Every_Tool_Should_Declare_Exactly_Its_OpenApi_Query_Parameters()
+    {
+        using JsonDocument spec = LoadOpenApiDocument();
+        JsonElement paths = spec.RootElement.GetProperty("paths");
+
+        // Every tool is covered, so an emptiness guard is enough here — there is no filter to escape.
+        ToolCatalog.All.Count.ShouldBeGreaterThan(30);
+
+        // Accumulate rather than throwing on the first mismatch, so one run reports the WHOLE drift set.
+        // Failing fast here would hand back one tool at a time and invite fixing them one at a time,
+        // which is how the #754 twin stayed hidden in the first place.
+        List<string> drift = [];
+
+        foreach (ToolDefinition tool in ToolCatalog.All)
+        {
+            paths.TryGetProperty(tool.PathTemplate, out JsonElement pathItem)
+                .ShouldBeTrue($"{tool.Name}: {tool.PathTemplate} is not in the OpenAPI document");
+
+            string verb = tool.HttpMethod.Method.ToLowerInvariant();
+            pathItem.TryGetProperty(verb, out JsonElement operation)
+                .ShouldBeTrue($"{tool.Name}: {verb.ToUpperInvariant()} {tool.PathTemplate} is not in the OpenAPI document");
+
+            IReadOnlySet<string> declared = tool.QueryParameters ?? new HashSet<string>(StringComparer.Ordinal);
+            HashSet<string> accepted = QueryParameterNames(operation);
+
+            string[] missing = accepted.Except(declared, StringComparer.Ordinal).OrderBy(n => n, StringComparer.Ordinal).ToArray();
+            string[] phantom = declared.Except(accepted, StringComparer.Ordinal).OrderBy(n => n, StringComparer.Ordinal).ToArray();
+
+            if (missing.Length > 0 || phantom.Length > 0)
+            {
+                drift.Add(
+                    $"{tool.Name} ({verb.ToUpperInvariant()} {tool.PathTemplate}): "
+                    + $"unreachable={string.Join(",", missing)} phantom={string.Join(",", phantom)}");
+            }
+        }
+
+        // A parameter the endpoint accepts but the tool omits is UNREACHABLE, not merely undocumented:
+        // ToolArgumentValidator rejects undeclared arguments, so the caller cannot pass it at all
+        // (#616 hard-capped two paged tools exactly this way). A phantom is the reverse — the tool
+        // advertises something the endpoint ignores.
+        drift.ShouldBeEmpty();
+    }
+
+    private static HashSet<string> QueryParameterNames(JsonElement operation)
+    {
+        if (!operation.TryGetProperty("parameters", out JsonElement parameters))
+        {
+            return [];
+        }
+
+        return parameters.EnumerateArray()
+            .Where(p => p.TryGetProperty("in", out JsonElement location)
+                && string.Equals(location.GetString(), "query", StringComparison.Ordinal))
+            .Select(p => p.GetProperty("name").GetString())
+            .OfType<string>()
+            .ToHashSet(StringComparer.Ordinal);
+    }
+
+    // The body is every declared argument that is not routed elsewhere — mirroring exactly how
+    // ErsatzTvApiClient builds the request, so this test cannot disagree with the code it guards.
+    // DELETE is not compared: ErsatzTvApiClient sets hasBody for POST/PUT/PATCH only, so a body
+    // argument on a DELETE tool would be silently dropped. No DELETE tool has one today.
+    private static Dictionary<string, string> DeclaredBodyArguments(ToolDefinition tool)
+    {
+        var pathParameters = Regex.Matches(tool.PathTemplate, @"\{([^}]+)\}")
+            .Select(m => m.Groups[1].Value)
+            .ToHashSet(StringComparer.Ordinal);
+
+        IReadOnlySet<string> queryParameters = tool.QueryParameters ?? new HashSet<string>(StringComparer.Ordinal);
+
+        if (!tool.InputSchema.RootElement.TryGetProperty("properties", out JsonElement properties))
+        {
+            return [];
+        }
+
+        return properties.EnumerateObject()
+            .Where(p => !pathParameters.Contains(p.Name)
+                && !queryParameters.Contains(p.Name)
+                && !string.Equals(p.Name, "ifMatch", StringComparison.Ordinal))
+            .ToDictionary(p => p.Name, p => DeclaredType(p.Value), StringComparer.Ordinal);
+    }
+
+    // The tool schema's own shape: a plain "type", plus the array element type where there is one.
+    private static string DeclaredType(JsonElement property)
+    {
+        string type = property.GetProperty("type").GetString().ShouldNotBeNull();
+
+        return type == "array" && property.TryGetProperty("items", out JsonElement items)
+            ? $"array<{items.GetProperty("type").GetString()}>"
+            : type;
+    }
+
+    private static Dictionary<string, string> RequestBodyProperties(JsonDocument spec, JsonElement operation, string toolName)
+    {
+        // No request body at all (queue/scan POSTs) — the tool must send none either.
+        if (!operation.TryGetProperty("requestBody", out JsonElement requestBody))
+        {
+            return [];
+        }
+
+        JsonElement schema = requestBody
+            .GetProperty("content")
+            .GetProperty("application/json")
+            .GetProperty("schema");
+
+        // Every request body in this document is a plain $ref to a component schema. Anything else
+        // (allOf/inline/oneOf) is a contract shape this guard has not been taught to read, so fail
+        // loudly rather than comparing against an empty set and reporting a false pass.
+        schema.TryGetProperty("$ref", out JsonElement reference)
+            .ShouldBeTrue($"{toolName}: request body schema is not a $ref; teach this test the new shape");
+
+        JsonElement schemas = spec.RootElement.GetProperty("components").GetProperty("schemas");
+        string componentName = reference.GetString().ShouldNotBeNull().Split('/')[^1];
+
+        return schemas
+            .GetProperty(componentName)
+            .GetProperty("properties")
+            .EnumerateObject()
+            .ToDictionary(p => p.Name, p => SpecType(schemas, p.Value, toolName, p.Name), StringComparer.Ordinal);
+    }
+
+    // Normalize the generator's shapes onto the catalog's vocabulary. Two forms appear in this
+    // document: a nullable type as ["null", T] (the catalog has no nullable notion — optionality is
+    // carried by `required`), and a $ref to a component, which for the enum fields is a string enum
+    // and for `logo` is an object.
+    private static string SpecType(JsonElement schemas, JsonElement property, string toolName, string fieldName)
+    {
+        if (property.TryGetProperty("$ref", out JsonElement reference))
+        {
+            string componentName = reference.GetString().ShouldNotBeNull().Split('/')[^1];
+            return SpecType(schemas, schemas.GetProperty(componentName), toolName, fieldName);
+        }
+
+        JsonElement type = property.GetProperty("type");
+
+        string[] types = type.ValueKind == JsonValueKind.Array
+            ? type.EnumerateArray().Select(t => t.GetString()).OfType<string>().Where(t => t != "null").ToArray()
+            : [type.GetString().ShouldNotBeNull()];
+
+        // More than one non-null type is a shape this guard has not been taught to read; fail rather
+        // than picking one and reporting a comparison that means nothing.
+        types.Length.ShouldBe(1, $"{toolName}.{fieldName}: unexpected OpenAPI type union [{string.Join(", ", types)}]");
+
+        // The element schema is resolved through the same normalization: an array's items can itself be
+        // a $ref to a component (ReplaceRemoteLibraryPreferencesRequest.libraries), which the catalog
+        // declares as an object array.
+        return types[0] == "array" && property.TryGetProperty("items", out JsonElement items)
+            ? $"array<{SpecType(schemas, items, toolName, fieldName)}>"
+            : types[0];
+    }
+
+    private static JsonDocument LoadOpenApiDocument()
+    {
+        string path = Path.Combine(AppContext.BaseDirectory, "openapi", "v1.json");
+
+        // A missing spec would make every assertion above vacuous, so it is an explicit failure.
+        File.Exists(path).ShouldBeTrue(
+            $"OpenAPI document not found at {path}; the test project links it from ErsatzTV/wwwroot/openapi/v1.json");
+
+        return JsonDocument.Parse(File.ReadAllText(path));
+    }
 }

ErsatzTV.Mcp/ToolCatalog.cs

@@ -29,14 +29,32 @@ public static class ToolCatalog
         Get("ersatztv_list_schedules", "List schedules.", "/api/v1/schedules"),
         Get("ersatztv_get_schedule", "Get a schedule by id.", "/api/v1/schedules/{id}", IdPath("Schedule id.")),
         Get("ersatztv_get_schedule_items", "Get a schedule's items. Emits the schedule ETag.", "/api/v1/schedules/{id}/items", IdPath("Schedule id.")),
-        Get("ersatztv_list_playouts", "List playouts (paged).", "/api/v1/playouts", [], Page()),
+        Get(
+            "ersatztv_list_playouts",
+            "List playouts (paged), optionally filtered by channel name.",
+            "/api/v1/playouts",
+            [],
+            [
+                Str(
+                    "query",
+                    "Optional case-insensitive substring match on the CHANNEL name (not the playout or schedule name); omit for all playouts.",
+                    arg: In.Query),
+                .. Page()
+            ]),
         Get("ersatztv_get_playout", "Get a playout by id.", "/api/v1/playouts/{id}", IdPath("Playout id.")),
         Get(
             "ersatztv_get_playout_items",
             "Get upcoming items (and unscheduled gaps) for a playout (paged).",
             "/api/v1/playouts/{id}/items",
             [IdPath("Playout id.")],
-            Page()),
+            [
+                Bool(
+                    "showFiller",
+                    "Include items whose filler kind is not None (pre/mid/post-roll, tail, fallback, guide-mode, deco); "
+                    + "default false returns only non-filler items.",
+                    arg: In.Query),
+                .. Page()
+            ]),
         Get("ersatztv_list_ffmpeg_profiles", "List FFmpeg profiles.", "/api/v1/ffmpeg/profiles"),
         Get("ersatztv_get_ffmpeg_profile", "Get an FFmpeg profile by id.", "/api/v1/ffmpeg/profiles/{id}", IdPath("FFmpeg profile id.")),
         Get(
@@ -132,7 +150,8 @@ public static class ToolCatalog
             [Str("name", "Schedule name.", required: true), .. ScheduleFlags()]),
         Put(
             "ersatztv_update_schedule",
-            "Update a program schedule's settings.",
+            "Update a program schedule. Send the full desired state: every field is applied, so omitting "
+            + "padToNearestMinute CLEARS a configured pad (GET the schedule first to copy current values).",
             "/api/v1/schedules/{id}",
             [IdPath("Schedule id."), Str("name", "Schedule name.", required: true), .. ScheduleFlags()]),
         Delete("ersatztv_delete_schedule", "Delete a program schedule.", "/api/v1/schedules/{id}", IdPath("Schedule id.")),
@@ -159,9 +178,22 @@ public static class ToolCatalog
             ChannelFields()),
         Put(
             "ersatztv_update_channel",
-            "Update a channel. Send the full desired state; enum fields take the enum name (GET the channel first to copy current values).",
+            "Update a channel. Send the full desired state; enum fields take the enum name (GET the channel first to copy current values). "
+            + "graphicsElementIds is part of that state: omitting it DETACHES every graphics element (e.g. the On Now/Next overlay), "
+            + "so copy it from ersatztv_get_channel unless you mean to clear it.",
             "/api/v1/channels/{id}",
-            [IdPath("Channel id."), .. ChannelFields()]),
+            [
+                IdPath("Channel id."),
+                .. ChannelFields(),
+
+                // Update-only: UpdateChannelRequest carries GraphicsElementIds, CreateChannelRequest does
+                // not, so this cannot move into the shared ChannelFields() without making create send an
+                // unknown property. PUT is a full replace, so omitting it detaches every attached element
+                // with no error — issue #754.
+                IntArray(
+                    "graphicsElementIds",
+                    "Ids of the graphics elements attached to the channel. Full replace: omit or send [] to detach all.")
+            ]),
         Post(
             "ersatztv_reset_channel_playout",
             "Queue a rebuild of a channel's playout (202 Accepted; 409 if a build is already running).",
@@ -297,7 +329,12 @@ public static class ToolCatalog
         Bool("treatCollectionsAsShows", "Treat collections as shows."),
         Bool("shuffleScheduleItems", "Shuffle schedule items."),
         Bool("randomStartPoint", "Use a random start point."),
-        Str("fixedStartTimeBehavior", "Fixed start-time behavior (enum name; GET a schedule to see valid values).")
+        Str("fixedStartTimeBehavior", "Fixed start-time behavior (enum name; GET a schedule to see valid values)."),
+
+        // Both Create- and UpdateScheduleRequest carry this, so it belongs in the shared helper. The
+        // update PUT is a full replace that writes the value unconditionally, so omitting it used to
+        // clear a configured pad silently — the same #754 shape as channel graphicsElementIds.
+        Int("padToNearestMinute", "Pad each item to the nearest N minutes; omit or send null for no padding.")
     ];
 
     // ---- Tool factories ----

docs/decisions/README.md

@@ -96,6 +96,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera
 | `iptv.logo-drives-bug-preset` | One uploaded channel logo drives both the listing logo and the on-screen bug via a shared, seeded `ChannelLogo`-sourced watermark preset (`Channel Bug`), not new per-channel schema. | 2026-07-20 | [link](records/iptv/logo-drives-bug-preset.md) |
 | `locking.entitylocker-atomic-flags` | `EntityLocker` uses `Interlocked.CompareExchange`-guarded atomic flags plus a documented single-owner-release discipline (no owner tokens/leases); `Unlock*` on an already-unlocked slot returns `false` and logs a Warning rather than throwing. | 2026-07-11 | [link](records/locking/entitylocker-atomic-flags.md) |
 | `mcp.server-foundation` | `ErsatzTV.Mcp` is a fresh stdio JSON-RPC server wrapping frozen `/api/v1` with explicit narrow per-endpoint tools, read-only-by-default enforced at runtime (`ERSATZTV_ALLOW_WRITES`), machine-key auth, and opt-in `If-Match`. | 2026-07-20 | [link](records/mcp/server-foundation.md) |
+| `mcp.tool-schema-openapi-parity` | Every POST/PUT/PATCH tool in `ToolCatalog` declares exactly the request-body properties its endpoint accepts, each with a matching type, and EVERY tool (read and write) declares exactly its endpoint's query parameters, both asserted against the generated `ErsatzTV/wwwroot/openapi/v1.json` (linked into `ErsatzTV.Mcp.Tests`) by `Every_Write_Tool_Should_Declare_Exactly_Its_OpenApi_Request_Body_Fields` and `Every_Tool_Should_Declare_Exactly_Its_OpenApi_Query_Parameters`. A field the endpoint accepts but the tool omits is a DEFECT, not a deferral: on the full-replace tools (channel update, schedule update, custom-order) the omission is silently applied as a clear. The write tools are NOT uniformly full-replace — add-collection-items is additive, and several leave an omitted field unchanged — so each tool description states its own semantics. An omitted query parameter is UNREACHABLE, not merely undocumented, because `ToolArgumentValidator` rejects undeclared arguments. | 2026-08-06 | [link](records/mcp/tool-schema-openapi-parity.md) |
 | `media.lastscan-null-boundary` | A never-scanned `LastScan` surfaces as `null` at the API/MCP boundary, not the `0001-01-01` MinValue sentinel — enforced by an ongoing read-boundary coercion plus a one-time data migration cleanup. | 2026-07-18 | [link](records/media/lastscan-null-boundary.md) |
 | `media.remote-stream-probe` | `ValidatePlayoutItemPath` probes the Plex/Jellyfin/Emby remote-stream URL via `IRemoteStreamProber` before returning it; only a redirected 404 fails closed (`PlayoutItemNotAvailableFromMediaServer`), everything else fails open, and there is no toggle. | 2026-07-19 | [link](records/media/remote-stream-probe.md) |
 | `media.remote-stream-probe-externaljson` | External-JSON playout channels' `StreamRemotely` now probes the remote-stream URL through the same `IRemoteStreamProber` seam as the generated-playout path, closing the #473 scope gap for a channel kind with no DB `PlayoutItem` rows. | 2026-07-20 | [link](records/media/remote-stream-probe-externaljson.md) |

docs/decisions/records/mcp/tool-schema-openapi-parity.md

@@ -0,0 +1,104 @@
+---
+key: mcp.tool-schema-openapi-parity
+title: '2026-08-06 — every MCP tool declares exactly its endpoint''s OpenAPI request-body fields and query parameters, asserted in CI (#754, #757)'
+status: active
+since: '2026-08-06'
+supersedes: none
+superseded-by: none
+rule: 'Every POST/PUT/PATCH tool in `ToolCatalog` declares exactly the request-body properties its endpoint accepts, each with a matching type, and EVERY tool (read and write) declares exactly its endpoint''s query parameters, both asserted against the generated `ErsatzTV/wwwroot/openapi/v1.json` (linked into `ErsatzTV.Mcp.Tests`) by `Every_Write_Tool_Should_Declare_Exactly_Its_OpenApi_Request_Body_Fields` and `Every_Tool_Should_Declare_Exactly_Its_OpenApi_Query_Parameters`. A field the endpoint accepts but the tool omits is a DEFECT, not a deferral: on the full-replace tools (channel update, schedule update, custom-order) the omission is silently applied as a clear. The write tools are NOT uniformly full-replace — add-collection-items is additive, and several leave an omitted field unchanged — so each tool description states its own semantics. An omitted query parameter is UNREACHABLE, not merely undocumented, because `ToolArgumentValidator` rejects undeclared arguments.'
+signals: 'MCP tool schema drift, full-replace write, silently dropped field, graphicsElementIds, padToNearestMinute, additionalProperties false · paths: `ErsatzTV.Mcp/ToolCatalog.cs`, `ErsatzTV.Mcp.Tests/ToolCatalogTests.cs`, `docs/mcp.md` · issues: #754, #757, #58, #616'
+mechanics: '`ErsatzTV.Mcp.Tests/ToolCatalogTests.cs`; `ErsatzTV.Mcp.Tests.csproj` links `openapi/v1.json`'
+---
+
+`ToolCatalog.ChannelFields()` declared 27 of `UpdateChannelRequest`'s 28 properties. The missing one
+was `graphicsElementIds`, which attaches channel-level graphics elements including the built-in On
+Now/Next overlay (`graphics.channel-level-attachment`).
+
+The cost was not "one field you cannot set". `PUT /api/v1/channels/{id}` is a **full replace**, and
+the tool's own description instructs the caller to *"send the full desired state"* — which the schema
+could not express. An agent that faithfully GET-edit-PUT a channel detached every attached graphics
+element, with a `200` and no error. Nothing surfaced until the overlay stopped rendering at the next
+transition, hours later. That is the `optional-parameter-on-shared-primitive-is-opt-out` shape: the
+omission is invisible at the call site and only observable as missing pixels.
+
+Fixing the one field would have left the mechanism intact, and the mechanism had already produced a
+second instance: `ScheduleFlags()` omitted `padToNearestMinute`, which both `CreateScheduleRequest`
+and `UpdateScheduleRequest` carry and `UpdateProgramScheduleHandler` writes unconditionally — so
+`ersatztv_update_schedule` silently cleared a configured pad the same way. Nothing tied a tool's
+declared arguments to the contract it wraps, so the next added DTO property would have drifted too.
+
+So the guard is the decision, and it is asserted against the **generated OpenAPI document** rather
+than the DTO types: `v1.json` is the actual wire contract, it is already regenerated by
+`scripts/update-openapi.sh` as part of the API checklist, and asserting against it keeps
+`ErsatzTV.Mcp.Tests` free of a project reference to the whole ASP.NET host. The test derives each
+tool's body set exactly as `ErsatzTvApiClient` does — declared arguments minus path parameters, minus
+query parameters, minus the reserved `ifMatch` header — so the guard cannot disagree with the routing
+it guards.
+
+Three anti-vacuity properties are deliberate, per the repo's standing "a test that filters on the
+property it asserts cannot see what is missing" rule:
+
+- The **covered write-tool set is pinned by name**, not merely filtered. A tool that stops being a
+  write verb, or a new one that is added, changes this list rather than silently leaving the loop.
+- A **missing or unrecognised spec is a failure**, never an empty comparison: an absent `v1.json`
+  fails with the path it looked in, and a request body that is not a plain `$ref` (an `allOf`,
+  `oneOf`, or inline schema), or a property whose type is a union this guard has not been taught,
+  fails asking to be taught the shape instead of comparing against `{}`.
+- **Names are compared with types**, not alone. A name-only guard is the same defect one level down:
+  the tool would advertise `string` for an `int?`, the agent would send `"30"`, and the API would
+  reject it — green test, broken tool. The generator's `["null", T]` nullable form and its `$ref`
+  (enum → `string`, model → `object`) are normalized onto the catalog's vocabulary, arrays down to
+  their element type.
+
+All were verified by mutation rather than assumed: dropping `graphicsElementIds`, dropping
+`padToNearestMinute`, retyping either field, drifting an array's element type, and removing the
+copied spec each turn the suite red, and each failure names the field or path at fault.
+
+**Query parameters are guarded the same way, across every tool (#757).** A second test compares each
+tool's routed `QueryParameters` against the spec's `parameters[in=query]` for its path and verb, reads
+included — the drift that existed when this was written was entirely on reads. An omitted parameter
+there is worse than an undeclared body field: `additionalProperties:false` means the caller cannot
+pass it *at all*, so the capability is unreachable rather than merely undocumented (`ersatztv_list_playouts`
+had lost its channel-name `query` filter and `ersatztv_get_playout_items` its `showFiller`; #616 was
+the same shape with paging). That test **accumulates** its mismatches and asserts once, so a run
+reports the whole drift set — failing on the first would invite fixing one tool at a time, which is
+how the twin in this very issue stayed hidden.
+
+It also **composes with** the older `Every_Query_Parameter_Should_Be_A_Declared_Property`, and the pair
+is the clearest illustration in this repo of why "a test that filters on the property it asserts cannot
+see what is missing" is a rule. That older test filters `Where(t => t.QueryParameters is { Count: > 0 })`
+— so a tool that lost its query parameters entirely escaped it, which is exactly how `list_playouts` and
+`get_playout_items` hid. The new test has no filter and reports them as *unreachable*; the old one then
+checks that a routed parameter is also a declared argument. Neither subsumes the other, and the inner
+duplicate of the old check was deliberately removed from the new test rather than kept as a second copy.
+
+**Scope, stated so it is not mistaken for more.** Request bodies are compared for POST/PUT/PATCH only.
+DELETE is uncovered because `ErsatzTvApiClient` builds a body for POST/PUT/PATCH only, so a body
+argument on a DELETE tool would be silently dropped; no tool has one today. Header arguments (`ifMatch`)
+and per-parameter *descriptions* are not compared either — `api.paging-zero-based` is pinned by its own
+test.
+
+The type comparison is **lossy by design, at the catalog's ceiling**: the catalog's vocabulary is
+`{string, integer, number, boolean, object, array<T>}`, so every object component collapses to `object`
+and every enum to `string`. Swapping one model or enum for another is therefore invisible here
+(verified by repointing `logo` at a structurally unrelated model — the suite stays green), as is
+`format` (`int32` vs `int64`). That is the right ceiling rather than a gap to close: comparing deeper
+than the catalog can express would assert a distinction no tool schema carries, and an opaque object
+like `logo` is copied through from a GET verbatim, so nested drift cannot cause the silent-clear this
+record exists to prevent. `integer` vs `number` IS distinguished. The `>1` non-null type-union
+assertion is a fail-loud guard for a shape this generator does not currently emit, so it is deliberate
+but **unexercised**.
+
+The guard is also a **two-job conjunction**, not self-contained: it compares against a checked-in
+`v1.json`, so it is only as fresh as the regeneration. What keeps it honest is the `api-docs` CI job,
+whose `^ErsatzTV/Controllers/Api/` path filter covers the directory every request DTO lives in — a
+new DTO property cannot leave `v1.json` stale without that job going red. That holds for a DTO's OWN
+properties and no further: a NESTED model such as `ArtworkContentTypeModel` lives in
+`ErsatzTV.Application/Artworks/`, outside that filter, so changing it can leave `v1.json` stale without
+the job firing. Pre-existing, and harmless to this guard only because nested shape is not compared.
+
+`graphicsElementIds` is declared on the **update tool only**, not in the shared `ChannelFields()`:
+`CreateChannelRequest` has no such property, and the tool schemas are `additionalProperties:false`,
+so sharing it would make every create call send an unknown property. `padToNearestMinute` is on both
+schedule requests, so it does belong in the shared `ScheduleFlags()`. The parity test is what makes
+that per-field placement checkable rather than a matter of care.

docs/mcp.md

@@ -186,12 +186,50 @@ Re-adding an already-present item is an **idempotent no-op** (no duplicate rows,
 referenced id does not exist the whole batch is rejected (`422`). So the flow is: search → add ids →
 re-run to confirm idempotence.
 
+### Full-replace writes drop what you omit (`mcp.tool-schema-openapi-parity`)
+
+**Check each tool's own description — the write tools are not uniform, and one is not uniform with
+itself.** Three are full replaces, where a field you leave out is not "left unchanged" but written as
+empty: `ersatztv_update_channel`, `ersatztv_update_schedule`, `ersatztv_update_collection_custom_order`.
+
+`ersatztv_update_playout` is **mixed, and this is the easy one to get wrong**: `scheduleFile` is
+leave-unchanged, but `dailyRebuildTime` is always applied — `UpdatePlayoutHandler` sets it to `null`
+unconditionally before re-applying a supplied value, so calling this tool to set `scheduleFile` while
+omitting `dailyRebuildTime` **silently clears the daily reset**. Send both, or neither.
+
+The rest are additive or leave-unchanged and say so: `ersatztv_add_collection_items` is an idempotent
+add (it does **not** replace membership), `ersatztv_update_collection` leaves an omitted
+`useCustomPlaybackOrder` alone, and `ersatztv_enable_jellyfin_library_sync` leaves an absent row
+untouched.
+
+For the full-replace ones, the GET → edit one field → PUT flow is only safe if the tool can express
+the whole state, and `ersatztv_update_channel` could not — it omitted `graphicsElementIds`, so that flow
+silently detached every graphics element (including the On Now/Next overlay) with a `200` and no
+error, visible only as missing pixels at the next transition. `ersatztv_update_schedule` cleared
+`padToNearestMinute` the same way (ersatztv#754).
+
+Both are fixed, and the class is now guarded by two tests in `ToolCatalogTests`, comparing against the
+generated `ErsatzTV/wwwroot/openapi/v1.json`:
+
+- Every POST/PUT/PATCH tool declares **exactly** the request-body fields its endpoint accepts, each
+  with a matching type. A new property on a request DTO fails until the catalog declares it.
+- Every tool — read **and** write — declares **exactly** its endpoint's query parameters. An omitted
+  one is not merely undocumented but *unreachable*, since `ToolArgumentValidator` rejects undeclared
+  arguments; that is how #616 hard-capped two paged tools at the first page, and how
+  `ersatztv_list_playouts` (`query`) and `ersatztv_get_playout_items` (`showFiller`) lost their
+  filters until ersatztv#757.
+
+When adding a write tool, regenerate the spec (`./scripts/update-openapi.sh`) and add the tool to the
+pinned list in the body test.
+
 ## Deferred
 
-Channel create/update (`ersatztv_create_channel` / `ersatztv_update_channel`) wrap a 28-field DTO with
-nine enum fields. Only `name`/`number`/`ffmpegProfileId` are required; the rest have server-side
+Channel create/update (`ersatztv_create_channel` / `ersatztv_update_channel`) wrap a large DTO with
+nine enum fields — 27 body fields on create, and 28 on update, which additionally carries
+`graphicsElementIds`. Only `name`/`number`/`ffmpegProfileId` are required; the rest have server-side
 defaults, and the enum fields take the enum **name** (the API validates them). Discover an existing
-channel's shape and current enum values with `ersatztv_get_channel` before creating/updating.
+channel's shape and current enum values with `ersatztv_get_channel` before creating/updating — and
+copy its `graphicsElementIds` through unless you mean to detach them.
 
 Deliberately **not** exposed in this cautious first write pass: