From 7bddb65cb485e2c2233042c65a782a8b28741b2f Mon Sep 17 00:00:00 2001
From: Igor Lins e Silva <4753812+igorls@users.noreply.github.com>
Date: Wed, 20 May 2026 15:31:57 -0300
Subject: [PATCH 1/3] fix: stale-PID timeout, MCP idle exit, structured errors
 (#1552)

- Add MEMPALACE_MINE_TIMEOUT_HOURS (default 2h): PID files now record
  '{pid} {unix_timestamp}'; _mine_already_running() treats alive-but-old
  processes as stale, unblocking queued mines after a ChromaDB hang.
  Backward-compatible: bare-PID files (old format) treated as stale.
- Add MEMPALACE_MCP_IDLE_HOURS (default 8h): daemon watchdog thread in
  mcp_server calls sys.exit(0) after the configured idle period, preventing
  accumulation of stale server processes holding ChromaDB/HNSW file handles.
  Set to 0 to disable.
- Enrich _internal_tool_error() with optional exc parameter: adds
  data: {error_class, message} to JSON-RPC error body so callers can
  distinguish lock contention, ChromaDB transients, and segfaults without
  scraping the message string. MineAlreadyRunning handler in tool_sync()
  adds error_class: 'LockHeldByOtherProcess' to the result dict.
- Update miner._cleanup_mine_pid_file() to parse first whitespace token
  as PID (handles both old and new PID file formats).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 mempalace/hooks_cli.py   |  66 +++++++++++++++++++++---
 mempalace/mcp_server.py  |  79 ++++++++++++++++++++++++++---
 mempalace/miner.py       |   7 ++-
 tests/test_hooks_cli.py  |  92 +++++++++++++++++++++++++++++----
 tests/test_mcp_server.py | 106 ++++++++++++++++++++++++++++++++++++---
 5 files changed, 321 insertions(+), 29 deletions(-)

diff --git a/mempalace/hooks_cli.py b/mempalace/hooks_cli.py
index 158a0fa0f0..d0c741b798 100644
--- a/mempalace/hooks_cli.py
+++ b/mempalace/hooks_cli.py
@@ -12,6 +12,7 @@
 import re
 import subprocess
 import sys
+import time
 from datetime import datetime
 from pathlib import Path
 from typing import Optional
@@ -281,6 +282,31 @@ def _get_mine_targets() -> list[tuple[str, str]]:
 # own slot on exit without scanning the whole directory.
 _MINE_PID_FILE_ENV = "MEMPALACE_MINE_PID_FILE"
 
+# Maximum wall-clock hours a mine subprocess is allowed to run before its
+# PID slot is treated as stale (even if the process is still alive).  A
+# wedged mine — e.g. one that is blocking indefinitely on ChromaDB
+# cold-init under concurrent Windows load (#1552) — would otherwise hold
+# its slot forever.  Set MEMPALACE_MINE_TIMEOUT_HOURS=0 to disable the
+# timeout (slots are reclaimed only when the PID is dead).
+_MINE_TIMEOUT_HOURS_ENV = "MEMPALACE_MINE_TIMEOUT_HOURS"
+_MINE_TIMEOUT_HOURS_DEFAULT = 2.0
+
+
+def _mine_slot_timeout_secs() -> float:
+    """Return the configured mine-slot timeout in seconds.
+
+    Reads ``MEMPALACE_MINE_TIMEOUT_HOURS`` from the environment (float).
+    Returns 0 if the env var is set to 0 or is not parseable.
+    """
+    raw = os.environ.get(_MINE_TIMEOUT_HOURS_ENV, "")
+    if raw:
+        try:
+            hours = float(raw)
+            return max(0.0, hours) * 3600
+        except ValueError:
+            pass
+    return _MINE_TIMEOUT_HOURS_DEFAULT * 3600
+
 
 def _pid_file_for_cmd(cmd: list[str]) -> Path:
     """Return the per-target PID file path for a mine subcommand.
@@ -333,23 +359,51 @@ def _pid_alive(pid: int) -> bool:
 
 
 def _mine_already_running(cmd: list[str]) -> bool:
-    """Return True if a previous mine for ``cmd``'s target is still alive."""
+    """Return True if a previous mine for ``cmd``'s target is still alive.
+
+    The PID file format is ``{pid} {unix_timestamp}`` (timestamp added in
+    #1552 to detect wedged subprocesses).  Old-format files (bare ``{pid}``)
+    are treated as having a start time of 0 — effectively "infinitely old" —
+    so they are immediately considered stale once the configured timeout
+    elapses.
+
+    A process is considered stale (and this function returns False) when:
+    - the PID is dead, OR
+    - the configured mine timeout is > 0 AND the process has been running
+      longer than the timeout.
+    """
     pid_file = _pid_file_for_cmd(cmd)
     try:
         recorded = pid_file.read_text().strip()
     except OSError:
         return False
-    if not recorded.isdigit():
+    if not recorded:
+        return False
+    parts = recorded.split(None, 1)
+    if not parts[0].isdigit():
+        return False
+    pid = int(parts[0])
+    if not _pid_alive(pid):
         return False
-    return _pid_alive(int(recorded))
+    timeout_secs = _mine_slot_timeout_secs()
+    if timeout_secs > 0:
+        start_ts = float(parts[1]) if len(parts) > 1 and parts[1] else 0.0
+        if time.time() - start_ts > timeout_secs:
+            return False
+    return True
 
 
 def _create_mine_slot_with_placeholder(pid_file: Path) -> Path:
-    """Atomically create a mine PID slot and write this hook PID into it."""
+    """Atomically create a mine PID slot and write this hook PID into it.
+
+    The slot body is ``{pid} {unix_timestamp}`` so that stale-by-age
+    detection in ``_mine_already_running`` can determine how long the
+    recorded process has been running (#1552).
+    """
     fd = os.open(str(pid_file), os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o600)
     try:
         with os.fdopen(fd, "w", encoding="ascii") as f:
-            f.write(str(os.getpid()))
+            f.write(f"{os.getpid()} {int(time.time())}")
     except OSError:
         try:
             os.close(fd)
@@ -437,7 +491,7 @@ def _spawn_mine(cmd: list) -> None:
                 pass
             raise
     try:
-        pid_file.write_text(str(proc.pid))
+        pid_file.write_text(f"{proc.pid} {int(time.time())}")
     except OSError:
         pass
 
diff --git a/mempalace/mcp_server.py b/mempalace/mcp_server.py
index 41cf9c0c21..90dfdda7fa 100644
--- a/mempalace/mcp_server.py
+++ b/mempalace/mcp_server.py
@@ -176,6 +176,27 @@ def _parse_args():
 _kg_cache_lock = threading.Lock()
 _palace_flag_given: bool = bool(_args.palace)
 
+# MCP server idle auto-exit (#1552).  Stale MCP servers from ended Claude
+# Code sessions do not self-terminate, accumulating ChromaDB/HNSW file
+# handles on Windows.  When MEMPALACE_MCP_IDLE_HOURS is set (or defaults
+# to 8 h), a background daemon thread exits the process once no request
+# has been handled for that long.  Set to 0 to disable.
+_MCP_IDLE_HOURS_ENV = "MEMPALACE_MCP_IDLE_HOURS"
+_MCP_IDLE_HOURS_DEFAULT = 8.0
+_last_request_time: float = time.monotonic()
+
+
+def _mcp_idle_timeout_secs() -> float:
+    """Return the configured MCP idle timeout in seconds (0 = disabled)."""
+    raw = os.environ.get(_MCP_IDLE_HOURS_ENV, "")
+    if raw:
+        try:
+            hours = float(raw)
+            return max(0.0, hours) * 3600
+        except ValueError:
+            pass
+    return _MCP_IDLE_HOURS_DEFAULT * 3600
+
 
 def _resolve_kg_path() -> str:
     if _palace_flag_given:
@@ -1204,7 +1225,11 @@ def tool_sync(project_dir: str = None, wing: str = None, apply: bool = False):
         # below, otherwise MineAlreadyRunning and ValueError fall into the
         # generic "sync failed" branch and break the structured-error tests.
         except MineAlreadyRunning as exc:
-            return {"success": False, "error": f"another mine is in progress: {exc}"}
+            return {
+                "success": False,
+                "error": f"another mine is in progress: {exc}",
+                "error_class": "LockHeldByOtherProcess",
+            }
         except ValueError as exc:
             return {"success": False, "error": str(exc)}
         except Exception as exc:
@@ -2286,22 +2311,30 @@ def tool_reconnect():
 ]
 
 
