Skip to content

Conversation

kamath
Copy link
Contributor

@kamath kamath commented Aug 20, 2025

Added MCP UI and cleaned up a few small things to support latest spec

Built-in.Retina.Display.mp4

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR introduces MCP UI functionality to the Browserbase MCP server, enabling visual browser session interfaces directly within MCP clients. The changes transform the server from a command-line only tool to one that supports embedded UI components.

Key changes include:

  • UI Integration: Added @mcp-ui/server dependency and integrated UI resource creation in the session management tool, allowing users to view Browserbase debugger interfaces as embedded iframes
  • Module System Modernization: Updated TypeScript configuration from Node16 to ESNext with bundler resolution to support modern UI libraries and bundling tools
  • Import Compatibility: Changed dotenv import from default to namespace import (import * as dotenv) for better ESM compatibility
  • Code Reorganization: Cleaned up debug URL display in the navigate tool, consolidating this functionality into dedicated session management where it's more appropriate
  • Server Architecture Enhancement: Restructured the main index.ts to export a default function with Zod-based configuration validation and standardized tool registration

These changes integrate seamlessly with the existing browser automation capabilities while adding a visual layer that enhances user experience. The UI resources use standard iframe embedding to display Browserbase's debugger interface, and the modular approach ensures backward compatibility with existing MCP protocol implementations.

Confidence score: 3/5

  • This PR introduces significant architectural changes that could impact module resolution and runtime behavior
  • Score reflects concerns about TypeScript configuration changes that may affect Node.js compatibility and the type assertion workaround in session.ts
  • Pay close attention to tsconfig.json and src/tools/session.ts for potential runtime issues

5 files reviewed, 1 comment

Edit Code Review Bot Settings | Greptile

uri: "ui://analytics-dashboard/main",
content: { type: "externalUrl", iframeUrl: debugUrl },
encoding: "text",
}) as unknown as TextContent,
Copy link

Choose a reason for hiding this comment

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

style: Type assertion as unknown as TextContent suggests type incompatibility. Consider updating the content array type to properly support UI resources or create a union type.

@kamath kamath changed the title add mcp ui add mcp ui to embed browser in compatible clients Aug 20, 2025
Copy link
Collaborator

@Kylejeong2 Kylejeong2 left a comment

Choose a reason for hiding this comment

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

Small changes, mostly due to keeping feature requests.

Also wondering about the ESNext stuff, why we need to change. Does the dockerfile need to change for us to be able to deploy to smithery as well?

apiKey: context.config.browserbaseApiKey,
});
const debugUrl = (await bb.sessions.debug(sessionId))
.debuggerFullscreenUrl;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We had a feature request to include the liveview/debug url in the output from these tool calls which is why they're still in here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah these are now in session create instead of on navigate; redundant to have it on every navigate call within a session

"module": "Node16",
"moduleResolution": "Node16",
"module": "ESNext",
"moduleResolution": "bundler",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason why we're changing to ESNext here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah node 16 is super old and past end of life. we can use nodenext here as well or you can pin the version

);

return {
content: [
{
type: "text",
text: `Browserbase Live Session View URL: https://www.browserbase.com/sessions/${session.sessionId}\nBrowserbase Live Debugger URL: ${debugUrl}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same thing as above, can technically change this to be another text output as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's what i did, are you saying you prefer it this way?

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah oops didn't see

return {
content: [
{
type: "text",
text: `Navigated to: ${params.url}`,
},
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unnecessary on navigate tool

apiKey: context.config.browserbaseApiKey,
});
const debugUrl = (await bb.sessions.debug(sessionId))
.debuggerFullscreenUrl;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah these are now in session create instead of on navigate; redundant to have it on every navigate call within a session

"module": "Node16",
"moduleResolution": "Node16",
"module": "ESNext",
"moduleResolution": "bundler",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah node 16 is super old and past end of life. we can use nodenext here as well or you can pin the version

);

return {
content: [
{
type: "text",
text: `Browserbase Live Session View URL: https://www.browserbase.com/sessions/${session.sessionId}\nBrowserbase Live Debugger URL: ${debugUrl}`,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's what i did, are you saying you prefer it this way?

@kamath kamath requested a review from Kylejeong2 August 21, 2025 05:08
Copy link
Collaborator

@Kylejeong2 Kylejeong2 left a comment

Choose a reason for hiding this comment

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

yeah looks good, the only thing is just pnpm install to get the right version of dotenv to pass our ci

);

return {
content: [
{
type: "text",
text: `Browserbase Live Session View URL: https://www.browserbase.com/sessions/${session.sessionId}\nBrowserbase Live Debugger URL: ${debugUrl}`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

ah oops didn't see

@kamath kamath requested a review from Kylejeong2 August 24, 2025 18:30
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.

2 participants