Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
agent-memorydoes not start on Windows.core/locking.pyimportsfcntlat module level, so every entry point —mem,mem-hook,mem-mcp— dies withModuleNotFoundError: No module named 'fcntl'before it can do anything.This PR makes the two POSIX-only spots portable. Both are narrow.
core/locking.pystore_lockusedfcntl.flockfor the pipeline-level advisory lock. It now switches onos.name:msvcrt.locking, taking one byte at position 0 withLK_NBLCKand releasing it withLK_UNLCK.fcntl.flockcalls as before.The contention branch now catches
OSErrorrather thanBlockingIOError. That is deliberate:msvcrt.lockingraises a plainOSErrorwhen the region is held, andBlockingIOErroris a subclass ofOSError, so the POSIX path keeps behaving exactly as it did.adapters/hook_entry.py_armand_disarmusedsignal.SIGALRMandsignal.setitimer, neither of which exists on Windows. Both are now guarded withhasattr, 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
fcntlcalls run on the same paths. The Windows branch is only reachable whenos.name == "nt".Verification
Windows 11 ARM64, CPython 3.12.
mem init,mem record,mem recallcomplete a full round trip, and BM25 recall returns the seeded entries with sensible ranking.mem-mcpanswers aninitializeplustools/listhandshake over stdio with its five tools.The 11 remaining failures are unrelated to this change and were present in the same form before it:
memcoreharness attempting to exec a Linux binary (OSError: [WinError 193]).fact/rogue.md) where the library returnsos.seppaths (fact\rogue.md).That second group points at a separate portability wrinkle: relative paths in
IndexReportare 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.