diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 4310615079..aabfef33b0 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -205,7 +205,7 @@ The vector store API is experimental under the shared `VECTOR_STORES` feature ID - **`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`, carrying the matching file name, a context snippet, and the matching lines with 1-based line numbers. Implementers should report each matching line verbatim, including its own terminator, so it can be reused as a `file_access_replace_lines` `new_line`; the pattern itself is matched against the line with its whole terminator removed, so `^`/`$` anchor to the line's text on a CRLF file as they already did on an LF one. A custom store populates these DTOs from its own `search`; the verbatim text is a recommendation, but the line number is not — it must address `split_lines`. - **`FileStoreEntry`** - `SerializationMixin` DTO returned by `list_children`, carrying an entry `name` and `type` (`"file"` or `"directory"`). -- **`FileAccessProvider`** - `ContextProvider` that adds shared file-access tools (`file_access_write`, `file_access_read`, `file_access_read_lines`, `file_access_delete`, `file_access_ls`, `file_access_grep`, `file_access_replace`, `file_access_replace_lines`) plus default usage instructions to each invocation. `file_access_ls` enumerates direct children (both files and subdirectories) as `{name, type}` entries with an optional `glob_pattern`, so the agent can walk the tree level by level; `file_access_grep` searches recursively from an optional base `directory` and returns relative `file_name` paths, scoped via an `fnmatch` `glob_pattern` (where `*` crosses `/`, e.g. `*.md`, `reports/*`). `file_access_replace` substitutes `old_string` with `new_string` (failing if not found, or if multiple matches and `replace_all` is false); `file_access_replace_lines` replaces whole 1-based lines with literal text (each `new_line` includes its own trailing newline; an empty `new_line` deletes the line, including its line break). `file_access_read_lines` returns a 1-based inclusive line range, one line per row as `\t`; `end_line` may be omitted to read to the end of the file, and an `end_line` past the last line clamps to it. Everything after the tab is verbatim, including the line's own terminator (which therefore doubles as the row separator), so a row's text can be fed straight back as a `file_access_replace_lines` `new_line` without losing a `\r\n`. Its line numbering comes from the same `_split_lines_keepends` split as `file_access_replace_lines` and as the stores in this package, so with one of those a number reported by grep addresses the same line in all three tools, including the trailing empty line of a newline-terminated file; grep itself runs through `AgentFileStore.search`, which must number by the same split but does not inherit it, so a store overriding `search` owns its numbering and nothing verifies it at run time. All tools are registered with `approval_mode="always_require"` by default, so every file operation needs host approval. Pass `disable_write_tools=True` to advertise only the read-only tools. To run unattended you can disable approval at the source with `disable_readonly_tool_approval=True` (read, read_lines, ls, grep) and/or `disable_write_tool_approval=True` (write, delete, replace, replace_lines), which register the affected tools with `approval_mode="never_require"`; alternatively, keep approval on and 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, read_lines, ls, grep), while `FileAccessProvider.all_tools_auto_approval_rule` approves every file-access tool including the write tools. 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 (`WRITE_TOOL_NAME`, `READ_TOOL_NAME`, `READ_LINES_TOOL_NAME`, `DELETE_TOOL_NAME`, `LS_TOOL_NAME`, `GREP_TOOL_NAME`, `REPLACE_TOOL_NAME`, `REPLACE_LINES_TOOL_NAME`). Unlike `MemoryContextProvider`, the store is intentionally shared across sessions and agents. +- **`FileAccessProvider`** - `ContextProvider` that adds shared file-access tools (`file_access_write`, `file_access_read`, `file_access_read_lines`, `file_access_delete`, `file_access_ls`, `file_access_grep`, `file_access_replace`, `file_access_replace_lines`) plus default usage instructions to each invocation. `file_access_ls` enumerates direct children (both files and subdirectories) as `{name, type}` entries with an optional `glob_pattern`, so the agent can walk the tree level by level; `file_access_grep` searches recursively from an optional base `directory` and returns relative `file_name` paths, scoped via an `fnmatch` `glob_pattern` (where `*` crosses `/`, e.g. `*.md`, `reports/*`). `file_access_replace` substitutes `old_string` with `new_string` (failing if not found, or if multiple matches and `replace_all` is false); `file_access_replace_lines` replaces whole 1-based lines with literal text (each `new_line` includes its own trailing newline; an empty `new_line` deletes the line, including its line break). `file_access_read_lines` returns a 1-based inclusive line range, one line per row as `\t`; `end_line` may be omitted to read to the end of the file, and an `end_line` past the last line clamps to it. Everything after the tab is verbatim, including the line's own terminator (which therefore doubles as the row separator), so a row's text can be fed straight back as a `file_access_replace_lines` `new_line` without losing a `\r\n`. Its line numbering comes from the same `_split_lines_keepends` split as `file_access_replace_lines` and as the stores in this package, so with one of those a number reported by grep addresses the same line in all three tools, including the trailing empty line of a newline-terminated file; grep itself runs through `AgentFileStore.search`, which must number by the same split but does not inherit it, so a store overriding `search` owns its numbering and nothing verifies it at run time. All tools are registered with `approval_mode="always_require"` by default, so every file operation needs host approval. Pass `disable_write_tools=True` to advertise only the read-only tools. To run unattended you can disable approval at the source with `disable_readonly_tool_approval=True` (read, read_lines, ls, grep) and/or `disable_write_tool_approval=True` (write, delete, replace, replace_lines), which register the affected tools with `approval_mode="never_require"`; alternatively, keep approval on and 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, read_lines, ls, grep), while `FileAccessProvider.all_tools_auto_approval_rule` approves every file-access tool including the write tools. 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 (`WRITE_TOOL_NAME`, `READ_TOOL_NAME`, `READ_LINES_TOOL_NAME`, `DELETE_TOOL_NAME`, `LS_TOOL_NAME`, `GREP_TOOL_NAME`, `REPLACE_TOOL_NAME`, `REPLACE_LINES_TOOL_NAME`). Unlike `MemoryContextProvider`, the store is intentionally shared across sessions and agents; pass `session_scoped=True` (with an optional explicit `scope`) to confine tool operations to a working folder derived from the session id or scope via the shared `_storage_key_segment` derivation (the provider fails closed when neither is available). ### File Memory Harness (`_harness/_file_memory.py`) diff --git a/python/packages/core/agent_framework/_harness/_agent.py b/python/packages/core/agent_framework/_harness/_agent.py index e5abc1c4dc..fbb9536b90 100644 --- a/python/packages/core/agent_framework/_harness/_agent.py +++ b/python/packages/core/agent_framework/_harness/_agent.py @@ -154,6 +154,7 @@ def _assemble_context_providers( disable_file_memory: bool, file_memory_store: AgentFileStore | None, file_access_store: AgentFileStore | None, + file_access_session_scoped: bool, file_access_disable_write_tools: bool, file_access_disable_readonly_tool_approval: bool, file_access_disable_write_tool_approval: bool, @@ -196,6 +197,7 @@ def _assemble_context_providers( 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, + session_scoped=file_access_session_scoped, ) ) @@ -330,6 +332,7 @@ def create_harness_agent( disable_file_memory: bool = False, file_memory_store: AgentFileStore | None = None, file_access_store: AgentFileStore | None = None, + file_access_session_scoped: bool = False, file_access_disable_write_tools: bool = False, file_access_disable_readonly_tool_approval: bool = False, file_access_disable_write_tool_approval: bool = False, @@ -454,6 +457,10 @@ def create_harness_agent( 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_session_scoped: When True, the FileAccessProvider confines tool operations + to a working folder derived from the active session id, so files are isolated per + session instead of shared across sessions. When False (default), the shared-store + semantics are preserved. Only used when file_access_store is set. file_access_disable_write_tools: When True, the FileAccessProvider advertises only its read-only tools (read, read_lines, ls, grep); the write tools (write, delete, replace, replace_lines) are hidden. When False (default), all tools are advertised. Only @@ -608,6 +615,7 @@ def create_harness_agent( disable_file_memory=disable_file_memory, file_memory_store=file_memory_store, file_access_store=file_access_store, + file_access_session_scoped=file_access_session_scoped, file_access_disable_write_tools=file_access_disable_write_tools, file_access_disable_readonly_tool_approval=file_access_disable_readonly_tool_approval, file_access_disable_write_tool_approval=file_access_disable_write_tool_approval, diff --git a/python/packages/core/agent_framework/_harness/_agent.pyi b/python/packages/core/agent_framework/_harness/_agent.pyi index 0c561f9235..a56ee836ff 100644 --- a/python/packages/core/agent_framework/_harness/_agent.pyi +++ b/python/packages/core/agent_framework/_harness/_agent.pyi @@ -71,6 +71,7 @@ def create_harness_agent( disable_file_memory: bool = False, file_memory_store: AgentFileStore | None = None, file_access_store: AgentFileStore | None = None, + file_access_session_scoped: bool = False, file_access_disable_write_tools: bool = False, file_access_disable_readonly_tool_approval: bool = False, file_access_disable_write_tool_approval: bool = False, diff --git a/python/packages/core/agent_framework/_harness/_file_access.py b/python/packages/core/agent_framework/_harness/_file_access.py index 5bfc6f592a..cdda1a1923 100644 --- a/python/packages/core/agent_framework/_harness/_file_access.py +++ b/python/packages/core/agent_framework/_harness/_file_access.py @@ -30,13 +30,13 @@ from abc import ABC, abstractmethod from collections.abc import Awaitable, Mapping, MutableMapping from pathlib import Path -from typing import Annotated, Any, ClassVar, Protocol, cast +from typing import Annotated, Any, ClassVar, Final, Protocol, cast import regex from pydantic import BaseModel, Field from .._feature_stage import ExperimentalFeature, experimental -from .._filesystem import _is_link_or_reparse_point # pyright: ignore[reportPrivateUsage] +from .._filesystem import _is_link_or_reparse_point, _storage_key_segment # pyright: ignore[reportPrivateUsage] from .._serialization import SerializationMixin from .._sessions import AgentSession, ContextProvider, SessionContext from .._telemetry import FeatureIndex, mark_feature_used @@ -65,6 +65,18 @@ "`file_access_replace_lines`. Reading the whole file first is rarely necessary." ) +# Prefix for session-derived working folders in session-scoped mode. Kept +# distinct from the other component prefixes so the same identifier under a +# different component never shares a storage location. +_ENCODED_FILE_ACCESS_SESSION_PREFIX: Final[str] = "~access-" + +# Instruction suffix appended when session-scoped mode is enabled, so the model +# does not assume files are shared outside the resolved workspace. +_SESSION_SCOPED_INSTRUCTIONS_SUFFIX = ( + "\n- Your file workspace is isolated to the current session or configured scope: files written " + "here are not visible outside that workspace." +) + # Maximum number of characters of context to include on either side of the first # regex match when building a result snippet. _SEARCH_SNIPPET_RADIUS = 50 @@ -233,6 +245,15 @@ async def _run_search_with_timeout( raise ValueError(_search_timeout_message()) from exc +def _combine_paths(base_path: str, relative_path: str) -> str: + """Join a working-folder path with a relative path using forward slashes.""" + if not base_path: + return relative_path + if not relative_path: + return base_path + return f"{base_path.rstrip('/')}/{relative_path.lstrip('/')}" + + def _normalize_relative_path(path: str, *, is_directory: bool = False) -> str: """Normalize and validate a relative store path. @@ -1683,10 +1704,12 @@ class FileAccessProvider(ContextProvider): Unlike :class:`~agent_framework.MemoryContextProvider`, which provides session-scoped memory that may be isolated per session, - :class:`FileAccessProvider` operates on a shared, persistent store whose - 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. + :class:`FileAccessProvider` operates by default on a shared, persistent + store whose contents are visible across sessions and agents. Pass + ``session_scoped=True`` (with an optional explicit ``scope``) to confine + tool operations to a workspace derived from the session id or scope + instead. The store is passed in by the caller and should already be scoped + to the desired folder or storage location. By default all tools require approval: each is registered with ``approval_mode="always_require"`` so the host must approve every file @@ -1766,6 +1789,8 @@ def __init__( disable_write_tools: bool = False, disable_readonly_tool_approval: bool = False, disable_write_tool_approval: bool = False, + session_scoped: bool = False, + scope: str | None = None, ) -> None: """Initialize the file access provider. @@ -1792,6 +1817,19 @@ def __init__( ``file_access_replace``, ``file_access_replace_lines``) are registered with ``approval_mode="never_require"`` so they run without host approval. Defaults to ``False`` (approval required). + session_scoped: When ``True``, tool operations are confined to a + working folder derived from the active session id (or the + explicit ``scope``), so files are isolated to that workspace: + per session by default, or shared across sessions when an + explicit ``scope`` is set. Defaults to ``False``, preserving + the shared-store semantics. + scope: The namespace that logically groups and isolates files + (for example, a user or tenant id). Only used when + ``session_scoped`` is ``True``; when ``None`` (the default), + the active session's ``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`. """ super().__init__(source_id) self.store = store @@ -1799,6 +1837,8 @@ def __init__( 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 # Serializes mutating tool operations (write/delete/replace/replace_lines). # The provider is shared across sessions/agents, so read-modify-write tools # (replace/replace_lines) could otherwise interleave and lose updates. Note @@ -1806,6 +1846,32 @@ def __init__( # processes sharing a FileSystemAgentFileStore on disk. self._write_lock = asyncio.Lock() + def _resolve_session_key(self, context: SessionContext) -> str: + """Resolve the working folder key for session-scoped mode. + + Uses the configured ``scope`` when set, otherwise the session id. The + value is an opaque namespace key, not a path: it is mapped to exactly + one folder name by :func:`~agent_framework._filesystem._storage_key_segment`. + That derivation is injective except for pathologically long values, + which fall back to a collision-resistant digest. Two byte-distinct + scopes or session ids therefore do not resolve to the same working + folder, so a caller authorized for one of them cannot reach another's + files. + + Raises: + ValueError: When neither ``scope`` nor the session id yields a + namespace. Without one there is nothing to isolate on, and + falling back to the store root would expose every other + session's files. + """ + raw_scope = self.scope or context.session_id or "" + if not raw_scope: + raise ValueError( + "FileAccessProvider session-scoped mode requires a scope: pass an explicit 'scope' or run with a " + "session that has a 'session_id'. Without one, files cannot be isolated from other sessions." + ) + return _storage_key_segment(raw_scope, encoded_prefix=_ENCODED_FILE_ACCESS_SESSION_PREFIX) + @staticmethod def _is_local_tool_call(function_call: Content) -> bool: """Return whether a function call targets this provider's local tools. @@ -1909,13 +1975,26 @@ async def before_run( readonly_approval: ApprovalMode = "never_require" if self.disable_readonly_tool_approval else "always_require" write_approval: ApprovalMode = "never_require" if self.disable_write_tool_approval else "always_require" + session_key = self._resolve_session_key(context) if self.session_scoped else "" + if session_key: + logger.debug("Session-scoped file access using working folder %r.", session_key) + await self.store.create_directory(session_key) + + def _session_path(relative_path: str) -> str: + return _combine_paths(session_key, relative_path) + + instructions = self.instructions + if self.session_scoped: + instructions += _SESSION_SCOPED_INSTRUCTIONS_SUFFIX + @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: normalized = _normalize_relative_path(file_name) + store_path = _session_path(normalized) async with self._write_lock: - await self.store.write(normalized, content, overwrite=overwrite) + await self.store.write(store_path, content, overwrite=overwrite) except FileExistsError: return f"File '{file_name}' already exists. To replace it, write again with overwrite set to true." except ValueError as exc: @@ -1929,7 +2008,7 @@ 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: normalized = _normalize_relative_path(file_name) - content = await self.store.read(normalized) + content = await self.store.read(_session_path(normalized)) except ValueError as exc: return f"Could not read file '{file_name}': {exc}" except OSError as exc: @@ -1945,7 +2024,7 @@ async def file_access_read_lines(file_name: str, start_line: int, end_line: int 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] try: normalized = _normalize_relative_path(file_name) - content = await self.store.read(normalized) + content = await self.store.read(_session_path(normalized)) if content is None: return f"File '{file_name}' not found." sliced = _slice_lines(content, start_line, end_line) @@ -1961,8 +2040,9 @@ async def file_access_delete(file_name: str) -> str: """Delete a file by name.""" try: normalized = _normalize_relative_path(file_name) + store_path = _session_path(normalized) async with self._write_lock: - deleted = await self.store.delete(normalized) + deleted = await self.store.delete(store_path) except ValueError as exc: return f"Could not delete file '{file_name}': {exc}" except OSError as exc: @@ -1976,8 +2056,9 @@ async def file_access_ls( ) -> list[dict[str, str]] | str: """List the direct child files and subdirectories of a directory. Omit ``directory`` (or pass an empty string) to list the root. To enumerate a subdirectory, pass its relative path, for example ``"reports"`` or ``"reports/2024"``. Optionally filter entries with a ``glob_pattern`` (e.g. ``"*.md"``). Subdirectories are listed before files, and each entry is ``{"name": , "type": "file"|"directory"}``.""" # ruff:ignore[line-too-long] target = directory if directory and directory.strip() else "" + store_target = _session_path(target) if target else session_key try: - listed = await self.store.list_children(target) + listed = await self.store.list_children(store_target) except ValueError as exc: return f"Could not list directory '{directory or ''}': {exc}" except OSError as exc: @@ -1996,12 +2077,13 @@ async def file_access_replace( """Replace occurrences of old_string with new_string in a file. Fails if old_string is not found, or if it occurs more than once and replace_all is false. Returns the number of occurrences replaced.""" # ruff:ignore[line-too-long] try: normalized = _normalize_relative_path(file_name) + store_path = _session_path(normalized) async with self._write_lock: - content = await self.store.read(normalized) + content = await self.store.read(store_path) if content is None: return f"File '{file_name}' not found." new_content, count = _apply_replace(content, old_string, new_string, replace_all) - await self.store.write(normalized, new_content, overwrite=True) + await self.store.write(store_path, new_content, overwrite=True) except ValueError as exc: return f"Could not replace in file '{file_name}': {exc}" except OSError as exc: @@ -2017,12 +2099,13 @@ async def file_access_replace_lines(file_name: str, edits: list[_LineEdit]) -> s r"""Replace lines in a file. Provide a list of edits, each with a 1-based line_number and a literal new_line (include your own trailing newline); an empty new_line deletes the line, including its line break. Fails on out-of-range or duplicate line numbers. 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) + store_path = _session_path(normalized) async with self._write_lock: - content = await self.store.read(normalized) + content = await self.store.read(store_path) if content is None: return f"File '{file_name}' not found." new_content = _apply_replace_lines(content, _line_edits(edits)) - await self.store.write(normalized, new_content, overwrite=True) + await self.store.write(store_path, new_content, overwrite=True) except ValueError as exc: return f"Could not edit file '{file_name}': {exc}" except OSError as exc: @@ -2055,8 +2138,9 @@ async def file_access_grep( """ glob_filter = glob_pattern if glob_pattern and glob_pattern.strip() else None target = directory if directory and directory.strip() else "" + store_target = _session_path(target) if target else session_key try: - results = await self.store.search(target, regex_pattern, glob_filter, recursive=True) + results = await self.store.search(store_target, regex_pattern, glob_filter, recursive=True) except ValueError as exc: return f"Could not search files: {exc}" except OSError as exc: @@ -2072,7 +2156,7 @@ async def file_access_grep( output.append(entry) return output - context.extend_instructions(self.source_id, [self.instructions]) + context.extend_instructions(self.source_id, [instructions]) tools = [file_access_read, file_access_read_lines, file_access_ls, file_access_grep] if not self.disable_write_tools: tools.extend([file_access_write, file_access_delete, file_access_replace, file_access_replace_lines]) diff --git a/python/packages/core/tests/core/test_harness_agent.py b/python/packages/core/tests/core/test_harness_agent.py index 41030e3bd0..ae7ff32020 100644 --- a/python/packages/core/tests/core/test_harness_agent.py +++ b/python/packages/core/tests/core/test_harness_agent.py @@ -255,6 +255,28 @@ def test_create_harness_agent_file_access_approval_opt_outs() -> None: assert access_provider.disable_write_tool_approval is True +def test_create_harness_agent_file_access_session_scoped_flag() -> None: + """The file_access_session_scoped flag should reach the FileAccessProvider.""" + default_agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + file_access_store=InMemoryAgentFileStore(), + ) + default_provider = next(p for p in default_agent.context_providers if isinstance(p, FileAccessProvider)) + assert default_provider.session_scoped is False + + scoped_agent = create_harness_agent( + client=_FakeChatClient(), # type: ignore[arg-type] + max_context_window_tokens=128_000, + max_output_tokens=16_384, + file_access_store=InMemoryAgentFileStore(), + file_access_session_scoped=True, + ) + scoped_provider = next(p for p in scoped_agent.context_providers if isinstance(p, FileAccessProvider)) + assert scoped_provider.session_scoped is True + + def test_create_harness_agent_default_file_stores_are_filesystem( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> 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 68ef1e775c..5bf5b07e36 100644 --- a/python/packages/core/tests/core/test_harness_file_access.py +++ b/python/packages/core/tests/core/test_harness_file_access.py @@ -28,9 +28,13 @@ FunctionTool, InMemoryAgentFileStore, Message, + SessionContext, SupportsChatGetResponse, ) -from agent_framework._filesystem import _is_link_or_reparse_point +from agent_framework._filesystem import ( # pyright: ignore[reportPrivateUsage] + _is_link_or_reparse_point, + _storage_key_segment, +) from agent_framework._harness import _file_access as _file_access_module from agent_framework._harness._file_access import ( _SEARCH_SNIPPET_RADIUS, @@ -44,6 +48,7 @@ ) from .conftest import create_junction_or_skip +from .test_filesystem import COLLIDING_IDENTIFIERS async def _list_files(store: AgentFileStore, directory: str = "") -> list[str]: @@ -1273,14 +1278,19 @@ async def _prepare_access_tools( disable_write_tools: bool = False, disable_readonly_tool_approval: bool = False, disable_write_tool_approval: bool = False, + session_id: str | None = "session-1", + session_scoped: bool = False, + scope: str | None = None, ) -> list[object]: """Prepare a FileAccessProvider and return its registered tools.""" - session = AgentSession(session_id="session-1") + session = AgentSession(session_id=session_id) provider = FileAccessProvider( store=store if store is not None else InMemoryAgentFileStore(), disable_write_tools=disable_write_tools, disable_readonly_tool_approval=disable_readonly_tool_approval, disable_write_tool_approval=disable_write_tool_approval, + session_scoped=session_scoped, + scope=scope, ) agent = Agent(client=chat_client_base, context_providers=[provider]) _, options = await agent._prepare_session_and_messages( # pyright: ignore[reportPrivateUsage] @@ -1998,3 +2008,119 @@ async def test_grep_matches_a_lone_carriage_return_as_content( payload = json.loads(_text((await grep.invoke(arguments={"regex_pattern": r"alpha\r$"}))[0])) assert payload[0]["matching_lines"][0]["line_number"] == 1 + + +# region Session isolation: session-scoped mode derives an injective working folder +# from the session id (or explicit scope) via _storage_key_segment, mirroring +# FileMemoryProvider. Default mode keeps the shared-store semantics untouched. + + +async def test_session_scoped_provider_isolates_sessions(chat_client_base: SupportsChatGetResponse) -> None: + """Two sessions must write the same file name into distinct working folders.""" + store = InMemoryAgentFileStore() + tools_a = await _prepare_access_tools(chat_client_base, store=store, session_id="session-a", session_scoped=True) + tools_b = await _prepare_access_tools(chat_client_base, store=store, session_id="session-b", session_scoped=True) + + await _tool_by_name(tools_a, "file_access_write").invoke(arguments={"file_name": "notes.txt", "content": "AAA"}) + await _tool_by_name(tools_b, "file_access_write").invoke(arguments={"file_name": "notes.txt", "content": "BBB"}) + + dirs = await _list_dirs(store) + assert len(dirs) == 2 + assert dirs[0] != dirs[1] + assert await _list_files(store) == [] + + read_a = await _tool_by_name(tools_a, "file_access_read").invoke(arguments={"file_name": "notes.txt"}) + read_b = await _tool_by_name(tools_b, "file_access_read").invoke(arguments={"file_name": "notes.txt"}) + assert _text(read_a[0]) == "AAA" + assert _text(read_b[0]) == "BBB" + + +@pytest.mark.parametrize("other_id", COLLIDING_IDENTIFIERS[1:]) +async def test_session_scoped_provider_isolates_colliding_session_ids( + chat_client_base: SupportsChatGetResponse, other_id: str +) -> None: + """Identifiers a lossy normalizer would fold must map to distinct working folders.""" + store = InMemoryAgentFileStore() + tools_base = await _prepare_access_tools( + chat_client_base, store=store, session_id="customer-42", session_scoped=True + ) + tools_other = await _prepare_access_tools(chat_client_base, store=store, session_id=other_id, session_scoped=True) + + await _tool_by_name(tools_base, "file_access_write").invoke(arguments={"file_name": "f.txt", "content": "BASE"}) + await _tool_by_name(tools_other, "file_access_write").invoke(arguments={"file_name": "f.txt", "content": "OTHER"}) + + assert len(await _list_dirs(store)) == 2 + read_base = await _tool_by_name(tools_base, "file_access_read").invoke(arguments={"file_name": "f.txt"}) + read_other = await _tool_by_name(tools_other, "file_access_read").invoke(arguments={"file_name": "f.txt"}) + assert _text(read_base[0]) == "BASE" + assert _text(read_other[0]) == "OTHER" + + +async def test_session_scoped_provider_fails_closed_without_session_or_scope() -> None: + """Session-scoped mode must raise instead of falling back to the shared store root.""" + provider = FileAccessProvider(store=InMemoryAgentFileStore(), session_scoped=True) + session = AgentSession() + context = SessionContext(session_id=None, input_messages=[]) + + with pytest.raises(ValueError, match="session"): + await provider.before_run(agent=None, session=session, context=context, state={}) + + +async def test_session_scoped_provider_scope_overrides_session(chat_client_base: SupportsChatGetResponse) -> None: + """An explicit scope must group files across sessions and win over the session id.""" + store = InMemoryAgentFileStore() + tools_a = await _prepare_access_tools( + chat_client_base, store=store, session_id="session-a", session_scoped=True, scope="tenant-1" + ) + tools_b = await _prepare_access_tools( + chat_client_base, store=store, session_id="session-b", session_scoped=True, scope="tenant-1" + ) + + await _tool_by_name(tools_a, "file_access_write").invoke(arguments={"file_name": "shared.md", "content": "SHARED"}) + read_b = await _tool_by_name(tools_b, "file_access_read").invoke(arguments={"file_name": "shared.md"}) + + assert _text(read_b[0]) == "SHARED" + assert len(await _list_dirs(store)) == 1 + + +async def test_session_scoped_provider_grep_returns_session_relative_names( + chat_client_base: SupportsChatGetResponse, +) -> None: + """Grep results must stay relative to the session root, not the store root or the session folder.""" + store = InMemoryAgentFileStore() + tools = await _prepare_access_tools(chat_client_base, store=store, session_id="session-a", session_scoped=True) + + await _tool_by_name(tools, "file_access_write").invoke( + arguments={"file_name": "docs/notes.txt", "content": "hello world"} + ) + payload = json.loads( + _text((await _tool_by_name(tools, "file_access_grep").invoke(arguments={"regex_pattern": "hello"}))[0]) + ) + + assert payload[0]["file_name"] == "docs/notes.txt" + + +async def test_session_scoped_disabled_keeps_shared_store_semantics( + chat_client_base: SupportsChatGetResponse, +) -> None: + """Default mode must keep writing to the shared store root without session folders.""" + store = InMemoryAgentFileStore() + tools_a = await _prepare_access_tools(chat_client_base, store=store, session_id="session-a") + tools_b = await _prepare_access_tools(chat_client_base, store=store, session_id="session-b") + + await _tool_by_name(tools_a, "file_access_write").invoke(arguments={"file_name": "notes.txt", "content": "AAA"}) + await _tool_by_name(tools_b, "file_access_write").invoke( + arguments={"file_name": "notes.txt", "content": "BBB", "overwrite": True} + ) + + assert await _list_dirs(store) == [] + assert await _list_files(store) == ["notes.txt"] + read_b = await _tool_by_name(tools_b, "file_access_read").invoke(arguments={"file_name": "notes.txt"}) + assert _text(read_b[0]) == "BBB" + + +def test_session_scoped_provider_uses_file_access_namespace_prefix() -> None: + """The session key derivation must use the file-access-specific encoded prefix.""" + assert _storage_key_segment("Session-a", encoded_prefix="~access-") != _storage_key_segment( + "Session-a", encoded_prefix="~scope-" + )