-def _internal_tool_error(req_id, tool_name: str) -> dict:
+def _internal_tool_error(req_id, tool_name: str, exc: BaseException = None) -> dict:
     logger.exception(f"Tool error in {tool_name}")
+    error: dict = {"code": -32000, "message": "Internal tool error"}
+    if exc is not None:
+        error["data"] = {
+            "error_class": type(exc).__name__,
+            "message": str(exc),
+        }
     return {
         "jsonrpc": "2.0",
         "id": req_id,
-        "error": {"code": -32000, "message": "Internal tool error"},
+        "error": error,
     }
 
 
 def handle_request(request):
+    global _last_request_time
     if not isinstance(request, dict):
         return {
             "jsonrpc": "2.0",
             "id": None,
             "error": {"code": -32600, "message": "Invalid Request"},
         }
+    _last_request_time = time.monotonic()
     method = request.get("method") or ""
     params = request.get("params") or {}
     req_id = request.get("id")
@@ -2449,9 +2482,9 @@ def handle_request(request):
                             "message": f"Missing required {word} {quoted} for tool {tool_name}",
                         },
                     }
-            return _internal_tool_error(req_id, tool_name)
-        except Exception:
-            return _internal_tool_error(req_id, tool_name)
+            return _internal_tool_error(req_id, tool_name, e)
+        except Exception as exc:
+            return _internal_tool_error(req_id, tool_name, exc)
 
     # Notifications (missing id) must never get a response
     if req_id is None:
@@ -2626,6 +2659,37 @@ def _maybe_eager_warmup_embedder() -> None:
         )
 
 
+def _start_idle_exit_watchdog() -> None:
+    """Start a daemon thread that exits the process after an idle period.
+
+    When no request has been handled for ``MEMPALACE_MCP_IDLE_HOURS``
+    (default 8 h), the thread calls ``sys.exit(0)`` so that stale MCP
+    servers from ended Claude Code sessions do not accumulate ChromaDB /
+    HNSW file handles on Windows (#1552).
+
+    Set ``MEMPALACE_MCP_IDLE_HOURS=0`` to disable the watchdog.
+    """
+    timeout = _mcp_idle_timeout_secs()
+    if timeout <= 0:
+        return
+    check_interval = min(60.0, timeout / 4)
+
+    def _watchdog() -> None:
+        while True:
+            time.sleep(check_interval)
+            idle = time.monotonic() - _last_request_time
+            if idle >= timeout:
+                logger.info(
+                    "MCP server idle for %.1f h (limit %.1f h); exiting to release file handles.",
+                    idle / 3600,
+                    timeout / 3600,
+                )
+                sys.exit(0)
+
+    t = threading.Thread(target=_watchdog, name="mcp-idle-watchdog", daemon=True)
+    t.start()
+
+
 def main():
     """MCP server entry point for the ``mempalace-mcp`` console script.
 
@@ -2661,6 +2725,9 @@ def main():
     # does not pay the ONNX/CoreML cold-load tax under the MCP client
     # timeout (#1495). Default off — preserves current startup latency.
     _maybe_eager_warmup_embedder()
+    # Idle auto-exit: release ChromaDB file handles from stale servers
+    # that outlived their Claude Code session (#1552).
+    _start_idle_exit_watchdog()
     while True:
         try:
             line = sys.stdin.readline()
diff --git a/mempalace/miner.py b/mempalace/miner.py
index 0eab9dc72e..0a86d965ae 100644
--- a/mempalace/miner.py
+++ b/mempalace/miner.py
@@ -1323,7 +1323,12 @@ def _cleanup_mine_pid_file() -> None:
         if not pid_file.exists():
             return
         recorded = pid_file.read_text().strip()
-        if recorded and recorded.isdigit() and int(recorded) == os.getpid():
+        # PID file format: "{pid} {unix_timestamp}" (timestamp added in
+        # #1552 for stale-by-age detection).  Old-format files (bare
+        # "{pid}") are also handled: split on whitespace and take the
+        # first token as the PID.
+        pid_token = recorded.split()[0] if recorded else ""
+        if pid_token and pid_token.isdigit() and int(pid_token) == os.getpid():
             pid_file.unlink()
     except OSError:
         # Best-effort cleanup; never fail the mine over PID bookkeeping.
diff --git a/tests/test_hooks_cli.py b/tests/test_hooks_cli.py
index be485c0615..f22ca68867 100644
--- a/tests/test_hooks_cli.py
+++ b/tests/test_hooks_cli.py
@@ -694,7 +694,9 @@ def test_claim_mine_slot_writes_live_placeholder_pid(tmp_path):
         pid_file = _claim_mine_slot(cmd)
 
         assert pid_file == _pid_file_for_cmd(cmd)
-        assert pid_file.read_text().strip() == str(os.getpid())
+        # Format: "{pid} {unix_timestamp}" — first token must be our PID.
+        content = pid_file.read_text().strip()
+        assert content.split()[0] == str(os.getpid())
         assert _mine_already_running(cmd) is True
         assert _claim_mine_slot(cmd) is None
 
@@ -715,7 +717,8 @@ def test_claim_mine_slot_reclaimed_slot_writes_live_placeholder_pid(tmp_path):
         reclaimed = _claim_mine_slot(cmd)
 
         assert reclaimed == pid_file
-        assert pid_file.read_text().strip() == str(os.getpid())
+        # Format: "{pid} {unix_timestamp}" — first token must be our PID.
+        assert pid_file.read_text().strip().split()[0] == str(os.getpid())
 
 
 def test_maybe_auto_ingest_ignores_transcript_arg_path(tmp_path):
@@ -789,7 +792,8 @@ def test_maybe_auto_ingest_skips_when_mine_running(tmp_path):
                 ]
                 pid_file = _pid_file_for_cmd(cmd)
                 pid_file.parent.mkdir(parents=True, exist_ok=True)
-                pid_file.write_text(str(os.getpid()))
+                import time as _time
+                pid_file.write_text(f"{os.getpid()} {int(_time.time())}")
                 with patch("mempalace.hooks_cli._mempalace_python", return_value=sys.executable):
                     with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen:
                         _maybe_auto_ingest()
@@ -855,6 +859,8 @@ def test_spawn_mine_uses_detached_kwargs(tmp_path):
 
 def test_spawn_mine_skips_when_target_running(tmp_path):
     """A second spawn for the same cmd target while the first is alive must skip."""
+    import time as _time
+
     pid_dir = tmp_path / "mine_pids"
     with patch("mempalace.hooks_cli.STATE_DIR", tmp_path):
         with patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir):
@@ -863,7 +869,7 @@ def test_spawn_mine_skips_when_target_running(tmp_path):
             cmd = ["mempalace", "mine", "/tmp/proj", "--mode", "projects"]
             pid_file = _pid_file_for_cmd(cmd)
             pid_file.parent.mkdir(parents=True, exist_ok=True)
-            pid_file.write_text(str(os.getpid()))  # live PID
+            pid_file.write_text(f"{os.getpid()} {int(_time.time())}")  # live PID, fresh
 
             with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen:
                 _spawn_mine(cmd)
@@ -901,8 +907,9 @@ def test_spawn_mine_reclaims_stale_slot(tmp_path):
                 mock_popen.return_value.pid = 4242
                 _spawn_mine(cmd)
                 mock_popen.assert_called_once()
-                # New PID is recorded in the reclaimed slot.
-                assert pid_file.read_text().strip() == "4242"
+                # New PID is recorded in the reclaimed slot (format: "{pid} {timestamp}").
+                content = pid_file.read_text().strip()
+                assert content.split()[0] == "4242"
 
 
 def test_spawn_mine_releases_slot_on_oserror(tmp_path):
@@ -978,7 +985,8 @@ def test_ingest_transcript_skips_when_target_running(tmp_path):
                 ]
                 pid_file = _pid_file_for_cmd(expected_cmd)
                 pid_file.parent.mkdir(parents=True, exist_ok=True)
-                pid_file.write_text(str(os.getpid()))  # live target
+                import time as _time
+                pid_file.write_text(f"{os.getpid()} {int(_time.time())}")  # live target, fresh
 
                 with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen:
                     _ingest_transcript(str(transcript))
@@ -1016,14 +1024,75 @@ def test_mine_already_running_dead_pid(tmp_path):
 
 
 def test_mine_already_running_live_pid(tmp_path):
-    """Returns True when the slot's recorded PID is alive."""
+    """Returns True when the slot's recorded PID is alive (new {pid ts} format)."""
+    import time as _time
+
     pid_dir = tmp_path / "mine_pids"
     cmd = ["mempalace", "mine", "/tmp/x", "--mode", "projects"]
