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 and who may close shared Redis clients; behavior shift for code using set_client() with external clients, but no protocol or data-path changes.

Overview
Fixes incorrect Redis client lifecycle when SearchIndex swaps connections via deprecated connect() or set_client().

set_client() now marks injected clients as not owned (_owns_redis_client = False), so disconnect() and GC finalizers no longer close caller-managed clients. When replacing an index-owned client, it detaches the finalizer and closes the previous client first.

connect() builds the new client before mutating state, so a failed reconnect leaves the existing client intact. On success it closes any previously owned client, assigns the new one, and keeps index ownership for factory-created clients.

Unit tests cover sync replacement, failed connect(), and async set_client() not taking ownership—aligned with constructor-injected client semantics.

^{Reviewed by Cursor Bugbot for commit 19689d4. 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.}

[mikemikimike]

Fixed in 19689d4. connect() now creates the replacement client before detaching or closing the currently owned client, so ConnectionError or missing configuration preserves the live client and ownership state. Added regression coverage for failed replacement. The focused test file is blocked in this machine because its autouse fixture requires Docker Compose and the Docker Desktop Linux engine is unavailable; git diff --check passes.

[vishal-bala]

Hi, and thanks for your contribution! As discussed in the previous PR for this issue, we actually would prefer to have the deprecated client management functionality removed as the more stable fix for this issue rather than continuing to patch it further.
See #661 (review)

That would be my request for this PR if you're looking to make that change, although it hasn't been scoped as such into the issue yet.