From 62d3e48447ee2b25886bb7a211d139923e8990db Mon Sep 17 00:00:00 2001 From: Simin Yuan <281164480+simin-yuan@users.noreply.github.com> Date: Sat, 7 Nov 2026 03:17:42 +0800 Subject: [PATCH 1/2] Python: report an over-long Windows path as ENAMETOOLONG, not a missing file On Windows without long-path support, FileSystemAgentFileStore.write() reported a path that merely exceeded the OS limit as FileNotFoundError, which reads as a missing file and sends callers looking for a bug in their own path handling. The limit is now used only to explain an error the OS already raised: when write() fails and a prefix of the path that is still inside the limit exists, the failure is re-raised as OSError(errno.ENAMETOOLONG), chained to the original error. Paths are never rejected up front, so a host with long-path support keeps its current behaviour and every in-limit path is untouched. Directories use the 248-character directory limit, files the 260-character one. The within-bound half of test_filesystem_store_matches_in_memory_complexity_bound now expects ENAMETOOLONG on a Windows host with long paths disabled. New tests cover both call sites with a pinned platform flag, check that unrelated FileNotFoundErrors still propagate unchanged, and check that a path past the limit is still written and read when the host supports long paths. Fixes #10141 --- .../agent_framework/_harness/_file_access.py | 53 +++++++ .../tests/core/test_harness_file_access.py | 133 +++++++++++++++++- 2 files changed, 183 insertions(+), 3 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_file_access.py b/python/packages/core/agent_framework/_harness/_file_access.py index 65b6036bb13..cb8f4937077 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,17 @@ _MAX_PATH_SEGMENTS: Final[int] = 255 _MAX_PATH_LENGTH: Final[int] = 4096 +# Windows path length limits. 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 characters 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-character 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 +1342,32 @@ async def create_directory(self, path: str) -> None: self._explicit_dirs[key] = display +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. + + 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 len(str(path)) < limit: + return + prefix = path + while len(str(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 +1389,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 + characters 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 +1549,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 +1568,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 17ef59fd5c6..15102274bfb 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,109 @@ 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 add a file name + # that pushes the absolute path past the file limit, so only the open fails. + segments: list[str] = [] + current = len(str(tmp_path)) + while current + 1 + 34 <= 247: + segments.append("d" * 34) + current += 1 + 34 + key = "/".join([*segments, "f" * 20]) + 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 From b2fd616dc6af4211827bd10a1a37f17719687bc8 Mon Sep 17 00:00:00 2001 From: Simin Yuan <281164480+simin-yuan@users.noreply.github.com> Date: Tue, 10 Nov 2026 23:38:11 +0800 Subject: [PATCH 2/2] Python: address long-path review feedback Count Windows path limits in UTF-16 code units instead of Python code points, restrict the ENAMETOOLONG rewrite to FileNotFoundError so collisions and permission failures keep their own types, and size the regression test's file name from the temporary directory so the path always crosses MAX_PATH. --- .../agent_framework/_harness/_file_access.py | 38 +++++++--- .../tests/core/test_harness_file_access.py | 73 ++++++++++++++++++- 2 files changed, 99 insertions(+), 12 deletions(-) diff --git a/python/packages/core/agent_framework/_harness/_file_access.py b/python/packages/core/agent_framework/_harness/_file_access.py index cb8f4937077..fb75ac1d78a 100644 --- a/python/packages/core/agent_framework/_harness/_file_access.py +++ b/python/packages/core/agent_framework/_harness/_file_access.py @@ -100,11 +100,13 @@ _MAX_PATH_SEGMENTS: Final[int] = 255 _MAX_PATH_LENGTH: Final[int] = 4096 -# Windows path length limits. 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 characters 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-character directory limit comes from. +# 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 @@ -1342,6 +1344,17 @@ 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`. @@ -1349,16 +1362,23 @@ def _raise_if_path_too_long(exc: OSError, path: Path, *, limit: int) -> None: 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 len(str(path)) < limit: + if not _WINDOWS_PATH_LIMITS_APPLY or not isinstance(exc, FileNotFoundError): + return + if _windows_path_length(path) < limit: return prefix = path - while len(str(prefix)) >= limit: + while _windows_path_length(prefix) >= limit: parent = prefix.parent if parent == prefix: return @@ -1390,8 +1410,8 @@ class FileSystemAgentFileStore(AgentFileStore): 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 - characters for a file and 248 for a directory unless long-path support is enabled + 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 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 15102274bfb..10628779582 100644 --- a/python/packages/core/tests/core/test_harness_file_access.py +++ b/python/packages/core/tests/core/test_harness_file_access.py @@ -3081,14 +3081,15 @@ def length_aware_mkdir(self: Path, mode: int = 0o777, parents: bool = False, exi monkeypatch.setattr(Path, "mkdir", length_aware_mkdir) else: - # Grow the parent until it is just inside the directory limit, then add a file name - # that pushes the absolute path past the file limit, so only the open fails. + # 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" * 20]) + key = "/".join([*segments, "f" * max(1, 260 - current - 1)]) monkeypatch.setattr(os, "open", raise_too_long) with pytest.raises(OSError) as exc_info: @@ -3132,3 +3133,69 @@ def raise_missing(*_args: object, **_kwargs: object) -> None: 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