fix(relations): T13 review round 3 - guard bare-name disarm, fix rollback test

Two findings from a third review pass, both confirmed reproducible before
fixing:

1. ConfigureCollection's re-arm call (added in round 2 to keep
   validate_on_write's relationsEnforced snapshot fresh) was unconditional,
   so a raw-gRPC caller sending a BARE collection name (e.g. "executions"
   instead of "default:executions") would find zero relations under that
   exact string via relationsWithChild's exact-match comparison, and
   armRelationsForChild's set_relations(bare, {}) would erase the map
   entry keyed by the SAME bare name the real, qualified arming used -
   silently disarming both the reverse index and validate_on_write for
   that collection, for the life of the process. Reproduced live with
   grpcurl against the raw ConfigureCollectionRequest proto (no in-tree
   client can send a bare name): a missing-parent write that should have
   been rejected was silently accepted after the bare-name call. Fixed by
   only calling armRelationsForChild when relationsWithChild(...) is
   non-empty for the string the caller actually sent; DropRelation's own
   call stays unconditional since it legitimately needs the erase.
   Re-verified live: the same sequence now correctly stays enforced.

2. test_validate_on_write_rolls_back_an_already_applied_sibling_relation's
   discriminating assertion did not discriminate: its "already-applied"
   sub-db was created for the first time inside the same aborted
   transaction, so it hit the exact "sub-db never existed" nullopt
   shortcut the comment claimed was unavailable. Fixed by committing a
   sibling child in its own successful write first, so the sub-db is a
   real, already-existing one holding a real posting before the rejected
   write runs; the assertion now checks the count STAYS at 1 rather than
   becoming 2. Verified by making the put survive (commented out the
   throw): the assertion correctly failed, showing the count read 2 with
   the fix disabled. Rewrote the comment to state precisely what the test
   proves, matching round 2's coordinator instruction not to overclaim a
   third time.

service/src/database_grpc_impl.cpp

@@ -3716,15 +3716,35 @@ grpc::Status DatabaseGrpcImpl::ConfigureCollection(
         response->set_success(ok);
         if (!ok) {
             response->set_error(err);
-        } else if (request->config().has_relations_enforced()) {
+        } else if (request->config().has_relations_enforced() &&
+                   !relation_manager_.relationsWithChild(request->collection()).empty()) {
             // v2.11.0 T13 round 2 — RelationRef::relationsEnforced is a
             // snapshot taken at arm time (the storage layer cannot read
             // CollectionConfigManager itself - see RelationRef's comment), so
             // flipping the flag here must re-arm this collection's relations
             // or validate_on_write keeps consulting the OLD value until
-            // something else happens to re-declare a relation. A no-op
-            // (empty refs) when this collection is not a child in any
-            // relation - armRelationsForChild handles that already.
+            // something else happens to re-declare a relation.
+            //
+            // v2.11.0 T13 round 3 (review finding) — gated on
+            // relationsWithChild(...).empty(), NOT unconditional.
+            // relationsWithChild compares `r.child` to `request->collection()`
+            // by EXACT string (relation_manager.cpp), no qualification
+            // normalisation. Every in-tree caller sends the qualified form
+            // (client.cpp's setRelationsEnforced), but a raw-gRPC caller that
+            // sends a bare name (e.g. "executions" instead of
+            // "default:executions") would find zero relations under that
+            // exact string, and armRelationsForChild's set_relations(bare,
+            // {}) then ERASES the map entry keyed by the SAME bare name the
+            // real, qualified arming used - silently disarming both the
+            // reverse index AND validate_on_write for that collection, for
+            // the life of the process, logged only as "re-armed 0
+            // relation(s)". This guard means a malformed/bare collection name
+            // on this RPC can no longer touch armed state at all; it can
+            // still, correctly, fail to re-snapshot relationsEnforced for
+            // that malformed name, but that is a no-op, not a disarm.
+            // DropRelation's own armRelationsForChild call stays
+            // unconditional - it legitimately needs the erase when the last
+            // relation for a child is removed.
             armRelationsForChild(request->collection());
         }
         return grpc::Status::OK;

tests/test_relation_enforcement.cpp

@@ -1440,23 +1440,28 @@ void test_validate_on_write_unrelated_update_not_rechecked() {
           "the update itself still applied");
 }
 
-// v2.11.0 T13 round 2 (review finding) — rollback of an ALREADY-APPLIED
+// v2.11.0 T13 round 3 (review finding) — rollback of an ALREADY-APPLIED
 // index mutation within the same write, not merely "the mutation never
 // happened because validation ran first."
 //
-// The earlier "left no posting behind" assertions are structurally weak on
-// their own: validate_on_write's check runs before that RELATION's own
-// index sub-db is even opened, so on rejection the sub-db often never
-// exists and relation_index_child_count returns 0 through the nullopt path
-// regardless of whether LMDB actually rolled anything back. This test
-// forces a real rollback to matter: `executions` declares TWO relations.
-// The first (wf_rel, validateOnWrite=false) has its posting WRITTEN - a
-// real mdb_put against a real sub-db, inside the loop, before the second
-// relation is even considered. The second (owner_rel, validateOnWrite=true)
-// then rejects. Both relations share the one write transaction `put()`
-// opens, so the abort must undo wf_rel's already-applied mdb_put along with
-// everything else - there is no sub-db-never-existed shortcut available
-// here, because it demonstrably did exist and did get written to.
+// round 2's version of this test made `_relidx_wf_rel` itself only exist
+// inside the SAME aborted transaction (nothing had committed a wf_rel
+// posting beforehand), so relation_index_child_count("wf_rel", "wf-1")
+// still returned 0 through the "sub-db was never created"
+// (`if (!dbi_opt) return 0;`) shortcut regardless of whether the abort
+// rolled anything back - the exact case the comment claimed was
+// unavailable. Fixed by committing a SIBLING child (e0) first, in its own,
+// separate, successful write: `_relidx_wf_rel` is a real, already-existing
+// sub-db holding one committed posting (from e0) BEFORE the rejected write
+// (e1) runs. `executions` declares TWO relations: wf_rel
+// (validateOnWrite=false) and owner_rel (validateOnWrite=true). e1's write
+// applies wf_rel's mdb_put for real - into that already-existing sub-db -
+// before owner_rel's check runs and rejects. The assertion that matters is
+// that the count STAYS AT 1 (e0's), not 2: reading 2 would mean e1's
+// already-applied wf_rel mutation survived the abort. Because the sub-db
+// demonstrably existed beforehand (asserted directly, see the sanity check
+// below), the nullopt shortcut is not in play here, and the assertion
+// actually discriminates a rollback from a no-op.
 void test_validate_on_write_rolls_back_an_already_applied_sibling_relation() {
     TmpEnv t("validate-rollback-sibling");
     LmdbDocumentStore store(t.env);
@@ -1468,7 +1473,20 @@ void test_validate_on_write_rolls_back_an_already_applied_sibling_relation() {
     Document w; w.id = "wf-1"; w.collection = "workflows";
     w.set_data({{"name", "real workflow"}});
     store.put("workflows", "wf-1", w);
-    // Deliberately no "users/u-ghost" - owner_rel's parent never exists.
+
+    Document uReal; uReal.id = "u-real"; uReal.collection = "users";
+    uReal.set_data({{"name", "a real user"}});
+    store.put("users", "u-real", uReal);
+    // Deliberately no "users/u-ghost" - owner_rel's parent for e1 never exists.
+
+    // Commit a sibling child FIRST, in its own successful write, so
+    // `_relidx_wf_rel` is a real, already-existing sub-db with one committed
+    // posting before the rejected write below ever runs.
+    Document e0; e0.id = "e0"; e0.collection = "executions";
+    e0.set_data({{"workflowId", "wf-1"}, {"ownerId", "u-real"}});
+    store.put("executions", "e0", e0);
+    check(store.relation_index_child_count("wf_rel", "wf-1") == 1,
+          "sanity: wf_rel's sub-db already exists and holds e0's committed posting");
 
     Document e; e.id = "e1"; e.collection = "executions";
     e.set_data({{"workflowId", "wf-1"}, {"ownerId", "u-ghost"}});
@@ -1482,16 +1500,16 @@ void test_validate_on_write_rolls_back_an_already_applied_sibling_relation() {
     check(!store.get("executions", "e1").has_value(),
           "the document itself was rolled back");
     check(store.relation_index_child_count("owner_rel", "u-ghost") == 0,
-          "owner_rel (the relation that rejected) has no posting");
-    check(store.relation_index_child_count("wf_rel", "wf-1") == 0,
-          "wf_rel (the EARLIER, already-applied sibling relation) was rolled "
-          "back too - LMDB's transaction abort undid a real mdb_put, not "
-          "just 'the sub-db never got created'");
+          "owner_rel (the relation that rejected) has no posting for the ghost id");
+    check(store.relation_index_child_count("wf_rel", "wf-1") == 1,
+          "wf_rel's count STAYS AT 1 (e0's) rather than becoming 2 - e1's "
+          "already-applied mdb_put into this REAL, already-existing sub-db "
+          "was rolled back by the same transaction abort that rejected "
+          "owner_rel. A count of 2 here would mean the sibling relation's "
+          "mutation survived the abort.");
 
     // Once owner_rel's parent exists, the identical write succeeds and BOTH
-    // relations end up with their postings - confirming the rollback above
-    // was real and not a side effect of some other bug losing wf_rel's
-    // posting permanently.
+    // relations end up with their postings - e0's plus e1's.
     Document u; u.id = "u-ghost"; u.collection = "users";
     u.set_data({{"name", "real user, now created"}});
     store.put("users", "u-ghost", u);
@@ -1502,8 +1520,9 @@ void test_validate_on_write_rolls_back_an_already_applied_sibling_relation() {
         threw = true;
     }
     check(!threw, "once owner_rel's parent exists too, the write succeeds");
-    check(store.relation_index_child_count("wf_rel", "wf-1") == 1, "wf_rel posted");
-    check(store.relation_index_child_count("owner_rel", "u-ghost") == 1, "owner_rel posted");
+    check(store.relation_index_child_count("wf_rel", "wf-1") == 2,
+          "wf_rel now has BOTH e0's (pre-existing) and e1's (just landed) postings");
+    check(store.relation_index_child_count("owner_rel", "u-ghost") == 1, "owner_rel posted for e1");
 }
 
 // v2.11.0 T13 round 2 (review finding 1) — relationsEnforced is the