Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions src/mcp/tools/firestore/converter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,21 @@ import { logger } from "../../../logger";

/**
* Takes an arbitrary value from a user and returns a FirestoreValue equivalent.
* @param {any} inputValue the JSON object input value.
* return FirestoreValue a firestorevalue object used in the Firestore API.
* @param inputValue the JSON object input value.
* @param key the schema field name driving the conversion (e.g. `reference_value`). When
* set to `reference_value`, the string is treated as a document path/resource name and
* converted to a `referenceValue` rather than a `stringValue`.
* @param projectId the active project id, used to build a full resource name from a
* relative document path when `key === "reference_value"`.
* @param databaseId the active database id (defaults to `(default)`).
* @return a FirestoreValue object used in the Firestore API.
*/
export function convertInputToValue(inputValue: any): FirestoreValue {
export function convertInputToValue(
inputValue: any,
key?: string,
projectId?: string,
databaseId: string = "(default)",
): FirestoreValue {
if (inputValue === null) {
return { nullValue: null };
} else if (typeof inputValue === "boolean") {
Expand All @@ -19,9 +30,19 @@ export function convertInputToValue(inputValue: any): FirestoreValue {
return { doubleValue: inputValue };
}
} else if (typeof inputValue === "string") {
// This is a simplification. In a real-world scenario, you might want to
// check for specific string formats like timestamp, bytes, or referenceValue.
// For now, it defaults to stringValue.
if (key === "reference_value") {
if (inputValue.startsWith("projects/")) {
return { referenceValue: inputValue };
}
if (!projectId) {
throw new Error("projectId is required to convert a relative reference_value path.");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to the repository style guide (line 30), expected user-facing errors should throw a FirebaseError instead of a generic Error. Note that you will need to import FirebaseError from ../../../error.

Suggested change
throw new Error("projectId is required to convert a relative reference_value path.");
throw new FirebaseError("projectId is required to convert a relative reference_value path.");
References
  1. Throw FirebaseError for expected, user-facing errors. (link)

}
const root = `projects/${projectId}/databases/${databaseId}/documents`;
return { referenceValue: `${root}/${inputValue.replace(/^\/+/, "")}` };
}
if (key === "timestamp_value") {
return { timestampValue: inputValue };
}
return { stringValue: inputValue };
} else if (Array.isArray(inputValue)) {
const arrayValue: { values?: FirestoreValue[] } = {
Expand Down
137 changes: 137 additions & 0 deletions src/mcp/tools/firestore/query_collection.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
import { expect } from "chai";
import * as sinon from "sinon";
import { query_collection } from "./query_collection";
import * as firestore from "../../../gcp/firestore";
import { McpContext } from "../../types";

describe("query_collection tool", () => {
const projectId = "test-project";
const ctx = { projectId } as McpContext;

let queryCollectionStub: sinon.SinonStub;

beforeEach(() => {
queryCollectionStub = sinon.stub(firestore, "queryCollection").resolves({ documents: [] });
});

afterEach(() => {
sinon.restore();
});

describe("reference_value filter", () => {
it("expands a relative document path to a full resource name", async () => {
await query_collection.fn(
{
collection_path: "posts",
filters: [
{
field: "author",
op: "EQUAL",
compare_value: { reference_value: "users/abc123" },
},
],
use_emulator: false,
},
ctx,
);

const [, structuredQuery] = queryCollectionStub.firstCall.args;
expect(structuredQuery.where.compositeFilter.filters[0].fieldFilter.value).to.deep.equal({
referenceValue: `projects/${projectId}/databases/(default)/documents/users/abc123`,
});
});

it("respects a non-default database id when expanding the path", async () => {
await query_collection.fn(
{
database: "my-db",
collection_path: "posts",
filters: [
{
field: "author",
op: "EQUAL",
compare_value: { reference_value: "users/abc123" },
},
],
use_emulator: false,
},
ctx,
);

const [, structuredQuery] = queryCollectionStub.firstCall.args;
expect(structuredQuery.where.compositeFilter.filters[0].fieldFilter.value).to.deep.equal({
referenceValue: `projects/${projectId}/databases/my-db/documents/users/abc123`,
});
});

it("strips a leading slash from a relative document path", async () => {
await query_collection.fn(
{
collection_path: "posts",
filters: [
{
field: "author",
op: "EQUAL",
compare_value: { reference_value: "/users/abc123" },
},
],
use_emulator: false,
},
ctx,
);

const [, structuredQuery] = queryCollectionStub.firstCall.args;
expect(structuredQuery.where.compositeFilter.filters[0].fieldFilter.value).to.deep.equal({
referenceValue: `projects/${projectId}/databases/(default)/documents/users/abc123`,
});
});

it("passes through a full resource name unchanged", async () => {
const fullName = "projects/other-project/databases/(default)/documents/users/abc123";
await query_collection.fn(
{
collection_path: "posts",
filters: [
{
field: "author",
op: "EQUAL",
compare_value: { reference_value: fullName },
},
],
use_emulator: false,
},
ctx,
);

const [, structuredQuery] = queryCollectionStub.firstCall.args;
expect(structuredQuery.where.compositeFilter.filters[0].fieldFilter.value).to.deep.equal({
referenceValue: fullName,
});
});
});

describe("timestamp_value filter", () => {
it("encodes the value as a Firestore timestampValue", async () => {
const iso = "2026-05-09T12:34:56Z";
await query_collection.fn(
{
collection_path: "posts",
filters: [
{
field: "publishedAt",
op: "GREATER_THAN",
compare_value: { timestamp_value: iso },
},
],
use_emulator: false,
},
ctx,
);

const [, structuredQuery] = queryCollectionStub.firstCall.args;
expect(structuredQuery.where.compositeFilter.filters[0].fieldFilter.value).to.deep.equal({
timestampValue: iso,
});
});
});
});
19 changes: 17 additions & 2 deletions src/mcp/tools/firestore/query_collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,18 @@ export const query_collection = tool(
.optional()
.describe("The integer value to compare against."),
double_value: z.number().optional().describe("The double value to compare against."),
reference_value: z
.string()
.optional()
.describe(
"A document reference value to compare against. Accepts either a document path (e.g. `users/abc123`) or a full resource name (e.g. `projects/{projectId}/databases/{databaseId}/documents/users/abc123`).",
),
timestamp_value: z
.string()
.optional()
.describe(
"A timestamp value to compare against, in RFC 3339/ISO 8601 format (e.g. `2026-05-09T00:00:00Z`).",
),
})
.describe("One and only one value may be specified per filters object."),
field: z.string().describe("the field searching against"),
Expand Down Expand Up @@ -108,18 +120,21 @@ export const query_collection = tool(
f.compare_value.double_value &&
f.compare_value.integer_value &&
f.compare_value.string_array_value &&
f.compare_value.string_value
f.compare_value.string_value &&
f.compare_value.reference_value &&
f.compare_value.timestamp_value
) {
throw mcpError("One and only one value may be specified per filters object.");
}
const out = Object.entries(f.compare_value).filter(([, value]) => {
return value !== null && value !== undefined;
});
const [key, value] = out[0];
Comment on lines 120 to +132
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The validation logic for ensuring exactly one comparison value is provided is incorrect. The current implementation uses the && operator, which only triggers an error if all fields are present simultaneously, failing to catch cases where two or more fields are provided. Furthermore, if no fields are provided, out[0] will be undefined, leading to a runtime crash during destructuring. A more robust approach is to check the length of the out array, which also simplifies the code and ensures boolean_value (currently missing from the check) is accounted for.

            const out = Object.entries(f.compare_value).filter(([, value]) => {
              return value !== null && value !== undefined;
            });
            if (out.length !== 1) {
              throw mcpError("One and only one value may be specified per filters object.");
            }
            const [key, value] = out[0];

return {
fieldFilter: {
field: { fieldPath: f.field },
op: f.op,
value: convertInputToValue(out[0][1]),
value: convertInputToValue(value, key, projectId, database),
},
};
}),
Expand Down