fix: preserve Redis client ownership boundaries

[mikemikimike]

Summary

Fixes #660.

SearchIndex.set_client() could keep ownership of a caller-provided Redis client and close it during disconnect or garbage collection. It also left the previously owned client open when replacing it. The sync connect() path had the corresponding replacement-lifecycle gap.

This change closes a previously owned client before replacement, marks clients supplied through set_client() as unowned, and keeps clients created by connect() owned by the index. Async ownership behavior is preserved and covered by a focused regression test.

Compatibility

Explicitly injected clients are no longer closed by the index, matching constructor injection semantics. Clients created by connect() remain index-owned and are closed as before. No Redis protocol or data behavior changes.

Validation

• .venv\\Scripts\\python.exe -m pytest tests/unit/test_connection_normalization.py -q --confcutdir=tests/unit -p pytest_asyncio — 12 passed.
• .venv\\Scripts\\python.exe -m pytest tests/unit/test_index_gc_finalizer.py -q --confcutdir=tests/unit -p pytest_asyncio — 11 passed.
• python -m ruff check redisvl/index/index.py tests/unit/test_connection_normalization.py tests/unit/test_index_gc_finalizer.py — passed.
• python -m ruff format --check redisvl/index/index.py tests/unit/test_connection_normalization.py — passed.
• git diff --check — passed.

The repository Docker-based test fixture was attempted, but pulling redis:8.4 failed with a registry EOF before any service started. The changed paths are covered by service-free unit tests; full Docker-backed integration tests were not run.

---

Note

Medium Risk
Changes connection teardown semantics for apps using deprecated connect()/set_client(); injected clients will no longer be closed by the index, which fixes a bug but may leave connections open if callers relied on the old behavior.

Overview
Fixes incorrect Redis client lifecycle on sync SearchIndex when swapping connections: set_client() no longer treats caller-supplied clients as index-owned, and connect() now closes and detaches a previously index-owned client before opening a new factory connection.

SearchIndex.connect() — If the index currently owns its client, the change detaches the GC finalizer, closes that client, then builds a new connection and sets _owns_redis_client = True so disconnect() still closes factory-created clients.

SearchIndex.set_client() — Same cleanup when replacing an owned client, then sets _owns_redis_client = False so injected clients are never closed by disconnect() or ownership finalizers (aligned with constructor redis_client= behavior).

Unit tests in test_connection_normalization.py lock in: owned client closed on set_client, external client left open on disconnect; connect() closes the old owned client and keeps ownership of the new one; async set_client does not take ownership.

^{Reviewed by Cursor Bugbot for commit 9dc39ae. Bugbot is set up for automated code reviews on this repo. Configure here.}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit 9dc39ae. Configure here.}

[cursor]

Failed connect closes the live client

Medium Severity

A failed connect() closes the currently owned Redis client before the replacement is created, so a ConnectionError or missing REDIS_URL leaves the index pointing at a closed client. Later operations keep using that dead connection instead of recovering.

 

^{Reviewed by Cursor Bugbot for commit 9dc39ae. Configure here.}