-    _seed_slot(pid_dir, cmd, str(os.getpid()))  # current process is alive
+    # Use a recent timestamp so the default 2 h timeout does not trigger.
+    _seed_slot(pid_dir, cmd, f"{os.getpid()} {int(_time.time())}")
     with patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir):
         assert _mine_already_running(cmd) is True
 
 
+def test_mine_already_running_live_pid_bare_format(tmp_path):
+    """Old bare-PID format (no timestamp) is still recognized as alive."""
+    pid_dir = tmp_path / "mine_pids"
+    cmd = ["mempalace", "mine", "/tmp/x", "--mode", "projects"]
+    _seed_slot(pid_dir, cmd, str(os.getpid()))  # old format: bare PID
+    with patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir):
+        # No timestamp → treated as start_ts=0 (infinitely old) → stale.
+        # Verify that the default 2 h timeout fires and returns False.
+        assert _mine_already_running(cmd) is False
+
+
+def test_mine_already_running_live_pid_exceeds_timeout(tmp_path):
+    """Returns False when PID is alive but has exceeded the configured timeout (#1552)."""
+    import time as _time
+
+    pid_dir = tmp_path / "mine_pids"
+    cmd = ["mempalace", "mine", "/tmp/x", "--mode", "projects"]
+    # Timestamp far in the past so any positive timeout fires immediately.
+    old_ts = int(_time.time()) - 3601  # 1 second past 1-hour mark
+    _seed_slot(pid_dir, cmd, f"{os.getpid()} {old_ts}")
+    with (
+        patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir),
+        patch.dict("os.environ", {"MEMPALACE_MINE_TIMEOUT_HOURS": "1"}),
+    ):
+        assert _mine_already_running(cmd) is False
+
+
+def test_mine_already_running_live_pid_within_timeout(tmp_path):
+    """Returns True when PID is alive and has NOT exceeded the configured timeout."""
+    import time as _time
+
+    pid_dir = tmp_path / "mine_pids"
+    cmd = ["mempalace", "mine", "/tmp/x", "--mode", "projects"]
+    recent_ts = int(_time.time()) - 60  # only 1 minute old
+    _seed_slot(pid_dir, cmd, f"{os.getpid()} {recent_ts}")
+    with (
+        patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir),
+        patch.dict("os.environ", {"MEMPALACE_MINE_TIMEOUT_HOURS": "2"}),
+    ):
+        assert _mine_already_running(cmd) is True
+
+
+def test_mine_already_running_timeout_zero_disables_check(tmp_path):
+    """MEMPALACE_MINE_TIMEOUT_HOURS=0 disables the age-based stale check."""
+    import time as _time
+
+    pid_dir = tmp_path / "mine_pids"
+    cmd = ["mempalace", "mine", "/tmp/x", "--mode", "projects"]
+    old_ts = int(_time.time()) - 86400  # 24 hours ago — stale under any non-zero timeout
+    _seed_slot(pid_dir, cmd, f"{os.getpid()} {old_ts}")
+    with (
+        patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir),
+        patch.dict("os.environ", {"MEMPALACE_MINE_TIMEOUT_HOURS": "0"}),
+    ):
+        # Timeout disabled — alive PID is always considered running.
+        assert _mine_already_running(cmd) is True
+
+
 def test_mine_already_running_corrupt_file(tmp_path):
     """Returns False when the slot contains non-integer content."""
     pid_dir = tmp_path / "mine_pids"
