Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a new visual interface for initializing Firebase products (Firestore and Authentication) within a workspace. It includes a new MCP application with HTML/TypeScript components, a Vite configuration for building the single-file app, and a new server resource to serve the UI. The review feedback highlights several critical improvements for type safety and error handling: specifically, the code should avoid using any as an escape hatch per the repository style guide, and it must explicitly check the isError property of server tool responses before accessing data. Additionally, reducing the project list page size is recommended to avoid potential API performance issues.
a06ac6e to
ebdd314
Compare
### Description - Adds mcpapps experiment flag to src/experiments.ts. - Adds applyAppMeta helper function to src/mcp/util.ts to conditionally add UI metadata. - Adds unit tests for applyAppMeta in src/mcp/util.spec.ts. ### Scenarios Tested - Unit tests passed. - Build succeeds.
### Description - Fixes applyAppMeta to preserve existing metadata. - Moves mcpapps flag to be grouped with other MCP experiments. - Removes as any in util.spec.ts by importing CallToolResult. ### Scenarios Tested - Build succeeds. - Lint passes for modified files (ignoring pre-existing warnings). - Unit tests for applyAppMeta pass.
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. - Verified build and file changes.
### Description Adds the Update Environment MCP App, allowing users to switch projects and directories from the UI. ### Scenarios Tested - Verified build and file changes.
### Description Adds the Init MCP App for interactive initialization of Firestore and Auth products. Includes support for Google Sign-In configuration and project selection. ### Scenarios Tested - Verified build and file changes.
- Remove non-existent login_ui and deploy_ui from resources index to fix build. - Avoid using any in mcp-app.ts, use specific interfaces. - Check isError on tool results. - Reduce page_size to 1000. - Fix lint warnings for catch blocks.
| const statusBox = document.getElementById("status-box") as HTMLDivElement; | ||
|
|
||
| let projects: any[] = []; | ||
| let filteredProjects: any[] = []; |
There was a problem hiding this comment.
Would love to have these typed
| <input type="text" id="search-input" placeholder="Type to filter projects..." /> | ||
| </div> | ||
|
|
||
| <div class="dropdown-container"> |
There was a problem hiding this comment.
Any reason this isn't a select -> option?
https://www.w3schools.com/tags/tag_select.asp
| } | ||
|
|
||
| searchInput.oninput = () => { | ||
| const query = searchInput.value.toLowerCase().trim(); |
There was a problem hiding this comment.
It'd be awesome to have some kind of debounce + query whenever the user types so that you don't have to get the full list immediately
| const result = await app.callServerTool({ | ||
| name: "firebase_update_environment", | ||
| arguments: { active_project: selectedProjectId }, | ||
| }); | ||
|
|
||
| const textContent = result.content?.find((c: any) => c.type === "text"); | ||
| const text = textContent ? (textContent as any).text : "Update complete."; | ||
|
|
||
| if (result.isError) { | ||
| showStatus(text, "error"); | ||
| submitBtn.disabled = false; | ||
| } else { | ||
| showStatus(text, "success"); | ||
| } |
There was a problem hiding this comment.
Looks like a lot of this is re-used from mcp-app. I wonder if you can just put it in a util?
|
|
||
| // Fetch projects on load | ||
| const result = await app.callServerTool({ name: "firebase_list_projects", arguments: {} }); | ||
| const data = result.structuredContent as any; |
| // Fetch current environment | ||
| try { | ||
| const envResult = await app.callServerTool({ | ||
| name: "firebase_get_environment", | ||
| arguments: {}, | ||
| }); | ||
| const envData = envResult.structuredContent as any; | ||
| if (envData) { | ||
| envProjectIdEl.textContent = envData.projectId || "<NONE>"; | ||
| envUserEl.textContent = envData.authenticatedUser || "<NONE>"; | ||
| } | ||
| } catch (err: any) { | ||
| console.error("Failed to fetch environment:", err); | ||
| showStatus(`Failed to fetch environment: ${err.message}`, "error"); |
There was a problem hiding this comment.
A lot of the same content as in mcp-app. Could we put this in a separate file?
| }, | ||
| "scripts": { | ||
| "build": "tsc && npm run copyfiles", | ||
| "build:mcp-apps": "vite build --config src/mcp/apps/update_environment/vite.config.ts && vite build --config src/mcp/apps/init/vite.config.ts", |
There was a problem hiding this comment.
You can create separate entry files in vite configs as well to make this simpler
Adds the Init MCP App. (Chained off #10259)