console: MCP Server connection instructions in the console#35902
console: MCP Server connection instructions in the console#35902leedqin merged 1 commit intoMaterializeInc:mainfrom
Conversation
|
Thanks for opening this PR! Here are a few tips to help make the review process smooth for everyone. PR title guidelines
Pre-merge checklist
|
jubrad
left a comment
There was a problem hiding this comment.
If MCP is not enabled by default should we not show these by default, or at least add a disclaimer about how to enable or to contact support to enable?
| lineHeight="1.15" | ||
| > | ||
| Service | ||
| Services and agents |
There was a problem hiding this comment.
If this is a title both should be capitalized
Services and Agents
| after you reload! | ||
| <Text pt={2} textStyle="text-base" color={colors.foreground.primary}> | ||
| Write this down; you will not be able to see your app password or | ||
| token again after you reload! |
There was a problem hiding this comment.
token again after you reload
I believe this is just after you close this modal, but this is probably fine.
| > | ||
| Copy this app password to somewhere safe. App passwords cannot be | ||
| displayed after initial creation. | ||
| Copy these to somewhere safe. They cannot be displayed after initial |
There was a problem hiding this comment.
Either way what we're displaying is an app password, just formatting changes
I think we're ok with
Copy this somewhere safe. App passwords cannot be displayed after initial creation.
|
|
||
| const mcpConfigJson = ( | ||
| baseUrl: string, | ||
| endpoint: "agents" | "observatory", |
There was a problem hiding this comment.
This name might be changing to developer along with the endpoint
|
Resolved some feedback from Slack and edits here in the latest commit |
jubrad
left a comment
There was a problem hiding this comment.
I actually think this is pretty unintuitive... you click click "create app password" at the bottom of the modal and the thing that gets added there is the app_password, but the mcp token gets added to the instructions and it's very easy to miss.
I think there's a more serious error around service accounts not being handled correctly.
| ); | ||
| }; | ||
|
|
||
| export default McpConnectModal; |
| }: SecretBoxProps) => { | ||
| const { colors } = useTheme<MaterializeTheme>(); | ||
|
|
||
| const base64Token = btoa(`${userEmail}:${password}`); |
|
Sorry about the accidental close, hit the wrong button. |
| overflow="visible" | ||
| message={ | ||
| isCloud | ||
| ? "The MCP server is not enabled by default. Contact Materialize support to enable the MCP endpoints for your environment." |
There was a problem hiding this comment.
If launching in Public Preview (https://materializeinc.slack.com/archives/C09ERRA0YAK/p1775590203042079) ... will it not be enabled by default?
There was a problem hiding this comment.
For some self managed customers, they might still need to enable it via mz_system
There was a problem hiding this comment.
Depending on the answer to your question https://materializeinc.slack.com/archives/C09ERRA0YAK/p1775773024131799?thread_ts=1775758256.275809&cid=C09ERRA0YAK ... possibly may need to change.
|
|
||
| <VStack alignItems="stretch" spacing="2"> | ||
| <Text textStyle="heading-xs">Step 2. Connect your client</Text> | ||
| <Text fontSize="sm" color={colors.foreground.secondary}> |
There was a problem hiding this comment.
I'm wondering ... would having a radio button group that specifies
- Agent endpoint
- Developer endpoint
be easier for our users to copy/paste?
That way, we don't need to have https://github.com/MaterializeInc/materialize/pull/35902/changes#diff-96cf50a06807c795164be7144f6cd56d25db3eedf76ed6bfb298610d50679e56R179-R186
?
There was a problem hiding this comment.
Good point! added that
| <Text textStyle="heading-xs">Step 1. Get your Base64 token</Text> | ||
| <Text fontSize="sm" color={colors.foreground.secondary}> | ||
| {isCloud | ||
| ? "Create an app password (under Account > App Passwords), then run the following in your terminal:" |
There was a problem hiding this comment.
I think we can do both. For future purposes; especially for service accounts they would need to go through the Account > App Password flow
| <Text fontSize="sm" color={colors.foreground.secondary}> | ||
| {isCloud | ||
| ? "Create an app password (under Account > App Passwords), then run the following in your terminal:" | ||
| : "Use a SQL role with LOGIN and PASSWORD privileges. Run the following in your terminal:"} |
There was a problem hiding this comment.
The SQL role with LOGIN and PASSWORD privileges ... not quite privileges ...
I see in the docs "Create roles with login/password/superuser privileges (Self-Managed only)." ... the privileges was only for the superuser part.
I would probably go "Use a login role with password attribute" or "Use a role with login and password attributes" or something.
cb7cb16 to
a3f2ff0
Compare
| }, | ||
| { | ||
| title: "Claude Desktop", | ||
| contents: mcpConfigJson(baseUrl, endpoint), |
There was a problem hiding this comment.
We don't tell where to save this content.
| }, | ||
| { | ||
| title: "Cursor", | ||
| contents: mcpConfigJson(baseUrl, endpoint), |
| <Radio value="agents" size="sm"> | ||
| Agents | ||
| </Radio> |
There was a problem hiding this comment.
So ... per https://materializeinc.slack.com/archives/C09ERRA0YAK/p1775758256275809 ... we probably want to hide the agents for now?
| overflow="visible" | ||
| message={ | ||
| isCloud | ||
| ? "The MCP server is not enabled by default. Contact Materialize support to enable the MCP endpoints for your environment." |
There was a problem hiding this comment.
Depending on the answer to your question https://materializeinc.slack.com/archives/C09ERRA0YAK/p1775773024131799?thread_ts=1775758256.275809&cid=C09ERRA0YAK ... possibly may need to change.
| const [currentEnvironment] = useAtom(currentEnvironmentState); | ||
| const appConfig = useAppConfig(); | ||
| const isCloud = appConfig.mode === "cloud"; | ||
| const [endpoint, setEndpoint] = useState<"agents" | "developer">("agents"); |
There was a problem hiding this comment.
maybe need to default to developer per the release schedule?
(also heads up on the agents endpoint: #35930)
fc3bdfe to
b51903b
Compare
| mcpBase64Token, | ||
| ...props | ||
| }: McpConnectInstructionsProps) => { | ||
| const { colors } = useTheme<MaterializeTheme>(); |
There was a problem hiding this comment.
Something to consider for the future (since the docs for MCP aren't published yet):
In https://github.com/MaterializeInc/materialize/pull/35902/changes#diff-0186f9eab0349780bb63a74a9a9e5a60fbba30089003fd88a0b9f3099ded0910R84-R92, we link people to integrations/sql-clients.
We might want to in the Mcp ConnectInstructions include a link to /docs/integrations/mcp-server/ as well.
b51903b to
2cc88d1
Compare
Refactor AppPasswordsPage and ConnectModal for consistency; update McpConnectInstructions layout - Changed "Services and agents" to "Service" for clarity in AppPasswordsPage. - Updated helper text to reflect the singular "Service" in AppPasswordsPage. - Added overflow and minWidth properties to SecretBox components in both AppPasswordsPage and ConnectModal for better UI handling. - Improved layout in McpConnectInstructions by consolidating code blocks and enhancing descriptions for clarity. Enhance MCP token handling and UI updates across components - Added support for pre-computed Base64 MCP tokens in ConnectInstructions and McpConnectInstructions. - Updated ConnectModal to pass the MCP token to relevant components. - Improved layout and descriptions in AppPasswordsPage and McpConnectInstructions for better clarity and user experience. - Adjusted text prompts to emphasize the importance of saving credentials after creation. Fix MCP token encoding and service account handling; - Use unicode-safe base64 encoding (TextEncoder) instead of btoa, which fails on non-ASCII service account names - Use the service account user instead of the logged-in user's email when generating MCP tokens for service account app passwords - Show MCP token under app password only when MCP Server tab is active - Add Agents/Developer radio group to MCP instructions so users can switch endpoints without a separate observatory section - Rename observatory endpoint references to developer (per MaterializeInc#35918) - Add -w0 flag to base64 command for Linux compatibility small cleanups letting the user know where to save the mcp config Removing disclaimer and radio button for "Agents"
2cc88d1 to
aff0522
Compare
### Motivation Adding MCP connection instructions in the console. Linear Issue [CNS-45](https://linear.app/materializeinc/issue/CNS-45/add-mcp-connection-instructions-in-the-console): ### Description Added MCP connect tab for the connect modal. Tested it on various auth modes: **Cloud** <img width="2654" height="2032" alt="image" src="https://github.com/user-attachments/assets/80ab2593-c017-49ff-aec0-24f9303485d0" /> **OIDC** <img width="2970" height="2016" alt="image" src="https://github.com/user-attachments/assets/ea61fac7-f393-4787-9a9c-c337f35e6106" /> **password** <img width="2850" height="1990" alt="image" src="https://github.com/user-attachments/assets/be494be0-6ec2-4346-a237-9eb60c659c34" /> ### Verification How do you know this change is correct? Describe new or existing automated tests, or manual steps you took.




Motivation
Adding MCP connection instructions in the console. Linear Issue CNS-45:
Description
Added MCP connect tab for the connect modal. Tested it on various auth modes:
Cloud
OIDC

password

Verification
How do you know this change is correct? Describe new or existing automated
tests, or manual steps you took.