feat(mcp): Add support for filtering on reference_value and timestamp_value in firestore_query_collection#10481
feat(mcp): Add support for filtering on reference_value and timestamp_value in firestore_query_collection#10481MrAlek wants to merge 2 commits intofirebase:mainfrom
Conversation
Adds a `reference_value` field to the `compare_value` schema so MCP clients can filter Firestore collections by document reference. Relative document paths are expanded to a full resource name using the active project and database; full resource names are passed through unchanged.
Adds a `timestamp_value` field to the `compare_value` schema, encoded as a Firestore `timestampValue` (RFC 3339/ISO 8601) so MCP clients can filter collections by date/time fields.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enhances the Firestore tool by adding support for reference_value and timestamp_value filters in the query_collection tool, including automatic expansion of relative document paths to full resource names. Feedback highlights a critical bug in the filter validation logic that could lead to runtime crashes or incorrect filtering when multiple values are provided. Additionally, a violation of the repository style guide was noted regarding the use of generic errors instead of FirebaseError for user-facing exceptions.
| 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]; |
There was a problem hiding this comment.
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 { referenceValue: inputValue }; | ||
| } | ||
| if (!projectId) { | ||
| throw new Error("projectId is required to convert a relative reference_value path."); |
There was a problem hiding this comment.
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.
| 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
- Throw FirebaseError for expected, user-facing errors. (link)
2 participants
Description
Adds the ability to filter on reference values and timestamps in the firestore MCP.
Scenarios Tested
Tested through MCP Inspector.
Sample Commands