Skip to content

Conversation

@Rodriguespn
Copy link
Contributor

@Rodriguespn Rodriguespn commented Oct 27, 2025

Summary

Implements MCP elicitation for the apply_migration tool to request user confirmation before applying database migrations.

Implementation

Uses a try-catch pattern to gracefully fallback when clients don't support elicitation:

  • Supporting clients: Show confirmation dialog with migration details and SQL preview
  • Non-supporting clients: Proceed with migration without confirmation (existing behavior)

Client Support

Currently supported: VS Code Copilot, Cursor, Postman, VT Code, fast-agent, mcp-use, MCPJam

Not yet supported: Claude Desktop, Claude.ai, GitHub Copilot coding agent

Related

AI-142: Explore MCP Elicitations

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds user confirmation prompts for database migrations using MCP's elicitation feature. When applying migrations, users will now be presented with the SQL to review and must explicitly confirm before the migration executes.

Key Changes:

  • Adds elicitation/create request to prompt users for confirmation before applying migrations
  • Passes server instance to database tools to enable elicitation requests
  • Maintains backwards compatibility by proceeding without confirmation if elicitation is not supported

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/mcp-server-supabase/src/tools/database-operation-tools.ts Implements elicitation request for user confirmation with SQL review before migration execution
packages/mcp-server-supabase/src/server.ts Passes server instance to getDatabaseTools to enable elicitation functionality

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

},
},
},
// @ts-ignore - elicitation types might not be available
Copy link

Copilot AI Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using @ts-ignore suppresses all type checking for the next line. Consider using @ts-expect-error instead, which will fail if the error no longer exists, helping track when types become available.

Suggested change
// @ts-ignore - elicitation types might not be available
// @ts-expect-error - elicitation types might not be available

Copilot uses AI. Check for mistakes.
@Rodriguespn
Copy link
Contributor Author

Rodriguespn commented Oct 27, 2025

@mattrossman I've implemented elicitation on apply_migration as a POC. Would love to discuss the implementation and testing approach before rolling it out to the other high-priority tools (execute_sql, merge_branch, etc.).

@Rodriguespn Rodriguespn force-pushed the pedrorodrigues/ai-142-explore-mcp-elicitations branch from 378ec95 to 9d0ff02 Compare October 30, 2025 17:48
@Rodriguespn Rodriguespn changed the title feat: Add MCP elicitation for apply_migration tool feat: Add MCP elicitation for create project and branch tools Oct 30, 2025
@Rodriguespn
Copy link
Contributor Author

@mattrossman Added elicitation request to create_project and create_branch tools and hide the confirm_cost tool when the client supports elicitations.

You can check that the tool confirm_cost is not listed on a client that supports elicitation (VS Code) and is there for Claude Code (doesn't support elicitation.

VS Code Copilot (Supports Elicitation)

image

Claude Code (doesn't support elicitation)

image

export function getAccountTools({
account,
readOnly,
server,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Think this is unused now that it's accessing the server via ToolExecuteContext

.enum(AWS_REGION_CODES)
.describe('The region to create the project in.'),
organization_id: z.string(),
confirm_cost_id: z
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parameter is what forces the agent to confirm cost prior to running create_project, I think we'd still need this at least for clients who don't support elicitation

Rodriguespn and others added 5 commits October 31, 2025 17:55
- Implemented user confirmation dialog for apply_migration using MCP elicitation protocol
- Added graceful fallback for clients that don't support elicitation
- Tests pass with fallback behavior in test environment
- Maintains backwards compatibility with all MCP clients
@Rodriguespn Rodriguespn force-pushed the pedrorodrigues/ai-142-explore-mcp-elicitations branch from ebc5843 to b739cfe Compare October 31, 2025 17:55
Copy link
Collaborator

@gregnr gregnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since elicitations are a major new feature, maybe it'd be best to implement them in just one place first and iterate in future PRs? e.g. this PR could only focus on apply_migration since that is relatively straightforward.

'Gets the cost of creating a new project or branch. Never assume organization as costs can be different for each.',
get_and_confirm_cost: tool({
description: async () => {
const clientCapabilities = server?.getClientCapabilities();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getClientCapabilities() will not be reliable in our stateless http setup. Capabilities are sent on the init message, which will be lost on future messages. @mattrossman is capturing this in the session ID JWT though - do you have have thoughts on how we can inject this back into the server instance every request?

get_cost: tool({
description:
'Gets the cost of creating a new project or branch. Never assume organization as costs can be different for each.',
get_and_confirm_cost: tool({
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally thinking:

  • If elicitations are supported, hide get_cost and confirm_cost tools, and run the elicitation directly on create_project and create_branch
  • If elicitations are not supported, keep both get_cost and confirm_cost as is

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice having the elicitation in create_project felt odd to me because you have to approve a create_project tool call before knowing how much the project will cost or that a safety net will follow (Slack context).

Comment on lines +225 to +245
const result = await server.request(
{
method: 'elicitation/create',
params: {
message: `You are about to apply migration "${name}" to project ${project_id}. This will modify your database schema.\n\nPlease review the SQL:\n\n${query}\n\nDo you want to proceed?`,
requestedSchema: {
type: 'object',
properties: {
confirm: {
type: 'boolean',
title: 'Confirm Migration',
description:
'I have reviewed the SQL and approve this migration',
},
},
required: ['confirm'],
},
},
},
ElicitResultSchema
);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to be careful about code flow with elicitations on stateless HTTP. Stateless HTTP is simulating bidirectional communication via requests and responses, but each new request has a brand new state so we need to make sure our mental model matches this in our implementations.

IIUC, elicitations are sent back via HTTP response? If yes, the result of the elicitation will be sent as a brand new request to the server and will never land back at this point in the code flow. Perhaps the best way to confirm this is to develop/test against a remote MCP server instead of stdio.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment suggests elicitations don't work with stateless :/

I'd think it run both requests in parallel (the outer tool call and the inner elicitation) but I guess keeping the tool call long-running w/o streaming isn't great.

@mattrossman
Copy link
Contributor

Tested in Cursor and Claude Code, the flow feels good in both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants