From 81c72af6bf97cd727a8cca540572cdb5861d20f0 Mon Sep 17 00:00:00 2001 From: Cleo Schneider Date: Wed, 7 Jan 2026 19:29:02 -0500 Subject: [PATCH 1/2] Update --only behavior to bypass availability checks --- src/mcp/index.ts | 7 ++- src/mcp/prompt.spec.ts | 17 +++++--- src/mcp/prompt.ts | 10 +++-- src/mcp/prompts/index.spec.ts | 81 +++++++++++++++++++++++++++++++++++ src/mcp/prompts/index.ts | 6 ++- src/mcp/tool.spec.ts | 20 ++++++--- src/mcp/tool.ts | 10 +++-- src/mcp/tools/index.spec.ts | 11 ++++- src/mcp/tools/index.ts | 9 +++- 9 files changed, 140 insertions(+), 31 deletions(-) create mode 100644 src/mcp/prompts/index.spec.ts diff --git a/src/mcp/index.ts b/src/mcp/index.ts index e89280e3e1c..62b35ec26db 100644 --- a/src/mcp/index.ts +++ b/src/mcp/index.ts @@ -182,6 +182,7 @@ export class FirebaseMcpServer { async detectProjectSetup(): Promise { await this.detectProjectRoot(); + if (this.activeFeatures?.length || this.enabledTools?.length) return; // Detecting active features requires that the project directory has been appropriately set await this.detectActiveFeatures(); } @@ -248,13 +249,12 @@ export class FirebaseMcpServer { } async getAvailableTools(): Promise { - const features = this.activeFeatures?.length ? this.activeFeatures : this.detectedFeatures; // We need a project ID and user for the context, but it's ok if they're empty. const projectId = (await this.getProjectId()) || ""; const accountEmail = await this.getAuthenticatedUser(); const isBillingEnabled = projectId ? await checkBillingEnabled(projectId) : false; const ctx = this._createMcpContext(projectId, accountEmail, isBillingEnabled); - return availableTools(ctx, features, this.enabledTools); + return availableTools(ctx, this.activeFeatures, this.detectedFeatures, this.enabledTools); } async getTool(name: string): Promise { @@ -263,13 +263,12 @@ export class FirebaseMcpServer { } async getAvailablePrompts(): Promise { - const features = this.activeFeatures?.length ? this.activeFeatures : this.detectedFeatures; // We need a project ID and user for the context, but it's ok if they're empty. const projectId = (await this.getProjectId()) || ""; const accountEmail = await this.getAuthenticatedUser(); const isBillingEnabled = projectId ? await checkBillingEnabled(projectId) : false; const ctx = this._createMcpContext(projectId, accountEmail, isBillingEnabled); - return availablePrompts(ctx, features); + return availablePrompts(ctx, this.activeFeatures, this.detectedFeatures); } async getPrompt(name: string): Promise { diff --git a/src/mcp/prompt.spec.ts b/src/mcp/prompt.spec.ts index c26e9a29c70..afd4db05a26 100644 --- a/src/mcp/prompt.spec.ts +++ b/src/mcp/prompt.spec.ts @@ -40,24 +40,26 @@ describe("prompt", () => { expect(testPrompt.fn).to.equal(testFn); }); - it("should use the default availability check for the feature if none is provided", () => { + it("should use the default availability check for the feature if none is provided", async () => { // Arrange: Prepare a fake default check function to be returned by our stub. - const fakeDefaultCheck = async () => true; + const fakeDefaultCheck = sandbox.stub().returns(true); getDefaultFeatureAvailabilityCheckStub.withArgs("core").returns(fakeDefaultCheck); // Act: Create a prompt WITHOUT providing an isAvailable function. const testPrompt = prompt("core", { name: "test_prompt" }, async () => []); + const isAvailable = await testPrompt.isAvailable(mockContext); + expect(isAvailable).to.be.true; // Assert: The prompt's isAvailable function should be the one our stub provided. - expect(testPrompt.isAvailable).to.equal(fakeDefaultCheck); + expect(fakeDefaultCheck.called).to.be.true; // Assert: The factory function should have called the stub to get the default. expect(getDefaultFeatureAvailabilityCheckStub.calledOnceWith("core")).to.be.true; }); it("should override the default and use the provided availability check", async () => { - const fakeDefaultCheck = async () => true; - const overrideCheck = async () => false; + const fakeDefaultCheck = sandbox.stub().returns(true); + const overrideCheck = sandbox.stub().returns(false); getDefaultFeatureAvailabilityCheckStub.withArgs("core").returns(fakeDefaultCheck); const testPrompt = prompt( @@ -69,11 +71,12 @@ describe("prompt", () => { overrideCheck, ); - expect(testPrompt.isAvailable).to.equal(overrideCheck); - const isAvailable = await testPrompt.isAvailable(mockContext); expect(isAvailable).to.be.false; + expect(fakeDefaultCheck.notCalled).to.be.true; + expect(overrideCheck.called).to.be.true; + expect(getDefaultFeatureAvailabilityCheckStub.notCalled).to.be.true; }); }); diff --git a/src/mcp/prompt.ts b/src/mcp/prompt.ts index 43c01947e23..2661dd6d9cc 100644 --- a/src/mcp/prompt.ts +++ b/src/mcp/prompt.ts @@ -26,12 +26,14 @@ export function prompt( fn: ServerPrompt["fn"], isAvailable?: (ctx: McpContext) => Promise, ): ServerPrompt { - // default to the feature level availability check, but allow override - const isAvailableFunc = isAvailable || getDefaultFeatureAvailabilityCheck(feature); - return { mcp: options, fn, - isAvailable: isAvailableFunc, + isAvailable: (ctx) => { + // default to the feature level availability check, but allow override + // We resolve this at runtime to allow for easier testing/mocking + const isAvailableFunc = isAvailable || getDefaultFeatureAvailabilityCheck(feature); + return isAvailableFunc(ctx); + }, }; } diff --git a/src/mcp/prompts/index.spec.ts b/src/mcp/prompts/index.spec.ts new file mode 100644 index 00000000000..8fb4c38ea21 --- /dev/null +++ b/src/mcp/prompts/index.spec.ts @@ -0,0 +1,81 @@ +import * as sinon from "sinon"; +import { expect } from "chai"; +import { McpContext } from "../types"; +import { availablePrompts } from "./index"; +// We import the *module* so we can stub the exported function on it +import * as availabilityUtil from "../util/availability"; + +describe("availablePrompts", () => { + let sandbox: sinon.SinonSandbox; + let getDefaultFeatureAvailabilityCheckStub: sinon.SinonStub; + + beforeEach(() => { + sandbox = sinon.createSandbox(); + // Default stub checks to return false + getDefaultFeatureAvailabilityCheckStub = sandbox.stub( + availabilityUtil, + "getDefaultFeatureAvailabilityCheck", + ); + + getDefaultFeatureAvailabilityCheckStub.withArgs("crashlytics").returns(async () => false); + getDefaultFeatureAvailabilityCheckStub.callThrough(); + }); + + afterEach(() => { + sandbox.restore(); + }); + + const mockContext: McpContext = { + projectId: "test-project", + accountEmail: "test@example.com", + config: {} as any, + host: { + logger: { + debug: () => void 0, + info: () => void 0, + warn: () => void 0, + error: () => void 0, + }, + } as any, + rc: {} as any, + firebaseCliCommand: "firebase", + isBillingEnabled: true, + }; + + it("should return core prompts by default", async () => { + const prompts = await availablePrompts(mockContext, [], []); + const corePrompt = prompts.find((p) => p.mcp._meta?.feature === "core"); + expect(corePrompt).to.exist; + }); + + it("should include feature-specific prompts when activeFeatures is provided", async () => { + const prompts = await availablePrompts(mockContext, ["crashlytics"]); + const crashPrompt = prompts.find((p) => p.mcp._meta?.feature === "crashlytics"); + + // Should exist because we bypass availability check + expect(crashPrompt).to.exist; + + // getDefaultFeatureAvailabilityCheck execution is deferred/lazy-loaded in prompt.ts + // Since activeFeatures bypasses checking .isAvailable on the prompt, the stub should NOT be called. + expect(getDefaultFeatureAvailabilityCheckStub.called).to.be.false; + }); + + it("should not include feature prompts if not in activeFeatures", async () => { + const prompts = await availablePrompts(mockContext, [], []); + const crashPrompt = prompts.find((p) => p.mcp._meta?.feature === "crashlytics"); + expect(crashPrompt).to.not.exist; + }); + + it("should fallback to detectedFeatures if activeFeatures is empty", async () => { + // For this test, we want availability to be true + getDefaultFeatureAvailabilityCheckStub.withArgs("crashlytics").returns(async () => true); + + const prompts = await availablePrompts(mockContext, [], ["crashlytics"]); + const crashPrompt = prompts.find((p) => p.mcp._meta?.feature === "crashlytics"); + + expect(crashPrompt).to.exist; + + // Fallback logic calls isAvailable(), which invokes our lazy check, calling getDefault... + expect(getDefaultFeatureAvailabilityCheckStub.called).to.be.true; + }); +}); diff --git a/src/mcp/prompts/index.ts b/src/mcp/prompts/index.ts index 7ea672dc63f..60e7b87f781 100644 --- a/src/mcp/prompts/index.ts +++ b/src/mcp/prompts/index.ts @@ -45,9 +45,13 @@ function namespacePrompts( export async function availablePrompts( ctx: McpContext, activeFeatures?: ServerFeature[], + detectedFeatures?: ServerFeature[], ): Promise { - const allPrompts = getAllPrompts(activeFeatures); + if (activeFeatures?.length) { + return getAllPrompts(activeFeatures); + } + const allPrompts = getAllPrompts(detectedFeatures); const availabilities = await Promise.all( allPrompts.map((p) => { if (p.isAvailable) { diff --git a/src/mcp/tool.spec.ts b/src/mcp/tool.spec.ts index 7dbd342f9a8..c1fa78bf0da 100644 --- a/src/mcp/tool.spec.ts +++ b/src/mcp/tool.spec.ts @@ -42,21 +42,25 @@ describe("tool", () => { expect(testTool.fn).to.equal(testFn); }); - it("should use the default availability check for the feature if none is provided", () => { - const fakeDefaultCheck = async () => true; + it("should use the default availability check for the feature if none is provided", async () => { + const fakeDefaultCheck = sandbox.stub().returns(true); getDefaultFeatureAvailabilityCheckStub.withArgs("core").returns(fakeDefaultCheck); const testTool = tool("core", { name: "test_tool", inputSchema: z.object({}) }, async () => ({ content: [], })); - expect(testTool.isAvailable).to.equal(fakeDefaultCheck); + const isAvailable = await testTool.isAvailable(mockContext); + + expect(isAvailable).to.be.true; + expect(fakeDefaultCheck.called).to.be.true; expect(getDefaultFeatureAvailabilityCheckStub.calledOnceWith("core")).to.be.true; }); it("should override the default and use the provided availability check", async () => { - const fakeDefaultCheck = async () => true; - const overrideCheck = async () => false; // This will be the override. + const fakeDefaultCheck = sandbox.stub().returns(true); + const overrideCheck = sandbox.stub().returns(false); + // This will be the override. getDefaultFeatureAvailabilityCheckStub.withArgs("core").returns(fakeDefaultCheck); const testTool = tool( @@ -69,10 +73,12 @@ describe("tool", () => { async () => ({ content: [] }), ); - expect(testTool.isAvailable).to.equal(overrideCheck); - const isAvailable = await testTool.isAvailable(mockContext); expect(isAvailable).to.be.false; + + expect(fakeDefaultCheck.notCalled).to.be.true; + expect(overrideCheck.called).to.be.true; + expect(getDefaultFeatureAvailabilityCheckStub.notCalled).to.be.true; }); }); diff --git a/src/mcp/tool.ts b/src/mcp/tool.ts index fa8105b5d4d..fc911d12893 100644 --- a/src/mcp/tool.ts +++ b/src/mcp/tool.ts @@ -54,12 +54,14 @@ export function tool( ): ServerTool { const { isAvailable, ...mcpOptions } = options; - // default to the feature level availability check, but allow override - const isAvailableFunc = isAvailable || getDefaultFeatureAvailabilityCheck(feature); - return { mcp: { ...mcpOptions, inputSchema: cleanSchema(zodToJsonSchema(options.inputSchema)) }, fn, - isAvailable: isAvailableFunc, + isAvailable: (ctx: McpContext) => { + // default to the feature level availability check, but allow override + // We resolve this at runtime to allow for easier testing/mocking + const isAvailableFunc = isAvailable || getDefaultFeatureAvailabilityCheck(feature); + return isAvailableFunc(ctx); + }, }; } diff --git a/src/mcp/tools/index.spec.ts b/src/mcp/tools/index.spec.ts index c6ec9efe702..53a349a2222 100644 --- a/src/mcp/tools/index.spec.ts +++ b/src/mcp/tools/index.spec.ts @@ -21,14 +21,14 @@ describe("availableTools", () => { }; it("should return specific tools when enabledTools is provided", async () => { - const tools = await availableTools(mockContext, [], ["firebase_login"]); + const tools = await availableTools(mockContext, [], [], ["firebase_login"]); expect(tools).to.have.length(1); expect(tools[0].mcp.name).to.equal("firebase_login"); }).timeout(2000); it("should return core tools by default", async () => { - const tools = await availableTools(mockContext, []); + const tools = await availableTools(mockContext, [], []); // an example of a core tool const loginTool = tools.find((t) => t.mcp.name === "firebase_login"); @@ -48,4 +48,11 @@ describe("availableTools", () => { expect(firestoreTool).to.not.exist; }).timeout(2000); + + it("should fallback to detected features if activeFeatures is empty", async () => { + const tools = await availableTools(mockContext, [], ["firestore"]); + const firestoreTool = tools.find((t) => t.mcp.name.startsWith("firestore_")); + + expect(firestoreTool).to.exist; + }); }); diff --git a/src/mcp/tools/index.ts b/src/mcp/tools/index.ts index 0885cc9b116..2f82fccffee 100644 --- a/src/mcp/tools/index.ts +++ b/src/mcp/tools/index.ts @@ -62,7 +62,7 @@ function getToolsByName(names: string[]): ServerTool[] { return Array.from(selectedTools); } -function getToolsByFeature(serverFeatures?: ServerFeature[]): ServerTool[] { +export function getToolsByFeature(serverFeatures?: ServerFeature[]): ServerTool[] { const features = new Set( serverFeatures?.length ? serverFeatures : (Object.keys(tools) as ServerFeature[]), ); @@ -79,13 +79,18 @@ function getToolsByFeature(serverFeatures?: ServerFeature[]): ServerTool[] { export async function availableTools( ctx: McpContext, activeFeatures?: ServerFeature[], + detectedFeatures?: ServerFeature[], enabledTools?: string[], ): Promise { if (enabledTools?.length) { return getToolsByName(enabledTools); } - const allTools = getToolsByFeature(activeFeatures); + if (activeFeatures?.length) { + return getToolsByFeature(activeFeatures); + } + + const allTools = getToolsByFeature(detectedFeatures); const availabilities = await Promise.all( allTools.map((t) => { if (t.isAvailable) { From 60c3833b8280639dcd024cd20d3ea7d061f874b8 Mon Sep 17 00:00:00 2001 From: Cleo Schneider Date: Fri, 9 Jan 2026 10:27:50 -0500 Subject: [PATCH 2/2] Respond to review: update tests and add a changelog entry --- CHANGELOG.md | 1 + src/mcp/prompts/index.spec.ts | 10 ++++------ 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e69de29bb2d..941b6be2970 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -0,0 +1 @@ +- Fixes an issue where the `--only` flag was not always respected for `firebase mcp` diff --git a/src/mcp/prompts/index.spec.ts b/src/mcp/prompts/index.spec.ts index 8fb4c38ea21..a7e7983797e 100644 --- a/src/mcp/prompts/index.spec.ts +++ b/src/mcp/prompts/index.spec.ts @@ -50,10 +50,9 @@ describe("availablePrompts", () => { it("should include feature-specific prompts when activeFeatures is provided", async () => { const prompts = await availablePrompts(mockContext, ["crashlytics"]); - const crashPrompt = prompts.find((p) => p.mcp._meta?.feature === "crashlytics"); - // Should exist because we bypass availability check - expect(crashPrompt).to.exist; + const features = [...new Set(prompts.map((p) => p.mcp._meta?.feature))]; + expect(features).to.have.members(["core", "crashlytics"]); // getDefaultFeatureAvailabilityCheck execution is deferred/lazy-loaded in prompt.ts // Since activeFeatures bypasses checking .isAvailable on the prompt, the stub should NOT be called. @@ -71,9 +70,8 @@ describe("availablePrompts", () => { getDefaultFeatureAvailabilityCheckStub.withArgs("crashlytics").returns(async () => true); const prompts = await availablePrompts(mockContext, [], ["crashlytics"]); - const crashPrompt = prompts.find((p) => p.mcp._meta?.feature === "crashlytics"); - - expect(crashPrompt).to.exist; + const features = [...new Set(prompts.map((p) => p.mcp._meta?.feature))]; + expect(features).to.have.members(["core", "crashlytics"]); // Fallback logic calls isAvailable(), which invokes our lazy check, calling getDefault... expect(getDefaultFeatureAvailabilityCheckStub.called).to.be.true;