From 5e3150ada76062f9ed450d877bfa343bb44ca3f0 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Mon, 13 Jul 2026 15:47:57 +0000 Subject: [PATCH] Switch FileAcessProvider on Harness to opt-in --- python/packages/core/AGENTS.md | 2 +- .../core/agent_framework/_harness/_agent.py | 31 ++++++-------- .../core/agent_framework/_harness/_agent.pyi | 1 - .../core/tests/core/test_harness_agent.py | 40 +++++++++---------- .../harness/build_your_own_claw/README.md | 4 +- 5 files changed, 36 insertions(+), 42 deletions(-) diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 31124b70dff..c36817add01 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -125,7 +125,7 @@ agent_framework/ - **Scoping** - Memories are isolated per session by default: each session writes under a working folder derived from `context.session_id`. Pass an explicit `scope` (e.g. a user id) to group memories across sessions, mirroring `FoundryMemoryProvider`'s `scope` arg. - **Descriptions & index** - `file_memory_write` accepts an optional `description`, stored in a companion `_description.md` sidecar. After each write/delete the provider rebuilds a capped (50-entry) `memories.md` index, and `before_run` injects that index as a `user` context message so the model knows what memories exist. Sidecars and the index are internal files hidden from `file_memory_ls`/`file_memory_grep` and rejected as write targets. - **`DEFAULT_FILE_MEMORY_SOURCE_ID`** / **`DEFAULT_FILE_MEMORY_INSTRUCTIONS`** - Public defaults for the provider's source id and instruction banner. -- **Harness wiring** - `create_harness_agent` includes both `FileMemoryProvider` and `FileAccessProvider` by default. Disable via `disable_file_memory` / `disable_file_access`; override the backing store via `file_memory_store` / `file_access_store`. When no store is supplied, defaults are `FileSystemAgentFileStore` rooted at `{cwd}/agent-file-memory` (memory) and `{cwd}/working` (access), mirroring the .NET `HarnessAgent`. `create_harness_agent` also wires in `MessageInjectionMiddleware` by default (mirroring the .NET harness's `UseMessageInjection`); it is always on with no opt-out because it is a no-op when no messages are queued for the session. +- **Harness wiring** - `create_harness_agent` includes the `FileMemoryProvider` by default; the `FileAccessProvider` is opt-in and added only when a `file_access_store` is supplied (no implicit `{cwd}/working` store is created). Disable file memory via `disable_file_memory`; override its backing store via `file_memory_store`. When no file-memory store is supplied, the default is `FileSystemAgentFileStore` rooted at `{cwd}/agent-file-memory`. `create_harness_agent` also wires in `MessageInjectionMiddleware` by default (mirroring the .NET harness's `UseMessageInjection`); it is always on with no opt-out because it is a no-op when no messages are queued for the session. ### Tool Approval Harness (`_harness/_tool_approval.py`) diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index 603d822b4db..a29472e1b82 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -151,7 +151,6 @@ def _assemble_context_providers( mode_provider: AgentModeProvider | None, disable_file_memory: bool, file_memory_store: AgentFileStore | None, - disable_file_access: bool, file_access_store: AgentFileStore | None, file_access_disable_write_tools: bool, file_access_disable_readonly_tool_approval: bool, @@ -186,12 +185,11 @@ def _assemble_context_providers( memory_store = file_memory_store or FileSystemAgentFileStore(Path.cwd() / "agent-file-memory") providers.append(FileMemoryProvider(memory_store)) - # Shared file access (on by default). Default store is rooted at ``{cwd}/working``. - if not disable_file_access: - access_store = file_access_store or FileSystemAgentFileStore(Path.cwd() / "working") + # Shared file access (opt-in). Only added when a store is supplied. + if file_access_store is not None: providers.append( FileAccessProvider( - access_store, + file_access_store, disable_write_tools=file_access_disable_write_tools, disable_readonly_tool_approval=file_access_disable_readonly_tool_approval, disable_write_tool_approval=file_access_disable_write_tool_approval, @@ -295,7 +293,6 @@ def create_harness_agent( mode_provider: AgentModeProvider | None = None, disable_file_memory: bool = False, file_memory_store: AgentFileStore | None = None, - disable_file_access: bool = False, file_access_store: AgentFileStore | None = None, file_access_disable_write_tools: bool = False, file_access_disable_readonly_tool_approval: bool = False, @@ -327,7 +324,7 @@ def create_harness_agent( - **TodoProvider** — todo list management - **AgentModeProvider** — plan/execute mode tracking - **FileMemoryProvider** — file-based session memory (on by default) - - **FileAccessProvider** — shared file read/write tools (on by default) + - **FileAccessProvider** — shared file read/write tools (opt-in via ``file_access_store``) - **SkillsProvider** — skill discovery and progressive loading - **BackgroundAgentsProvider** — delegate work to background sub-agents - **Tool approval** — "don't ask again" standing approval rules plus heuristic @@ -408,23 +405,22 @@ def create_harness_agent( file_memory_store: Custom AgentFileStore backing the FileMemoryProvider. When None (and disable_file_memory is False), a FileSystemAgentFileStore rooted at ``{cwd}/agent-file-memory`` is created. Ignored when disable_file_memory is True. - disable_file_access: When True, skip the FileAccessProvider. When False (default), - a FileAccessProvider is added, giving the agent shared read/write file tools. - file_access_store: Custom AgentFileStore backing the FileAccessProvider. When None - (and disable_file_access is False), a FileSystemAgentFileStore rooted at - ``{cwd}/working`` is created. Ignored when disable_file_access is True. + file_access_store: AgentFileStore backing the FileAccessProvider. File access is + opt-in: when None (default), no FileAccessProvider is added and the agent has no + file access tools. When set, a FileAccessProvider is added, giving the agent shared + read/write file tools backed by the supplied store. file_access_disable_write_tools: When True, the FileAccessProvider advertises only its read-only tools (read, ls, grep); the write tools (write, delete, replace, - replace_lines) are hidden. When False (default), all tools are advertised. Ignored - when disable_file_access is True. + replace_lines) are hidden. When False (default), all tools are advertised. Only + used when file_access_store is set. file_access_disable_readonly_tool_approval: When True, the FileAccessProvider's read-only tools (read, ls, grep) are registered with ``approval_mode="never_require"`` so they - run without host approval. When False (default), they require approval. Ignored when - disable_file_access is True. + run without host approval. When False (default), they require approval. Only used when + file_access_store is set. file_access_disable_write_tool_approval: When True, the FileAccessProvider's write tools (write, delete, replace, replace_lines) are registered with ``approval_mode="never_require"`` so they run without host approval. When False - (default), they require approval. Ignored when disable_file_access is True. + (default), they require approval. Only used when file_access_store is set. skills_provider: Custom SkillsProvider instance for code-defined skills. Can be combined with ``skills_paths`` to aggregate file and code-based skills. **Security:** if the provider is configured with an external skill source (e.g. @@ -539,7 +535,6 @@ def create_harness_agent( mode_provider=mode_provider, disable_file_memory=disable_file_memory, file_memory_store=file_memory_store, - disable_file_access=disable_file_access, file_access_store=file_access_store, file_access_disable_write_tools=file_access_disable_write_tools, file_access_disable_readonly_tool_approval=file_access_disable_readonly_tool_approval, diff --git a/python/packages/core/agent_framework/_harness/_agent.pyi b/python/packages/core/agent_framework/_harness/_agent.pyi index a63fbd14fa5..3a9906e917b 100644 --- a/python/packages/core/agent_framework/_harness/_agent.pyi +++ b/python/packages/core/agent_framework/_harness/_agent.pyi @@ -68,7 +68,6 @@ def create_harness_agent( mode_provider: AgentModeProvider | None = None, disable_file_memory: bool = False, file_memory_store: AgentFileStore | None = None, - disable_file_access: bool = False, file_access_store: AgentFileStore | None = None, file_access_disable_write_tools: bool = False, file_access_disable_readonly_tool_approval: bool = False, diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index 19cb5a006b6..ecb0c9025fa 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -88,7 +88,7 @@ def test_create_harness_agent_with_defaults() -> None: def test_create_harness_agent_includes_all_default_providers(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: - """Default assembly should include history, compaction, todo, mode, file memory, and file access.""" + """Default assembly should include history, compaction, todo, mode, and file memory.""" monkeypatch.chdir(tmp_path) agent = create_harness_agent( client=_FakeChatClient(), @@ -103,7 +103,6 @@ def test_create_harness_agent_includes_all_default_providers(tmp_path: Path, mon assert TodoProvider in provider_types assert AgentModeProvider in provider_types assert FileMemoryProvider in provider_types - assert FileAccessProvider in provider_types assert SkillsProvider not in provider_types @@ -132,7 +131,7 @@ def test_create_harness_agent_disable_mode() -> None: def test_create_harness_agent_disable_file_memory() -> None: - """disable_file_memory=True should exclude only the FileMemoryProvider.""" + """disable_file_memory=True should exclude the FileMemoryProvider.""" agent = create_harness_agent( client=_FakeChatClient(), # type: ignore[arg-type] max_context_window_tokens=128_000, @@ -141,22 +140,28 @@ def test_create_harness_agent_disable_file_memory() -> None: ) provider_types = [type(p) for p in agent.context_providers] assert FileMemoryProvider not in provider_types - # The file access provider should remain active. - assert FileAccessProvider in provider_types -def test_create_harness_agent_disable_file_access() -> None: - """disable_file_access=True should exclude only the FileAccessProvider.""" - agent = create_harness_agent( +def test_create_harness_agent_file_access_is_opt_in() -> None: + """FileAccessProvider is absent by default and added only when file_access_store is set.""" + default_agent = create_harness_agent( client=_FakeChatClient(), # type: ignore[arg-type] max_context_window_tokens=128_000, max_output_tokens=16_384, - disable_file_access=True, ) - provider_types = [type(p) for p in agent.context_providers] - assert FileAccessProvider not in provider_types - # The file memory provider should remain active. - assert FileMemoryProvider in provider_types + assert FileAccessProvider not in [type(p) for p in default_agent.context_providers] + # The file memory provider should remain active by default. + assert FileMemoryProvider in [type(p) for p in default_agent.context_providers] + + access_store = InMemoryAgentFileStore() + opt_in_agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + file_access_store=access_store, + ) + access_provider = next(p for p in opt_in_agent.context_providers if isinstance(p, FileAccessProvider)) + assert access_provider.store is access_store def test_create_harness_agent_uses_custom_file_stores() -> None: @@ -183,6 +188,7 @@ def test_create_harness_agent_file_access_approval_opt_outs() -> None: client=_FakeChatClient(), # type: ignore[arg-type] max_context_window_tokens=128_000, max_output_tokens=16_384, + file_access_store=InMemoryAgentFileStore(), file_access_disable_readonly_tool_approval=True, file_access_disable_write_tool_approval=True, ) @@ -195,7 +201,7 @@ def test_create_harness_agent_file_access_approval_opt_outs() -> None: def test_create_harness_agent_default_file_stores_are_filesystem( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: - """Without custom stores, the providers default to FileSystemAgentFileStore rooted in cwd.""" + """Without a custom store, the FileMemoryProvider defaults to FileSystemAgentFileStore in cwd.""" monkeypatch.chdir(tmp_path) agent = create_harness_agent( client=_FakeChatClient(), # type: ignore[arg-type] @@ -204,11 +210,8 @@ def test_create_harness_agent_default_file_stores_are_filesystem( ) memory_provider = next(p for p in agent.context_providers if isinstance(p, FileMemoryProvider)) - access_provider = next(p for p in agent.context_providers if isinstance(p, FileAccessProvider)) assert isinstance(memory_provider.store, FileSystemAgentFileStore) - assert isinstance(access_provider.store, FileSystemAgentFileStore) assert memory_provider.store.root_path == (tmp_path / "agent-file-memory").resolve() - assert access_provider.store.root_path == (tmp_path / "working").resolve() def test_create_harness_agent_skills_paths_adds_provider() -> None: @@ -390,7 +393,6 @@ def build_agent(*, disable_compaction: bool) -> Any: disable_todo=True, disable_mode=True, disable_file_memory=True, - disable_file_access=True, ) def seeded_session(agent: Any) -> tuple[Any, int]: @@ -448,7 +450,6 @@ async def fake_get_response(*, messages: Sequence[Message], options: dict[str, A disable_todo=True, disable_mode=True, disable_file_memory=True, - disable_file_access=True, ) session = agent.create_session() @@ -535,7 +536,6 @@ async def recording_leaf(*, messages: Sequence[Message], options: dict[str, Any] disable_todo=True, disable_mode=True, disable_file_memory=True, - disable_file_access=True, ) session = agent.create_session() diff --git a/python/samples/02-agents/harness/build_your_own_claw/README.md b/python/samples/02-agents/harness/build_your_own_claw/README.md index daf38c64f58..28ca1aaf38d 100644 --- a/python/samples/02-agents/harness/build_your_own_claw/README.md +++ b/python/samples/02-agents/harness/build_your_own_claw/README.md @@ -68,8 +68,8 @@ Teaches the assistant to work with *your* data safely. ### What this sample demonstrates - **File access** — the agent reads a pre-populated `working/portfolio.csv` and writes reports - with the `file_access_*` tools. File access is on by default; the sample points its store at the - sample's `working/` folder via `create_harness_agent(file_access_store=...)`. + with the `file_access_*` tools. File access is opt-in; the sample enables it by pointing its + store at the sample's `working/` folder via `create_harness_agent(file_access_store=...)`. - **Approvals** — file-access tools require approval by default, but the sample wires the built-in `read_only_tools_auto_approval_rule` so reads/lists/searches are frictionless while saving and deleting still pause for approval. The `place_trade` tool is marked