fix(GEN-4808): satisfy the Rails destructive-delete gate from MCP - #28
Merged
Merged
Conversation
Rails (GEN-4797) gates PAT-authenticated destructive Vidsheet deletes behind a two-phase confirm: the first DELETE returns 428 with an authoritative would_destroy preview and a single-use, target-bound confirm_token. mcp.gen.pro sends the caller's PAT, so it is in the gated class by design — but 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 has therefore been failing in production. Fail-closed (nothing was destroyed), but the caller got an unactionable error. - client: GenConfirmationRequired carries would_destroy + confirm_token; _call raises it on 428 instead of the generic error. A 428 with no usable token still fails closed. - client: gated_delete() returns the preview so the model can show the user what would be deleted, then re-call with the token. It deliberately does NOT auto-confirm — Rails stays the authority for target drift, expiry and one-shot consumption, and a tool must never manufacture a user's approval. - server: gen_delete_column / gen_delete_layer / gen_delete_variable take an optional confirm_token and route through gated_delete. Mirrors the proven handler in gen-agentic rails_client._delete_with_destructive_confirmation. Verified: 149 tools load; 4/4 new tests pass; removing the confirm_token parameter makes test_delete_tools_accept_confirm_token fail (red-then-green). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Rails (GEN-4797) gates PAT-authenticated destructive Vidsheet deletes behind a two-phase confirm: the first
DELETEreturns 428 with an authoritativewould_destroypreview and a single-use, target-boundconfirm_token.mcp.gen.prosends the caller's PAT (client.py→X-API-Key), so it is in the gated class by design. But no delete tool accepted a token, andclient._callcollapsed the 428 into an opaqueGenApiError.Every MCP column/layer/variable delete against a populated target has been failing in production. Fail-closed — nothing was destroyed — but the caller got an unactionable error with no path forward.
Fix
GenConfirmationRequiredcarrieswould_destroy+confirm_token;_callraises it on 428 instead of the generic error. A 428 with no usable token still fails closed.gated_delete()returns the preview so the model can show the user exactly what would be deleted, then re-call with the token.gen_delete_column/gen_delete_layer/gen_delete_variabletake an optionalconfirm_token.Deliberately does NOT auto-confirm. Rails stays the authority for target drift, expiry and one-shot consumption; an MCP tool must never manufacture a user's approval. Mirrors the proven handler in gen-agentic
rails_client._delete_with_destructive_confirmation.Verification
mcp.list_tools()), all three exposeconfirm_tokenconfirm_tokenparameter makestest_delete_tools_accept_confirm_tokenfailFollow-up
GEN-4808 also flags the TypeScript SDK's
deleteColumn/deleteLayer/deleteVariable— not in this repo, tracked separately.🤖 Generated with Claude Code