fix: share SQLite storage across LCM engine clones

[grantjayy]

Summary

LCM engine clones currently construct a fresh set of SQLite-backed helpers. A clone therefore pays database initialization cost again and retains another set of SQLite file descriptors even though it is serving the same logical database as its prototype.

This change gives each clone family one reference-counted storage bundle while keeping mutable engine/session/model state clone-local. Independently constructed engines still own separate bundles, but bundles resolving to the same canonical database path share one in-process reentrant lock for helper construction, operations, backup/commit, and final teardown.

Fixes #463.

Hermes-Session: 20260804_101014_c509f5f7

Why this is needed

Hermes creates engine clones for child agents. Before this change, ten retained clones added 60 file descriptors on the measured macOS checkout and spent roughly 100 ms constructing duplicate SQLite helper sets. That multiplication raises the risk of descriptor exhaustion and repeated schema/WAL setup under parallel child-agent workloads.

The ownership boundary is the important part of the fix:

• clones share only the storage helpers;
• session IDs, cursors, runtime counters, model/provider metadata, adaptive retrieval state, and assertion extractors remain engine-local;
• every engine owns exactly one idempotent lease;
• owner-first shutdown releases one lease without closing storage still used by clones;
• final release attempts every helper close and reports combined failures;
• abandoned clones release their lease through finalization;
• failed clone construction rolls its acquired lease back.

This is complementary to #470's cleanup work. It does not replace #486's cross-process FTS bootstrap locking; the lock introduced here is process-local.

Implementation

• Add a reference-counted _SharedStorage bundle for message store, summary DAG, lifecycle state, optional assertion store, and optional query-view store.
• Pass the bundle into clone_for_agent() and preserve clone-local runtime/model state.
• Use a weak canonical-path lock registry so independently constructed bundles for the same database serialize helper initialization, operations, backup/commit, and teardown without sharing mutable bundle ownership.
• Allow each SQLite helper to adopt the shared reentrant lock.
• Synchronize every touched store/DAG operation, lifecycle debt cleanup, backup, close, and commit path that uses a shared connection.
• Add lifecycle, concurrency, rollback, finalization, profile-rebinding, aggregate-cleanup, and descriptor-regression tests.
• Add scripts/measure_clone_storage.py for reproducible before/after measurements against a selected checkout.

Reproduced benchmark

Environment: macOS, Python from the active Hermes environment, 25 samples, 10 retained clones. The benchmark imports each selected checkout using LCM_BENCH_REPO_ROOT.

Commands:

LCM_BENCH_REPO_ROOT=/private/tmp/hermes-lcm-baseline-check-20260804 \
  python scripts/measure_clone_storage.py --samples 25 --clones 10 --json

LCM_BENCH_REPO_ROOT=/private/tmp/hermes-lcm-shared-storage-impl-20260804 \
  python scripts/measure_clone_storage.py --samples 25 --clones 10 --json

Commits:

• Base: 6b7dbb1
• Feature: 074b9d8bd117bc092a9d77f2be5b5b8300d304d2

Results:

• Retained-clone file-descriptor delta: +60 → 0
• Median clone setup: 5.402319 ms → 0.166021 ms
• Ten-clone setup: 101.970458 ms → 1.952215 ms
• Median initial startup: 6.731890 ms → 7.287487 ms

Raw local receipts:

• /private/tmp/lcm-storage-benchmark-final-base-20260804.json
  • SHA-256 2bcd2a9430f2d63a5b4f7ef00a9386157a9475e27825d935c44e03d3d3c09c06
• /private/tmp/lcm-storage-benchmark-final-feature-20260804.json
  • SHA-256 ba969980fa6087ded140a811b1bcf91a2b725e9f64aae536f1484f7cc84a7626

Verification

Green feature/affected gates:

• tests/test_lcm_core.py::TestLCMEngineSharedStorage: 9 passed
• tests/test_lcm_engine.py: 751 passed, 1 skipped
• tests/test_lcm_core.py excluding one independently reproduced upstream failure: 316 passed
• tests/test_packaging_install.py: 38 passed in the grouped run before the known isolated baseline failure was reproduced
• tests/test_assertion_lifecycle_tools.py tests/test_assertion_store.py tests/test_query_view_store.py: 49 passed
• tests/test_lcm_core.py::TestLifecycleStateStore: 19 passed
• tests/test_assertion_store.py: 20 passed
• tests/test_query_view_store.py: 21 passed
• Ruff on every changed Python file: passed
• Python compilation on changed production/benchmark files: passed
• git diff --check: passed
• Independent adversarial concurrency/lifecycle review: PASS after two repair rounds
• Final closure review on 074b9d8: PASS; 133 affected storage tests passed locally

Full-suite result:

• The suite remains red on current main for pre-existing or load-sensitive tests. The observed failures were reproduced against pristine 6b7dbb1, including strict wall-clock embedding deadlines, macOS /var versus /private/var containment assertions, trajectory token/chunk expectations, provider-routing behavior, and an isolated packaging registration test.
• Three doctor failures initially exposed an incompatible post-shutdown alias-clearing change. That change was reverted; all three doctor tests pass on the final feature commit.
• No remaining full-suite failure was unique to this feature branch in the bounded base comparison.

Scope

This PR intentionally does not:

• share one bundle across independently constructed engines;
• provide cross-process locking;
• change session/runtime/model ownership;
• change public configuration;
• include the separate reasoning-control or preflight-maintenance features.

