fix(hindsight): guard client lifecycle with a leaf lock

[yingliang-zhang]

Summary

HindsightMemoryProvider._client was read and written from at least three threads — the retain writer thread, the background prefetch worker, and the turn/tool thread — with no lock. _get_client() is check-then-act (if self._client is None: self._client = <constructor>), and the embedded constructor takes seconds (runtime check, optional dependency install, daemon spawn). Two threads hitting a cold or just-nulled client both construct, one wins, and the loser's HindsightEmbedded is orphaned with an aiohttp ClientSession that is never closed — HindsightEmbedded.__del__ is deliberately neutered and _close_client only ever closed the current client. That orphan is a direct generator of the Unclosed client session / Unclosed connector noise in #11923.

The stale-daemon retry path in _run_hindsight_operation (self._client = None then _get_client()) widened the window from microseconds to seconds. The race pre-dates #64745 by five months (0ba6471dd1); #64745's single-prefetch-worker gate narrowed the prefetch side but leaves the writer, tool and daemon-start paths exposed.

The fix (+43/−15 in one file)

• self._client_lock, a leaf lock: _get_client() takes it only on the construction path; the fast path stays lock-free; the lock is never held across _run_sync/operation(client) and never nests with _prefetch_lock or _pending_retain_ops_lock.
• _get_client(*, recreate=False) — double-checked construction; recreate retires the client only if it is still the one the caller observed as broken (identity CAS), so a sibling thread's fresh rebuild is returned as-is instead of being orphaned.
• The retry path no longer nulls the client inline; it records _broken_client and rebuilds via the CAS. The loser is not closed inline (closing against a dead daemon can hang) — only shutdown() closes.
• shutdown() retires the client under the lock before closing it, so a concurrent _get_client() rebuilds instead of racing the close. _close_client → _close_client_of(client) (parameterised; single caller, grep-verified).

Tests

Three new tests in tests/plugins/memory/test_hindsight_provider.py:

• test_client_created_once_under_concurrent_first_access — 8 threads against a 0.2 s stubbed factory; exactly one construction, one shared object.
• test_retry_does_not_orphan_a_sibling_client — retriable-failure retry with 4 concurrent readers against a tracking factory; exactly one replacement client, zero orphans.
• test_shutdown_closes_retired_client_and_allows_rebuild — retired client closed exactly once, _client is None, and a post-shutdown _get_client() rebuilds.

Verification

Check	Result
tests/plugins/memory/test_hindsight_provider.py	95 passed, 1 skipped
tests/plugins/memory/ + tests/agent/test_memory_provider.py	526 passed; 3 failures, all reproduced identically on pristine 44e607be61 (2 × test_mem0_v3 No module named 'mem0', 1 × test_openviking_optional_peer — pre-existing, unrelated)
Mutation check	Reverting _get_client to check-then-act makes test_client_created_once_under_concurrent_first_access RED (8 constructions); restoring makes it green
ruff / py_compile / git diff --check	clean

Follow-up from the #64745 rewrite. Independent of it — this is a client-lifecycle invariant, not a prefetch one — but the two are complementary: #64745 fences what gets published, this fences what gets built.

[teknium1]

Closing: the bundled Hindsight provider this PR patches has moved out of this repo.

Thanks @yingliang-zhang for this contribution. In #119888 (merge 9d799e0531c; removal commit 4cbf862abe4) the in-tree plugins/memory/hindsight/ provider was removed — Hindsight now installs from the plugin catalog and its code lives in vectorize-io/hindsight hindsight-integrations/hermes (maintained by @nicoloboschi). There is no longer any code in this repo for the Hindsight half of this PR to patch, so we are closing every open PR against the bundled provider rather than leaving them stranded.

Triage notes:

• Touches only plugins/memory/hindsight/init.py + its tests: adds a leaf _client_lock with double-checked construction, an identity-CAS recreate path for the stale-daemon retry, and retire-before-close in shutdown(). Never landed (git log -S '_client_lock' / '_close_client_of' on plugins/memory/hindsight/ is empty; the diff also carries the unmerged fix(hindsight): fence prefetch publication to the owning session #64745 _prefetch_generation fence). The pinned upstream still has the unlocked check-then-act _get_client (hindsight-integrations/hermes/init.py:734-738), the inline null-and-rebuild retry (:754-755) and close-then-null shutdown (:1574-1576), so the race is still present there and belongs in the upstream fixes list.
• Still relevant at the catalog pin (dc750388)? Yes — the same code is at hindsight-integrations/hermes/__init__.py:734 in the upstream tree. It is listed with your credit in Fixes from Hermes-side PRs worth carrying into hindsight-integrations/hermes vectorize-io/hindsight#4662 so it is not lost; if you want to carry the fix yourself, please open it against vectorize-io/hindsight — it would be welcome there.

If you believe this was closed in error, comment and we will reopen.

(Bulk-closed in the hindsight-move close pass.)