[Simin Yuan (simin-yuan)]

Motivation & Context

Fixes #10137. On Windows, FileSessionStore.set() raises PermissionError: [WinError 5]
when the destination snapshot file is briefly unavailable to os.replace() — for example
right after a previous replacement. The store already serializes writes per snapshot with
a process-local lock, so this is not a locking gap: the replacement itself fails, and a
racing save raises instead of following the last-writer-wins behaviour documented on the
class.

Measured on Windows 11 23H2 (OS build 22631.6199, Python 3.11.16, main at 13956ad),
32 concurrent set() calls for one session id, six runs:

source	failures across six runs
without this change	7/32, 6/32, 6/32, 4/32, 7/32, 2/32
with this change	0/32 in all six runs

100 sequential set() calls fail zero times either way. The failure needs concurrent
saves; a second process saving every 50 ms while another reads did not reproduce it on
this machine, so the table reports the condition I could measure rather than a mechanism.

Description & Review Guide

• What are the major changes? set() now replaces the snapshot through
  _replace_session_file(), which retries os.replace() up to three times (10 ms, then
  20 ms) when it raises PermissionError, and re-raises the original error after the last
  attempt. The retry stays inside the existing per-file lock, so ordering guarantees are
  unchanged. The retry runs on Windows only (os.name == "nt"): on POSIX the same
  exception means the caller genuinely may not replace the file.
• What is the impact of these changes? A transient Windows replacement failure no
  longer reaches the caller; a persistent permission problem still fails, after at most
  ~30 ms of retries. POSIX behaviour is unchanged.
• What do you want reviewers to focus on? Whether a bounded retry is the right fix
  versus something like ReplaceFileW, whether the retry delays suit the store's perf
  profile, and whether the Windows-only gate is worth keeping — the alternative is to
  retry everywhere and accept a 30 ms worst case on a call that is about to fail anyway.
  The retry test pins the platform flag with monkeypatch, so both branches are covered
  on every CI OS.

New tests, both in python/packages/core/tests/core/test_sessions.py:

• test_set_retries_transient_permission_error_during_atomic_replace — the name and
  location the automated triage suggested; it fails with the original PermissionError
  when the retry is removed.
• test_set_reports_permission_error_without_retrying_off_windows — the error is raised
  after a single os.replace() attempt when the platform gate is off, and the previous
  snapshot is left intact.

Related Issue

Fixes #10137

AI Assistance

• This is an AI-assisted contribution. I reviewed, understood, and verified all submitted content and accept responsibility for it.

AI assistance details: implementation, tests, and this PR description were drafted with an AI assistant. I ran every verification listed below myself on the Windows machine described above.

Contribution Checklist

• python/packages/core/tests/core/test_sessions.py → 192 passed, 1 skipped
• The retry test fails without the change (PermissionError) and passes with it
• ruff format --check and ruff check are clean on both touched files
• pyright with pyrightconfig.tests.json → 0 errors, 0 warnings for both files
• Full core suite: not run locally for this revision — the workspace pins uv==0.13.5 and this machine has 0.11.7, so uv sync refuses without a global tool upgrade. Leaving it to CI.

One failure on this host is unrelated to this change:
test_harness_file_access.py::test_filesystem_store_matches_in_memory_complexity_bound
builds a path past MAX_PATH on a Windows host with LongPathsEnabled=0 and fails with or
without the change; it covers the file store rather than the session store, and the
long-path behaviour is tracked separately in #10141.

[Copilot]

🟡 Changes recommended

Persistent failure with Windows retries enabled lacks test coverage for three replacement attempts, PermissionError propagation, snapshot preservation, temporary-file cleanup, and 10/20 ms waits.

1 open issue

• Missing persistent-failure test for Windows retries · New

What changed in this PR

This PR addresses #10137 by adding bounded retries for transient Windows snapshot replacement failures in FileSessionStore, while preserving the existing per-file lock and POSIX behavior.

Changes:

• Retry Windows PermissionError failures for up to three total attempts, with 10 ms and 20 ms waits.
• Add tests for transient recovery and immediate failure when retries are disabled.

File	Description
python/​packages/​core/​tests/​core/​test_sessions.py	Tests retry recovery, snapshot preservation, and platform-gated behavior.
python/​packages/​core/​agent_framework/​_sessions.py	Adds the Windows-only retry helper and uses it during atomic snapshot writes.

_{Review effort: Balanced}

[Simin Yuan (simin-yuan)]

Eduard van Valkenburg (@eavanvalkenburg) thanks for the approval. On my side GitHub still reports mergeStateStatus=BLOCKED with reviewDecision=null, even though the latest run (42624267086) is green across Python 3.11-3.15, pre-commit, and merge-gatekeeper. Could you tell me which required check or approval is still missing — or merge it if nothing is? If you'd rather have Copilot's persistent-failure test first, I can add it (three replacement attempts, PermissionError propagation, snapshot preserved, temp-file cleanup, mocked 10/20 ms waits).

[Simin Yuan (simin-yuan)]

Added the test the remaining thread asked for in 7cf96aa: it pins the Windows retry path, asserts exactly three attempts, the 10 ms / 20 ms waits, PermissionError propagation, the previous snapshot still readable, and no temp file left behind. I resolved the thread.

One thing worth writing down, because it explains the earlier BLOCKED state: the Main ruleset requires required_review_thread_resolution, require_last_push_approval, a code-owner review, and squash-only merges through a merge queue (strict_required_status_checks_policy too). So the single unresolved thread was enough to block even with your approval; and this new commit resets the last-push approval, so it will need a re-approval after CI. That is the rule working as designed, not a request to bypass it — I would rather name the gate than keep guessing at it.

The test is not executed on my machine (no full checkout here); CI will run it.