Skip to content

fix(mcp): release the writer lease after a mutating-idle window (#1888) - #1966

Open
JasonAiassist wants to merge 1 commit into
MemPalace:developfrom
JasonAiassist:fix/mcp-writer-lease-idle-release
Open

fix(mcp): release the writer lease after a mutating-idle window (#1888)#1966
JasonAiassist wants to merge 1 commit into
MemPalace:developfrom
JasonAiassist:fix/mcp-writer-lease-idle-release

Conversation

@JasonAiassist

Copy link
Copy Markdown

Problem

The per-palace writer lease (#1818/#1823) is held for the whole MCP process lifetime. As #1888 reports, one interactive session therefore starves every other writer on the palace for hours:

  • hook-driven and manual mempalace mine runs exit with MineAlreadyRunning
  • queued-writes daemon jobs fail against the held flock
  • a second session's mutating tools are refused with -32001 until the first session ends

We hit this in production daily: multiple Claude Code sessions plus lifecycle hooks against one palace produced recurring sqlite→HNSW divergence (362 drawers in one day) before 3.5.0's lease, and with the lease the starvation above.

Fix — cooperative lease

Release the lease once no mutating tool has run for MEMPALACE_MCP_WRITER_LEASE_IDLE_S seconds (default 300; 0 restores the legacy hold-until-exit behavior).

The critical correctness piece: the release also runs _force_chroma_cache_reset(), so the next mutating call reopens the palace from disk. Stale in-memory HNSW state is why the lease was lifetime-scoped; dropping every cached Chroma handle at release time is what makes a re-acquire safe. Re-acquisition rides the existing self-heal retry path (#1960), so the first mutating tool after a release transparently wins the lease back — no restart, no user action.

Implementation notes:

  • A mutating-tool in-flight counter plus last-mutation timestamp (guarded by a leaf threading.Lock) drive the idle clock; the lease is never released mid-write.
  • The idle-exit watchdog thread performs the release check and now also starts when only the lease release is enabled (MEMPALACE_MCP_IDLE_HOURS=0 no longer disables it).
  • The atexit hook is registered once and reads the current lease, replacing the per-acquire lambda that would go stale across release/re-acquire cycles.

Tests

8 new tests in tests/test_mcp_server.py covering: idle release + cache reset, no release before the window / while a mutation is in flight / when disabled via env, env fallback on invalid values, re-acquire after release, in-flight tracking, and atexit idempotency. Full suite: 3267 passed, 20 skipped; ruff check/format clean.

Scope

Part of the #1963 concurrent-writer cluster (tactical tier — no architectural change; fully compatible with both the queued-writes daemon and the #1270 bridge direction). Fixes the starvation half of #1888; the thread-local re-entrancy half was already fixed by the process-wide holder set in palace.py.

Follow-up candidate (separate PR if wanted): service.run_mcp_tool executes write handlers without taking mine_palace_lock, so daemon-executed mcp_tool jobs could still interleave with a live lease holder once something starts submitting them.

🤖 Generated with Claude Code

…alace#1888)

The per-palace writer lease (MemPalace#1818/MemPalace#1823) is held for the whole MCP
process lifetime, so a single interactive session starves every other
writer on the palace — hook and manual mines exit with
MineAlreadyRunning, daemon jobs fail, and a second session's mutating
tools are refused — for hours at a time.

Release the lease once no mutating tool has run for
MEMPALACE_MCP_WRITER_LEASE_IDLE_S seconds (default 300; 0 restores the
legacy hold-until-exit behavior). The release also runs
_force_chroma_cache_reset() so the next mutating call reopens the
palace from disk: a re-acquire is only safe when no stale in-memory
HNSW state survives, which is the reason the lease was
lifetime-scoped in the first place. Re-acquisition rides the existing
self-heal retry path, so the first mutating tool after a release
transparently wins the lease back.

The idle-exit watchdog thread drives the release check and now also
starts when only the lease release is enabled. The atexit hook is
registered once and reads the current lease, replacing the
per-acquire lambda that would go stale across release/re-acquire
cycles.

Part of the MemPalace#1963 concurrent-writer cluster.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@fatkobra fatkobra left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: resetting Chroma caches only when the lease is released does not make lease handoff safe. A local read can reopen and cache a PersistentClient while this process is lease-free; a peer can then write; and when this process reacquires the lease, the first mutation reuses that stale client because no reset occurs after acquisition.

There is also a handoff race because the OS lease is released before _force_chroma_cache_reset() completes, so a new local mutator can acquire the lease and begin using a client that the previous releaser is about to reset.

Please reset and reopen after every successful lease acquisition before the first mutation, and coordinate release, reset, and reacquisition under one process-wide lifecycle guard. Add a regression covering: release -> local read reopens -> peer writes -> reacquire -> mutate.

@igorls

igorls commented Aug 15, 2026

Copy link
Copy Markdown
Member

Thanks for this contribution, and apologies for the slow turnaround.

develop has moved a fair way since this was opened and the branch no longer merges cleanly. If you're still interested in landing it, could you rebase onto current develop? Once it merges cleanly and CI is green I'll get it reviewed for the 3.8.0 cycle.

If you'd rather not pick it back up, no problem at all — just say so and I'll close it out, and thanks either way for taking the time to send it.

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.

4 participants