Skip to content

Commit 19fd481

Browse files
authored
fix(mcp): Update --only behavior to bypass availability checks (#9713)
* Update --only behavior to bypass availability checks * Respond to review: update tests and add a changelog entry
1 parent 958949e commit 19fd481

File tree

10 files changed

+139
-31
lines changed

10 files changed

+139
-31
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Fixes an issue where the `--only` flag was not always respected for `firebase mcp`

src/mcp/index.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ export class FirebaseMcpServer {
182182

183183
async detectProjectSetup(): Promise<void> {
184184
await this.detectProjectRoot();
185+
if (this.activeFeatures?.length || this.enabledTools?.length) return;
185186
// Detecting active features requires that the project directory has been appropriately set
186187
await this.detectActiveFeatures();
187188
}
@@ -248,13 +249,12 @@ export class FirebaseMcpServer {
248249
}
249250

250251
async getAvailableTools(): Promise<ServerTool[]> {
251-
const features = this.activeFeatures?.length ? this.activeFeatures : this.detectedFeatures;
252252
// We need a project ID and user for the context, but it's ok if they're empty.
253253
const projectId = (await this.getProjectId()) || "";
254254
const accountEmail = await this.getAuthenticatedUser();
255255
const isBillingEnabled = projectId ? await checkBillingEnabled(projectId) : false;
256256
const ctx = this._createMcpContext(projectId, accountEmail, isBillingEnabled);
257-
return availableTools(ctx, features, this.enabledTools);
257+
return availableTools(ctx, this.activeFeatures, this.detectedFeatures, this.enabledTools);
258258
}
259259

260260
async getTool(name: string): Promise<ServerTool | null> {
@@ -263,13 +263,12 @@ export class FirebaseMcpServer {
263263
}
264264

265265
async getAvailablePrompts(): Promise<ServerPrompt[]> {
266-
const features = this.activeFeatures?.length ? this.activeFeatures : this.detectedFeatures;
267266
// We need a project ID and user for the context, but it's ok if they're empty.
268267
const projectId = (await this.getProjectId()) || "";
269268
const accountEmail = await this.getAuthenticatedUser();
270269
const isBillingEnabled = projectId ? await checkBillingEnabled(projectId) : false;
271270
const ctx = this._createMcpContext(projectId, accountEmail, isBillingEnabled);
272-
return availablePrompts(ctx, features);
271+
return availablePrompts(ctx, this.activeFeatures, this.detectedFeatures);
273272
}
274273

275274
async getPrompt(name: string): Promise<ServerPrompt | null> {

src/mcp/prompt.spec.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,24 +40,26 @@ describe("prompt", () => {
4040
expect(testPrompt.fn).to.equal(testFn);
4141
});
4242

43-
it("should use the default availability check for the feature if none is provided", () => {
43+
it("should use the default availability check for the feature if none is provided", async () => {
4444
// Arrange: Prepare a fake default check function to be returned by our stub.
45-
const fakeDefaultCheck = async () => true;
45+
const fakeDefaultCheck = sandbox.stub().returns(true);
4646
getDefaultFeatureAvailabilityCheckStub.withArgs("core").returns(fakeDefaultCheck);
4747

4848
// Act: Create a prompt WITHOUT providing an isAvailable function.
4949
const testPrompt = prompt("core", { name: "test_prompt" }, async () => []);
50+
const isAvailable = await testPrompt.isAvailable(mockContext);
51+
expect(isAvailable).to.be.true;
5052

5153
// Assert: The prompt's isAvailable function should be the one our stub provided.
52-
expect(testPrompt.isAvailable).to.equal(fakeDefaultCheck);
54+
expect(fakeDefaultCheck.called).to.be.true;
5355

5456
// Assert: The factory function should have called the stub to get the default.
5557
expect(getDefaultFeatureAvailabilityCheckStub.calledOnceWith("core")).to.be.true;
5658
});
5759

5860
it("should override the default and use the provided availability check", async () => {
59-
const fakeDefaultCheck = async () => true;
60-
const overrideCheck = async () => false;
61+
const fakeDefaultCheck = sandbox.stub().returns(true);
62+
const overrideCheck = sandbox.stub().returns(false);
6163
getDefaultFeatureAvailabilityCheckStub.withArgs("core").returns(fakeDefaultCheck);
6264

6365
const testPrompt = prompt(
@@ -69,11 +71,12 @@ describe("prompt", () => {
6971
overrideCheck,
7072
);
7173

72-
expect(testPrompt.isAvailable).to.equal(overrideCheck);
73-
7474
const isAvailable = await testPrompt.isAvailable(mockContext);
7575
expect(isAvailable).to.be.false;
7676

77+
expect(fakeDefaultCheck.notCalled).to.be.true;
78+
expect(overrideCheck.called).to.be.true;
79+
7780
expect(getDefaultFeatureAvailabilityCheckStub.notCalled).to.be.true;
7881
});
7982
});

src/mcp/prompt.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,14 @@ export function prompt(
2626
fn: ServerPrompt["fn"],
2727
isAvailable?: (ctx: McpContext) => Promise<boolean>,
2828
): ServerPrompt {
29-
// default to the feature level availability check, but allow override
30-
const isAvailableFunc = isAvailable || getDefaultFeatureAvailabilityCheck(feature);
31-
3229
return {
3330
mcp: options,
3431
fn,
35-
isAvailable: isAvailableFunc,
32+
isAvailable: (ctx) => {
33+
// default to the feature level availability check, but allow override
34+
// We resolve this at runtime to allow for easier testing/mocking
35+
const isAvailableFunc = isAvailable || getDefaultFeatureAvailabilityCheck(feature);
36+
return isAvailableFunc(ctx);
37+
},
3638
};
3739
}

src/mcp/prompts/index.spec.ts

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
import * as sinon from "sinon";
2+
import { expect } from "chai";
3+
import { McpContext } from "../types";
4+
import { availablePrompts } from "./index";
5+
// We import the *module* so we can stub the exported function on it
6+
import * as availabilityUtil from "../util/availability";
7+
8+
describe("availablePrompts", () => {
9+
let sandbox: sinon.SinonSandbox;
10+
let getDefaultFeatureAvailabilityCheckStub: sinon.SinonStub;
11+
12+
beforeEach(() => {
13+
sandbox = sinon.createSandbox();
14+
// Default stub checks to return false
15+
getDefaultFeatureAvailabilityCheckStub = sandbox.stub(
16+
availabilityUtil,
17+
"getDefaultFeatureAvailabilityCheck",
18+
);
19+
20+
getDefaultFeatureAvailabilityCheckStub.withArgs("crashlytics").returns(async () => false);
21+
getDefaultFeatureAvailabilityCheckStub.callThrough();
22+
});
23+
24+
afterEach(() => {
25+
sandbox.restore();
26+
});
27+
28+
const mockContext: McpContext = {
29+
projectId: "test-project",
30+
accountEmail: "[email protected]",
31+
config: {} as any,
32+
host: {
33+
logger: {
34+
debug: () => void 0,
35+
info: () => void 0,
36+
warn: () => void 0,
37+
error: () => void 0,
38+
},
39+
} as any,
40+
rc: {} as any,
41+
firebaseCliCommand: "firebase",
42+
isBillingEnabled: true,
43+
};
44+
45+
it("should return core prompts by default", async () => {
46+
const prompts = await availablePrompts(mockContext, [], []);
47+
const corePrompt = prompts.find((p) => p.mcp._meta?.feature === "core");
48+
expect(corePrompt).to.exist;
49+
});
50+
51+
it("should include feature-specific prompts when activeFeatures is provided", async () => {
52+
const prompts = await availablePrompts(mockContext, ["crashlytics"]);
53+
54+
const features = [...new Set(prompts.map((p) => p.mcp._meta?.feature))];
55+
expect(features).to.have.members(["core", "crashlytics"]);
56+
57+
// getDefaultFeatureAvailabilityCheck execution is deferred/lazy-loaded in prompt.ts
58+
// Since activeFeatures bypasses checking .isAvailable on the prompt, the stub should NOT be called.
59+
expect(getDefaultFeatureAvailabilityCheckStub.called).to.be.false;
60+
});
61+
62+
it("should not include feature prompts if not in activeFeatures", async () => {
63+
const prompts = await availablePrompts(mockContext, [], []);
64+
const crashPrompt = prompts.find((p) => p.mcp._meta?.feature === "crashlytics");
65+
expect(crashPrompt).to.not.exist;
66+
});
67+
68+
it("should fallback to detectedFeatures if activeFeatures is empty", async () => {
69+
// For this test, we want availability to be true
70+
getDefaultFeatureAvailabilityCheckStub.withArgs("crashlytics").returns(async () => true);
71+
72+
const prompts = await availablePrompts(mockContext, [], ["crashlytics"]);
73+
const features = [...new Set(prompts.map((p) => p.mcp._meta?.feature))];
74+
expect(features).to.have.members(["core", "crashlytics"]);
75+
76+
// Fallback logic calls isAvailable(), which invokes our lazy check, calling getDefault...
77+
expect(getDefaultFeatureAvailabilityCheckStub.called).to.be.true;
78+
});
79+
});

src/mcp/prompts/index.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,13 @@ function namespacePrompts(
4545
export async function availablePrompts(
4646
ctx: McpContext,
4747
activeFeatures?: ServerFeature[],
48+
detectedFeatures?: ServerFeature[],
4849
): Promise<ServerPrompt[]> {
49-
const allPrompts = getAllPrompts(activeFeatures);
50+
if (activeFeatures?.length) {
51+
return getAllPrompts(activeFeatures);
52+
}
5053

54+
const allPrompts = getAllPrompts(detectedFeatures);
5155
const availabilities = await Promise.all(
5256
allPrompts.map((p) => {
5357
if (p.isAvailable) {

src/mcp/tool.spec.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,25 @@ describe("tool", () => {
4242
expect(testTool.fn).to.equal(testFn);
4343
});
4444

45-
it("should use the default availability check for the feature if none is provided", () => {
46-
const fakeDefaultCheck = async () => true;
45+
it("should use the default availability check for the feature if none is provided", async () => {
46+
const fakeDefaultCheck = sandbox.stub().returns(true);
4747
getDefaultFeatureAvailabilityCheckStub.withArgs("core").returns(fakeDefaultCheck);
4848

4949
const testTool = tool("core", { name: "test_tool", inputSchema: z.object({}) }, async () => ({
5050
content: [],
5151
}));
5252

53-
expect(testTool.isAvailable).to.equal(fakeDefaultCheck);
53+
const isAvailable = await testTool.isAvailable(mockContext);
54+
55+
expect(isAvailable).to.be.true;
56+
expect(fakeDefaultCheck.called).to.be.true;
5457
expect(getDefaultFeatureAvailabilityCheckStub.calledOnceWith("core")).to.be.true;
5558
});
5659

5760
it("should override the default and use the provided availability check", async () => {
58-
const fakeDefaultCheck = async () => true;
59-
const overrideCheck = async () => false; // This will be the override.
61+
const fakeDefaultCheck = sandbox.stub().returns(true);
62+
const overrideCheck = sandbox.stub().returns(false);
63+
// This will be the override.
6064
getDefaultFeatureAvailabilityCheckStub.withArgs("core").returns(fakeDefaultCheck);
6165

6266
const testTool = tool(
@@ -69,10 +73,12 @@ describe("tool", () => {
6973
async () => ({ content: [] }),
7074
);
7175

72-
expect(testTool.isAvailable).to.equal(overrideCheck);
73-
7476
const isAvailable = await testTool.isAvailable(mockContext);
7577
expect(isAvailable).to.be.false;
78+
79+
expect(fakeDefaultCheck.notCalled).to.be.true;
80+
expect(overrideCheck.called).to.be.true;
81+
7682
expect(getDefaultFeatureAvailabilityCheckStub.notCalled).to.be.true;
7783
});
7884
});

src/mcp/tool.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,14 @@ export function tool<InputSchema extends ZodTypeAny>(
5454
): ServerTool {
5555
const { isAvailable, ...mcpOptions } = options;
5656

57-
// default to the feature level availability check, but allow override
58-
const isAvailableFunc = isAvailable || getDefaultFeatureAvailabilityCheck(feature);
59-
6057
return {
6158
mcp: { ...mcpOptions, inputSchema: cleanSchema(zodToJsonSchema(options.inputSchema)) },
6259
fn,
63-
isAvailable: isAvailableFunc,
60+
isAvailable: (ctx: McpContext) => {
61+
// default to the feature level availability check, but allow override
62+
// We resolve this at runtime to allow for easier testing/mocking
63+
const isAvailableFunc = isAvailable || getDefaultFeatureAvailabilityCheck(feature);
64+
return isAvailableFunc(ctx);
65+
},
6466
};
6567
}

src/mcp/tools/index.spec.ts

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ describe("availableTools", () => {
2121
};
2222

2323
it("should return specific tools when enabledTools is provided", async () => {
24-
const tools = await availableTools(mockContext, [], ["firebase_login"]);
24+
const tools = await availableTools(mockContext, [], [], ["firebase_login"]);
2525

2626
expect(tools).to.have.length(1);
2727
expect(tools[0].mcp.name).to.equal("firebase_login");
2828
}).timeout(2000);
2929

3030
it("should return core tools by default", async () => {
31-
const tools = await availableTools(mockContext, []);
31+
const tools = await availableTools(mockContext, [], []);
3232
// an example of a core tool
3333
const loginTool = tools.find((t) => t.mcp.name === "firebase_login");
3434

@@ -48,4 +48,11 @@ describe("availableTools", () => {
4848

4949
expect(firestoreTool).to.not.exist;
5050
}).timeout(2000);
51+
52+
it("should fallback to detected features if activeFeatures is empty", async () => {
53+
const tools = await availableTools(mockContext, [], ["firestore"]);
54+
const firestoreTool = tools.find((t) => t.mcp.name.startsWith("firestore_"));
55+
56+
expect(firestoreTool).to.exist;
57+
});
5158
});

src/mcp/tools/index.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ function getToolsByName(names: string[]): ServerTool[] {
6262
return Array.from(selectedTools);
6363
}
6464

65-
function getToolsByFeature(serverFeatures?: ServerFeature[]): ServerTool[] {
65+
export function getToolsByFeature(serverFeatures?: ServerFeature[]): ServerTool[] {
6666
const features = new Set(
6767
serverFeatures?.length ? serverFeatures : (Object.keys(tools) as ServerFeature[]),
6868
);
@@ -79,13 +79,18 @@ function getToolsByFeature(serverFeatures?: ServerFeature[]): ServerTool[] {
7979
export async function availableTools(
8080
ctx: McpContext,
8181
activeFeatures?: ServerFeature[],
82+
detectedFeatures?: ServerFeature[],
8283
enabledTools?: string[],
8384
): Promise<ServerTool[]> {
8485
if (enabledTools?.length) {
8586
return getToolsByName(enabledTools);
8687
}
8788

88-
const allTools = getToolsByFeature(activeFeatures);
89+
if (activeFeatures?.length) {
90+
return getToolsByFeature(activeFeatures);
91+
}
92+
93+
const allTools = getToolsByFeature(detectedFeatures);
8994
const availabilities = await Promise.all(
9095
allTools.map((t) => {
9196
if (t.isAvailable) {

0 commit comments

Comments
 (0)