@@ -1035,10 +1104,13 @@ def test_mine_already_running_corrupt_file(tmp_path):
 
 def test_mine_already_running_distinct_cmds_independent(tmp_path):
     """Slots are keyed per cmd; an alive entry for cmd A doesn't shadow cmd B."""
+    import time as _time
+
     pid_dir = tmp_path / "mine_pids"
     cmd_a = ["mempalace", "mine", "/tmp/a", "--mode", "projects"]
     cmd_b = ["mempalace", "mine", "/tmp/b", "--mode", "projects"]
-    _seed_slot(pid_dir, cmd_a, str(os.getpid()))
+    recent_ts = int(_time.time())
+    _seed_slot(pid_dir, cmd_a, f"{os.getpid()} {recent_ts}")
     with patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir):
         assert _mine_already_running(cmd_a) is True
         assert _mine_already_running(cmd_b) is False
diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py
index ecd60afea4..e7ddbda0fe 100644
--- a/tests/test_mcp_server.py
+++ b/tests/test_mcp_server.py
@@ -2309,13 +2309,107 @@ def test_multi_tenant_env_switch(self, tmp_path, monkeypatch):
         )
         assert add_result.get("success") is True, add_result
 
-        monkeypatch.setenv("MEMPALACE_PALACE_PATH", str(tmp_b))
-        query_b = mcp_server.tool_kg_query(entity="alice_secret")
-        assert query_b.get("count", 0) == 0, f"tenant B leaked tenant A's fact: {query_b}"
 
