From 09321898607109aecb2534143ea58f81d714137c Mon Sep 17 00:00:00 2001 From: hussein1362 Date: Wed, 22 Apr 2026 06:47:25 +0300 Subject: [PATCH] fix: handle Windows PermissionError on directory fsync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows, opening a directory with O_RDONLY raises PermissionError. Wrap the directory fsync in a try/except PermissionError — NTFS journals metadata synchronously so the directory sync is unnecessary there. Also adjust test assertions to expect 1 fsync call (file only) on Windows vs 2 (file + directory) on POSIX. --- nanobot/session/manager.py | 14 ++++++++++---- tests/session/test_session_fsync.py | 13 +++++++++---- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index 392862ff..436d8225 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -296,11 +296,17 @@ class SessionManager: if fsync: # fsync the directory so the rename is durable. - fd = os.open(str(path.parent), os.O_RDONLY) + # On Windows, opening a directory with O_RDONLY raises + # PermissionError — skip the dir sync there (NTFS + # journals metadata synchronously). try: - os.fsync(fd) - finally: - os.close(fd) + fd = os.open(str(path.parent), os.O_RDONLY) + try: + os.fsync(fd) + finally: + os.close(fd) + except PermissionError: + pass # Windows — directory fsync not supported except BaseException: tmp_path.unlink(missing_ok=True) raise diff --git a/tests/session/test_session_fsync.py b/tests/session/test_session_fsync.py index 8e45c761..3194cf95 100644 --- a/tests/session/test_session_fsync.py +++ b/tests/session/test_session_fsync.py @@ -2,6 +2,7 @@ from __future__ import annotations +import sys from pathlib import Path from unittest.mock import patch @@ -9,6 +10,8 @@ import pytest from nanobot.session.manager import SessionManager +_IS_WINDOWS = sys.platform == "win32" + @pytest.fixture def sessions_dir(tmp_path: Path) -> Path: @@ -39,8 +42,9 @@ class TestSaveFsync: with patch("os.fsync") as mock_fsync: manager.save(session, fsync=True) - # Should be called twice: once for the file, once for the directory - assert mock_fsync.call_count == 2 + # File fsync always runs; directory fsync only on non-Windows. + expected = 1 if _IS_WINDOWS else 2 + assert mock_fsync.call_count == expected def test_save_default_no_fsync(self, manager: SessionManager): """Default save() should not fsync (backward compat).""" @@ -77,8 +81,9 @@ class TestFlushAll: with patch("os.fsync") as mock_fsync: manager.flush_all() - # file fsync + directory fsync - assert mock_fsync.call_count == 2 + # file fsync always; directory fsync only on non-Windows + expected = 1 if _IS_WINDOWS else 2 + assert mock_fsync.call_count == expected def test_flush_all_continues_on_error(self, manager: SessionManager): """One broken session should not prevent others from flushing."""