Python: Add tool name prefixes to FileAccessProvider

python/packages/core/agent_framework/_harness/_file_access.py

@@ -31,7 +31,7 @@
 import unicodedata
 import weakref
 from abc import ABC, abstractmethod
-from collections.abc import Awaitable, Mapping, MutableMapping
+from collections.abc import Awaitable, Callable, Mapping, MutableMapping
 from pathlib import Path
 from typing import Annotated, Any, ClassVar, Final, Protocol, cast
 
@@ -1680,8 +1680,9 @@ class _LineEdit(BaseModel):
             default=None,
             description=(
                 "Optional: the text you believe is currently on that line, as reported by "
-                "file_access_grep. Give the line's own text only: file_access_read_lines prefixes "
-                "each line with its number and a tab, and that prefix is not part of the line. "
+                "`file_access_grep`, which may have the same configured prefix as the enclosing tool. "
+                "Give the line's own text only: `file_access_read_lines`, with the same optional prefix, "
+                "prefixes each line with its number and a tab, and that prefix is not part of the line. "
                 "When supplied, the edit is rejected unless it matches, which catches an "
                 "out-of-date line number or a file that changed since you looked. The trailing "
                 "newline is ignored in the comparison."
@@ -1877,6 +1878,7 @@ def __init__(
         disable_write_tool_approval: bool = False,
         session_scoped: bool = False,
         scope: str | None = None,
+        tool_name_prefix: str | None = None,
     ) -> None:
         """Initialize the file access provider.
 
@@ -1917,15 +1919,21 @@ def __init__(
                 ``session_id`` is used. The value is treated as an opaque
                 key rather than a path: it is mapped onto exactly one folder
                 by :func:`~agent_framework._filesystem._storage_key_segment`.
+            tool_name_prefix: Optional prefix for tool names to avoid collisions.
         """
         super().__init__(source_id)
         self.store = store
-        self.instructions = instructions or DEFAULT_FILE_ACCESS_INSTRUCTIONS
+        self.instructions = instructions or (
+            DEFAULT_FILE_ACCESS_INSTRUCTIONS.replace("file_access_", f"{tool_name_prefix}_file_access_")
+            if tool_name_prefix
+            else DEFAULT_FILE_ACCESS_INSTRUCTIONS
+        )
         self.disable_write_tools = disable_write_tools
         self.disable_readonly_tool_approval = disable_readonly_tool_approval
         self.disable_write_tool_approval = disable_write_tool_approval
         self.session_scoped = session_scoped
         self.scope = scope
+        self.tool_name_prefix = tool_name_prefix
 
     def _resolve_session_key(self, context: SessionContext) -> str:
         """Resolve the working folder key for session-scoped mode.
@@ -2046,6 +2054,27 @@ def all_tools_auto_approval_rule(function_call: Content) -> bool:
             and function_call.name in FileAccessProvider._ALL_TOOL_NAMES
         )
 
+    def _prefixed_tool_name(self, name: str) -> str:
+        return f"{self.tool_name_prefix}_{name}" if self.tool_name_prefix else name
+
+    def get_read_only_tools_auto_approval_rule(self) -> Callable[[Content], bool]:
+        """Create an auto-approval rule for this provider's read-only tool names."""
+        tool_names = frozenset(self._prefixed_tool_name(name) for name in self._READ_ONLY_TOOL_NAMES)
+
+        def rule(function_call: Content) -> bool:
+            return self._is_local_tool_call(function_call) and function_call.name in tool_names
+
+        return rule
+
+    def get_all_tools_auto_approval_rule(self) -> Callable[[Content], bool]:
+        """Create an auto-approval rule for all of this provider's tool names."""
+        tool_names = frozenset(self._prefixed_tool_name(name) for name in self._ALL_TOOL_NAMES)
+
+        def rule(function_call: Content) -> bool:
+            return self._is_local_tool_call(function_call) and function_call.name in tool_names
+
+        return rule
+
     async def before_run(
         self,
         *,
@@ -2071,7 +2100,11 @@ def _session_path(relative_path: str) -> str:
         if session_key:
             instructions += _SESSION_SCOPED_INSTRUCTIONS_SUFFIX
 
-        @tool(name=FileAccessProvider.WRITE_TOOL_NAME, schema=_WriteFileInput, approval_mode=write_approval)
+        @tool(
+            name=self._prefixed_tool_name(FileAccessProvider.WRITE_TOOL_NAME),
+            schema=_WriteFileInput,
+            approval_mode=write_approval,
+        )
         async def file_access_write(file_name: str, content: str, overwrite: bool = False) -> str:
             """Write a file with the given name and content. By default, does not overwrite an existing file unless overwrite is set to true."""  # ruff:ignore[line-too-long]
             try:
@@ -2096,7 +2129,11 @@ async def file_access_write(file_name: str, content: str, overwrite: bool = Fals
                 return f"Could not write file '{file_name}': {exc.strerror or exc}"
             return f"File '{file_name}' written."
 
-        @tool(name=FileAccessProvider.READ_TOOL_NAME, schema=_ReadFileInput, approval_mode=readonly_approval)
+        @tool(
+            name=self._prefixed_tool_name(FileAccessProvider.READ_TOOL_NAME),
+            schema=_ReadFileInput,
+            approval_mode=readonly_approval,
+        )
         async def file_access_read(file_name: str) -> str:
             r"""Read the content of a file by name. Returns the file content or a message indicating the file could not be read. Line numbers count lines split on \n only: a lone \r never starts a new line, each line keeps its own terminator, and content ending in a newline has a final empty line."""  # ruff:ignore[line-too-long]
             try:
@@ -2109,12 +2146,12 @@ async def file_access_read(file_name: str) -> str:
             return content if content is not None else f"File '{file_name}' not found."
 
         @tool(
-            name=FileAccessProvider.READ_LINES_TOOL_NAME,
+            name=self._prefixed_tool_name(FileAccessProvider.READ_LINES_TOOL_NAME),
             schema=_ReadLinesInput,
             approval_mode=readonly_approval,
         )
         async def file_access_read_lines(file_name: str, start_line: int, end_line: int | None = None) -> str:
-            r"""Read part of a file by 1-based inclusive line number; omit end_line to read to the end of the file, and an end_line past the last line is clamped. Each line is prefixed with its number and a tab; everything after that tab is verbatim, including the line's own terminator, so it can be reused as a file_access_replace_lines new_line. Line numbers count lines split on \n only: a lone \r never starts a new line, each line keeps its own terminator, and content ending in a newline has a final empty line."""  # ruff:ignore[line-too-long]
+            r"""Read part of a file by 1-based inclusive line number; omit end_line to read to the end of the file, and an end_line past the last line is clamped. Each line is prefixed with its number and a tab; everything after that tab is verbatim, including the line's own terminator, so it can be reused as a `new_line` value by `file_access_replace_lines`, which may have the same configured prefix as this tool. Line numbers count lines split on \n only: a lone \r never starts a new line, each line keeps its own terminator, and content ending in a newline has a final empty line."""  # ruff:ignore[line-too-long]
             try:
                 normalized = _normalize_relative_path(file_name)
                 content = await self.store.read(_session_path(normalized))
@@ -2128,7 +2165,11 @@ async def file_access_read_lines(file_name: str, start_line: int, end_line: int
             # Each line keeps its terminator, so it doubles as the row separator.
             return "".join(f"{number}\t{line}" for number, line in enumerate(sliced, start_line))
 
-        @tool(name=FileAccessProvider.DELETE_TOOL_NAME, schema=_DeleteFileInput, approval_mode=write_approval)
+        @tool(
+            name=self._prefixed_tool_name(FileAccessProvider.DELETE_TOOL_NAME),
+            schema=_DeleteFileInput,
+            approval_mode=write_approval,
+        )
         async def file_access_delete(file_name: str) -> str:
             """Delete a file by name."""
             try:
@@ -2142,7 +2183,11 @@ async def file_access_delete(file_name: str) -> str:
                 return f"Could not delete file '{file_name}': {exc.strerror or exc}"
             return f"File '{file_name}' deleted." if deleted else f"File '{file_name}' not found."
 
-        @tool(name=FileAccessProvider.LS_TOOL_NAME, schema=_ListInput, approval_mode=readonly_approval)
+        @tool(
+            name=self._prefixed_tool_name(FileAccessProvider.LS_TOOL_NAME),
+            schema=_ListInput,
+            approval_mode=readonly_approval,
+        )
         async def file_access_ls(
             directory: str | None = None,
             glob_pattern: str | None = None,
@@ -2164,7 +2209,11 @@ async def file_access_ls(
                 {"name": entry.name, "type": entry.type} for entry in listed if _matches_glob(entry.name, glob_pattern)
             ]
 
-        @tool(name=FileAccessProvider.REPLACE_TOOL_NAME, schema=_ReplaceInput, approval_mode=write_approval)
+        @tool(
+            name=self._prefixed_tool_name(FileAccessProvider.REPLACE_TOOL_NAME),
+            schema=_ReplaceInput,
+            approval_mode=write_approval,
+        )
         async def file_access_replace(
             file_name: str,
             old_string: str,
@@ -2188,7 +2237,7 @@ async def file_access_replace(
             return f"Replaced {count} occurrence(s) in '{file_name}'."
 
         @tool(
-            name=FileAccessProvider.REPLACE_LINES_TOOL_NAME,
+            name=self._prefixed_tool_name(FileAccessProvider.REPLACE_LINES_TOOL_NAME),
             schema=_ReplaceLinesInput,
             approval_mode=write_approval,
         )
@@ -2209,7 +2258,11 @@ async def file_access_replace_lines(file_name: str, edits: list[_LineEdit]) -> s
                 return f"Could not edit file '{file_name}': {exc.strerror or exc}"
             return f"Replaced {len(edits)} line(s) in '{file_name}'."
 
-        @tool(name=FileAccessProvider.GREP_TOOL_NAME, schema=_SearchFilesInput, approval_mode=readonly_approval)
+        @tool(
+            name=self._prefixed_tool_name(FileAccessProvider.GREP_TOOL_NAME),
+            schema=_SearchFilesInput,
+            approval_mode=readonly_approval,
+        )
         async def file_access_grep(
             regex_pattern: str,
             glob_pattern: str | None = None,
@@ -2225,10 +2278,12 @@ async def file_access_grep(
             or ``"reports/*"`` to restrict the search to the ``reports`` subtree.
             Leave empty or omit to search all files.
             Returns matching results whose file_name values are paths relative to the store root
-            (directly usable with file_access_read), along with snippets and matching lines with line numbers.
+            (directly usable with `file_access_read`, which may have the same configured prefix as this tool),
+            along with snippets and matching lines with line numbers.
             Stores are expected to report each matching line verbatim, including its own line
-            terminator, so it can normally be reused as a file_access_replace_lines new_line; a
-            custom store may not, so prefer file_access_read_lines when the exact text matters.
+            terminator, so it can normally be reused as a `new_line` value by `file_access_replace_lines`
+            with the same optional prefix; a custom store may not, so prefer `file_access_read_lines`
+            with the same optional prefix when the exact text matters.
             Line numbers count lines split on \n only, with a final empty line when content ends in a
             newline.
             The regex_pattern must be 256 characters or fewer.

python/packages/core/tests/core/test_harness_file_access.py

@@ -844,13 +844,15 @@ async def test_filesystem_store_does_not_create_root_until_write(tmp_path: Path)
     assert await store.read("a.txt") == "alpha"
 
 
+@pytest.mark.parametrize("tool_name_prefix", [None, "", "spec_files"])
 async def test_file_access_provider_registers_tools_and_instructions(
     chat_client_base: SupportsChatGetResponse,
+    tool_name_prefix: str | None,
 ) -> None:
-    """``FileAccessProvider.before_run`` should add the canonical instructions and all tools."""
+    """Register all tools with the configured prefix while preserving default names."""
     session = AgentSession(session_id="session-1")
     store = InMemoryAgentFileStore()
-    provider = FileAccessProvider(store=store)
+    provider = FileAccessProvider(store=store, tool_name_prefix=tool_name_prefix)
     agent = Agent(client=chat_client_base, context_providers=[provider])
 
     _, options = await agent._prepare_session_and_messages(  # pyright: ignore[reportPrivateUsage]
@@ -870,13 +872,94 @@ async def test_file_access_provider_registers_tools_and_instructions(
         "file_access_replace",
         "file_access_replace_lines",
     }
-    assert {getattr(tool, "name", None) for tool in tools} >= expected_names
-
+    if tool_name_prefix:
+        expected_names = {f"{tool_name_prefix}_{name}" for name in expected_names}
+    assert {getattr(tool, "name", None) for tool in tools} == expected_names
+
+    expected_instructions = (
+        DEFAULT_FILE_ACCESS_INSTRUCTIONS.replace("file_access_", f"{tool_name_prefix}_file_access_")
+        if tool_name_prefix
+        else DEFAULT_FILE_ACCESS_INSTRUCTIONS
+    )
     instructions = options.get("instructions")
     if isinstance(instructions, str):
-        assert DEFAULT_FILE_ACCESS_INSTRUCTIONS in instructions
+        assert expected_instructions in instructions
     else:
-        assert any(DEFAULT_FILE_ACCESS_INSTRUCTIONS in chunk for chunk in (instructions or []))
+        assert any(expected_instructions in chunk for chunk in (instructions or []))
+
+
+@pytest.mark.parametrize(
+    ("spec_prefix", "sdk_prefix"),
+    [(None, None), ("spec_files", "sdk_files"), ("spec_files", None)],
+    ids=["duplicate-names", "distinct-prefixes", "prefixed-and-default"],
+)
+async def test_file_access_providers_with_multiple_stores(
+    chat_client_base: SupportsChatGetResponse,
+    spec_prefix: str | None,
+    sdk_prefix: str | None,
+) -> None:
+    """Reproduce #9646 and allow an agent run when the stores have distinct tool names."""
+    agent = Agent(
+        client=chat_client_base,
+        context_providers=[
+            FileAccessProvider(
+                InMemoryAgentFileStore(),
+                source_id="spec_files",
+                tool_name_prefix=spec_prefix,
+                disable_readonly_tool_approval=True,
+                disable_write_tool_approval=True,
+            ),
+            FileAccessProvider(
+                InMemoryAgentFileStore(),
+                source_id="sdk_files",
+                tool_name_prefix=sdk_prefix,
+                disable_readonly_tool_approval=True,
+                disable_write_tool_approval=True,
+            ),
+        ],
+    )
+
+    if spec_prefix is None and sdk_prefix is None:
+        with pytest.raises(ValueError, match="Duplicate tool name 'file_access_read'"):
+            await agent.run("test")
+    else:
+        response = await agent.run("test")
+        assert response.text == "test response - test"
+
+
+async def test_file_access_provider_prefixed_tools_route_to_their_own_store(
+    chat_client_base: SupportsChatGetResponse,
+) -> None:
+    """Prefixed tools must keep same-named files in different stores independent."""
+    spec_store = InMemoryAgentFileStore()
+    sdk_store = InMemoryAgentFileStore()
+    agent = Agent(
+        client=chat_client_base,
+        context_providers=[
+            FileAccessProvider(spec_store, source_id="spec_files", tool_name_prefix="spec"),
+            FileAccessProvider(sdk_store, source_id="sdk_files", tool_name_prefix="sdk"),
+        ],
+    )
+    _, options = await agent._prepare_session_and_messages(  # pyright: ignore[reportPrivateUsage]
+        session=AgentSession(session_id="session-1"),
+        input_messages=[Message(role="user", contents=["work with files"])],
+    )
+    tools = options["tools"]
+    assert isinstance(tools, list)
+    assert len(tools) == 16
+
+    for prefix, content in (("spec", "specification"), ("sdk", "implementation")):
+        write = _tool_by_name(tools, f"{prefix}_file_access_write")
+        result = await write.invoke(arguments={"file_name": "shared.txt", "content": content})
+        assert "written" in _text(result[0])
+
+    for prefix, content in (("spec", "specification"), ("sdk", "implementation")):
+        read = _tool_by_name(tools, f"{prefix}_file_access_read")
+        result = await read.invoke(arguments={"file_name": "shared.txt"})
+        assert _text(result[0]) == content
+
+    assert await spec_store.read("shared.txt") == "specification"
+    assert await sdk_store.read("shared.txt") == "implementation"
 
 
 async def test_file_access_provider_all_tools_require_approval(
@@ -979,6 +1062,28 @@ def test_read_only_tools_auto_approval_rule() -> None:
         assert FileAccessProvider.read_only_tools_auto_approval_rule(hosted) is False
 
 
+def test_prefixed_provider_read_only_auto_approval_rule() -> None:
+    """A provider-bound rule should approve only that provider's prefixed read-only tools."""
+    provider = FileAccessProvider(InMemoryAgentFileStore(), tool_name_prefix="spec")
+    rule = provider.get_read_only_tools_auto_approval_rule()
+
+    prefixed_read = Content("function_call", call_id="c1", name="spec_file_access_read", arguments="{}")
+    unprefixed_read = Content("function_call", call_id="c2", name="file_access_read", arguments="{}")
+    prefixed_write = Content("function_call", call_id="c3", name="spec_file_access_write", arguments="{}")
+    hosted_read = Content(
+        "function_call",
+        call_id="c4",
+        name="spec_file_access_read",
+        arguments="{}",
+        hosted=True,
+    )
+
+    assert rule(prefixed_read) is True
+    assert rule(unprefixed_read) is False
+    assert rule(prefixed_write) is False
+    assert rule(hosted_read) is False
+
+
 def test_all_tools_auto_approval_rule() -> None:
     """The all-tools rule approves every file-access tool but nothing else."""
     for name in (
@@ -1007,6 +1112,31 @@ def test_all_tools_auto_approval_rule() -> None:
     assert FileAccessProvider.all_tools_auto_approval_rule(unrelated) is False
 
 
+def test_prefixed_provider_all_tools_auto_approval_rule() -> None:
+    """A provider-bound rule should approve only that provider's prefixed file-access tools."""
+    provider = FileAccessProvider(InMemoryAgentFileStore(), tool_name_prefix="spec")
+    rule = provider.get_all_tools_auto_approval_rule()
+    tool_names = (
+        FileAccessProvider.WRITE_TOOL_NAME,
+        FileAccessProvider.READ_TOOL_NAME,
+        FileAccessProvider.READ_LINES_TOOL_NAME,
+        FileAccessProvider.DELETE_TOOL_NAME,
+        FileAccessProvider.LS_TOOL_NAME,
+        FileAccessProvider.GREP_TOOL_NAME,
+        FileAccessProvider.REPLACE_TOOL_NAME,
+        FileAccessProvider.REPLACE_LINES_TOOL_NAME,
+    )
+
+    for name in tool_names:
+        prefixed = Content("function_call", call_id="c1", name=f"spec_{name}", arguments="{}")
+        unprefixed = Content("function_call", call_id="c2", name=name, arguments="{}")
+        hosted = Content("function_call", call_id="c3", name=f"spec_{name}", arguments="{}", hosted=True)
+
+        assert rule(prefixed) is True
+        assert rule(unprefixed) is False
+        assert rule(hosted) is False
+
+
 async def test_file_access_provider_tools_round_trip_files(
     chat_client_base: SupportsChatGetResponse,
 ) -> None: