fix(GEN-4808): satisfy the Rails destructive-delete gate from MCP

src/gen_mcp_server/client.py

@@ -38,6 +38,31 @@ class GenApiError(Exception):
     """Raised on a non-2xx GEN API response. Message mirrors the TS server."""
 
 
+class GenConfirmationRequired(Exception):
+    """Rails returned 428 for a destructive Vidsheet operation (GEN-4797).
+
+    Rails gates PAT-authenticated destructive Vidsheet deletes behind a
+    two-phase confirm: the first DELETE returns 428 with an authoritative
+    ``would_destroy`` preview and a short-lived, single-use ``confirm_token``
+    bound to (operation, target-state fingerprint, acting user + agent).
+
+    MCP callers are interactive humans holding a PAT, so they are *in* the
+    gated class by design. This exception carries the preview back to the
+    caller instead of collapsing it into an opaque ``GenApiError``, so the
+    model can show what would be destroyed and re-call with ``confirm_token``.
+
+    We deliberately do NOT auto-confirm: Rails is the authority for target
+    drift, expiry and one-shot consumption, and an MCP tool must never
+    manufacture a user's approval.
+    """
+
+    def __init__(self, would_destroy: Any, confirm_token: str, body: Any) -> None:
+        self.would_destroy = would_destroy
+        self.confirm_token = confirm_token
+        self.body = body
+        super().__init__("confirmation_required")
+
+
 def _pat_from_http_request() -> str | None:
     """In hosted (HTTP) mode, read the caller's PAT from the request headers.
 
@@ -98,6 +123,18 @@ async def _call(
         data: Any = json.loads(text)
     except json.JSONDecodeError:
         data = {"raw": text}
+    if res.status_code == httpx.codes.PRECONDITION_REQUIRED:
+        # GEN-4808: Rails' two-phase destructive confirm. Surface the preview
+        # + token rather than the generic error, which is unactionable and made
+        # every gated MCP delete look like an opaque server failure.
+        token = data.get("confirm_token") if isinstance(data, dict) else None
+        if isinstance(token, str) and token:
+            raise GenConfirmationRequired(
+                would_destroy=data.get("would_destroy") if isinstance(data, dict) else None,
+                confirm_token=token,
+                body=data,
+            )
+        # A 428 without a usable token is a real error — fail closed.
     if not res.is_success:
         # Same shape as TS: "API error <status>: <json>"
         raise GenApiError(f"API error {res.status_code}: {json.dumps(data)}")
@@ -161,3 +198,33 @@ async def integration_api_call(
 def json_result(data: Any) -> str:
     """FastMCP returns the string as the tool's text content (mirrors jsonResult)."""
     return json.dumps(data, indent=2)
+
+
+async def gated_delete(path: str, *, confirm_token: str | None = None) -> str:
+    """DELETE a destructive Vidsheet resource through Rails' two-phase gate.
+
+    GEN-4808. Without ``confirm_token`` Rails answers 428 with the
+    authoritative ``would_destroy`` preview; we return that preview so the
+    caller can show the user exactly what would be deleted and then re-call
+    the same tool with the returned ``confirm_token``.
+
+    The token is minted and validated by Rails and is single-use, expiring,
+    and bound to the target's state fingerprint — so a stale preview fails
+    closed rather than deleting something the user never saw.
+    """
+    try:
+        data = await api_call("DELETE", path)
+    except GenConfirmationRequired as gate:
+        return json_result(
+            {
+                "status": "confirmation_required",
+                "would_destroy": gate.would_destroy,
+                "confirm_token": gate.confirm_token,
+                "next_step": (
+                    "Show the user what would_destroy lists. If they confirm, call this "
+                    "same tool again with confirm_token set to the value above. The token "
+                    "is single-use and expires; do not reuse or cache it."
+                ),
+            }
+        )
+    return json_result(data)

src/gen_mcp_server/server.py

@@ -9,7 +9,7 @@
 from fastmcp import FastMCP
 from pydantic import Field
 
-from .client import api_call, agent_api_call, agent_core_api_call, form_call, integration_api_call, json_result
+from .client import api_call, agent_api_call, agent_core_api_call, form_call, gated_delete, integration_api_call, json_result
 from .generation_types import resolve_generation_type
 from .reference import API_REFERENCE
 from .vidsheet_contract import enum_values, model_facing_fields, require_enum_value
@@ -825,14 +825,17 @@ async def gen_reorder_columns(
     )
     return json_result(data)
 
-@mcp.tool(name="gen_delete_column", description="Step 4 (Edit & Generate): Delete a column from a vidsheet. Only ingredient-role columns can be deleted.")
+@mcp.tool(name="gen_delete_column", description="Step 4 (Edit & Generate): Delete a column from a vidsheet. Only ingredient-role columns can be deleted. Destructive: the first call returns a would_destroy preview plus a confirm_token; show the preview to the user, then call again with confirm_token to actually delete.")
 async def gen_delete_column(
     engine_id: Annotated[str, Field(description="The engine ID")],
     column_id: Annotated[str, Field(description="The column ID to delete")],
     agent_id: Annotated[str, Field(description="The agent ID that owns the engine")],
+    confirm_token: Annotated[str | None, Field(description="Single-use token from this tool's previous confirmation_required response. Omit on the first call.")] = None,
 ) -> str:
-    data = await api_call("DELETE", f"/vidsheet/{engine_id}/columns/{column_id}?agent_id={agent_id}")
-    return json_result(data)
+    path = f"/vidsheet/{engine_id}/columns/{column_id}?agent_id={agent_id}"
+    if confirm_token:
+        path = f"{path}&confirm_token={confirm_token}"
+    return await gated_delete(path)
 
 @mcp.tool(name="gen_list_rows", description="Step 4 (Edit & Generate): List all rows in a vidsheet. A row is one piece of content; cells across its columns are its ingredients and generated outputs.")
 async def gen_list_rows(
@@ -944,15 +947,18 @@ async def gen_reorder_layers(
     )
     return json_result(data)
 
-@mcp.tool(name="gen_delete_layer", description="Step 4 (Edit & Generate): Delete a layer from a cell.")
+@mcp.tool(name="gen_delete_layer", description="Step 4 (Edit & Generate): Delete a layer from a cell. Destructive: the first call returns a would_destroy preview plus a confirm_token; show the preview to the user, then call again with confirm_token to actually delete.")
 async def gen_delete_layer(
     engine_id: Annotated[str, Field(description="The engine ID")],
     cell_id: Annotated[str, Field(description="The cell ID")],
     layer_id: Annotated[str, Field(description="The layer ID to delete")],
     agent_id: Annotated[str, Field(description="The agent ID that owns the engine")],
+    confirm_token: Annotated[str | None, Field(description="Single-use token from this tool's previous confirmation_required response. Omit on the first call.")] = None,
 ) -> str:
-    data = await api_call("DELETE", f"/vidsheet/{engine_id}/cells/{cell_id}/layers/{layer_id}?agent_id={agent_id}")
-    return json_result(data)
+    path = f"/vidsheet/{engine_id}/cells/{cell_id}/layers/{layer_id}?agent_id={agent_id}"
+    if confirm_token:
+        path = f"{path}&confirm_token={confirm_token}"
+    return await gated_delete(path)
 
 @mcp.tool(name="gen_list_variables", description="Step 4 (Edit & Generate): Get global variables for a vidsheet. Variables are key-value pairs used for template substitution in prompts and content (e.g. {{brand_name}}).")
 async def gen_list_variables(
@@ -1872,14 +1878,17 @@ async def gen_update_variable(
     data = await api_call("PATCH", f"/vidsheet/{engine_id}/global_variables/{variable_id}?agent_id={agent_id}", body)
     return json_result(data)
 
-@mcp.tool(name="gen_delete_variable", description="Step 4 (Edit & Generate): Delete a global variable from a vidsheet.")
+@mcp.tool(name="gen_delete_variable", description="Step 4 (Edit & Generate): Delete a global variable from a vidsheet. Destructive: the first call returns a would_destroy preview plus a confirm_token; show the preview to the user, then call again with confirm_token to actually delete.")
 async def gen_delete_variable(
     engine_id: Annotated[str, Field(description="The vidsheet/engine ID")],
     variable_id: Annotated[str, Field(description="The variable ID to delete")],
     agent_id: Annotated[str, Field(description="The agent ID that owns the engine")],
+    confirm_token: Annotated[str | None, Field(description="Single-use token from this tool's previous confirmation_required response. Omit on the first call.")] = None,
 ) -> str:
-    data = await api_call("DELETE", f"/vidsheet/{engine_id}/global_variables/{variable_id}?agent_id={agent_id}")
-    return json_result(data)
+    path = f"/vidsheet/{engine_id}/global_variables/{variable_id}?agent_id={agent_id}"
+    if confirm_token:
+        path = f"{path}&confirm_token={confirm_token}"
+    return await gated_delete(path)
 
 # ─── Vidsheet undo / redo (change-set history) ───────────────────────────────
 # Backs the undo/redo surface: every grouped write to a vidsheet is

tests/test_destructive_confirm.py

@@ -0,0 +1,102 @@
+"""GEN-4808: MCP delete tools must satisfy Rails' two-phase destructive gate.
+
+Rails (GEN-4797) gates PAT-authenticated destructive Vidsheet deletes: the
+first DELETE returns 428 with an authoritative ``would_destroy`` preview and a
+single-use ``confirm_token``. mcp.gen.pro sends the caller's PAT, so it is in
+the gated class by design — but before this fix no delete tool accepted a
+token and ``client._call`` collapsed the 428 into an opaque ``GenApiError``.
+Every MCP column/layer/variable delete against a populated target therefore
+failed in production (fail-closed: nothing was destroyed, but nothing worked).
+
+These tests fail on a tree without the fix:
+- no GenConfirmationRequired / gated_delete export
+- the 428 body is swallowed, so would_destroy + confirm_token never reach the caller
+- the delete tools expose no confirm_token parameter
+"""
+from __future__ import annotations
+
+import asyncio
+import json
+
+import httpx
+import pytest
+
+from gen_mcp_server import client as client_mod
+from gen_mcp_server.client import (
+    GenApiError,
+    GenConfirmationRequired,
+    gated_delete,
+)
+
+PREVIEW = {
+    "would_destroy": {"video_layers": [{"id": 42, "name": "Old Logo"}]},
+    "confirm_token": "tok_abc123",
+}
+
+
+def _stub_call(monkeypatch, *, status: int, body: dict) -> list[str]:
+    """Replace api_call with one that mimics client._call's status handling."""
+    seen: list[str] = []
+
+    async def fake_api_call(method: str, path: str, body_arg=None):
+        seen.append(path)
+        if status == httpx.codes.PRECONDITION_REQUIRED:
+            token = body.get("confirm_token")
+            if isinstance(token, str) and token:
+                raise GenConfirmationRequired(
+                    would_destroy=body.get("would_destroy"),
+                    confirm_token=token,
+                    body=body,
+                )
+            raise GenApiError(f"API error {status}: {json.dumps(body)}")
+        return body
+
+    monkeypatch.setattr(client_mod, "api_call", fake_api_call)
+    return seen
+
+
+def test_gated_delete_surfaces_preview_instead_of_opaque_error(monkeypatch):
+    """A 428 must return the preview + token, not raise GenApiError."""
+    _stub_call(monkeypatch, status=httpx.codes.PRECONDITION_REQUIRED, body=PREVIEW)
+
+    result = json.loads(asyncio.run(gated_delete("/vidsheet/1/columns/2?agent_id=a")))
+
+    assert result["status"] == "confirmation_required"
+    assert result["confirm_token"] == "tok_abc123"
+    assert result["would_destroy"] == PREVIEW["would_destroy"]
+    # The model must be told what to do next, or it will retry blindly.
+    assert "confirm_token" in result["next_step"]
+
+
+def test_gated_delete_passes_through_a_successful_delete(monkeypatch):
+    """With a valid token Rails 2xxes; the tool returns the real body."""
+    _stub_call(monkeypatch, status=200, body={"deleted": True})
+
+    result = json.loads(
+        asyncio.run(gated_delete("/vidsheet/1/columns/2?agent_id=a&confirm_token=tok_abc123"))
+    )
+
+    assert result == {"deleted": True}
+
+
+def test_428_without_a_token_fails_closed(monkeypatch):
+    """A malformed preview must NOT be treated as approval."""
+    _stub_call(
+        monkeypatch,
+        status=httpx.codes.PRECONDITION_REQUIRED,
+        body={"would_destroy": {"video_layers": []}},  # no confirm_token
+    )
+
+    with pytest.raises(GenApiError):
+        asyncio.run(gated_delete("/vidsheet/1/columns/2?agent_id=a"))
+
+
+def test_delete_tools_accept_confirm_token():
+    """All three gated delete tools must expose the token parameter."""
+    import gen_mcp_server.server as server
+
+    tools = {t.name: t for t in asyncio.run(server.mcp.list_tools())}
+    for name in ("gen_delete_column", "gen_delete_layer", "gen_delete_variable"):
+        assert name in tools, f"{name} missing from the tool registry"
+        params = (getattr(tools[name], "parameters", None) or {}).get("properties", {})
+        assert "confirm_token" in params, f"{name} cannot satisfy the Rails 428 gate"