[jtstothard]

PR Review

Strengths

This is a comprehensive fix that addresses the root cause of the FD leak:

1. Reference-counted storage - Prevents leak at source via _SharedStorage
2. Thread-safe - Proper locking with weakref registry for DB paths
3. Finalizer protection - weakref.finalize ensures cleanup even if shutdown missed
4. Comprehensive tests - 4 new test cases covering clone lifecycle and concurrent access
5. Backwards compatible - Existing code continues to work

Suggested Enhancements

1. Add FD leak regression test

Direct test that verifies FD count doesn't grow under clone churn:

def test_clone_does_not_leak_fds(self, tmp_path):
    from pathlib import Path
    import os

    baseline = len(list(Path(f"/proc/{os.getpid()}/fd").iterdir()))
    prototype = self._engine(tmp_path)
    clones = [prototype.clone_for_agent() for _ in range(10)]

    after_clones = len(list(Path(f"/proc/{os.getpid()}/fd").iterdir()))

    for clone in clones:
        clone.shutdown()
    prototype.shutdown()

    after_shutdown = len(list(Path(f"/proc/{os.getpid()}/fd").iterdir()))

    assert after_clones - baseline < 5  # Small increase acceptable
    assert after_shutdown == baseline  # Clean return to baseline

Why: Demonstrates fix prevents FD exhaustion under real clone churn patterns.

2. Add inline lifecycle comments

Document reference counting flow in _SharedStorage:

def acquire(self) -> "_SharedStorage":
    # Increment reference count, prevent double-acquire on closed storage
    with self._lock:
        if self._closed or self._closing:
            raise RuntimeError("cannot acquire closed LCM storage")
        self._owners += 1
    return self

def release(self) -> None:
    # Decrement reference count, trigger close when last owner releases
    with self._lock:
        if self._owners <= 0:
            return
        self._owners -= 1
        if self._owners:
            return  # Still shared, keep alive
        self._closing = True  # Last owner releasing, trigger teardown

Why: Future maintainers can trace ownership lifecycle quickly.

3. Add concurrent stress test (optional)

Multi-threaded clone churn test to verify locks prevent race conditions:

def test_concurrent_clone_churn(self, tmp_path):
    prototype = self._engine(tmp_path)
    barrier = threading.Barrier(11)
    errors = []

    def worker():
        barrier.wait()
        try:
            for _ in range(10):
                clone = prototype.clone_for_agent()
                clone.shutdown()
        except Exception as e:
            errors.append(e)

    threads = [threading.Thread(target=worker) for _ in range(10)]
    for t in threads:
        t.start()
    barrier.wait()
    for t in threads:
        t.join()

    prototype.shutdown()
    assert errors == []

Why: Verifies thread safety under high-concurrency scenarios.

4. Handle SQLite close errors specifically

Catch sqlite3.OperationalError separately during close:

for helper in (self.store, self.dag, self.lifecycle, ...):
    close = getattr(helper, "close", None)
    if callable(close):
        try:
            close()
        except sqlite3.OperationalError as exc:
            # WAL checkpoint can fail if connection state inconsistent
            failures.append(exc)
        except BaseException as exc:
            failures.append(exc)

Why: Better error reporting and debugging for SQLite-specific issues.

---

Verdict

Ready to approve with these enhancements. High-quality fix with minimal additions needed.

The FD regression test (suggestion #1) is the highest-value addition - it directly demonstrates that the issue is fixed and provides a regression guard for future changes.

---

Context

This PR addresses issue #463 (file descriptor leak on macOS). I encountered the same issue on Linux and confirmed:

• Leak rate: ~1 fd/min
• Current FD count: 135 fds (before local patch)
• After local patch (similar approach): Stable at 7 fds

Our local verification confirms the fix is correct. PR #501 is more comprehensive than our stopgap patch and is ready for merge with the suggested enhancements.

[grantjayy]

Thanks for the detailed review and the Linux confirmation, @jtstothard.

Pushed d4eeb4c addressing the feedback:

1. FD leak regression test — added test_clone_churn_does_not_leak_fds. It uses /proc/self/fd with a /dev/fd fallback (this repo's CI and primary dev environment include macOS, which has no procfs), warms up once to keep lazy imports out of the baseline, churns 10 clone/shutdown cycles, then holds 10 live clones. It asserts clones add zero new descriptors (they share the prototype's storage bundle) and that shutdown returns the process exactly to baseline.

2. Lifecycle comments — added inline comments documenting the acquire/release reference-counting flow in _SharedStorage.

3. Concurrent stress test — added test_concurrent_clone_churn_is_thread_safe: 10 threads × 10 clone/shutdown cycles through a shared barrier, asserting no errors, the prototype's lease survives the churn, and final close happens exactly once.

4. SQLite-specific close handling — left as-is intentionally: the close loop already catches BaseException per helper and aggregates all failures into a BaseExceptionGroup, so an sqlite3.OperationalError is preserved with its type and message in the raised group. A separate except sqlite3.OperationalError branch would append to the same list and change no behavior. Happy to add it if you want the explicit branch for documentation value.

Verification: full tests/test_lcm_core.py suite passes on this branch (320 passed; the one failure, test_summary_call_keeps_unresolved_direct_slug_model_only, also fails on the unmodified branch head and is unrelated to this change). Ruff clean.