fix(storage): close review findings on txn-accepting LmdbDocumentStore overloads

1. test_subdb_identity's abort-path index/relation assertions were
   vacuous: before any collection had ever committed, index_lookup_eq()
   and a relation lookup return nullopt/empty purely from a cold cache,
   regardless of whether an aborted posting rolled back. Declare a
   relation on 'children' and re-write the SAME name/parentId values
   post-abort so the follow-up assertions prove a singleton (not "empty
   or absent"), covering both the index and relation halves of the
   all-or-nothing claim.

2. put(WriteTxn&, ...) recorded the collection's own dbi only after
   mdb_put succeeded, unlike del(WriteTxn&, ...) and
   maintainIndexes/maintainRelations, which record immediately after
   open_for_write. A throw from mdb_put on a brand-new collection left
   the collection's own handle out of to_cache while index/relation
   handles were in it - move the emplace_back to right after
   open_for_write, matching every other call site.

3. del(WriteTxn&, ...) treated a collection created earlier in the SAME
   uncommitted caller transaction as nonexistent, since cachedDbi() only
   sees already-committed collections - which would silently drop a
   cascade delete instead of routing it through open_for_write. Also
   check presence in the caller's own to_cache vector, which by
   construction already holds any handle opened earlier in this
   transaction.

service/src/storage/document_store_lmdb.cpp

@@ -524,6 +524,17 @@ void LmdbDocumentStore::put(WriteTxn& wtxn,
                              std::vector<std::pair<std::string, unsigned int>>& to_cache) {
     std::string payload = encode_document(doc);
     unsigned int dbi = open_for_write(wtxn, collection);
+    // Recorded immediately, not after mdb_put succeeds: del() and the
+    // maintainIndexes/maintainRelations helpers all record their handle right
+    // after opening it, and a handle from a transaction that later aborts is
+    // harmless to have recorded (the caller only ever applies to_cache after
+    // ITS commit succeeds). Recording it late, only after mdb_put, meant a
+    // throw from mdb_put on a brand-new collection left the collection's own
+    // handle out of to_cache while any index/relation handles it. A caller
+    // that somehow committed after catching would then have committed a
+    // sub-db whose handle nothing had cached - a repeat of the v2.8.1 class
+    // of bug ("callers must remember" is not an enforcement mechanism).
+    to_cache.emplace_back(std::string(collection), dbi);
     MDB_val k = to_val(id);
 
     // v2.9.0 — index maintenance runs in THIS transaction, so a write that
@@ -549,8 +560,7 @@ void LmdbDocumentStore::put(WriteTxn& wtxn,
     MDB_val v = to_val(payload);
     mdb_check(mdb_put(wtxn.raw(), dbi, &k, &v, 0), "put");
     // NOT cached here - see the header contract. The caller caches every
-    // entry in to_cache (this one included) only after ITS commit succeeds.
-    to_cache.emplace_back(std::string(collection), dbi);
+    // entry in to_cache (recorded above) only after ITS commit succeeds.
 }
 
 
@@ -1468,8 +1478,19 @@ bool LmdbDocumentStore::del(WriteTxn& wtxn,
     if (id.empty()) return false;
     // Same no-op-avoidance as the no-txn form above: don't spring an empty
     // sub-db into existence for a collection that has never been written.
-    // Cache-only, so this costs nothing extra inside the caller's txn.
-    if (!cachedDbi(collection)) return false;
+    // cachedDbi() alone only sees ALREADY-COMMITTED collections, though - a
+    // collection this same caller-owned transaction created earlier (via an
+    // uncommitted put(wtxn, ...) call sharing this to_cache) would not be in
+    // the process-wide cache yet, and treating it as "doesn't exist" would
+    // silently drop the delete instead of routing it through open_for_write.
+    // By construction, anything opened earlier in THIS transaction is
+    // already in to_cache (every txn-accepting overload records its handle
+    // there), so checking both closes that gap rather than merely
+    // documenting it.
+    const bool known_this_txn = std::any_of(
+        to_cache.begin(), to_cache.end(),
+        [&](const auto& p) { return p.first == collection; });
+    if (!cachedDbi(collection) && !known_this_txn) return false;
 
     unsigned int dbi = open_for_write(wtxn, collection);
     MDB_val k = to_val(id);

tests/test_subdb_identity.cpp

@@ -41,6 +41,7 @@ using smartbotic::db::storage::LmdbEnv;
 using smartbotic::db::storage::LmdbEnvOpts;
 using smartbotic::db::storage::read_subdb_identity;
 using smartbotic::db::storage::ReadTxn;
+using smartbotic::db::storage::RelationRef;
 using smartbotic::db::storage::verify_subdb_identity;
 using smartbotic::db::storage::write_subdb_identity;
 using smartbotic::db::storage::WriteTxn;
@@ -1729,6 +1730,11 @@ void test_txn_accepting_writes_are_atomic_across_collections() {
     TmpEnv t("txn-atomic");
     LmdbDocumentStore store(t.env);
     store.set_indexed_fields("parents", {"name"});
+    // A relation declared on 'children' as the CHILD side, so put(wtxn, ...)
+    // on 'children' also maintains a reverse-index posting - the brief's "no
+    // relation entry survives" half needs one declared to be testable at
+    // all, and it shares the identical to_cache mechanism as the index path.
+    store.set_relations("children", {RelationRef{"par_child", "parentId"}});
 
     Document pd;
     pd.id = "p1";
@@ -1740,7 +1746,8 @@ void test_txn_accepting_writes_are_atomic_across_collections() {
     cd.collection = "children";
     cd.set_data(nlohmann::json{{"parentId", "p1"}});
 
-    // --- Abort path: neither document, nor the index entry, may survive. ---
+    // --- Abort path: neither document, nor the index entry, nor the
+    // relation posting, may survive. ---
     {
         WriteTxn wtxn(t.env);
         std::vector<std::pair<std::string, unsigned int>> to_cache;
@@ -1755,17 +1762,22 @@ void test_txn_accepting_writes_are_atomic_across_collections() {
           "aborted txn: the parent document does not exist");
     check(!store.get("children", "c1").has_value(),
           "aborted txn: the child document does not exist");
-    auto idx = store.index_lookup_eq("parents", "name", nlohmann::json("alice"));
-    check(!idx.has_value() || idx->empty(),
-          "aborted txn: no index entry survives for the never-committed parent");
 
     // --- The collection must not be poisoned by the aborted transaction: a
     // later write, scan, count and get on EITHER collection must still work.
     // This is precisely the v2.8.0 failure mode - caching a handle before
-    // commit leaves a closed handle behind an aborted caller transaction. ---
+    // commit leaves a closed handle behind an aborted caller transaction.
+    // It also DOUBLES as the only way to make the index/relation-rollback
+    // checks below non-vacuous: before either collection has ever committed
+    // successfully, index_lookup_eq()/relation_index_children() report
+    // nullopt/empty purely because their sub-db was never cached - that
+    // would pass whether or not the aborted posting actually rolled back.
+    // Writing a NEW row under the SAME key values the aborted write used
+    // (name="alice", parentId="p1") warms those caches for real, so a
+    // survived posting from the aborted write would show up alongside it. ---
     bool ok_put = true;
     try {
-        store.put("parents", "p2", pd);
+        store.put("parents", "p2", pd);   // same name: "alice" as the aborted p1
     } catch (const std::exception&) {
         ok_put = false;
     }
@@ -1773,7 +1785,7 @@ void test_txn_accepting_writes_are_atomic_across_collections() {
 
     bool ok_put2 = true;
     try {
-        store.put("children", "c2", cd);
+        store.put("children", "c2", cd);  // same parentId: "p1" as the aborted c1
     } catch (const std::exception&) {
         ok_put2 = false;
     }
@@ -1791,6 +1803,20 @@ void test_txn_accepting_writes_are_atomic_across_collections() {
     check(store.count("parents") == 1, "count() on 'parents' is right after the abort");
     check(store.count("children") == 1, "count() on 'children' is right after the abort");
 
+    // Now the real proof: the index for "alice" holds ONLY p2, and the
+    // relation's postings for parent "p1" hold ONLY c2. If the aborted
+    // write's postings (p1 under "alice", c1 under "p1") had survived, either
+    // of these would report two ids instead of one - unlike the vacuous
+    // "nullopt/empty" checks a cold cache would also produce.
+    auto idx = store.index_lookup_eq("parents", "name", nlohmann::json("alice"));
+    check(idx.has_value() && idx->size() == 1 && (*idx)[0] == "p2",
+          "aborted txn: the index under 'alice' holds only the post-abort "
+          "write (p2) - the aborted p1 posting did not survive");
+    auto children_of_p1 = store.relation_index_children("par_child", "p1", 10);
+    check(children_of_p1.size() == 1 && children_of_p1[0] == "c2",
+          "aborted txn: parent 'p1' has only the post-abort child (c2) in the "
+          "relation index - the aborted c1 posting did not survive");
+
     // --- Commit path: both documents AND the index entry land together,
     // once the caller re-primes to pick up the handles it chose not to
     // cache itself (see the next block for why that step is mandatory in