-        monkeypatch.setenv("MEMPALACE_PALACE_PATH", str(tmp_a))
-        query_a = mcp_server.tool_kg_query(entity="alice_secret")
-        assert query_a.get("count", 0) >= 1, f"tenant A lost its own fact: {query_a}"
+# ── Structured error codes + MineAlreadyRunning (#1552) ─────────────────
+
+
+class TestStructuredErrors:
+    """Verify that _internal_tool_error and MineAlreadyRunning return
+    machine-readable structured data (#1552)."""
+
+    def test_internal_tool_error_without_exc_has_no_data_field(self):
+        """Backward-compat: callers that omit exc still get a valid error dict."""
+        from mempalace.mcp_server import _internal_tool_error
+
+        try:
+            raise ValueError("test error")
+        except ValueError:
+            resp = _internal_tool_error("req-1", "mempalace_search")
+
+        assert resp["jsonrpc"] == "2.0"
+        assert resp["id"] == "req-1"
+        err = resp["error"]
+        assert err["code"] == -32000
+        assert err["message"] == "Internal tool error"
+        assert "data" not in err
+
+    def test_internal_tool_error_with_exc_includes_structured_data(self):
+        """When exc is supplied, the error body must include data.error_class
+        and data.message so callers can distinguish error types (#1552)."""
+        from mempalace.mcp_server import _internal_tool_error
+
+        exc = RuntimeError("chromadb cold init wedge")
+        try:
+            raise exc
+        except RuntimeError:
+            resp = _internal_tool_error("req-2", "mempalace_add_drawer", exc)
+
+        err = resp["error"]
+        assert err["code"] == -32000
+        assert "data" in err
+        assert err["data"]["error_class"] == "RuntimeError"
+        assert "chromadb cold init wedge" in err["data"]["message"]
+
+    def test_internal_tool_error_exception_dispatch_passes_exc(self, monkeypatch):
+        """handle_request's Exception branch must pass exc to _internal_tool_error."""
+        from mempalace import mcp_server
+
+        captured = {}
+
+        def fake_handler(**kwargs):
+            raise OSError("fake disk error")
+
+        fake_tool_entry = {
+            "handler": fake_handler,
+            "input_schema": {"type": "object", "properties": {}},
+        }
+        monkeypatch.setattr(
+            mcp_server,
+            "TOOLS",
+            {"mempalace_fake": fake_tool_entry},
+        )
+
+        original = mcp_server._internal_tool_error
+
+        def spy_error(req_id, tool_name, exc=None):
+            captured["exc"] = exc
+            return original(req_id, tool_name, exc)
+
+        monkeypatch.setattr(mcp_server, "_internal_tool_error", spy_error)
+
+        req = {
+            "jsonrpc": "2.0",
+            "id": "r1",
+            "method": "tools/call",
+            "params": {"name": "mempalace_fake", "arguments": {}},
+        }
+        resp = mcp_server.handle_request(req)
+        assert resp["error"]["code"] == -32000
+        assert isinstance(captured.get("exc"), OSError)
+        assert "data" in resp["error"]
+        assert resp["error"]["data"]["error_class"] == "OSError"
+
+    def test_tool_sync_mine_already_running_returns_error_class(self, monkeypatch, tmp_path):
+        """tool_sync MineAlreadyRunning path returns error_class: LockHeldByOtherProcess."""
+        from mempalace import mcp_server
+        from mempalace.palace import MineAlreadyRunning
+
+        cfg = MagicMock()
+        cfg.palace_path = str(tmp_path / "palace")
+        monkeypatch.setattr(mcp_server, "_config", cfg)
+        monkeypatch.setattr(mcp_server, "_get_kg", lambda *a, **kw: MagicMock())
+
+        def _raise_locked(*args, **kwargs):
+            raise MineAlreadyRunning("pid=12345")
+
+        import mempalace.sync as sync_mod
+
+        monkeypatch.setattr(sync_mod, "sync_palace", _raise_locked, raising=False)
+
+        result = mcp_server.tool_sync()
+        assert result["success"] is False
+        assert "another mine is in progress" in result["error"]
+        assert result.get("error_class") == "LockHeldByOtherProcess"
 
     def test_cache_thread_safe(self, tmp_path, monkeypatch):
         """Concurrent _get_kg() for the same path yields one instance."""

From 1c818d5cdd95bc3275e0d9a21791d27a13ffa3e3 Mon Sep 17 00:00:00 2001
From: Igor Lins e Silva <4753812+igorls@users.noreply.github.com>
Date: Wed, 20 May 2026 15:59:19 -0300
Subject: [PATCH 2/3] fix: address stale hook review feedback

- Disable mine and MCP idle timeouts when env values are invalid.
- Use bare-PID slot file mtime as the compatibility timestamp instead of treating old slots as infinitely stale.
- Treat malformed PID-slot timestamps as stale without crashing hook execution.
- Use process-level idle watchdog termination so stale MCP servers actually release file handles.
- Restore tenant-isolation assertions accidentally removed from the KG cache test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 mempalace/hooks_cli.py   | 18 +++++++++++++-----
 mempalace/mcp_server.py  |  6 +++---
 tests/test_hooks_cli.py  | 37 ++++++++++++++++++++++++++++++++++---
 tests/test_mcp_server.py | 15 +++++++++++++++
 4 files changed, 65 insertions(+), 11 deletions(-)

