feat: neighborhood lock system + MCP integration - #43
Conversation
Adds a backend-agnostic locking layer so wiki-worker agents can take exclusive control of a slug (plus optional 1-hop neighbors) while they edit, with monotonic fencing tokens guarding against split-brain writes after stale leases are reclaimed. - memu/neighborhood_lock.py introduces the LockRegistry Protocol, SqliteLockRegistry (reuses the backend's SQLite connection for the lock tables alongside PR #21's node schema), MarkdownLockRegistry (sidecar files under .memu/locks/ with fencing counters in .memu/locks/_fence/<slug>), the NeighborhoodLock async context manager with an async-task renewer, and NeighborhoodConflict that subclasses the existing LaneContestedError so lane-lock callers catch wiki contention without extra except clauses. - The markdown backend now honors fencing_token: renew and release compare the caller's token against the on-disk counter and refuse the operation on mismatch, closing the TODO. - StorageBackend.get_lock_registry() lands on the protocol and the sqlite + markdown backends, keeping the context manager agnostic. - tests/storage/test_neighborhood_lock.py parametrizes both registries across seven design scenarios (basic acquire/release, concurrent conflict, subclass compatibility with LaneContestedError, monotonic fence, expired-lease reclaim, wrong-owner rejection, context-manager multi-lock with renewal), plus markdown-specific fence tampering and backend wiring checks. Cross-process SQLite is intentionally deferred. https://claude.ai/code/session_01VFVFZ2inY8WJ9tn7Rq8ruq
The former tests/test_memu_search.py issued a blocking requests.post to http://localhost:8000 at module import time, which made pytest exit 2 (collection error) on any environment without a live memU server — including CI's baseline job. It was never a real test, just a manual smoke script mis-filed under tests/. Moved to scripts/search_smoke.py with a __main__ guard so importing it no longer has side effects. https://claude.ai/code/session_01VFVFZ2inY8WJ9tn7Rq8ruq
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e08963071c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tmp = path.with_suffix(path.suffix + ".tmp") | ||
| tmp.write_text(content, encoding="utf-8") | ||
| os.replace(tmp, path) |
There was a problem hiding this comment.
When two wiki-worker processes use separate MarkdownLockRegistry instances for the same vault, _mu does not coordinate them, but every fence update uses the same <slug>.tmp path. In that cross-process acquire race, one writer can replace/unlink the temp file while the other is between write_text and os.replace, surfacing FileNotFoundError instead of a NeighborhoodConflict; even without the exception, the read-increment-write sequence can lose a fence increment. This undermines the filesystem concurrency guarantee for the new Markdown lock registry.
Useful? React with 👍 / 👎.
| self._conn.execute( | ||
| "INSERT INTO neighborhood_locks" | ||
| " (slug, holder_id, fencing_token, acquired_at, expires_at)" | ||
| " VALUES (?, ?, ?, ?, ?)", |
There was a problem hiding this comment.
For SQLite-backed agents running in separate processes/connections, the live-lock check, fence bump, and lock insert are not wrapped in a database transaction or protected by a database-level lock. If two agents acquire a free slug concurrently, both can observe no row before either inserts; the loser then gets a raw SQLite lock/unique-constraint error at this insert rather than the promised NeighborhoodConflict, and the fence update may already have been applied. Use an atomic transaction (for example BEGIN IMMEDIATE) and map the losing insert to NeighborhoodConflict.
Useful? React with 👍 / 👎.
2 participants
Combined rebased PR from #22 and #23.
Closes #22, closes #23