Conversation
### Description Adds support for running the MCP server in SSE (HTTP) mode, in addition to the default Stdio transport. This allows clients to connect over network or via tools that support SSE. ### Scenarios Tested - Started server in SSE mode and verified log output.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces Server-Sent Events (SSE) support to the MCP server, enabling it to operate over HTTP via new --sse and --port CLI options. Key changes include the implementation of an Express-based server within the start method to handle SSE connections and message routing, as well as updates to tool calls for progress token support and refined GA4 event tracking. Review feedback suggests improving code quality by replacing inline require calls with top-level imports and replacing any types with specific interfaces to comply with the project's TypeScript style guide.
### Description Fixes a type error where progressToken was not defined on McpContext. ### Scenarios Tested - Verified build succeeds.
### Description Addresses PR comments by: - Moving inline require calls to top-level imports. - Replacing any types with specific interfaces or unknown. ### Scenarios Tested - Verified build succeeds.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for Server-Sent Events (SSE) in the MCP server, allowing it to run in HTTP mode alongside the default Stdio transport. It adds new CLI options for --sse and --port, implements an Express-based SSE server with session management, and includes minor improvements to error handling and progress token support. Several issues were identified during the review: a regression in GA4 tracking for resource listing, potential concurrency issues with the shared server instance in SSE mode, violations of the repository style guide regarding the use of console and any types, and a security concern regarding the default network binding address.
### Description - Reverts accidental GA4 tracking change in mcpListResources. - Replaces console.error with this.logger calls for better logging. - Changes default server binding from 0.0.0.0 to 127.0.0.1 for security. ### Scenarios Tested - Verified build succeeds.
### Description - Applied auto-formatting fixes from npm run format. ### Scenarios Tested - Verified build succeeds.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces SSE (Server-Sent Events) support for the MCP server, allowing it to run in HTTP mode in addition to the default Stdio mode. It also adds progress token support to the MCP context and improves error handling in the billing check by replacing 'any' with 'unknown' and using proper type narrowing. I have one piece of feedback regarding the use of a typecast in the SSE transport initialization; please define a proper interface or type for the transport object instead of using an 'any' cast to maintain type safety as per the repository style guide.
* feat: add infrastructure for MCP Apps ### Description Adds support for returning structured content from tools, which is used by MCP Apps to pass complex data to the host. Also updates the resource index. ### Scenarios Tested - Verified build and file changes. * fix: resolve build errors and address review comments on infra ### Description - Removes imports and registry entries for UI resources that are not yet available in this branch (login, update_environment, deploy, init). - Replaces as any in toContent with an intersection type for better type safety. ### Scenarios Tested - Verified build succeeds.
| rc: RC; | ||
| firebaseCliCommand: string; | ||
| isBillingEnabled: boolean; | ||
| progressToken?: string | number; |
There was a problem hiding this comment.
What is progressToken? Is it a common concept for MCP server? If not, some comment here can be helpful
### Description - Defines a local interface extending SSEServerTransport to avoid using when accessing . ### Scenarios Tested - Build succeeds. - Lint passes for modified lines.
### Description - Replaced boolean flag with string flag (defaults to 'stdio'). - Added validation for to accept only 'stdio' or 'sse'. - Fixed build errors by adding to interface and removing missing resource. ### Scenarios Tested - Build succeeds. - Lint passes with no new errors.
chkuang-g
left a comment
chkuang-g
left a comment
There was a problem hiding this comment.
LGTM. But I will let the Firebase CLI and Javascript expert to approve this :)
4 participants
Adds support for running the MCP server in SSE (HTTP) mode. This opens up some interesting use cases, but mostly it makes it much easier to test out the MCP apps work that is incoming.
I also opened up the typing to support structured content returned from tools - minor, but makes ti much easier to have apps read tool responses later.