diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index ad1f215f51..7b5581e419 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -98,7 +98,7 @@ agent_framework/ - **`InMemoryAgentFileStore`** - Dict-backed store suitable for tests and lightweight scenarios. - **`FileSystemAgentFileStore`** - Disk-backed store rooted under a configurable directory. Enforces relative-path normalization, root containment, and rejects symlink/reparse-point segments to prevent escape. - **`FileSearchResult`** / **`FileSearchMatch`** - `SerializationMixin` DTOs returned by `search_files`, carrying the matching file name, a context snippet, and the matching lines with 1-based line numbers. -- **`FileAccessProvider`** - `ContextProvider` that adds shared file-access tools (`file_access_save_file`, `file_access_read_file`, `file_access_delete_file`, `file_access_list_files`, `file_access_list_subdirectories`, `file_access_search_files`) plus default usage instructions to each invocation. `file_access_list_files`/`file_access_list_subdirectories` enumerate direct children (files / subdirectories) so the agent can walk the tree level by level; `file_access_search_files` searches recursively from the store root and returns store-root-relative `file_name` paths, scoped via an `fnmatch` glob (where `*` crosses `/`, e.g. `*.md`, `reports/*`). Unlike `MemoryContextProvider`, the store is intentionally shared across sessions and agents. +- **`FileAccessProvider`** - `ContextProvider` that adds shared file-access tools (`file_access_save_file`, `file_access_read_file`, `file_access_delete_file`, `file_access_list_files`, `file_access_list_subdirectories`, `file_access_search_files`) plus default usage instructions to each invocation. `file_access_list_files`/`file_access_list_subdirectories` enumerate direct children (files / subdirectories) so the agent can walk the tree level by level; `file_access_search_files` searches recursively from the store root and returns store-root-relative `file_name` paths, scoped via an `fnmatch` glob (where `*` crosses `/`, e.g. `*.md`, `reports/*`). All six tools are registered with `approval_mode="always_require"`, so every file operation needs host approval. To run unattended, pass one of the static auto-approval rules to `ToolApprovalMiddleware` (via `auto_approval_rules`): `FileAccessProvider.read_only_tools_auto_approval_rule` approves only the read-only tools (read, list files, list subdirectories, search), while `FileAccessProvider.all_tools_auto_approval_rule` approves every file-access tool including save and delete. Both rules reject any call carrying a `server_label` so they stay scoped to this provider's local tools and never auto-approve a same-named hosted tool. The tool names are also exposed as class constants (`SAVE_FILE_TOOL_NAME`, `READ_FILE_TOOL_NAME`, `DELETE_FILE_TOOL_NAME`, `LIST_FILES_TOOL_NAME`, `LIST_SUBDIRECTORIES_TOOL_NAME`, `SEARCH_FILES_TOOL_NAME`). Unlike `MemoryContextProvider`, the store is intentionally shared across sessions and agents. ### File Memory Harness (`_harness/_file_memory.py`) diff --git a/python/packages/core/agent_framework/_harness/_file_access.py b/python/packages/core/agent_framework/_harness/_file_access.py index a78209d442..c5f2ed92b0 100644 --- a/python/packages/core/agent_framework/_harness/_file_access.py +++ b/python/packages/core/agent_framework/_harness/_file_access.py @@ -37,7 +37,8 @@ from .._feature_stage import ExperimentalFeature, experimental from .._serialization import SerializationMixin from .._sessions import AgentSession, ContextProvider, SessionContext -from .._tools import ApprovalMode, tool +from .._tools import tool +from .._types import Content logger = logging.getLogger(__name__) @@ -1075,15 +1076,72 @@ class FileAccessProvider(ContextProvider): contents are visible across sessions and agents. The store is passed in by the caller and should already be scoped to the desired folder or storage location. + + All six tools always require approval: each is registered with + ``approval_mode="always_require"`` so the host must approve every file + operation the model proposes. In the auto-invocation flow this means the + model's calls to these tools are converted into + ``function_approval_request`` items and the tool does **not** execute until + the host supplies a matching ``function_approval_response``. Consumers that + use the base agent directly must install + :class:`~agent_framework.ToolApprovalMiddleware` (or use + :func:`~agent_framework.create_harness_agent`, which wires it in by default) + to drive that handshake; otherwise these tools never run. To run unattended, + supply one of the static auto-approval rules to + :class:`~agent_framework.ToolApprovalMiddleware` via its + ``auto_approval_rules``: + + - :meth:`read_only_tools_auto_approval_rule` — auto-approves only the + read-only tools (read, list files, list subdirectories, search), while + still prompting for the tools that modify the store (save and delete). + - :meth:`all_tools_auto_approval_rule` — auto-approves every file-access + tool, including save and delete. + + For example, to auto-approve only the read-only tools:: + + create_harness_agent( + chat_client, + auto_approval_rules=[FileAccessProvider.read_only_tools_auto_approval_rule], + ) """ + #: Name of the tool that saves a file. + SAVE_FILE_TOOL_NAME = "file_access_save_file" + #: Name of the tool that reads a file. + READ_FILE_TOOL_NAME = "file_access_read_file" + #: Name of the tool that deletes a file. + DELETE_FILE_TOOL_NAME = "file_access_delete_file" + #: Name of the tool that lists the files in a directory. + LIST_FILES_TOOL_NAME = "file_access_list_files" + #: Name of the tool that lists the subdirectories of a directory. + LIST_SUBDIRECTORIES_TOOL_NAME = "file_access_list_subdirectories" + #: Name of the tool that searches file contents. + SEARCH_FILES_TOOL_NAME = "file_access_search_files" + + #: Names of the tools that only read from (never modify) the file store. + _READ_ONLY_TOOL_NAMES: frozenset[str] = frozenset({ + READ_FILE_TOOL_NAME, + LIST_FILES_TOOL_NAME, + LIST_SUBDIRECTORIES_TOOL_NAME, + SEARCH_FILES_TOOL_NAME, + }) + + #: Names of all tools exposed by this provider. + _ALL_TOOL_NAMES: frozenset[str] = frozenset({ + SAVE_FILE_TOOL_NAME, + READ_FILE_TOOL_NAME, + DELETE_FILE_TOOL_NAME, + LIST_FILES_TOOL_NAME, + LIST_SUBDIRECTORIES_TOOL_NAME, + SEARCH_FILES_TOOL_NAME, + }) + def __init__( self, store: AgentFileStore, *, source_id: str = DEFAULT_FILE_ACCESS_SOURCE_ID, instructions: str | None = None, - require_delete_approval: bool = True, ) -> None: """Initialize the file access provider. @@ -1096,17 +1154,78 @@ def __init__( source_id: Unique source ID for the provider. instructions: Optional instruction override. When ``None`` the default file-access instructions are used. - require_delete_approval: When ``True`` (the default) the - ``file_access_delete_file`` tool is registered with - ``approval_mode="always_require"`` so the host must approve every - delete the model proposes. Set to ``False`` to opt out and allow - the agent to delete files autonomously (matching the .NET - ``FileAccessProvider``, which has no approval mechanism). """ super().__init__(source_id) self.store = store self.instructions = instructions or DEFAULT_FILE_ACCESS_INSTRUCTIONS - self.require_delete_approval = require_delete_approval + + @staticmethod + def _is_local_tool_call(function_call: Content) -> bool: + """Return whether a function call targets this provider's local tools. + + Hosted-tool calls carry a ``server_label`` in their + ``additional_properties`` and are a separate server-scoped approval + boundary that must be passed through untouched (see + :func:`agent_framework._tools._is_hosted_tool_approval`). These rules + only ever auto-approve the provider's own local tools, so any call that + carries a ``server_label`` is rejected even if its name collides with a + file-access tool name. + """ + return not function_call.additional_properties.get("server_label") + + @staticmethod + def read_only_tools_auto_approval_rule(function_call: Content) -> bool: + """Auto-approval rule that approves only the read-only file-access tools. + + The tools exposed by :class:`FileAccessProvider` always require approval. + Pass this rule to :class:`~agent_framework.ToolApprovalMiddleware` (via + ``auto_approval_rules``) to automatically approve the tools that read + from the store (``file_access_read_file``, ``file_access_list_files``, + ``file_access_list_subdirectories``, and ``file_access_search_files``), + while still prompting for the tools that modify it + (``file_access_save_file`` and ``file_access_delete_file``). + + Hosted-tool calls (those carrying a ``server_label``) are never + auto-approved, even when their name matches a file-access tool, so the + rule stays scoped to this provider's local tools. + + Args: + function_call: The pending ``function_call`` content. + + Returns: + ``True`` for read-only file-access tools, ``False`` otherwise so that + subsequent rules continue to be evaluated. + """ + return ( + FileAccessProvider._is_local_tool_call(function_call) + and function_call.name in FileAccessProvider._READ_ONLY_TOOL_NAMES + ) + + @staticmethod + def all_tools_auto_approval_rule(function_call: Content) -> bool: + """Auto-approval rule that approves every file-access tool. + + The tools exposed by :class:`FileAccessProvider` always require approval. + Pass this rule to :class:`~agent_framework.ToolApprovalMiddleware` (via + ``auto_approval_rules``) to automatically approve every file-access tool, + including the tools that modify the store (``file_access_save_file`` and + ``file_access_delete_file``). + + Hosted-tool calls (those carrying a ``server_label``) are never + auto-approved, even when their name matches a file-access tool, so the + rule stays scoped to this provider's local tools. + + Args: + function_call: The pending ``function_call`` content. + + Returns: + ``True`` for any file-access tool, ``False`` otherwise so that + subsequent rules continue to be evaluated. + """ + return ( + FileAccessProvider._is_local_tool_call(function_call) + and function_call.name in FileAccessProvider._ALL_TOOL_NAMES + ) async def before_run( self, @@ -1118,7 +1237,7 @@ async def before_run( ) -> None: """Inject file-access tools and instructions before the model runs.""" - @tool(name="file_access_save_file", schema=_SaveFileInput, approval_mode="never_require") + @tool(name=FileAccessProvider.SAVE_FILE_TOOL_NAME, schema=_SaveFileInput, approval_mode="always_require") async def file_access_save_file(file_name: str, content: str, overwrite: bool = False) -> str: """Save a file with the given name and content. By default, does not overwrite an existing file unless overwrite is set to true.""" # noqa: E501 try: @@ -1132,7 +1251,7 @@ async def file_access_save_file(file_name: str, content: str, overwrite: bool = return f"Could not save file '{file_name}': {exc.strerror or exc}" return f"File '{file_name}' saved." - @tool(name="file_access_read_file", schema=_ReadFileInput, approval_mode="never_require") + @tool(name=FileAccessProvider.READ_FILE_TOOL_NAME, schema=_ReadFileInput, approval_mode="always_require") async def file_access_read_file(file_name: str) -> str: """Read the content of a file by name. Returns the file content or a message indicating the file could not be read.""" # noqa: E501 try: @@ -1144,9 +1263,7 @@ async def file_access_read_file(file_name: str) -> str: return f"Could not read file '{file_name}': {exc.strerror or exc}" return content if content is not None else f"File '{file_name}' not found." - delete_approval_mode: ApprovalMode = "always_require" if self.require_delete_approval else "never_require" - - @tool(name="file_access_delete_file", schema=_DeleteFileInput, approval_mode=delete_approval_mode) + @tool(name=FileAccessProvider.DELETE_FILE_TOOL_NAME, schema=_DeleteFileInput, approval_mode="always_require") async def file_access_delete_file(file_name: str) -> str: """Delete a file by name.""" try: @@ -1158,7 +1275,7 @@ async def file_access_delete_file(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="file_access_list_files", schema=_ListFilesInput, approval_mode="never_require") + @tool(name=FileAccessProvider.LIST_FILES_TOOL_NAME, schema=_ListFilesInput, approval_mode="always_require") async def file_access_list_files(directory: str | None = None) -> list[str] | str: """List the direct child file names of a directory. Omit ``directory`` (or pass an empty string) to list the root. To enumerate files in a subdirectory, pass its relative path, for example ``"reports"`` or ``"reports/2024"``.""" # noqa: E501 target = directory if directory and directory.strip() else "" @@ -1169,7 +1286,11 @@ async def file_access_list_files(directory: str | None = None) -> list[str] | st except OSError as exc: return f"Could not list directory '{directory or ''}': {exc.strerror or exc}" - @tool(name="file_access_list_subdirectories", schema=_ListSubdirectoriesInput, approval_mode="never_require") + @tool( + name=FileAccessProvider.LIST_SUBDIRECTORIES_TOOL_NAME, + schema=_ListSubdirectoriesInput, + approval_mode="always_require", + ) async def file_access_list_subdirectories(directory: str | None = None) -> list[str] | str: """List the direct child subdirectory names of a directory. @@ -1186,7 +1307,7 @@ async def file_access_list_subdirectories(directory: str | None = None) -> list[ except OSError as exc: return f"Could not list directory '{directory or ''}': {exc.strerror or exc}" - @tool(name="file_access_search_files", schema=_SearchFilesInput, approval_mode="never_require") + @tool(name=FileAccessProvider.SEARCH_FILES_TOOL_NAME, schema=_SearchFilesInput, approval_mode="always_require") async def file_access_search_files( regex_pattern: str, file_pattern: str | None = None, diff --git a/python/packages/core/tests/core/test_harness_file_access.py b/python/packages/core/tests/core/test_harness_file_access.py index 4c50c8ae0d..34c03bf489 100644 --- a/python/packages/core/tests/core/test_harness_file_access.py +++ b/python/packages/core/tests/core/test_harness_file_access.py @@ -497,10 +497,10 @@ async def test_file_access_provider_registers_tools_and_instructions( assert any(DEFAULT_FILE_ACCESS_INSTRUCTIONS in chunk for chunk in (instructions or [])) -async def test_file_access_provider_delete_approval_defaults_to_always_require( +async def test_file_access_provider_all_tools_require_approval( chat_client_base: SupportsChatGetResponse, ) -> None: - """By default ``file_access_delete_file`` should require host approval.""" + """Every file-access tool should require host approval.""" session = AgentSession(session_id="session-1") provider = FileAccessProvider(store=InMemoryAgentFileStore()) agent = Agent(client=chat_client_base, context_providers=[provider]) @@ -512,36 +512,72 @@ async def test_file_access_provider_delete_approval_defaults_to_always_require( tools = options["tools"] assert isinstance(tools, list) - delete_file = _tool_by_name(tools, "file_access_delete_file") - assert delete_file.approval_mode == "always_require" - # The non-destructive tools should remain autonomous. for name in ( - "file_access_save_file", - "file_access_read_file", - "file_access_list_files", - "file_access_list_subdirectories", - "file_access_search_files", + FileAccessProvider.SAVE_FILE_TOOL_NAME, + FileAccessProvider.READ_FILE_TOOL_NAME, + FileAccessProvider.DELETE_FILE_TOOL_NAME, + FileAccessProvider.LIST_FILES_TOOL_NAME, + FileAccessProvider.LIST_SUBDIRECTORIES_TOOL_NAME, + FileAccessProvider.SEARCH_FILES_TOOL_NAME, ): - assert _tool_by_name(tools, name).approval_mode == "never_require" + assert _tool_by_name(tools, name).approval_mode == "always_require" -async def test_file_access_provider_delete_approval_opt_out( - chat_client_base: SupportsChatGetResponse, -) -> None: - """``require_delete_approval=False`` should drop delete to ``never_require``.""" - session = AgentSession(session_id="session-1") - provider = FileAccessProvider(store=InMemoryAgentFileStore(), require_delete_approval=False) - agent = Agent(client=chat_client_base, context_providers=[provider]) - - _, options = await agent._prepare_session_and_messages( # pyright: ignore[reportPrivateUsage] - session=session, - input_messages=[Message(role="user", contents=["work with files"])], - ) - - tools = options["tools"] - assert isinstance(tools, list) - delete_file = _tool_by_name(tools, "file_access_delete_file") - assert delete_file.approval_mode == "never_require" +def test_read_only_tools_auto_approval_rule() -> None: + """The read-only rule approves only the non-mutating tools.""" + approved = { + FileAccessProvider.READ_FILE_TOOL_NAME, + FileAccessProvider.LIST_FILES_TOOL_NAME, + FileAccessProvider.LIST_SUBDIRECTORIES_TOOL_NAME, + FileAccessProvider.SEARCH_FILES_TOOL_NAME, + } + rejected = { + FileAccessProvider.SAVE_FILE_TOOL_NAME, + FileAccessProvider.DELETE_FILE_TOOL_NAME, + "some_other_tool", + } + for name in approved: + call = Content("function_call", call_id="c1", name=name, arguments="{}") + assert FileAccessProvider.read_only_tools_auto_approval_rule(call) is True + for name in rejected: + call = Content("function_call", call_id="c1", name=name, arguments="{}") + assert FileAccessProvider.read_only_tools_auto_approval_rule(call) is False + # A hosted tool with the same name (carrying a server_label) is NOT auto-approved. + for name in approved: + hosted = Content( + "function_call", + call_id="c1", + name=name, + arguments="{}", + additional_properties={"server_label": "remote"}, + ) + assert FileAccessProvider.read_only_tools_auto_approval_rule(hosted) 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 ( + FileAccessProvider.SAVE_FILE_TOOL_NAME, + FileAccessProvider.READ_FILE_TOOL_NAME, + FileAccessProvider.DELETE_FILE_TOOL_NAME, + FileAccessProvider.LIST_FILES_TOOL_NAME, + FileAccessProvider.LIST_SUBDIRECTORIES_TOOL_NAME, + FileAccessProvider.SEARCH_FILES_TOOL_NAME, + ): + call = Content("function_call", call_id="c1", name=name, arguments="{}") + assert FileAccessProvider.all_tools_auto_approval_rule(call) is True + # A hosted tool with the same name (carrying a server_label) is NOT auto-approved. + hosted = Content( + "function_call", + call_id="c1", + name=name, + arguments="{}", + additional_properties={"server_label": "remote"}, + ) + assert FileAccessProvider.all_tools_auto_approval_rule(hosted) is False + + unrelated = Content("function_call", call_id="c1", name="some_other_tool", arguments="{}") + assert FileAccessProvider.all_tools_auto_approval_rule(unrelated) is False async def test_file_access_provider_tools_round_trip_files( diff --git a/python/samples/02-agents/context_providers/file_access_data_processing/data_processing.py b/python/samples/02-agents/context_providers/file_access_data_processing/data_processing.py index aac04f6ea1..49624d8eda 100644 --- a/python/samples/02-agents/context_providers/file_access_data_processing/data_processing.py +++ b/python/samples/02-agents/context_providers/file_access_data_processing/data_processing.py @@ -7,6 +7,12 @@ data, perform analysis, and write summary output back to the same folder via the ``file_access_*`` tools. +The file-access tools all require approval (``approval_mode="always_require"``), +so a base ``Agent`` installs :class:`ToolApprovalMiddleware` to drive the +approval handshake. Because this sample is non-interactive, it auto-approves +every file-access tool via +:meth:`FileAccessProvider.all_tools_auto_approval_rule`. + The sibling ``working/`` folder contains ``sales.csv`` — ~50 rows of sales transactions (date, product, category, quantity, unit_price, region, salesperson). The agent is asked, in a single session, to: list available @@ -22,7 +28,7 @@ import os from pathlib import Path -from agent_framework import Agent, FileAccessProvider, FileSystemAgentFileStore +from agent_framework import Agent, FileAccessProvider, FileSystemAgentFileStore, ToolApprovalMiddleware from agent_framework.foundry import FoundryChatClient from azure.identity import AzureCliCredential from dotenv import load_dotenv @@ -92,13 +98,19 @@ async def main() -> None: # agent for the duration of each run. file_access = FileAccessProvider(store=FileSystemAgentFileStore(working_dir)) - # 4. Create the agent and attach the provider. + # 4. Create the agent and attach the provider. The file-access tools all + # require approval (approval_mode="always_require"). Developers can + # present these to the user for approval, or like in this case, auto-approve + # them via FileAccessProvider.all_tools_auto_approval_rule. Note that + # to use tool approval rules, the agent must have ToolApprovalMiddleware + # in its middleware stack. async with Agent( client=client, name="DataAnalyst", description="A data analyst assistant that reads, analyzes, and processes data files.", instructions=INSTRUCTIONS, context_providers=[file_access], + middleware=[ToolApprovalMiddleware(auto_approval_rules=[FileAccessProvider.all_tools_auto_approval_rule])], ) as agent: # 5. Run all prompts inside one session so the conversation remains # coherent across turns. diff --git a/python/samples/02-agents/harness/README.md b/python/samples/02-agents/harness/README.md index bd2af5b7ed..410bd34759 100644 --- a/python/samples/02-agents/harness/README.md +++ b/python/samples/02-agents/harness/README.md @@ -29,6 +29,7 @@ Each feature can be disabled or customized via keyword arguments. | File | Description | |------|-------------| | `harness_research.py` | Interactive research assistant with web search, a plan/execute workflow, and an execute-mode loop that re-invokes the agent until every todo is complete | +| `harness_data_processing.py` | Data-processing assistant over a folder of CSV files, demonstrating file-access tools and tool approval | ## Running @@ -40,10 +41,30 @@ export FOUNDRY_MODEL="your-model-deployment-name" # Authenticate with Azure (required for AzureCliCredential) az login -# Run the research sample -python samples/02-agents/harness/harness_research.py +# Run a sample against the released agent-framework (PEP 723 isolated env) +uv run samples/02-agents/harness/harness_research.py ``` +### Running against the local repo + +To run a sample against your **local** `agent-framework` checkout (so it picks +up uncommitted changes), use the workspace environment instead of the isolated +PEP 723 env. From the `python/` directory, run the script with `uv run python` +and add the `textual` UI dependency the harness console needs: + +```bash +uv run --with textual python samples/02-agents/harness/harness_research.py +uv run --with textual python samples/02-agents/harness/harness_data_processing.py +``` + +The workspace environment already provides the editable `agent-framework` +packages plus the samples' other dependencies (`rich`, `python-dotenv`, +`azure-identity`); only `textual` needs to be supplied with `--with`. + +> Note: invoking `uv run python