- 
                Notifications
    You must be signed in to change notification settings 
- Fork 748
improvement: mcp server ui, auth, and rules #6659
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| The latest updates on your projects. Learn more about Vercel for GitHub. 
 | 
| ) | ||
| # Add to Claude code | ||
| print_tabbed( | ||
| f"{_utf8('➜')} {green('Add to Claude Code')}: claude mcp add --transport http marimo {mcp_url}" | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we also include 'add to Cursor'? Since the most likely use cases for this are Claude Code and Cursor. It can be something like:
print_tabbed(
    f"{_utf8('➜')} {green('Add to Cursor')}: Add the following to ~/.cursor/mcp.json:\n"
    f"""  {{
    "mcpServers": {{
      "marimo": {{
        "url": "{mcp_url}"
      }}
    }}
  }}"""
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that seems a bit verbose for the CLI. we could look into making the CLI more interface in the future (e.g. press i for more info)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that makes sense
| source_url: str = MARIMO_RULES_URL | ||
|  | ||
|  | ||
| class GetMarimoRules(ToolBase[EmptyArgs, GetMarimoRulesOutput]): | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really good! Maybe we should add this to the next_steps= suggestion for some of the other tools?
My suggestions would be:
- GetNotebookErrors()if there are any notebook specific or marimo specific errors
- GetLightweightCellMap()and- GetActiveNotebooks()because these are entry points.
- GetDatabaseTables()because the rules contain some SQL-specific guidelines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe we can do this in a followup. it hard to tell why an AI would ask for rules so im not sure if the next steps would veer it off track
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok sounds good. I'll keep this incase we decide to update it later. We can see first if the Chat LLMs either don't pull rules initially or pull it early on in the conversation then forget it.
| Also not sure if this is intentional or not but since all the other tools have tests, might be nice to include some tests too for  | 
| mcp_app = mcp.streamable_http_app() | ||
|  | ||
| # Middleware to require edit scope | ||
| class RequiresEditMiddleware(BaseHTTPMiddleware): | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RequiresEditMiddleware() doesn't seem to be mcp specific at all. Maybe it would be better to move this logic to marimo/_server/api/middleware.py?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is actually just for MCP. we already support auth on other endpoints with @requires
Uh oh!
There was an error while loading. Please reload this page.