Repository navigation
Python: fix(core): shorten FileSessionStore atomic temp file path on Windows - #9739
chengrm (chengwudi1) wants to merge 1 commit into
Conversation
Fixes microsoft#9682. In FileSessionStore.set(), the temporary file was named .ile_path.name.uuid.tmp, unnecessarily appending 38 characters to the snapshot name. For long session IDs in deep directories, this caused atomic writes to exceed Windows MAX_PATH (260 characters) and raise FileNotFoundError, even though the final snapshot path was within limits. Use a compact, ID-independent temporary filename .uuid.tmp so temp paths remain short and never exceed snapshot path length.
|
chengrm (@chengwudi1) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
🟡 Changes recommended
The regression test must enforce the path-length constraint independently of host configuration to reliably detect the original bug.
1 open issue
What changed in this PR
Addresses #9682 by shortening temporary filenames used by Python’s FileSessionStore, avoiding Windows path-limit failures when saving long session IDs.
Changes:
- Uses a compact, session-ID-independent temporary filename while preserving atomic replacement.
- Adds a regression test covering long session IDs, round-trip persistence, and temporary-file cleanup.
| File | Description |
|---|---|
python/packages/core/tests/core/test_sessions.py |
Adds a long-session-ID regression test. |
python/packages/core/agent_framework/_sessions.py |
Shortens atomic-write temporary filenames. |
Review effort: Balanced
| session = AgentSession(session_id=session_id) | ||
| session.state["key"] = "value" | ||
|
|
||
| await store.set(session_id, session) |
Simin Yuan (simin-yuan)
left a comment
There was a problem hiding this comment.
Ran this where the issue's condition holds: Windows 11, LongPathsEnabled = 0, Python 3.14.9.
Method: the repro from #9682 unchanged — session ID at MAX_SESSION_ID_LENGTH, storage path sized so the snapshot path is exactly 240 characters (fits, while the old temp name did not).
| agent-framework-core at | set() result |
|---|---|
67ba500 (this PR's base) |
FileNotFoundError, errno 2 |
d1c38940 (this PR) |
saved, no error |
Same script, same machine, same storage path both times.
With the change, the temp name is .<uuid>.tmp (33 characters) instead of .<session id>.json.<uuid>.tmp (up to 171), so the temp path stays shorter than the snapshot path regardless of session ID length.
tests/core/test_sessions.py with this PR applied on the same machine: all tests pass, no failures (exit code 0).
One caveat on scope: this covers only the long-paths-disabled configuration that CI does not exercise (windows-latest images set LongPathsEnabled = 1). I did not run this on POSIX.
Reporting the measurement only.

Summary
Fixes #9682.
In
FileSessionStore.set(), the temporary file was created using:This redundant inclusion of
{file_path.name}appended 38 characters (.+ 32-char hex +.tmp+.) to an already long filename. For long session IDs (up toMAX_SESSION_ID_LENGTH = 128) residing in moderately deep directory trees on Windows whereLongPathsEnabledis not set, this caused atomic writes to exceed the WindowsMAX_PATHlimit (260 characters) and fail withFileNotFoundError, even when the final snapshot file path was well within limits.Changes
FileSessionStore.set()to.{uuid.uuid4().hex}.tmp._workflows/_checkpoint.pyand_filesystem.py.MAX_PATHif the destination path is valid..*.tmpglob pattern expected by cleanup assertions in tests.test_set_long_session_id_atomic_write_does_not_overflow_pathinpython/packages/core/tests/core/test_sessions.py.Verification
uv run pytest packages/core/tests/core/test_sessions.py -v-> 188 passed (including new regression test).uv run poe syntax -P core-> All tasks passed.uv run poe pyright -P core-> 0 errors, 0 warnings.