diff --git a/mempalace/hooks_cli.py b/mempalace/hooks_cli.py
index d0c741b798..33c93f241f 100644
--- a/mempalace/hooks_cli.py
+++ b/mempalace/hooks_cli.py
@@ -304,7 +304,7 @@ def _mine_slot_timeout_secs() -> float:
             hours = float(raw)
             return max(0.0, hours) * 3600
         except ValueError:
-            pass
+            return 0.0
     return _MINE_TIMEOUT_HOURS_DEFAULT * 3600
 
 
@@ -363,9 +363,8 @@ def _mine_already_running(cmd: list[str]) -> bool:
 
     The PID file format is ``{pid} {unix_timestamp}`` (timestamp added in
     #1552 to detect wedged subprocesses).  Old-format files (bare ``{pid}``)
-    are treated as having a start time of 0 — effectively "infinitely old" —
-    so they are immediately considered stale once the configured timeout
-    elapses.
+    use the PID file's mtime as the approximate start time so a still-running
+    pre-upgrade mine is not immediately misclassified as stale.
 
     A process is considered stale (and this function returns False) when:
     - the PID is dead, OR
@@ -387,7 +386,16 @@ def _mine_already_running(cmd: list[str]) -> bool:
         return False
     timeout_secs = _mine_slot_timeout_secs()
     if timeout_secs > 0:
-        start_ts = float(parts[1]) if len(parts) > 1 and parts[1] else 0.0
+        if len(parts) > 1 and parts[1]:
+            try:
+                start_ts = float(parts[1])
+            except ValueError:
+                return False
+        else:
+            try:
+                start_ts = pid_file.stat().st_mtime
+            except OSError:
+                return True
         if time.time() - start_ts > timeout_secs:
             return False
     return True
diff --git a/mempalace/mcp_server.py b/mempalace/mcp_server.py
index 90dfdda7fa..6e5a163ae2 100644
--- a/mempalace/mcp_server.py
+++ b/mempalace/mcp_server.py
@@ -194,7 +194,7 @@ def _mcp_idle_timeout_secs() -> float:
             hours = float(raw)
             return max(0.0, hours) * 3600
         except ValueError:
-            pass
+            return 0.0
     return _MCP_IDLE_HOURS_DEFAULT * 3600
 
 
@@ -2663,7 +2663,7 @@ def _start_idle_exit_watchdog() -> None:
     """Start a daemon thread that exits the process after an idle period.
 
     When no request has been handled for ``MEMPALACE_MCP_IDLE_HOURS``
-    (default 8 h), the thread calls ``sys.exit(0)`` so that stale MCP
+    (default 8 h), the thread terminates the process so that stale MCP
     servers from ended Claude Code sessions do not accumulate ChromaDB /
     HNSW file handles on Windows (#1552).
 
@@ -2684,7 +2684,7 @@ def _watchdog() -> None:
                     idle / 3600,
                     timeout / 3600,
                 )
-                sys.exit(0)
+                os._exit(0)
 
     t = threading.Thread(target=_watchdog, name="mcp-idle-watchdog", daemon=True)
     t.start()
diff --git a/tests/test_hooks_cli.py b/tests/test_hooks_cli.py
index f22ca68867..4270f1240f 100644
--- a/tests/test_hooks_cli.py
+++ b/tests/test_hooks_cli.py
@@ -1036,16 +1036,47 @@ def test_mine_already_running_live_pid(tmp_path):
 
 
 def test_mine_already_running_live_pid_bare_format(tmp_path):
-    """Old bare-PID format (no timestamp) is still recognized as alive."""
+    """Old bare-PID format uses file mtime for the stale-by-age check."""
     pid_dir = tmp_path / "mine_pids"
     cmd = ["mempalace", "mine", "/tmp/x", "--mode", "projects"]
     _seed_slot(pid_dir, cmd, str(os.getpid()))  # old format: bare PID
     with patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir):
-        # No timestamp → treated as start_ts=0 (infinitely old) → stale.
-        # Verify that the default 2 h timeout fires and returns False.
+        assert _mine_already_running(cmd) is True
+
+
+def test_mine_already_running_bare_pid_old_mtime_is_stale(tmp_path):
+    """Old bare-PID slots are reclaimed once their file mtime exceeds timeout."""
+    import time as _time
+
+    pid_dir = tmp_path / "mine_pids"
+    cmd = ["mempalace", "mine", "/tmp/x", "--mode", "projects"]
+    slot = _seed_slot(pid_dir, cmd, str(os.getpid()))
+    old_mtime = _time.time() - 3601
+    os.utime(slot, (old_mtime, old_mtime))
+    with (
+        patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir),
+        patch.dict("os.environ", {"MEMPALACE_MINE_TIMEOUT_HOURS": "1"}),
+    ):
+        assert _mine_already_running(cmd) is False
+
+
+def test_mine_already_running_malformed_timestamp_is_stale(tmp_path):
+    """Malformed timestamps fail soft instead of crashing hook execution."""
+    pid_dir = tmp_path / "mine_pids"
+    cmd = ["mempalace", "mine", "/tmp/x", "--mode", "projects"]
+    _seed_slot(pid_dir, cmd, f"{os.getpid()} not-a-timestamp")
+    with patch("mempalace.hooks_cli._MINE_PID_DIR", pid_dir):
         assert _mine_already_running(cmd) is False
 
 
+def test_mine_slot_timeout_invalid_env_disables_timeout():
+    """Invalid MEMPALACE_MINE_TIMEOUT_HOURS disables stale-by-age checks."""
+    from mempalace.hooks_cli import _mine_slot_timeout_secs
+
+    with patch.dict("os.environ", {"MEMPALACE_MINE_TIMEOUT_HOURS": "nope"}):
+        assert _mine_slot_timeout_secs() == 0.0
+
+
 def test_mine_already_running_live_pid_exceeds_timeout(tmp_path):
     """Returns False when PID is alive but has exceeded the configured timeout (#1552)."""
     import time as _time
diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py
index e7ddbda0fe..d3ba4aaeb4 100644
--- a/tests/test_mcp_server.py
+++ b/tests/test_mcp_server.py
@@ -2309,6 +2309,14 @@ def test_multi_tenant_env_switch(self, tmp_path, monkeypatch):
         )
         assert add_result.get("success") is True, add_result
 
+        monkeypatch.setenv("MEMPALACE_PALACE_PATH", str(tmp_b))
+        query_b = mcp_server.tool_kg_query(entity="alice_secret")
+        assert query_b.get("count", 0) == 0, f"tenant B leaked tenant A's fact: {query_b}"
+
+        monkeypatch.setenv("MEMPALACE_PALACE_PATH", str(tmp_a))
+        query_a = mcp_server.tool_kg_query(entity="alice_secret")
+        assert query_a.get("count", 0) >= 1, f"tenant A lost its own fact: {query_a}"
+
 
 # ── Structured error codes + MineAlreadyRunning (#1552) ─────────────────
 
@@ -2411,6 +2419,13 @@ def _raise_locked(*args, **kwargs):
         assert "another mine is in progress" in result["error"]
         assert result.get("error_class") == "LockHeldByOtherProcess"
 
+    def test_mcp_idle_timeout_invalid_env_disables_watchdog(self, monkeypatch):
+        """Invalid MEMPALACE_MCP_IDLE_HOURS disables idle auto-exit."""
+        from mempalace import mcp_server
+
+        monkeypatch.setenv("MEMPALACE_MCP_IDLE_HOURS", "not-a-float")
+        assert mcp_server._mcp_idle_timeout_secs() == 0.0
+
     def test_cache_thread_safe(self, tmp_path, monkeypatch):
         """Concurrent _get_kg() for the same path yields one instance."""
         import concurrent.futures

From 5f0a2140160a0a83cf6aa32829ef2db1efdadf49 Mon Sep 17 00:00:00 2001
From: Igor Lins e Silva <4753812+igorls@users.noreply.github.com>
Date: Wed, 20 May 2026 13:09:15 -0300
Subject: [PATCH 3/3] style: format hook tests

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 tests/test_hooks_cli.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/test_hooks_cli.py b/tests/test_hooks_cli.py
index 4270f1240f..44b70e4ee9 100644
--- a/tests/test_hooks_cli.py
+++ b/tests/test_hooks_cli.py
@@ -793,6 +793,7 @@ def test_maybe_auto_ingest_skips_when_mine_running(tmp_path):
                 pid_file = _pid_file_for_cmd(cmd)
                 pid_file.parent.mkdir(parents=True, exist_ok=True)
                 import time as _time
+
                 pid_file.write_text(f"{os.getpid()} {int(_time.time())}")
                 with patch("mempalace.hooks_cli._mempalace_python", return_value=sys.executable):
                     with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen:
@@ -986,6 +987,7 @@ def test_ingest_transcript_skips_when_target_running(tmp_path):
                 pid_file = _pid_file_for_cmd(expected_cmd)
                 pid_file.parent.mkdir(parents=True, exist_ok=True)
                 import time as _time
+
                 pid_file.write_text(f"{os.getpid()} {int(_time.time())}")  # live target, fresh
 
                 with patch("mempalace.hooks_cli.subprocess.Popen") as mock_popen: