fix(vector_stores): stop _get_vector_store_ids_from_tool_calls sharing and mutating its default list

Same defect class as the registry constructors, one method further down the file.
_get_vector_store_ids_from_tool_calls took `vector_store_ids: List[str] = []` and
extended it in place, so the def-time list would accumulate ids across every call
that omitted the argument and then return them as if they came from the request.

The single caller passes the argument explicitly, so nothing leaks today; this is
a latent landmine rather than an active bug. Fixing it alongside the constructors
keeps the file free of the pattern instead of leaving one instance for the next
caller to trip over.

Rebuilt as a single comprehension so it neither seeds a mutable default nor writes
back into a caller-owned list. Order is unchanged: seeded ids first, then ids
discovered in tool calls.
This commit is contained in:
Sarveswaran MG 2026-07-22 18:33:17 +05:30
parent 8040927673
commit 22417073de
2 changed files with 27 additions and 6 deletions

View file

@ -376,16 +376,15 @@ class VectorStoreRegistry:
return vector_stores_to_run
def _get_vector_store_ids_from_tool_calls(
self, tools: Optional[List[Dict]] = None, vector_store_ids: List[str] = []
self, tools: Optional[List[Dict]] = None, vector_store_ids: Optional[List[str]] = None
) -> List[str]:
"""
Returns the vector store ids from the tool calls
"""
if tools:
for tool in tools:
if "vector_store_ids" in tool:
vector_store_ids.extend(tool["vector_store_ids"])
return vector_store_ids
return [
*(vector_store_ids or ()),
*(vs_id for tool in (tools or ()) if "vector_store_ids" in tool for vs_id in tool["vector_store_ids"]),
]
def load_vector_stores_from_config(self, vector_stores_config: List[Dict]):
"""

View file

@ -279,3 +279,25 @@ def test_delete_vector_store_index_removes_by_name():
registry.delete_vector_store_index("idx_does_not_exist")
assert [index.index_name for index in registry.vector_store_indexes] == ["idx_b"]
def test_get_vector_store_ids_from_tool_calls_does_not_share_or_mutate():
"""
Regression test: _get_vector_store_ids_from_tool_calls took `vector_store_ids: List[str] = []`
and extended it in place, so the def-time default accumulated ids across every call that
omitted the argument, and a caller-supplied list was mutated behind the caller's back.
"""
registry = VectorStoreRegistry()
tools = [{"vector_store_ids": ["vs_a"]}]
first = registry._get_vector_store_ids_from_tool_calls(tools=tools)
second = registry._get_vector_store_ids_from_tool_calls(tools=tools)
assert first == ["vs_a"]
assert second == ["vs_a"]
caller_ids = ["vs_seed"]
result = registry._get_vector_store_ids_from_tool_calls(tools=tools, vector_store_ids=caller_ids)
assert result == ["vs_seed", "vs_a"]
assert caller_ids == ["vs_seed"]