-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(mcp): Adds MCP resources and a read_resources
tool.
#9149
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
- Adds `/firebase:init` prompt with placeholder guidance. - Resources are defined in `src/mcp/resources`. - Prompts and other output can "link" to resources by saying to use the `read_resources` tool with a particular URI.
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.
Really like the general idea here! The guides obviously need work, but the dynamic instruction loading is nice.
title: "Firestore Init Guide", | ||
description: "guides the coding agent through configuring Firestore in the current project", | ||
}, | ||
async (uri, ctx) => { |
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.
async (uri, ctx) => { | |
async (uri,_) => { |
src/mcp/util/detect.ts
Outdated
|
||
export type WorkspacePlatform = "web" | "android" | "ios" | "flutter" | "react-native" | "unity"; | ||
|
||
export function detectWorkspacePlatform(config: Config): WorkspacePlatform | null { |
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 super similar to getPlatformFromFolder in src/dataconnect/appFinder.ts. Can we pick one implementation and stick to that?
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.
Didn't realize both existed, I'll use that (it should probably get hoisted out of dataconnect
but I'll leave that for now.
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.
LGTM and mergable as is, but i think we might wanna consider moving to loading from MDs.
{ | ||
uri, | ||
type: "text", | ||
text: `Create ai.ts with import { AI } from "firebase";`, |
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.
Suggestion from shopping this around to folks on the FDC team - WDYT about moving this content into actual md files and loading it into these files? That would make it very easy for PMs/other folks to write, and it would be simpler to reuse content from cursor rules and other files
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.
My suggestion is we leave it for now. PMs should be able to manipulate TS files that are primarily just big strings, and it's important to make prompts and resources able to be dynamic. I suspect most prompts will be a combination of a big static hunk of text and some dynamic bits.
If we eventually want to go down this route, I'm going to bring Dotprompt to do the job properly 😄
- Adds `/firebase:init` prompt with placeholder guidance. - Resources are defined in `src/mcp/resources`. - Prompts and other output can "link" to resources by saying to use the `read_resources` tool with a particular URI. - Consolidates context into a single McpContext type. - Some additional refactoring and cleanup.
/firebase:init
prompt with placeholder guidance.src/mcp/resources
.read_resources
tool with a particular URI.