fix(state): a partial fcntl must disable the WAL guard, not kill every importer (#118269)

* fix(state): a partial fcntl must disable the WAL guard, not kill every importer

hermes_state_lockguard gated only on ImportError, treating "import fcntl
succeeded" as "full POSIX fcntl present". Stock CPython for Windows ships no
fcntl at all, so CI's Windows lane always took the except branch — but an
install whose venv has some other module importable as `fcntl` (flock/LOCK_*
only) reached the unguarded `fcntl.F_RDLCK` read and raised AttributeError at
import time.

hermes_state imports this module at module level, so that killed every
importer before supported() could report the guard as unavailable: `hermes
serve` (Desktop backend "exited before port announcement (1)"), `hermes
doctor`, the gateway and cron.

Gate on os.name == "nt" first — the module already promises to be a no-op on
Windows, and a lookalike must never arm it — and treat a partial module off
Windows like a missing one (ImportError, AttributeError), falling back to the
same no-op.

Closes #118026

* fix(state): a constants-only fcntl must not report the WAL guard as supported

Checking F_OFD_SETLK/F_RDLCK/F_UNLCK alone let a module carrying the constants
but no fcntl() callable pass the capability probe: supported() returned True and
the first hold() raised AttributeError from _ofd_lock(). Probe the callable in
the same block so that shape degrades to the no-op like a missing module, and
pin it with a constants-only stub that holds an fd open so hold() actually
reaches fcntl.fcntl().

* test(state): run the Windows branch of the lockguard gate on the Windows lane

The tests-os Windows job selects files by the windows_only marker, so the
os.name == "nt" branch had no runner coverage. Add a marked test asserting a
lookalike fcntl never arms the guard on native Windows.

hermes_state_lockguard.py

@@ -40,17 +40,33 @@ _SHARED_FIRST = _PENDING_BYTE + 2
 _SHARED_SIZE = 510
 _SHM_DMS_BYTE = 128
 
-try:
-    import fcntl
-    # CPython exports F_OFD_SETLK only from 3.12. The kernel ABI values are stable: 37 on every
-    # Linux arch (asm-generic/fcntl.h), 90 on XNU (bsd/sys/fcntl.h, documented in fcntl(2)).
-    _F_OFD_SETLK: Optional[int] = getattr(
-        fcntl, "F_OFD_SETLK", {"linux": 37, "darwin": 90}.get(sys.platform.rstrip("0123456789")))
-    _F_RDLCK, _F_UNLCK, _SEEK_SET = fcntl.F_RDLCK, fcntl.F_UNLCK, os.SEEK_SET
-except ImportError:  # Windows
+# Windows has no POSIX advisory locks, and the module promises to be a no-op there. Gate on the
+# platform FIRST: some Windows installs have a third-party module importable as `fcntl` (stock
+# CPython for Windows ships none), and letting the import decide would arm the guard on a
+# lookalike. Off Windows a partial module is equally fatal at import time — every importer of
+# hermes_state dies before supported() can say "no" (#118026) — so tolerate a missing attribute
+# the same way as a missing module and fall back to the no-op.
+if os.name == "nt":
     fcntl = None  # type: ignore[assignment]
-    _F_OFD_SETLK = None
+    _F_OFD_SETLK: Optional[int] = None
     _F_RDLCK = _F_UNLCK = _SEEK_SET = 0
+else:
+    try:
+        import fcntl
+        # CPython exports F_OFD_SETLK only from 3.12. The kernel ABI values are stable: 37 on every
+        # Linux arch (asm-generic/fcntl.h), 90 on XNU (bsd/sys/fcntl.h, documented in fcntl(2)).
+        _F_OFD_SETLK = getattr(
+            fcntl, "F_OFD_SETLK", {"linux": 37, "darwin": 90}.get(sys.platform.rstrip("0123456789")))
+        _F_RDLCK, _F_UNLCK, _SEEK_SET = fcntl.F_RDLCK, fcntl.F_UNLCK, os.SEEK_SET
+        # The constants alone do not make the guard usable: _ofd_lock() calls fcntl.fcntl(), so a
+        # module that has the constants but no callable would pass supported() and then raise
+        # from the first hold(). Probe the whole capability here, not just the symbols.
+        if not callable(getattr(fcntl, "fcntl", None)):
+            raise ImportError("fcntl module has no fcntl() callable")
+    except (ImportError, AttributeError):
+        fcntl = None  # type: ignore[assignment]
+        _F_OFD_SETLK = None
+        _F_RDLCK = _F_UNLCK = _SEEK_SET = 0
 
 # struct flock differs per libc: glibc/musl put type+whence first, Darwin/BSD last.
 _FLOCK_FORMAT = "@qqihh" if sys.platform == "darwin" or "bsd" in sys.platform else "@hhqqi"

tests/hermes_state/test_lockguard_fcntl_tolerance.py

@@ -0,0 +1,95 @@
+"""A partial or lookalike ``fcntl`` must disable the WAL lock guard, never kill the process.
+
+Regression for #118026: the module gated only on ``ImportError``, so a Windows install whose
+venv has some other module importable as ``fcntl`` (stock CPython for Windows ships none) hit an
+unguarded ``fcntl.F_RDLCK`` read at import time. ``hermes_state`` imports this module at module
+level, so the ``AttributeError`` killed every importer — ``hermes serve`` (Desktop backend
+"exited before port announcement (1)"), ``hermes doctor``, the gateway, cron — before
+``supported()`` existed to report the guard as unavailable.
+"""
+
+from __future__ import annotations
+
+import importlib
+import os
+import sys
+import types
+
+import pytest
+
+
+def _reimport_with_fcntl(monkeypatch, stub: types.ModuleType | None):
+    """Import the guard fresh with ``fcntl`` replaced (or absent), leaving sys.modules clean."""
+    monkeypatch.delitem(sys.modules, "hermes_state_lockguard", raising=False)
+
+    if stub is None:
+        monkeypatch.setitem(sys.modules, "fcntl", None)  # import fcntl -> ImportError
+    else:
+        monkeypatch.setitem(sys.modules, "fcntl", stub)
+
+    return importlib.import_module("hermes_state_lockguard")
+
+
+def _windows_fcntl_lookalike() -> types.ModuleType:
+    """A stand-in exposing only flock/LOCK_*, as reported on the affected Windows installs."""
+    stub = types.ModuleType("fcntl")
+    stub.flock = lambda *args, **kwargs: None
+    stub.LOCK_EX = 2
+    stub.LOCK_SH = 1
+    stub.LOCK_UN = 8
+    return stub
+
+
+def _constants_only_fcntl() -> types.ModuleType:
+    """The other partial shape: every lock constant present, no ``fcntl()`` callable. Passing the
+    constant checks alone would report ``supported()`` and then raise from the first hold()."""
+    stub = types.ModuleType("fcntl")
+    stub.F_OFD_SETLK = 37
+    stub.F_RDLCK = 0
+    stub.F_UNLCK = 2
+    return stub
+
+
+@pytest.mark.parametrize(
+    "stub",
+    [
+        pytest.param(None, id="absent"),
+        pytest.param(_windows_fcntl_lookalike(), id="partial"),
+        pytest.param(_constants_only_fcntl(), id="constants-only"),
+    ],
+)
+def test_guard_degrades_to_a_no_op_instead_of_killing_every_importer(monkeypatch, tmp_path, stub):
+    guard = _reimport_with_fcntl(monkeypatch, stub)
+
+    # An open descriptor on the file is what makes hold() reach fcntl.fcntl(); without one the
+    # constants-only shape would pass on a vacuous loop.
+    db = tmp_path / "state.db"
+    fd = os.open(db, os.O_RDWR | os.O_CREAT)
+    try:
+        # The contract the crash violated: the module imports, reports itself unavailable, and
+        # hold() hands back an empty guard so callers take the ordinary unguarded path.
+        assert guard.supported() is False
+        assert guard.hold(db) == {}
+        guard.release({})
+    finally:
+        os.close(fd)
+
+
+@pytest.mark.windows_only
+def test_windows_never_arms_the_guard_even_with_an_importable_fcntl(monkeypatch):
+    # Stock CPython for Windows has no fcntl, so the lookalike is the only way this branch can
+    # see an importable module; the platform gate must decide before the import does.
+    guard = _reimport_with_fcntl(monkeypatch, _windows_fcntl_lookalike())
+
+    assert guard.fcntl is None
+    assert guard.supported() is False
+    assert guard.hold("state.db") == {}
+
+
+def test_a_usable_fcntl_still_arms_the_guard(monkeypatch):
+    real_fcntl = pytest.importorskip("fcntl", reason="POSIX-only: nothing to arm on Windows")
+
+    guard = _reimport_with_fcntl(monkeypatch, real_fcntl)
+
+    # The tolerance above must not have disabled the guard everywhere.
+    assert guard.supported() is True