Skip to content

Make the store lock and the hook timeout work on Windows - #30

Open
sebplace wants to merge 1 commit into
tigerless-labs:mainfrom
sebplace:windows-portability
Open

sebplace wants to merge 1 commit into
tigerless-labs:mainfrom
sebplace:windows-portability

Conversation

@sebplace

@sebplace sebplace commented Sep 9, 2026

Copy link
Copy Markdown

agent-memory does not start on Windows. core/locking.py imports fcntl at module level, so every entry point — mem, mem-hook, mem-mcp — dies with ModuleNotFoundError: No module named 'fcntl' before it can do anything.

This PR makes the two POSIX-only spots portable. Both are narrow.

core/locking.py

store_lock used fcntl.flock for the pipeline-level advisory lock. It now switches on os.name:

  • Windows: msvcrt.locking, taking one byte at position 0 with LK_NBLCK and releasing it with LK_UNLCK.
  • Everything else: the same fcntl.flock calls as before.

The contention branch now catches OSError rather than BlockingIOError. That is deliberate: msvcrt.locking raises a plain OSError when the region is held, and BlockingIOError is a subclass of OSError, so the POSIX path keeps behaving exactly as it did.

adapters/hook_entry.py

_arm and _disarm used signal.SIGALRM and signal.setitimer, neither of which exists on Windows. Both are now guarded with hasattr, so on Windows the hook runs without a wall-clock guard instead of failing to start.

That felt like the right trade-off given the module's own docstring: a hook that breaks its host is worse than a hook that never fires. Happy to swap it for a thread-based timeout if you would rather keep the guard on every platform.

No behaviour change on POSIX

The same fcntl calls run on the same paths. The Windows branch is only reachable when os.name == "nt".

Verification

Windows 11 ARM64, CPython 3.12.

  • mem init, mem record, mem recall complete a full round trip, and BM25 recall returns the seeded entries with sensible ranking.
  • mem-mcp answers an initialize plus tools/list handshake over stdio with its five tools.
  • Test suite: 377 passed.

The 11 remaining failures are unrelated to this change and were present in the same form before it:

  • 9 are the memcore harness attempting to exec a Linux binary (OSError: [WinError 193]).
  • 2 are assertions expecting POSIX separators (fact/rogue.md) where the library returns os.sep paths (fact\rogue.md).

That second group points at a separate portability wrinkle: relative paths in IndexReport are built with the platform separator rather than normalised to /. It is arguably inconsistent with the portable-store promise, but it is a distinct concern and I left it out of this PR rather than widening the diff. Glad to open a follow-up if you would like it fixed.

agent-memory does not start on Windows: core/locking.py imports fcntl at
module level, so every entry point dies with ModuleNotFoundError before it
can do anything.

Two POSIX-only spots, both narrow:

- core/locking.py used fcntl.flock for the pipeline-level store lock. It now
  switches on os.name and uses msvcrt.locking on Windows, locking one byte at
  position 0 with LK_NBLCK and releasing with LK_UNLCK. Contention is caught
  as OSError, which still covers the BlockingIOError the POSIX path raises.

- adapters/hook_entry.py used signal.SIGALRM and signal.setitimer, neither of
  which exists on Windows. Both are now guarded with hasattr, so the hook runs
  without a wall-clock guard rather than failing to start. That matches the
  module's stated contract that a hook which breaks its host is worse than one
  that never fires.

No behaviour change on POSIX: the same fcntl calls run on the same paths.

Verified on Windows 11 ARM64, CPython 3.12: mem init, mem record and mem
recall complete a full round trip, and the MCP adapter answers a tools/list
handshake with its five tools. Test suite: 377 passed. The 11 remaining
failures are unrelated to this change: 9 are the memcore harness trying to
exec a Linux binary, and 2 are assertions expecting POSIX path separators
where the library returns os.sep paths.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant