From 97b4dfd800a922dcfe87b46f756ccc8c68316133 Mon Sep 17 00:00:00 2001 From: drknowhow Date: Tue, 22 Sep 2026 08:57:12 -0400 Subject: [PATCH] fix(embed): a corrupt embedding store no longer kills the MCP server (2.145.2) A damaged HNSW segment opens cleanly and faults on the first real read, which lands on the background c3-embed-index thread as a native access violation and kills the MCP server with no error. _open_chroma now probes with count() on the init path, quarantines a store that fails, and reopens empty (one retry); the hash file moves with it so the rebuild happens. Rebased from #170 onto 2.145.1; only version files and CHANGELOG conflicted. Verified: the 5 new tests fail on main's embedding_index.py and pass here. Claude-Session: https://claude.ai/code/session_01DEKoSbeEFepPGwPHQDHxAJ --- CHANGELOG.md | 23 +++ cli/c3.py | 2 +- pyproject.toml | 2 +- services/embedding_index.py | 89 ++++++++-- tests/test_embedding_index_corrupt_store.py | 187 ++++++++++++++++++++ 5 files changed, 288 insertions(+), 15 deletions(-) create mode 100644 tests/test_embedding_index_corrupt_store.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f89259..5a501c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,29 @@ All notable changes to Code Context Control (C3) are documented here. The format is loosely based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [2.145.2] - 2026-09-22 + +### Fixed — a corrupt embedding store no longer kills the MCP server (#170) + +A damaged HNSW segment does not surface when the store is opened: chromadb +hands back a client and a collection handle, then faults on the first real +read. That read lands on the background `c3-embed-index` thread +(`build` → `_remove_file_chunks` → `Collection.get`), and the fault is a native +access violation, so the whole process dies. The host sees a server that +connected, listed its tools and vanished; Claude Code reported `c3` as failed +to connect with no error to chase. It happened on 2026-07-17 and 2026-09-06. + +- `EmbeddingIndex._open_chroma` probes the store with `count()` on the init + path, where the failure is still a catchable Python exception, and treats a + failed probe as corruption. It closes the client (Windows will not release + `chroma.sqlite3` otherwise), moves the store and its hash file to + `.c3/embeddings/quarantine_corrupt_/`, and reopens empty. +- The hash file moves too. It claims vectors the fresh store does not have, so + leaving it would suppress the rebuild the quarantine exists to force. +- One retry, never a loop. If the replacement store also fails its probe, the + index degrades to unavailable instead of taking the server down. The store + is a rebuildable cache, so recovery costs one re-embed and no data. + ## [2.145.1] - 2026-09-22 ### Changed — the in-app guide describes the C3 you have installed diff --git a/cli/c3.py b/cli/c3.py index 08a0c08..62310a1 100644 --- a/cli/c3.py +++ b/cli/c3.py @@ -91,7 +91,7 @@ # Config CONFIG_DIR = ".c3" CONFIG_FILE = ".c3/config.json" -__version__ = "2.145.1" +__version__ = "2.145.2" # The PreToolUse matcher for native subagent calls (installer and hub migration). AGENT_MATCHER = "Agent|Task" diff --git a/pyproject.toml b/pyproject.toml index 5cb5839..afb8171 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta" [project] name = "code-context-control" -version = "2.145.1" +version = "2.145.2" description = "Local MCP code-intelligence for AI coding tools: surgical search/read/edit, agent-config version history, path-level access + masking guards, and a multi-project hub." readme = "README.md" requires-python = ">=3.10" diff --git a/services/embedding_index.py b/services/embedding_index.py index bc3a664..39c70a2 100644 --- a/services/embedding_index.py +++ b/services/embedding_index.py @@ -6,10 +6,13 @@ Falls back gracefully when Ollama or chromadb are unavailable. """ +import gc import hashlib import json import logging +import shutil import threading +from datetime import datetime from pathlib import Path log = logging.getLogger("c3.embedding_index") @@ -106,19 +109,7 @@ def warm(self): def _init_backends(self): """Initialize chromadb collection and check Ollama.""" try: - import chromadb - from chromadb.config import Settings - - persist_dir = str(self._index_dir / "chromadb") - Path(persist_dir).mkdir(parents=True, exist_ok=True) - self._chroma_client = chromadb.PersistentClient( - path=persist_dir, - settings=Settings(anonymized_telemetry=False), - ) - self._collection = self._chroma_client.get_or_create_collection( - name=COLLECTION_NAME, - metadata={"hnsw:space": "cosine"}, - ) + self._open_chroma() self._available = True self._drop_legacy_collection() except Exception as e: @@ -135,6 +126,78 @@ def _init_backends(self): self._model_ok = False self._ollama_ok = self._ollama_up and self._model_ok + def _open_chroma(self, _retried: bool = False) -> None: + """Open the persistent collection, quarantining a corrupt store once. + + A damaged HNSW segment does not surface at open time: the Rust backend + accepts the client and the collection handle, then faults on the first + real read. When that read happens on the background index thread it has + taken the whole MCP process down with an access violation, which the + host reports only as a dead server (seen 2026-07-17, 2026-09-06). + + So probe here, on the init path, where the failure is still a catchable + Python exception, and treat a failed probe as a corrupt store: close the + client, move the directory aside, and reopen empty. The store is a + rebuildable cache, so the cost is one re-embed, not lost data. + """ + import chromadb + from chromadb.config import Settings + + persist_dir = self._index_dir / "chromadb" + persist_dir.mkdir(parents=True, exist_ok=True) + self._chroma_client = chromadb.PersistentClient( + path=str(persist_dir), + settings=Settings(anonymized_telemetry=False), + ) + self._collection = self._chroma_client.get_or_create_collection( + name=COLLECTION_NAME, + metadata={"hnsw:space": "cosine"}, + ) + try: + self._collection.count() + except Exception as e: + if _retried: + raise + log.warning( + "embedding store failed its health probe (%s); " + "quarantining and rebuilding", + e, + ) + self._quarantine_store(persist_dir) + self._open_chroma(_retried=True) + + def _close_chroma(self) -> None: + """Drop the client so Windows releases its handle on chroma.sqlite3.""" + client = self._chroma_client + self._collection = None + self._chroma_client = None + try: + close = getattr(client, "close", None) + if callable(close): + close() + except Exception: + pass + gc.collect() + + def _quarantine_store(self, persist_dir: Path) -> None: + """Move a corrupt store and its hash file aside for post-mortem. + + The hashes go with it: they claim vectors that the fresh store does not + have, so leaving them behind would suppress the very rebuild this is + meant to trigger. + """ + self._close_chroma() + dest = self._index_dir / f"quarantine_corrupt_{datetime.now():%Y%m%d_%H%M%S}" + dest.mkdir(parents=True, exist_ok=True) + shutil.move(str(persist_dir), str(dest / persist_dir.name)) + try: + if self._hash_file.exists(): + shutil.move(str(self._hash_file), str(dest / self._hash_file.name)) + except OSError: + self._hash_file.unlink(missing_ok=True) + self._file_hashes = {} + log.warning("quarantined corrupt embedding store to %s", dest) + def _drop_legacy_collection(self) -> None: """Best-effort removal of the pre-2.108.0 collection and hash file. diff --git a/tests/test_embedding_index_corrupt_store.py b/tests/test_embedding_index_corrupt_store.py new file mode 100644 index 0000000..6e11dfd --- /dev/null +++ b/tests/test_embedding_index_corrupt_store.py @@ -0,0 +1,187 @@ +"""Regression tests for corrupt-store recovery in services/embedding_index.py. + +A damaged HNSW segment does not announce itself at open time. chromadb hands +back a client and a collection handle, then faults on the first real read -- +and when that read lands on the background index thread, the fault is a native +access violation that kills the whole MCP process. The host sees only a server +that connected, listed tools and died (observed 2026-07-17 and 2026-09-06). + +The fix probes the store on the init path, where the failure is still a +catchable Python exception, and treats a failed probe as corruption: close the +client, move the store aside, reopen empty. These tests pin that contract. +""" +import json + +import pytest + +from services import embedding_index as ei_mod +from services.embedding_index import COLLECTION_NAME, EmbeddingIndex + + +class _FakeCollection: + """A collection whose health probe can be made to fail.""" + + def __init__(self, count_error=None): + self._count_error = count_error + self.count_calls = 0 + + def count(self): + self.count_calls += 1 + if self._count_error is not None: + raise self._count_error + + +class _FakeClient: + def __init__(self, path, collection, on_close=None): + self.path = path + self._collection = collection + self._on_close = on_close + self.closed = False + + def get_or_create_collection(self, name=None, metadata=None, **kwargs): + assert name == COLLECTION_NAME + return self._collection + + def close(self): + self.closed = True + if self._on_close is not None: + self._on_close() + + +def _install_fake_chromadb(monkeypatch, collections): + """Patch chromadb.PersistentClient to hand out *collections* in order.""" + made = [] + remaining = list(collections) + + class _FakeChromadb: + @staticmethod + def PersistentClient(path=None, settings=None): + client = _FakeClient(path, remaining.pop(0)) + made.append(client) + return client + + class _FakeConfig: + @staticmethod + def Settings(**kwargs): + return object() + + import sys + import types + + mod = types.ModuleType("chromadb") + mod.PersistentClient = _FakeChromadb.PersistentClient + cfg = types.ModuleType("chromadb.config") + cfg.Settings = _FakeConfig.Settings + mod.config = cfg + monkeypatch.setitem(sys.modules, "chromadb", mod) + monkeypatch.setitem(sys.modules, "chromadb.config", cfg) + return made + + +def _make_index(tmp_path) -> EmbeddingIndex: + return EmbeddingIndex(str(tmp_path), ollama_client=None) + + +def test_healthy_store_is_not_quarantined(tmp_path, monkeypatch): + """A store that answers count() is left exactly where it is.""" + healthy = _FakeCollection() + made = _install_fake_chromadb(monkeypatch, [healthy]) + idx = _make_index(tmp_path) + + idx._open_chroma() + + assert healthy.count_calls == 1, "the health probe must actually run" + assert len(made) == 1, "a healthy store must not be reopened" + assert not made[0].closed + assert (tmp_path / ".c3" / "embeddings" / "chromadb").is_dir() + assert not list((tmp_path / ".c3" / "embeddings").glob("quarantine_corrupt_*")) + + +def test_corrupt_store_is_quarantined_and_reopened_empty(tmp_path, monkeypatch): + """A failed probe moves the store aside and reopens, rather than raising.""" + store = tmp_path / ".c3" / "embeddings" / "chromadb" + store.mkdir(parents=True, exist_ok=True) + (store / "chroma.sqlite3").write_text("damaged", encoding="utf-8") + + boom = RuntimeError( + "Error executing plan: Error sending backfill request to compactor: " + "Failed to apply logs to the hnsw segment writer" + ) + corrupt, fresh = _FakeCollection(count_error=boom), _FakeCollection() + made = _install_fake_chromadb(monkeypatch, [corrupt, fresh]) + idx = _make_index(tmp_path) + + idx._open_chroma() + + # It recovered onto the second client rather than propagating the error. + assert len(made) == 2 + assert made[0].closed, "the corrupt client must be closed before the move" + assert idx._collection is fresh + assert fresh.count_calls == 1, "the replacement store is probed too" + + # The damaged bytes were preserved for post-mortem, not deleted. + quarantines = list((tmp_path / ".c3" / "embeddings").glob("quarantine_corrupt_*")) + assert len(quarantines) == 1 + moved = quarantines[0] / "chromadb" / "chroma.sqlite3" + assert moved.read_text(encoding="utf-8") == "damaged" + + +def test_quarantine_takes_the_hash_file_with_it(tmp_path, monkeypatch): + """Stale hashes would suppress the rebuild the quarantine exists to force.""" + embeddings = tmp_path / ".c3" / "embeddings" + embeddings.mkdir(parents=True, exist_ok=True) + (embeddings / "chromadb").mkdir(exist_ok=True) + hash_file = embeddings / "file_hashes_v2.json" + hash_file.write_text(json.dumps({"cli/mcp_server.py": "deadbeef"}), encoding="utf-8") + + boom = RuntimeError("Failed to apply logs to the hnsw segment writer") + _install_fake_chromadb(monkeypatch, [_FakeCollection(count_error=boom), _FakeCollection()]) + idx = _make_index(tmp_path) + idx._file_hashes = {"cli/mcp_server.py": "deadbeef"} + + idx._open_chroma() + + assert not hash_file.exists(), "stale hashes must not survive the quarantine" + assert idx._file_hashes == {}, "in-memory hashes must be dropped too" + quarantined = next(embeddings.glob("quarantine_corrupt_*")) / "file_hashes_v2.json" + assert json.loads(quarantined.read_text(encoding="utf-8")) == { + "cli/mcp_server.py": "deadbeef" + } + + +def test_second_failure_raises_instead_of_looping(tmp_path, monkeypatch): + """One quarantine, then give up: a rebuild loop is worse than degrading.""" + boom = RuntimeError("Failed to apply logs to the hnsw segment writer") + made = _install_fake_chromadb( + monkeypatch, + [_FakeCollection(count_error=boom), _FakeCollection(count_error=boom)], + ) + idx = _make_index(tmp_path) + + with pytest.raises(RuntimeError): + idx._open_chroma() + + assert len(made) == 2, "exactly one retry, not an unbounded loop" + + +def test_init_backends_degrades_instead_of_killing_the_server(tmp_path, monkeypatch): + """Unrecoverable corruption marks the index unavailable; it never raises.""" + boom = RuntimeError("Failed to apply logs to the hnsw segment writer") + _install_fake_chromadb( + monkeypatch, + [_FakeCollection(count_error=boom), _FakeCollection(count_error=boom)], + ) + idx = _make_index(tmp_path) + + class _DeadOllama: + def is_available(self, timeout=None): + return False + + def has_model(self, model): + return False + + idx.ollama = _DeadOllama() + + idx._init_backends() # must not raise + + assert idx._available is False