diff --git a/python/packages/core/agent_framework/_harness/_file_access.py b/python/packages/core/agent_framework/_harness/_file_access.py index 65b6036bb1..fb75ac1d78 100644 --- a/python/packages/core/agent_framework/_harness/_file_access.py +++ b/python/packages/core/agent_framework/_harness/_file_access.py @@ -26,6 +26,7 @@ import fnmatch import logging import os +import sys import threading import time import unicodedata @@ -99,6 +100,19 @@ _MAX_PATH_SEGMENTS: Final[int] = 255 _MAX_PATH_LENGTH: Final[int] = 4096 +# Windows path length limits, counted the way Windows counts them: in UTF-16 code +# units, not Python code points (a supplementary character is two units). They are +# consulted only to explain an error the OS already raised (see +# :func:`_raise_if_path_too_long`); they never gate a path up front. ``MAX_PATH`` is +# 260 of those units including the drive, the separators and the terminating NUL, and +# ``CreateDirectory`` needs room for the 8.3 name it may have to synthesize, which is +# where the 248-unit directory limit comes from. +# https://learn.microsoft.com/windows/win32/fileio/maximum-file-path-limitation +_WINDOWS_MAX_PATH_CHARS: Final[int] = 260 +_WINDOWS_MAX_DIRECTORY_PATH_CHARS: Final[int] = 248 +# Pinned in a constant so tests can exercise both branches on any platform. +_WINDOWS_PATH_LIMITS_APPLY: Final[bool] = sys.platform == "win32" + # Hard cap on the length of a user-supplied search regex. The cap is a coarse bound on # how much work a single pattern can describe; it does not stop a short pathological # pattern, which is what :class:`_BoundedSearchPattern` is for. @@ -1330,6 +1344,50 @@ async def create_directory(self, path: str) -> None: self._explicit_dirs[key] = display +def _windows_path_length(path: Path) -> int: + """Return the length Windows measures for *path*, in UTF-16 code units. + + Windows path limits are expressed in UTF-16 code units, while ``len`` on a Python + string counts Unicode code points. A supplementary character such as an emoji is one + code point but two UTF-16 units, so counting code points would understate the path + and could miss a genuine length failure. + """ + return len(str(path).encode("utf-16-le", "surrogatepass")) // 2 + + +def _raise_if_path_too_long(exc: OSError, path: Path, *, limit: int) -> None: + """Re-raise an over-long Windows path failure as :data:`errno.ENAMETOOLONG`. + + Without long-path support Windows reports a path past its limit as a missing path, + which CPython surfaces as :class:`FileNotFoundError`. Callers then go looking for a + bug in their own path handling instead of shortening the key. + + Only that missing-path symptom is rewritten. A collision or a permission failure has + its own exception type and must keep it, even when the path happens to be over the + limit: with long-path support enabled Windows reports those directly, and relabeling + them as a length problem would hide the real cause. + + The length is used only to explain an error that already happened. Nothing here + rejects a path before it is used, so a host with long-path support behaves exactly as + before and every in-limit path stays untouched. The rewrite additionally requires a + prefix inside the limit to exist; without one the path is missing for an ordinary + reason and the original error is the accurate one. + """ + if not _WINDOWS_PATH_LIMITS_APPLY or not isinstance(exc, FileNotFoundError): + return + if _windows_path_length(path) < limit: + return + prefix = path + while _windows_path_length(prefix) >= limit: + parent = prefix.parent + if parent == prefix: + return + prefix = parent + if not prefix.exists(): + return + raise OSError(errno.ENAMETOOLONG, "File name too long", str(path)) from exc + + @experimental(feature_id=ExperimentalFeature.HARNESS) class FileSystemAgentFileStore(AgentFileStore): """A disk-backed :class:`AgentFileStore` rooted under a configurable directory. @@ -1351,6 +1409,15 @@ class FileSystemAgentFileStore(AgentFileStore): symlink between the probe and the file open. This store is designed for single-tenant or co-operating-tenant use; it is not a sandbox against a hostile process that shares the root directory. + + On Windows the length of the resolved path (root plus key) is limited to 260 UTF-16 + code units for a file and 248 for a directory unless long-path support is enabled + (``LongPathsEnabled``). :meth:`write` reports that limit as :class:`OSError` with + :data:`errno.ENAMETOOLONG` instead of the ``FileNotFoundError`` the OS raises, so a + caller can tell "too long" apart from "missing". The limit only renames a failure + the OS already produced; paths are never rejected up front, and a host with + long-path support behaves exactly as before. :class:`InMemoryAgentFileStore` has no + such limit, so the two backends still differ at that bound. """ # Case aliases can identify the same file while having different Path hashes. @@ -1502,6 +1569,11 @@ def _write_file_sync(full_path: Path, content: str, overwrite: bool) -> None: if parent_file is not None: raise NotADirectoryError(f"Parent path is not a directory: {parent_file}") from exc raise + except OSError as exc: + # Windows reports an over-long absolute path as FileNotFoundError; name the + # real cause before the caller reads it as a missing parent directory. + _raise_if_path_too_long(exc, full_path.parent, limit=_WINDOWS_MAX_DIRECTORY_PATH_CHARS) + raise encoded = content.encode("utf-8") flags = os.O_WRONLY | os.O_CREAT if overwrite: @@ -1516,6 +1588,7 @@ def _write_file_sync(full_path: Path, content: str, overwrite: bool) -> None: try: fd = os.open(full_path, flags, 0o644) except OSError as exc: + _raise_if_path_too_long(exc, full_path, limit=_WINDOWS_MAX_PATH_CHARS) # Windows reports PermissionError when opening a directory for # writing; POSIX exclusive creation can report FileExistsError. if isinstance(exc, (FileExistsError, PermissionError)) and full_path.is_dir(): 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 17ef59fd5c..1062877958 100644 --- a/python/packages/core/tests/core/test_harness_file_access.py +++ b/python/packages/core/tests/core/test_harness_file_access.py @@ -8,6 +8,7 @@ import os import re import stat +import sys import threading import time from contextlib import suppress @@ -2983,9 +2984,33 @@ async def test_in_memory_store_accepts_deep_paths_within_the_bound() -> None: assert retained < 200 * len(path) +def _windows_long_paths_disabled() -> bool: + """Return whether this host is Windows with long-path support turned off. + + Read-only: the registry value is queried, never written. The Windows default is + ``0`` when the value or key is absent. The concept does not apply elsewhere, so + this returns ``False`` on every other platform. + """ + if sys.platform != "win32": + return False + try: + import winreg + + with winreg.OpenKey(winreg.HKEY_LOCAL_MACHINE, r"SYSTEM\CurrentControlSet\Control\FileSystem") as key: + enabled, _ = winreg.QueryValueEx(key, "LongPathsEnabled") + except OSError: + return True + return not enabled + + async def test_filesystem_store_matches_in_memory_complexity_bound(tmp_path) -> None: """Both backends refuse the same over-bound path with the same errno, so - backend substitution is not observable on the bound either.""" + backend substitution is not observable on the bound either. + + That agreement stops at the OS bound. On Windows without long-path support a + 200-segment key is still legal for the store itself but not for the filesystem, + so the within-bound half expects the normalized ``ENAMETOOLONG`` there. + """ import errno fs_store = FileSystemAgentFileStore(str(tmp_path)) @@ -3001,7 +3026,176 @@ async def test_filesystem_store_matches_in_memory_complexity_bound(tmp_path) -> assert mem_info.value.errno == errno.ENAMETOOLONG within = "/".join(["d"] * 200 + ["f"]) - await fs_store.write(within, "x") await mem_store.write(within, "x") - assert await fs_store.read(within) == "x" assert await mem_store.read(within) == "x" + + if _windows_long_paths_disabled(): + with pytest.raises(OSError) as within_info: + await fs_store.write(within, "x") + assert within_info.value.errno == errno.ENAMETOOLONG + # The path is not reachable for the filesystem backend, so it reads as absent. + assert await fs_store.read(within) is None + else: + await fs_store.write(within, "x") + assert await fs_store.read(within) == "x" + + +@pytest.mark.parametrize("failure_point", ["mkdir", "open"]) +async def test_filesystem_store_normalizes_windows_path_too_long_error( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, failure_point: str +) -> None: + """An OS "path too long" failure is reported as ``ENAMETOOLONG``. + + Windows reports an absolute path past its limit as a missing path, which CPython + surfaces as ``FileNotFoundError``; without the rewrite the store reports a path that + is merely too long as a missing file. The failure is injected at both points where + the store touches the OS and the platform flag is pinned, so the test also runs on + CI hosts that have long paths enabled. + """ + import errno + + monkeypatch.setattr("agent_framework._harness._file_access._WINDOWS_PATH_LIMITS_APPLY", True) + store = FileSystemAgentFileStore(str(tmp_path)) + too_long = FileNotFoundError(errno.ENOENT, "The system cannot find the path specified", "too-long") + + def raise_too_long(*_args: object, **_kwargs: object) -> None: + raise too_long + + if failure_point == "mkdir": + # A directory past its limit is what a real host refuses, and it still creates the + # prefixes below the limit first; the rewrite checks for one of those. + real_mkdir = Path.mkdir + key = "/".join(["d" * 40] * 8) # absolute path well past both limits + + def length_aware_mkdir(self: Path, mode: int = 0o777, parents: bool = False, exist_ok: bool = False) -> None: + if len(str(self)) >= 248: + # Mimic pathlib's recursion: create the deepest prefix that is still legal, + # then refuse, exactly as the OS does. + prefix = self + while len(str(prefix)) >= 248 and prefix.parent != prefix: + prefix = prefix.parent + if prefix != self and len(str(prefix)) < 248: + real_mkdir(prefix, parents=True, exist_ok=True) + raise_too_long() + real_mkdir(self, mode, parents, exist_ok) + + monkeypatch.setattr(Path, "mkdir", length_aware_mkdir) + else: + # Grow the parent until it is just inside the directory limit, then size the file + # name from the current length so the absolute path is guaranteed to reach the + # file limit wherever ``tmp_path`` happens to live. + segments: list[str] = [] + current = len(str(tmp_path)) + while current + 1 + 34 <= 247: + segments.append("d" * 34) + current += 1 + 34 + key = "/".join([*segments, "f" * max(1, 260 - current - 1)]) + monkeypatch.setattr(os, "open", raise_too_long) + + with pytest.raises(OSError) as exc_info: + await store.write(key, "x") + + assert not isinstance(exc_info.value, FileNotFoundError) + assert exc_info.value.errno == errno.ENAMETOOLONG + assert exc_info.value.__cause__ is too_long + + +@pytest.mark.skipif(_windows_long_paths_disabled(), reason="this host rejects paths past MAX_PATH") +async def test_filesystem_store_never_rejects_a_path_past_the_windows_limit(tmp_path: Path) -> None: + """The Windows limits only rename a failure; they never reject a path up front. + + On a host that supports long paths, the key that fails on a legacy host must still be + written and read. A length check applied before the OS call would break exactly this + case, which is why the rewrite runs only inside an ``except`` block. + """ + store = FileSystemAgentFileStore(str(tmp_path)) + key = "/".join(["d" * 40] * 8) + await store.write(key, "payload") + assert await store.read(key) == "payload" + + +async def test_filesystem_store_propagates_other_file_not_found_errors( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A ``FileNotFoundError`` that is not the Windows over-long-path error keeps + its type and identity, so a genuinely missing parent still reads as missing.""" + import errno + + store = FileSystemAgentFileStore(str(tmp_path)) + missing = FileNotFoundError(errno.ENOENT, "The system cannot find the path specified") + + def raise_missing(*_args: object, **_kwargs: object) -> None: + raise missing + + monkeypatch.setattr(Path, "mkdir", raise_missing) + + with pytest.raises(FileNotFoundError) as exc_info: + await store.write("nested/file.txt", "x") + + assert exc_info.value is missing + + +async def test_filesystem_store_measures_long_paths_in_utf16_code_units( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The Windows limit is compared in UTF-16 code units, not Python code points. + + A supplementary character such as an emoji is one code point but two UTF-16 units. + A path one code point short of 260 can therefore already have reached the Windows + limit, and must still be reported as too long. + """ + import errno + + monkeypatch.setattr("agent_framework._harness._file_access._WINDOWS_PATH_LIMITS_APPLY", True) + store = FileSystemAgentFileStore(str(tmp_path)) + root = str(store.root_path) + key = "f" * (257 - len(root)) + "\U0001f600" + target = store.root_path / key + # 259 code points, 260 UTF-16 units: only the Windows length has reached the limit. + assert len(str(target)) == 259 + assert len(str(target).encode("utf-16-le")) // 2 == 260 + + too_long = FileNotFoundError(errno.ENOENT, "The system cannot find the path specified") + + def raise_too_long(*_args: object, **_kwargs: object) -> None: + raise too_long + + monkeypatch.setattr(os, "open", raise_too_long) + + with pytest.raises(OSError) as exc_info: + await store.write(key, "x") + + assert not isinstance(exc_info.value, FileNotFoundError) + assert exc_info.value.errno == errno.ENAMETOOLONG + assert exc_info.value.__cause__ is too_long + + +@pytest.mark.parametrize("error_type", [FileExistsError, PermissionError]) +async def test_filesystem_store_preserves_non_missing_errors_on_long_paths( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, error_type: type[OSError] +) -> None: + """Collisions and permission failures on an over-long path keep their own type. + + Only the legacy "missing path" symptom is rewritten. With long-path support enabled + Windows reports an existing file as ``FileExistsError`` and a refusal as + ``PermissionError``; relabeling either as a length problem would hide the real cause. + """ + import errno + + monkeypatch.setattr("agent_framework._harness._file_access._WINDOWS_PATH_LIMITS_APPLY", True) + store = FileSystemAgentFileStore(str(tmp_path)) + key = "f" * (260 - len(str(store.root_path)) - 1) + error_number = errno.EEXIST if error_type is FileExistsError else errno.EACCES + error = error_type(error_number, "original failure") + assert len(str(store.root_path / key)) == 260 + + def raise_error(*_args: object, **_kwargs: object) -> None: + raise error + + monkeypatch.setattr(os, "open", raise_error) + + with pytest.raises(OSError) as exc_info: + await store.write(key, "x") + + assert exc_info.value is error + assert exc_info.value.errno == error_number