From 281adcf8a9bc86240f9ce2657edc44e1cbd4a577 Mon Sep 17 00:00:00 2001 From: "praisonai-triage-agent[bot]" <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Date: Fri, 24 Apr 2026 09:50:38 +0000 Subject: [PATCH 1/5] fix: make session store fcntl import cross-platform compatible (fixes #1540) - Add conditional fcntl import like file_memory.py already has - Use _HAS_FCNTL flag to check fcntl availability before usage - Windows uses msvcrt for locking, Unix uses fcntl when available - Fixes Windows test collection failure due to missing fcntl module Co-authored-by: praisonai-triage-agent[bot] --- .../praisonaiagents/session/store.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/praisonai-agents/praisonaiagents/session/store.py b/src/praisonai-agents/praisonaiagents/session/store.py index 69e1fdbe6..516c80da6 100644 --- a/src/praisonai-agents/praisonaiagents/session/store.py +++ b/src/praisonai-agents/praisonaiagents/session/store.py @@ -5,7 +5,6 @@ Zero dependencies beyond stdlib. """ -import fcntl import json import logging from praisonaiagents._logging import get_logger @@ -14,6 +13,13 @@ import tempfile import threading import time + +# fcntl is Unix-only; on Windows, use msvcrt for file locking +if sys.platform != 'win32': + import fcntl + _HAS_FCNTL = True +else: + _HAS_FCNTL = False from dataclasses import dataclass, field from datetime import datetime, timezone from typing import Any, Dict, List, Optional @@ -150,7 +156,8 @@ def acquire(self) -> bool: msvcrt.locking(self._lock_file.fileno(), msvcrt.LK_NBLCK, 1) else: # Unix locking - fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + if _HAS_FCNTL: + fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) return True except (IOError, OSError, BlockingIOError): if self._lock_file: @@ -171,7 +178,8 @@ def release(self) -> None: import msvcrt msvcrt.locking(self._lock_file.fileno(), msvcrt.LK_UNLCK, 1) else: - fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_UN) + if _HAS_FCNTL: + fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_UN) except (IOError, OSError): pass finally: From 09f7f4e5f58a432cfbdf48750c8e0fb6131e5e67 Mon Sep 17 00:00:00 2001 From: "praisonai-triage-agent[bot]" <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Date: Fri, 24 Apr 2026 10:03:42 +0000 Subject: [PATCH 2/5] fix: add one-time warning for degraded file locking Addresses CodeRabbit feedback about silent no-op locking when fcntl is unavailable on non-Windows platforms. Adds module-level sentinel to emit warning only once, following AGENTS.md error handling principles. Co-authored-by: Mervin Praison --- .../praisonaiagents/session/store.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/praisonai-agents/praisonaiagents/session/store.py b/src/praisonai-agents/praisonaiagents/session/store.py index 516c80da6..1e8560afc 100644 --- a/src/praisonai-agents/praisonaiagents/session/store.py +++ b/src/praisonai-agents/praisonaiagents/session/store.py @@ -28,6 +28,9 @@ logger = get_logger(__name__) +# Module-level sentinel to track if we've warned about degraded locking +_WARNED_NO_FCNTL = False + # Default session directory (uses centralized paths - DRY) DEFAULT_SESSION_DIR = str(get_sessions_dir()) @@ -158,6 +161,15 @@ def acquire(self) -> bool: # Unix locking if _HAS_FCNTL: fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) + else: + # Warn once about degraded locking (should never happen with current platform detection) + global _WARNED_NO_FCNTL + if not _WARNED_NO_FCNTL: + logger.warning( + "File locking unavailable on this platform (no fcntl/msvcrt); " + "concurrent writers may corrupt session files." + ) + _WARNED_NO_FCNTL = True return True except (IOError, OSError, BlockingIOError): if self._lock_file: @@ -180,6 +192,7 @@ def release(self) -> None: else: if _HAS_FCNTL: fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_UN) + # Note: No warning needed in release() as it mirrors acquire() logic except (IOError, OSError): pass finally: From a9777bfe4ae828fb4ca934f0edc5fb873ce38d64 Mon Sep 17 00:00:00 2001 From: "praisonai-triage-agent[bot]" <272766704+praisonai-triage-agent[bot]@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:56:09 +0000 Subject: [PATCH 3/5] fix: improve fcntl import robustness and fix unreachable code MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace platform-based fcntl import with try/except for better cross-platform compatibility - Fix unreachable warning code branch that could never execute - Remove misleading msvcrt reference in Unix-only warning message - Now gracefully handles environments where fcntl is unavailable (GAE, Pyodide, etc.) - All reviewer feedback from Qodo, CodeRabbit, Gemini, Copilot, and Greptile addressed 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Mervin Praison --- src/praisonai-agents/praisonaiagents/session/store.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/praisonai-agents/praisonaiagents/session/store.py b/src/praisonai-agents/praisonaiagents/session/store.py index 1e8560afc..0633f4b8f 100644 --- a/src/praisonai-agents/praisonaiagents/session/store.py +++ b/src/praisonai-agents/praisonaiagents/session/store.py @@ -15,10 +15,10 @@ import time # fcntl is Unix-only; on Windows, use msvcrt for file locking -if sys.platform != 'win32': +try: import fcntl _HAS_FCNTL = True -else: +except ImportError: _HAS_FCNTL = False from dataclasses import dataclass, field from datetime import datetime, timezone @@ -162,11 +162,11 @@ def acquire(self) -> bool: if _HAS_FCNTL: fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) else: - # Warn once about degraded locking (should never happen with current platform detection) + # Warn once about degraded locking global _WARNED_NO_FCNTL if not _WARNED_NO_FCNTL: logger.warning( - "File locking unavailable on this platform (no fcntl/msvcrt); " + "File locking unavailable on this platform (no fcntl); " "concurrent writers may corrupt session files." ) _WARNED_NO_FCNTL = True From d79481b6965b0173da19247b518508ccb777b1ab Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:56:42 +0000 Subject: [PATCH 4/5] fix: harden optional fcntl import and add regression test --- .../praisonaiagents/session/store.py | 2 +- .../tests/unit/session/test_session_store.py | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/praisonai-agents/praisonaiagents/session/store.py b/src/praisonai-agents/praisonaiagents/session/store.py index 0633f4b8f..5932659a1 100644 --- a/src/praisonai-agents/praisonaiagents/session/store.py +++ b/src/praisonai-agents/praisonaiagents/session/store.py @@ -162,7 +162,7 @@ def acquire(self) -> bool: if _HAS_FCNTL: fcntl.flock(self._lock_file.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB) else: - # Warn once about degraded locking + # Warn once about degraded locking on non-Windows platforms without fcntl global _WARNED_NO_FCNTL if not _WARNED_NO_FCNTL: logger.warning( diff --git a/src/praisonai-agents/tests/unit/session/test_session_store.py b/src/praisonai-agents/tests/unit/session/test_session_store.py index a74bc71ab..4d4d9e22d 100644 --- a/src/praisonai-agents/tests/unit/session/test_session_store.py +++ b/src/praisonai-agents/tests/unit/session/test_session_store.py @@ -9,8 +9,11 @@ import tempfile import threading import time +import builtins +import importlib import pytest from concurrent.futures import ThreadPoolExecutor +from unittest.mock import patch from praisonaiagents.session.store import ( DefaultSessionStore, @@ -160,6 +163,22 @@ def worker(worker_id): if start_idx < other_start < end_idx: pytest.fail(f"Lock interleaving detected: {results}") + def test_import_without_fcntl(self): + """Test module import succeeds when fcntl is unavailable.""" + import praisonaiagents.session.store as store_module + original_import = builtins.__import__ + + def import_without_fcntl(name, *args, **kwargs): + if name == "fcntl": + raise ImportError("fcntl unavailable") + return original_import(name, *args, **kwargs) + + with patch("builtins.__import__", side_effect=import_without_fcntl): + reloaded_module = importlib.reload(store_module) + assert reloaded_module._HAS_FCNTL is False + + importlib.reload(store_module) + class TestDefaultSessionStore: """Tests for DefaultSessionStore.""" From 41a687808f327d5086cdc1079c35469fdaa39853 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 2 Jun 2026 12:58:50 +0000 Subject: [PATCH 5/5] test: cover missing fcntl import path for session store --- src/praisonai-agents/tests/unit/session/test_session_store.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/praisonai-agents/tests/unit/session/test_session_store.py b/src/praisonai-agents/tests/unit/session/test_session_store.py index 4d4d9e22d..4e852ffde 100644 --- a/src/praisonai-agents/tests/unit/session/test_session_store.py +++ b/src/praisonai-agents/tests/unit/session/test_session_store.py @@ -177,7 +177,8 @@ def import_without_fcntl(name, *args, **kwargs): reloaded_module = importlib.reload(store_module) assert reloaded_module._HAS_FCNTL is False - importlib.reload(store_module) + restored_module = importlib.reload(store_module) + assert restored_module._HAS_FCNTL is (importlib.util.find_spec("fcntl") is not None) class TestDefaultSessionStore: