fix(mcp_server): reset chromadb System cache on staleness reconnect (#2002)

[colorpanda82]

Fixes #2002. Part of the #1963 concurrent-writer epic; independent of the #1976 daemon/bridge work.

Thanks @fatkobra for the go-ahead on a focused PR.

Problem

_get_client() detects a peer writer's inode/mtime change on chroma.sqlite3 and rebuilds the client via ChromaBackend.make_client(). But chromadb caches its System, and therefore the live in-memory PersistentLocalHnswSegment, keyed by path. So the rebuilt "fresh" client is handed back the same stale segment, which on its next _persist() writes its outdated index over the on-disk one, destroying records other writers had already indexed.

Observed on a live multi-writer palace: the persisted HNSW count went backwards (4 to 3). Over one day, sqlite grew by ~107 drawers while the index grew by ~40, so roughly 63% of new drawers never reached the index and vector search silently degraded to BM25.

Fix

Call the existing _force_chroma_cache_reset() on the staleness path, before make_client(), so chromadb rebuilds the segment from the on-disk state. It is guarded by the existing inode_changed or mtime_changed check, so first-open is unaffected.

_force_chroma_cache_reset() already closes the client, drops the backend's per-palace cache, and calls SharedSystemClient.clear_system_cache(). It simply was not being called on this path (it is already used in the missing-DB and repair paths). clear_system_cache() is the accepted workaround for this class of path-keyed staleness in chromadb (cf. chroma-core/chroma#2536, chroma-core/chroma#5843).

Test

test_get_client_resets_chroma_system_cache_on_reconnect asserts the reset runs before make_client() on an mtime reconnect. It fails on develop (order is ['make_client']) and passes with the fix (['reset', 'make_client']). The existing test_get_client_rearms_quarantine_on_reconnect (#1573) and the inode/mtime invalidation tests still pass.

Scope

This removes one concrete data-loss path on the staleness reconnect. It does not make multi-writer safe on its own; chromadb PersistentClient is single-process by design, which is what #1976 / #1270 address at the architecture level.

Manual multi-process reproduction

With a long-lived writer plus a genuinely separate process:

1. long-lived writer indexes 3 records,
2. separate process indexes 2,
3. long-lived writer indexes 1.

Without the fix the persisted index count regresses; with the fix it ends at 6 (monotonic). Happy to add this as a @pytest.mark.slow integration test if you'd prefer it in-tree; I kept it out of the suite to keep CI deterministic.

[fatkobra]

Reviewed the reconnect path, _force_chroma_cache_reset(), and the new regression test.

The fix is correctly placed: the full Chroma system-cache reset runs only after an inode/mtime change is detected and before ChromaBackend.make_client() reconstructs the client. First-open behavior is unchanged.

The test is also meaningful: it uses a real palace, primes the client cache, simulates a peer-write mtime change, and verifies the required reset -> make_client ordering.

This is a focused and appropriate fix for #2002. It does not claim to make multi-writer Chroma safe generally, and it complements the daemon/single-writer work in #1976 rather than duplicating it.

Approved from my side.

Non-blocking notes:

• A future test could assert the internal SharedSystemClient.clear_system_cache() call directly.
• ChromaBackend._client() has its own inode/mtime reopen path; it may deserve a separate investigation for long-lived non-MCP callers rather than expanding this PR.
• GitHub flags hidden/bidirectional text in the touched files; the added hunk looks normal, but a control-character scan would be useful.

[colorpanda82]

Following up on the three non-blocking notes.

Hidden/bidirectional text. Ran a byte-level scan over the full diff. No bidi controls (U+202A-202E, U+2066-2069), no zero-width or BOM (U+200B-200D, U+2060, U+FEFF), and no other invisible/format or non-ASCII control characters, in the added lines or the surrounding context. The only non-ASCII codepoint in the whole patch is a single U+2014 em dash in a test docstring (tests/test_mcp_server.py), which is neither hidden nor bidirectional, so the flag does not correspond to anything in this patch. Glad to swap that em dash for ASCII if you'd prefer the touched file stay pure ASCII.

Direct assertion of the cache reset. Kept the test at the behavioral reset -> make_client ordering level to avoid coupling to chromadb internals, but I agree an explicit assertion on SharedSystemClient.clear_system_cache() is a reasonable strengthening. Happy to add it here, or fold it into the follow-up test mentioned below, whichever you prefer.

ChromaBackend._client() reopen path. Confirmed on develop: _client() (chroma.py L2044-2064) has its own inode/mtime reopen that reconstructs PersistentClient without a system-cache reset (_force_chroma_cache_reset does not appear in chroma.py), so long-lived non-MCP callers holding a backend across a rebuild would be exposed to the same #2002 class. Filed #2028 to track it separately rather than expand this PR.

[fatkobra]

Thanks for the thorough follow-up.

This addresses all three of my non-blocking notes:

• The byte-level Unicode scan is sufficient. The U+2014 em dash is normal visible punctuation, so there is no need to replace it.
• I agree that the current behavioral reset -> make_client ordering test is the right level for this PR. Since _force_chroma_cache_reset() already exists and this change adds it to the missing reconnect path, a direct assertion against Chroma’s internal SharedSystemClient would add coupling without being necessary for this regression.
• Filing ChromaBackend._client() inode/mtime reopen rebuilds without a chromadb system-cache reset (parallel to #2002) #2028 separately for ChromaBackend._client() is exactly the right scope. That path has a wider backend-level blast radius and deserves its own caller analysis, reproduction, and tests rather than expanding fix(mcp_server): reset chromadb System cache on staleness reconnect (#2002) #2026.

No further changes requested from my side. My approval stands.