Преглед изворни кода

docs(plan): re-plan relations from the corrected design, targeting v2.11.0

Replaces the superseded 3278-line v2.5.0 plan. 13 tasks: Phase A (Tasks 1-9) is
additive and shippable on its own - declaration, DUPSORT reverse index, restrict
and no_action, DescribeDelete, bootstrap scan, per-collection switches, and the
mandatory RPC-boundary e2e. Phase B (Tasks 10-13) does the write-path work that
relations and unique constraints both need.

The Global Constraints section carries the rules that cost this repo production
incidents, because a plan that omits them invites the same failures: register a
runtime-created sub-db with cacheCommittedDbi() or it is invisible to every read
since v2.8.1; never cache a dbi before commit; never call mdb_dbi_open from a read
txn; never open a second env on an open path; skip reserved keys with
is_index_meta_key(); page any loadFromStore because limit defaults to 100 and 0
returns nothing; add client methods, never struct members.

Task 11 activates the unique-constraint check that already exists. It specifies
the two things that make it correct rather than merely present: applyDualWriteMirror
must rethrow UniqueViolation WITHOUT touching mirror health (a rejected write is
the caller's business, and flipping health would send every read to MemoryStore -
the v2.8.1 fault), and MemoryStore must undo its in-memory mutation, since it
mutates before the mirror runs.

Task 12 makes WAL-first explicit for cascade, because MemoryStore is rebuilt from
snapshot + WAL and not from LMDB, so an LMDB-only cascade would have its child
deletions resurrected on the next restart.

The reverse index uses MDB_DUPSORT per the re-validated design, giving restrict
and DescribeDelete an O(1)-ish child count via mdb_cursor_count, and carries a key
format version in its name (_relidx1_) from the first commit.

Self-review recorded in the plan: every spec requirement is assigned to a task,
no placeholders, types consistent across tasks, and one deliberate gap noted
(where the delete-gate helper lives is left to the implementer; its behaviour is
fully specified).
fszontagh пре 1 месец
родитељ
комит
75a8a5e6ca

+ 4 - 2
docs/ROADMAP.md

@@ -146,8 +146,10 @@ from node configuration.
 **The design is re-validated and current** as of 2026-08-09:
 `docs/superpowers/specs/2026-08-03-relations-design.md`. Read its status section
 first - it lists what survived re-validation, what was wrong, and the constraints
-that post-date it. The 3278-line plan beside it is **superseded and must not be
-executed**; re-plan from the design.
+that post-date it. **The plan to execute is
+`docs/superpowers/plans/2026-08-09-relations-v2.11.0.md`** (13 tasks, Phase A
+additive and shippable alone, Phase B the write-path work). The 3278-line
+`2026-08-04-relations-v2.5.0.md` beside it is superseded and must not be executed.
 
 **Relations and unique constraints share one blocker**, so schedule them
 together. Both need `LmdbDocumentStore` operations to accept a caller's

+ 669 - 0
docs/superpowers/plans/2026-08-09-relations-v2.11.0.md

@@ -0,0 +1,669 @@
+# Relations + Unique Constraints Implementation Plan (v2.11.0)
+
+> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
+
+**Goal:** Give the database referential integrity - a parent delete can no longer silently leave children pointing at nothing - and activate the unique-constraint check that already exists but cannot currently be enforced.
+
+**Architecture:** A relation is a document in the `_relations` system collection, exactly parallel to `_views`. Each relation gets one `MDB_DUPSORT` sub-db in the project env, key = parent id, data = child id, maintained inside the same LMDB transaction as the child document. Phase A adds declaration, the reverse index, the non-destructive policies and the discoverability surface without touching the write path. Phase B gives `LmdbDocumentStore` txn-accepting overloads, which is what both atomic cascade and unique-violation propagation need.
+
+**Tech Stack:** C++20, LMDB (`MDB_DUPSORT`), gRPC/Protobuf, nlohmann::json at the API edge, yyjson on hot paths, spdlog. No new dependencies.
+
+**Source of truth:** `docs/superpowers/specs/2026-08-03-relations-design.md`, re-validated 2026-08-09. Read its status section first. This plan replaces `2026-08-04-relations-v2.5.0.md`, which is superseded.
+
+## Global Constraints
+
+Every task's requirements implicitly include these. They are not style preferences - each one is a production incident this repository already survived.
+
+- **A sub-db created at runtime MUST be registered with `cacheCommittedDbi()` after its transaction commits.** Since v2.8.1 `try_open_for_read` serves only from the primed cache and a miss means "no such sub-db". A `_relidx1_` sub-db created without that call is invisible to every later read, and relations would silently not enforce.
+- **Never cache an `MDB_dbi` before its transaction commits** (v2.8.0). LMDB closes a handle whose opening transaction aborts.
+- **Never call `mdb_dbi_open` from a read transaction** (v2.8.1). `lmdb.h` forbids concurrent use, and violating it silently rebinds cached handles.
+- **Never open a second `MDB_env` on a path this process already has open** (v2.4.4). POSIX locks are per-process; closing one fd drops them all.
+- **Skip reserved keys with `is_index_meta_key()`**, not `is_identity_key()`. There are two as of v2.10.0.
+- **Every sub-db carries the v2.4.4 identity sentinel**, written by `open_for_write`.
+- **`Query::limit` defaults to 100, and `limit = 0` returns NOTHING.** Any `loadFromStore` must page explicitly. This trap has now bitten `ViewManager`, `PolicyManager` and `CollectionConfigManager`.
+- **Public client structs must not change size.** Add methods or overloads, never members - a size change under an unchanged soname crashed the webserver in a restart loop.
+- **The client qualifies names.** Relations are keyed `<project>:<name>`. v2.4.2 lost views for two releases by qualifying `collection` but not `name`.
+- **No bare `assert()` in tests.** Use a real `check()` counter.
+- **Version target is 2.11.0.** Not v2.5.0 - that version never existed.
+
+---
+
+## File Structure
+
+| Path | Responsibility |
+|------|---------------|
+| `service/src/relations/relation_manager.hpp/.cpp` | **New.** `RelationInfo`, declaration CRUD, project-qualified cache, `loadFromStore` with paging. Mirrors `views/view_manager.*`. |
+| `service/src/relations/relation_index.hpp/.cpp` | **New.** Sub-db naming (`_relidx1_<name>`), key encoding, and the reverse-index read/write primitives. Kept separate from `relation_manager` so the storage shape can be tested without a MemoryStore. |
+| `service/src/storage/document_store_lmdb.hpp/.cpp` | Reverse-index maintenance inside the document's write txn; txn-accepting overloads (Phase B). |
+| `service/src/storage/dual_write_mirror.hpp` | Phase B: let `UniqueViolation` and `RelationViolation` through without flipping mirror health. |
+| `service/src/config/collection_config_manager.hpp/.cpp` | `CollectionCfg::relationsEnforced`, `CollectionCfg::uniqueFields`. |
+| `service/src/database_service.cpp/.hpp` | Own the `RelationManager`; re-arm relation and uniqueness declarations at boot. |
+| `service/src/database_grpc_impl.cpp/.hpp` | The five relation RPCs, `DescribeDelete`, and enforcement in `Delete`. |
+| `service/src/migrations/migration_runner.cpp` | `create_relation` migration op. |
+| `proto/database.proto` | Relation messages and RPCs; `optional bool relations_enforced` on `CollectionConfig`. |
+| `client/include/smartbotic/database/client.hpp`, `client/src/client.cpp` | Relation methods (methods only - no struct changes). |
+| `cli/main.cpp` | `relations`, `relations-check`, `describe-delete`. |
+| `tests/test_relation_index.cpp` | **New.** Storage shape: DUPSORT postings, child counts, reserved keys, handle caching. |
+| `tests/test_relation_manager.cpp` | **New.** Declaration, project scoping, paging, cross-project rejection. |
+| `tests/test_relation_enforcement.cpp` | **New.** Policies, dot-paths, arrays, absent/null. |
+| `tests/load_test/test_relations.sh` + `.cpp` | **New, mandatory.** End-to-end over the real RPC boundary, including a restart. |
+
+---
+
+# Phase A - declaration, reverse index, non-destructive policies
+
+No write-path restructuring. Everything here is additive and safe to ship alone.
+
+### Task 1: RelationInfo and RelationManager
+
+**Files:**
+- Create: `service/src/relations/relation_manager.hpp`, `service/src/relations/relation_manager.cpp`
+- Create: `tests/test_relation_manager.cpp`
+- Modify: `tests/CMakeLists.txt`, `service/CMakeLists.txt`
+
+**Interfaces:**
+- Consumes: `MemoryStore&` (as `ViewManager` does), `smartbotic::database::Query`
+- Produces:
+  ```cpp
+  namespace smartbotic::database {
+  enum class OnDelete { Restrict, Cascade, SetNull, NoAction };
+
+  struct RelationInfo {
+      std::string name;            // qualified "<project>:<name>"
+      std::string child;           // qualified "<project>:<collection>"
+      std::string childField;      // dot-path, may resolve to an array of ids
+      std::string parent;          // qualified "<project>:<collection>"
+      OnDelete onDelete = OnDelete::Restrict;
+      bool validateOnWrite = false;
+  };
+
+  class RelationManager {
+  public:
+      explicit RelationManager(MemoryStore& store);
+      void loadFromStore();
+      bool createRelation(const RelationInfo& r, std::string& errorOut);
+      bool dropRelation(const std::string& qualifiedName, std::string& errorOut);
+      std::optional<RelationInfo> getRelation(const std::string& qualifiedName) const;
+      std::vector<RelationInfo> listRelations(const std::string& project = "") const;
+      // Relations whose PARENT is this collection - what a delete must consult.
+      std::vector<RelationInfo> relationsWithParent(const std::string& qualifiedCollection) const;
+      // Relations whose CHILD is this collection - what a write must maintain.
+      std::vector<RelationInfo> relationsWithChild(const std::string& qualifiedCollection) const;
+  private:
+      MemoryStore& store_;
+      mutable std::shared_mutex mutex_;
+      std::unordered_map<std::string, RelationInfo> cache_;
+  };
+  }
+  ```
+
+- [ ] **Step 1: Write the failing test**
+
+```cpp
+// tests/test_relation_manager.cpp
+void test_relations_are_project_scoped_and_survive_reload() {
+    Fixture f;                       // MemoryStore, started
+    RelationManager rm(f.store);
+    rm.loadFromStore();
+
+    RelationInfo a;
+    a.name = "default:exec_wf";
+    a.child = "default:executions";
+    a.childField = "workflowId";
+    a.parent = "default:workflows";
+    std::string err;
+    check(rm.createRelation(a, err), "created in default");
+
+    RelationInfo b = a;              // SAME bare name, different project
+    b.name = "acme:exec_wf";
+    b.child = "acme:executions";
+    b.parent = "acme:workflows";
+    check(rm.createRelation(b, err), "the same name in another project is allowed");
+
+    check(rm.listRelations("default").size() == 1, "listing is project-filtered");
+    check(rm.listRelations().size() == 2, "empty project lists everything");
+
+    RelationManager fresh(f.store);   // restart
+    fresh.loadFromStore();
+    check(fresh.getRelation("default:exec_wf").has_value(), "survives reload");
+    check(fresh.getRelation("acme:exec_wf").has_value(), "both survive");
+}
+
+void test_cross_project_relation_is_refused() {
+    Fixture f;
+    RelationManager rm(f.store);
+    RelationInfo r;
+    r.name = "default:bad";
+    r.child = "default:executions";
+    r.childField = "workflowId";
+    r.parent = "acme:workflows";      // different env - no txn spans two
+    std::string err;
+    check(!rm.createRelation(r, err), "a cross-project relation is refused");
+    check(err.find("project") != std::string::npos, "and says why");
+}
+
+void test_more_than_one_page_of_relations_loads() {
+    Fixture f;
+    RelationManager rm(f.store);
+    for (int i = 0; i < 250; ++i) {          // Query::limit defaults to 100
+        RelationInfo r;
+        char buf[32];
+        std::snprintf(buf, sizeof(buf), "default:r%03d", i);
+        r.name = buf;
+        r.child = "default:c";
+        r.childField = "p";
+        r.parent = "default:p";
+        std::string err;
+        rm.createRelation(r, err);
+    }
+    RelationManager fresh(f.store);
+    fresh.loadFromStore();
+    check(fresh.listRelations().size() == 250,
+          "all 250 load - a bare Query would stop at 100, as it did for views, "
+          "policies and collection configs");
+}
+```
+
+- [ ] **Step 2: Run it and watch it fail**
+
+Run: `cmake --build build -j$(nproc) --target test_relation_manager && ./build/tests/test_relation_manager`
+Expected: does not compile - `relation_manager.hpp` does not exist.
+
+- [ ] **Step 3: Implement**
+
+Model on `service/src/views/view_manager.cpp`. Three points that are not boilerplate:
+
+```cpp
+// relation_manager.cpp
+constexpr const char* SYSTEM_COLLECTION = "_relations";
+
+void RelationManager::loadFromStore() {
+    std::unique_lock lock(mutex_);
+    cache_.clear();
+    CollectionOptions opts;
+    store_.createCollection(SYSTEM_COLLECTION, opts);
+
+    // Page explicitly. Query::limit defaults to 100 and limit=0 returns nothing.
+    constexpr uint32_t kPage = 500;
+    uint32_t offset = 0;
+    while (true) {
+        Query q;
+        q.limit = kPage;
+        q.offset = offset;
+        auto res = store_.find(SYSTEM_COLLECTION, q);
+        if (res.documents.empty()) break;
+        for (const auto& d : res.documents) {
+            if (d.id.empty()) continue;
+            cache_[d.id] = fromJson(d.data());
+        }
+        if (res.documents.size() < kPage) break;
+        offset += kPage;
+    }
+    spdlog::info("RelationManager: loaded {} relation(s)", cache_.size());
+}
+
+bool RelationManager::createRelation(const RelationInfo& r, std::string& errorOut) {
+    // Each project owns its own LMDB env and no transaction spans two, so a
+    // cross-project relation could never be enforced atomically.
+    const auto rn = resolveCollection(r.name);
+    const auto rc = resolveCollection(r.child);
+    const auto rp = resolveCollection(r.parent);
+    if (rc.project != rp.project || rc.project != rn.project) {
+        errorOut = "relation, child and parent must be in one project (no "
+                   "transaction spans two project envs)";
+        return false;
+    }
+    if (r.childField.empty()) { errorOut = "child_field is required"; return false; }
+    ...
+}
+```
+
+- [ ] **Step 4: Run the tests - all three pass**
+
+- [ ] **Step 5: Commit**
+
+```bash
+git add service/src/relations tests/test_relation_manager.cpp tests/CMakeLists.txt service/CMakeLists.txt
+git commit -m "feat(relations): RelationManager with project-qualified keys"
+```
+
+---
+
+### Task 2: the reverse index sub-db
+
+**Files:**
+- Create: `service/src/relations/relation_index.hpp`, `service/src/relations/relation_index.cpp`
+- Create: `tests/test_relation_index.cpp`
+- Modify: `tests/CMakeLists.txt`, `service/CMakeLists.txt`
+
+**Interfaces:**
+- Consumes: `WriteTxn&`/`ReadTxn&` from `storage/lmdb_txn.hpp`, `is_index_meta_key()` from `storage/secondary_index.hpp`
+- Produces:
+  ```cpp
+  namespace smartbotic::db::storage {
+  // The trailing digit is a KEY FORMAT VERSION. v2.9.1 had to bump _idx_ to
+  // _idx2_ when an encoding changed, so a stale index is never read under new
+  // rules. Paying that forward costs nothing.
+  inline constexpr std::string_view kRelIndexPrefix = "_relidx1_";
+  std::string relation_index_subdb(std::string_view relationBareName);
+  bool is_relation_index_subdb(std::string_view subdb);
+  }
+  ```
+
+- [ ] **Step 1: Write the failing test**
+
+```cpp
+// tests/test_relation_index.cpp — DUPSORT shape, per the re-validated design.
+void test_children_of_a_parent_are_a_dup_set() {
+    TmpEnv t("relidx");
+    LmdbDocumentStore store(t.env);
+
+    check(store.relation_index_add("r1", "wf-1", "exec-a"), "added a child");
+    check(store.relation_index_add("r1", "wf-1", "exec-b"), "and another");
+    check(store.relation_index_add("r1", "wf-2", "exec-c"), "under a second parent");
+
+    // THE operation restrict and DescribeDelete need: a count without reading
+    // the children. mdb_cursor_count makes it O(1)-ish.
+    check(store.relation_index_child_count("r1", "wf-1") == 2, "two children of wf-1");
+    check(store.relation_index_child_count("r1", "wf-2") == 1, "one child of wf-2");
+    check(store.relation_index_child_count("r1", "wf-none") == 0,
+          "an unreferenced parent has none, and that is not an error");
+
+    auto kids = store.relation_index_children("r1", "wf-1", 10);
+    std::sort(kids.begin(), kids.end());
+    check(kids == std::vector<std::string>{"exec-a", "exec-b"}, "children listed");
+
+    // Removing one pair must not remove the sibling.
+    check(store.relation_index_remove("r1", "wf-1", "exec-a"), "removed one pair");
+    check(store.relation_index_child_count("r1", "wf-1") == 1, "the sibling survives");
+
+    // Idempotent: re-adding the same pair is a no-op, not a duplicate.
+    store.relation_index_add("r1", "wf-1", "exec-b");
+    check(store.relation_index_child_count("r1", "wf-1") == 1, "no duplicate posting");
+}
+
+// The failure this repo is most likely to reproduce. Since v2.8.1 reads serve
+// only from the primed dbi cache, so a sub-db created at runtime is invisible
+// unless registered after commit - and relations would silently not enforce.
+void test_index_created_at_runtime_is_visible_after_reopen() {
+    const std::string path = make_tmpdir("relidx-visible");
+    const LmdbEnvOpts opts{path, 64ULL << 20, 256, 126, false};
+    {
+        LmdbEnv env(opts);
+        LmdbDocumentStore store(env);
+        store.relation_index_add("r1", "wf-1", "exec-a");
+        // Same process, no reopen: must be visible immediately.
+        check(store.relation_index_child_count("r1", "wf-1") == 1,
+              "visible in the process that created it - this is what "
+              "cacheCommittedDbi() buys");
+    }
+    LmdbEnv env2(opts);
+    LmdbDocumentStore store2(env2);      // priming runs in the constructor
+    check(store2.relation_index_child_count("r1", "wf-1") == 1,
+          "and after a restart, via prime_dbi_cache()");
+}
+```
+
+- [ ] **Step 2: Run it - fails to compile**
+
+- [ ] **Step 3: Implement on `LmdbDocumentStore`**
+
+```cpp
+// document_store_lmdb.cpp
+bool LmdbDocumentStore::relation_index_add(std::string_view relation,
+                                            std::string_view parentId,
+                                            std::string_view childId) {
+    const std::string sub = relation_index_subdb(relation);
+    WriteTxn wtxn(env_);
+    const unsigned int dbi = open_for_write(wtxn, sub, MDB_DUPSORT);
+    MDB_val k = to_val(parentId);
+    MDB_val v = to_val(childId);
+    const int rc = mdb_put(wtxn.raw(), dbi, &k, &v, MDB_NODUPDATA);
+    if (rc != MDB_SUCCESS && rc != MDB_KEYEXIST) throw_mdb(rc, "relation index add");
+    wtxn.commit();
+    cacheCommittedDbi(sub, dbi);   // MANDATORY - see Global Constraints
+    return true;
+}
+
+uint64_t LmdbDocumentStore::relation_index_child_count(std::string_view relation,
+                                                        std::string_view parentId) {
+    ReadTxn rtxn(env_);
+    auto dbi_opt = try_open_for_read(rtxn, relation_index_subdb(relation));
+    if (!dbi_opt) return 0;
+    MDB_cursor* cur = nullptr;
+    mdb_check(mdb_cursor_open(rtxn.raw(), *dbi_opt, &cur), "cursor_open (relidx)");
+    struct G { MDB_cursor* c; ~G() { if (c) mdb_cursor_close(c); } } g{cur};
+    MDB_val k = to_val(parentId);
+    MDB_val v{0, nullptr};
+    if (mdb_cursor_get(cur, &k, &v, MDB_SET) != MDB_SUCCESS) return 0;
+    size_t n = 0;
+    mdb_check(mdb_cursor_count(cur, &n), "cursor_count (relidx)");
+    return static_cast<uint64_t>(n);
+}
+```
+
+`relation_index_remove` uses `mdb_del(txn, dbi, &k, &v)` - passing the data removes only that pair. `relation_index_children` walks `MDB_FIRST_DUP`/`MDB_NEXT_DUP` and skips `is_index_meta_key`.
+
+- [ ] **Step 4: Run - both tests pass**
+
+- [ ] **Step 5: Commit**
+
+```bash
+git commit -am "feat(relations): DUPSORT reverse index with O(1) child counts"
+```
+
+---
+
+### Task 3: maintain the index on child writes
+
+**Files:**
+- Modify: `service/src/storage/document_store_lmdb.cpp` (`put`, `del`)
+- Modify: `service/src/storage/document_store_lmdb.hpp`
+- Create: `tests/test_relation_enforcement.cpp`
+
+**Interfaces:**
+- Consumes: `filter_eval::resolveFilterValue()` - the SAME resolver filters use, so a relation and a query cannot disagree about which field a path names
+- Produces: `void set_relations(std::string_view collection, std::vector<RelationRef> rels)` where `RelationRef { std::string name; std::string childField; }`
+
+- [ ] **Step 1: Write the failing test**
+
+```cpp
+void test_index_follows_the_child_field() {
+    TmpEnv t("rel-maint");
+    LmdbDocumentStore store(t.env);
+    store.set_relations("executions", {{"exec_wf", "workflowId"}});
+
+    auto put = [&](const std::string& id, const nlohmann::json& data) {
+        Document d; d.id = id; d.collection = "executions"; d.set_data(data);
+        store.put("executions", id, d);
+    };
+
+    put("e1", {{"workflowId", "wf-1"}});
+    put("e2", {{"workflowId", "wf-1"}});
+    check(store.relation_index_child_count("exec_wf", "wf-1") == 2, "two children");
+
+    put("e2", {{"workflowId", "wf-2"}});          // re-point
+    check(store.relation_index_child_count("exec_wf", "wf-1") == 1, "left the old parent");
+    check(store.relation_index_child_count("exec_wf", "wf-2") == 1, "joined the new one");
+
+    store.del("executions", "e1");
+    check(store.relation_index_child_count("exec_wf", "wf-1") == 0, "delete removes it");
+
+    // Absent and null are NOT references: they never block a delete and never
+    // count as dangling.
+    put("e3", {{"other", 1}});
+    put("e4", {{"workflowId", nullptr}});
+    check(store.relation_index_child_count("exec_wf", "wf-1") == 0, "absent adds nothing");
+
+    // An ARRAY-valued reference contributes one posting per element.
+    store.set_relations("nodes", {{"node_creds", "config.credentialIds"}});
+    Document n; n.id = "n1"; n.collection = "nodes";
+    n.set_data({{"config", {{"credentialIds", {"c1", "c2"}}}}});
+    store.put("nodes", "n1", n);
+    check(store.relation_index_child_count("node_creds", "c1") == 1, "array element 1");
+    check(store.relation_index_child_count("node_creds", "c2") == 1, "array element 2");
+}
+```
+
+- [ ] **Step 2: Run - fails**
+
+- [ ] **Step 3: Implement**
+
+Add to `put()` alongside the existing `maintainIndexes` call, inside the same `WriteTxn`, collecting handles for `cacheCommittedDbi` after commit. Diff old vs new reference sets exactly as `maintainIndexes` does - `std::set_difference` over sorted id lists - so an unchanged reference costs no index write.
+
+- [ ] **Step 4: Run - passes**
+
+- [ ] **Step 5: Commit**
+
+---
+
+### Task 4: `restrict` and `no_action` on delete
+
+**Files:**
+- Modify: `service/src/database_grpc_impl.cpp` (`Delete`, around line 769)
+- Modify: `tests/test_relation_enforcement.cpp`
+
+**Interfaces:**
+- Consumes: `RelationManager::relationsWithParent()`, `relation_index_child_count()`, `relation_index_children()`
+- Produces: `grpc::StatusCode::FAILED_PRECONDITION` naming the relation, the child count and up to five blocking ids
+
+- [ ] **Step 1: Write the failing test** (unit-level, against the manager + store; the RPC-level check lands in Task 9)
+
+```cpp
+void test_restrict_blocks_and_names_the_blockers() {
+    // ... parent wf-1 with two children ...
+    std::string err;
+    check(!enforcer.canDelete("default:workflows", "wf-1", err), "restrict blocks");
+    check(err.find("exec_wf") != std::string::npos, "names the relation");
+    check(err.find("2") != std::string::npos, "gives the child count");
+    check(err.find("e1") != std::string::npos, "samples a blocking id");
+
+    // no_action permits it and leaves the reference dangling, deliberately.
+    rel.onDelete = OnDelete::NoAction;
+    check(enforcer.canDelete("default:workflows", "wf-1", err), "no_action permits");
+}
+
+void test_relations_enforced_false_skips_the_check() {
+    cfg.relationsEnforced = false;      // per-collection switch, Task 8
+    std::string err;
+    check(enforcer.canDelete("default:workflows", "wf-1", err),
+          "enforcement disabled for this collection lets the delete through - and "
+          "the resulting dangling references will NOT be found by re-enabling it, "
+          "only by `relations check`");
+}
+```
+
+- [ ] **Step 2: Run - fails**
+- [ ] **Step 3: Implement** the check in `Delete` after the access gate and before `store_.remove()`.
+- [ ] **Step 4: Run - passes**
+- [ ] **Step 5: Commit**
+
+---
+
+### Task 5: `DescribeDelete`
+
+**Files:** `proto/database.proto`, `service/src/database_grpc_impl.{hpp,cpp}`, `client/*`, `cli/main.cpp`
+
+The design calls this the feature that makes the behaviour discoverable, and expects it to matter more day to day than cascade. A relational schema exposes constraints through DDL; a document store has none to read, so "what happens if I delete this?" has to be answerable as a query.
+
+- [ ] **Step 1: Proto**
+
+```proto
+message DescribeDeleteRequest { string collection = 1; string id = 2; }
+message RelationImpact {
+    string relation = 1;
+    string child_collection = 2;
+    string child_field = 3;
+    string on_delete = 4;        // restrict | cascade | set_null | no_action
+    uint64 child_count = 5;
+    repeated string sample_child_ids = 6;   // at most five
+    bool blocks = 7;
+}
+message DescribeDeleteResponse {
+    bool success = 1;
+    string error = 2;
+    bool would_be_blocked = 3;
+    repeated RelationImpact impacts = 4;
+}
+```
+
+- [ ] **Step 2-5:** failing test → implement → pass → commit. Counts come from `relation_index_child_count`, so this is cheap enough to call from a UI before every delete.
+
+---
+
+### Task 6: RPCs, client, CLI, boot re-arm
+
+**Files:** `proto/database.proto`, `service/src/database_grpc_impl.{hpp,cpp}`, `service/src/database_service.{hpp,cpp}`, `client/include/smartbotic/database/client.hpp`, `client/src/client.cpp`, `cli/main.cpp`
+
+**Interfaces:**
+- Produces (client - **methods only**, no struct members):
+  ```cpp
+  bool createRelation(const std::string& name, const std::string& childCollection,
+                      const std::string& childField, const std::string& parentCollection,
+                      const std::string& onDelete = "restrict",
+                      bool validateOnWrite = false);
+  bool dropRelation(const std::string& name);
+  struct RelationDefinition { std::string name, child, childField, parent, onDelete;
+                              bool validateOnWrite = false; };
+  std::vector<RelationDefinition> listRelations();
+  ```
+
+- [ ] **Step 1:** `CreateRelation`/`DropRelation`/`ListRelations`/`GetRelationInfo`, gated as **admin-only** - `_relations` is a system collection and a relation names another collection's schema.
+- [ ] **Step 2:** Client qualifies `name` **and** `child`/`parent`, and `unqualify()`s on the way back. This is the v2.4.2 bug; write the test first.
+- [ ] **Step 3:** `DatabaseService::applyRelationDeclarations()`, called next to `applyIndexDeclarations()`. A persisted declaration that is not applied at boot leaves the write path not maintaining an index the read path trusts - that is not bookkeeping, it is the failure mode.
+- [ ] **Step 4:** CLI `relations`, `relations-check <name>`.
+- [ ] **Step 5:** Commit.
+
+---
+
+### Task 7: bootstrap scan and `relations check`
+
+**Files:** `service/src/relations/relation_manager.cpp`, `service/src/database_grpc_impl.cpp`, `cli/main.cpp`
+
+- [ ] **Step 1:** Failing test: declaring a relation over a non-empty child collection builds the index over existing rows, idempotently (re-declaring does not double postings - `MDB_NODUPDATA` already guarantees this).
+- [ ] **Step 2:** Implement `build_relation_index(relation, childCollection, childField)` mirroring `build_index`.
+- [ ] **Step 3:** `relations check` reports dangling references **without changing anything**: walk the index, `mdb_get` each parent id in the parent sub-db, report misses.
+- [ ] **Step 4:** Note in the RPC docstring that a large collection blocks, so it belongs in a migration rather than a live call.
+- [ ] **Step 5:** Commit.
+
+---
+
+### Task 8: per-collection enable/disable
+
+**Files:** `service/src/config/collection_config_manager.{hpp,cpp}`, `proto/database.proto`, `service/src/database_grpc_impl.cpp`, `service/src/database_service.cpp`
+
+- [ ] **Step 1: Write the failing test**
+
+```cpp
+void test_relations_enforced_is_a_partial_update() {
+    CollectionCfg cfg = mgr.configFor("c");
+    check(cfg.relationsEnforced, "defaults to enforced - declaring a relation IS "
+                                 "the opt-in, so a declared constraint must not "
+                                 "silently do nothing");
+
+    // Change ONLY precision. A plain proto3 bool defaults to false and would
+    // silently disable enforcement here - the exact trap v2.4.5 hit with
+    // versioning_enabled, which is why the field is `optional`.
+    configureCollection("c", /*precision=*/"ms", /*relations_enforced=*/std::nullopt);
+    check(mgr.configFor("c").relationsEnforced,
+          "a precision-only call leaves enforcement alone");
+
+    configureCollection("c", "", /*relations_enforced=*/false);
+    check(!mgr.configFor("c").relationsEnforced, "and it can be turned off");
+    check(mgr.configFor("c").timestampPrecision == "ms",
+          "without resetting precision");
+}
+```
+
+- [ ] **Step 2:** Run - fails.
+- [ ] **Step 3:** Add `bool relationsEnforced = true;` and `std::vector<std::string> uniqueFields;` to `CollectionCfg`, serialise both in `toJson`/`fromJson` (absent → default, which is what every pre-2.11 record has), and add `optional bool relations_enforced` to the proto `CollectionConfig`.
+- [ ] **Step 4:** Run - passes.
+- [ ] **Step 5:** Commit.
+
+---
+
+### Task 9: end-to-end over the real RPC boundary (mandatory)
+
+**Files:** Create `tests/load_test/test_relations.cpp`, `tests/load_test/test_relations.sh`
+
+The v2.4.2 views bug survived two releases because in-process unit tests passed while the client/server boundary was broken. Relations have the same registry shape and the same client-qualifies-names surface.
+
+- [ ] **Step 1:** Driver with `setup` and `verify` phases, following `tests/load_test/test_indexes.{cpp,sh}`.
+- [ ] **Step 2:** `setup`: declare a relation from a client with a **non-default project**, insert parents and children, assert `restrict` blocks a parent delete with a message naming the relation, assert `DescribeDelete` reports the right counts, assert `no_action` permits.
+- [ ] **Step 3:** Restart the service in the shell script. `verify`: the declaration survived, a child written *after* the restart is indexed (proving maintenance is armed, not merely remembered), and `restrict` still blocks.
+- [ ] **Step 4:** Assert per-project isolation: an identically-named relation in another project does not see the first project's children.
+- [ ] **Step 5:** Commit.
+
+---
+
+# Phase B - write-path restructuring
+
+Everything here shares one prerequisite. Do Task 10 first; Tasks 11-13 depend on it.
+
+### Task 10: txn-accepting overloads on `LmdbDocumentStore`
+
+**Files:** `service/src/storage/document_store_lmdb.{hpp,cpp}`, `tests/test_document_store.cpp`
+
+Eight public operations open their own `WriteTxn`. Atomic cascade needs one transaction spanning the parent sub-db, each affected child sub-db and the index sub-dbs. Internal helpers (`open_for_write`, `maintainIndexes`) already take a `WriteTxn&`, so this is precedented rather than novel.
+
+- [ ] **Step 1:** Failing test: two documents in different collections written in ONE txn; if it aborts, neither exists and no index entry survives.
+- [ ] **Step 2:** Add `put(WriteTxn&, collection, id, doc)`, `del(WriteTxn&, collection, id)` and the vector equivalents. Existing no-txn forms become thin wrappers that open, call, commit and then `cacheCommittedDbi` - **handles must still be cached only after the caller's commit**, so the txn-taking forms return the handles they opened.
+- [ ] **Step 3:** Run - passes.
+- [ ] **Step 4:** Commit.
+
+---
+
+### Task 11: activate unique constraints
+
+**Files:** `service/src/storage/dual_write_mirror.hpp`, `service/src/memory_store.cpp`, `proto/database.proto`, `service/src/database_grpc_impl.cpp`, `client/*`, `cli/main.cpp`
+
+The check is already built and tested (`find_duplicate_values`, enforcement in `maintainIndexes`). It is unreachable because it throws from `put()` inside `applyDualWriteMirror`, which catches **every** exception, bumps drift and flips `mirror_healthy_` - so a rejection would be swallowed *and* would send every read in the process to MemoryStore, which is the v2.8.1 fault.
+
+- [ ] **Step 1: Write the failing test**
+
+```cpp
+void test_unique_violation_rejects_the_write_and_leaves_no_trace() {
+    // A duplicate insert must fail...
+    check(!store.insert("users", dupEmailDoc), "the duplicate write is rejected");
+    // ...and must NOT have landed in MemoryStore...
+    check(!store.get("users", "u2").has_value(), "no row was left behind");
+    // ...and must NOT have marked the mirror unhealthy, because a rejected
+    // write is the caller's business, not a storage fault. Flipping health here
+    // would send every read in the process to MemoryStore.
+    check(mirrorHealthy.load(), "mirror health is untouched");
+    check(driftCount.load() == 0, "and drift did not move");
+}
+```
+
+- [ ] **Step 2:** Run - fails: the exception is swallowed and the row is present.
+- [ ] **Step 3:** Implement. In `applyDualWriteMirror`, catch `UniqueViolation` **separately and rethrow** without touching health or drift. In `MemoryStore::insert`/`update`, wrap the mirror call so a rethrown violation undoes the in-memory mutation before propagating. Map it to `ALREADY_EXISTS` in the handlers.
+- [ ] **Step 4:** Run - passes. Then re-run `test_subdb_identity` and the index e2e: nothing about mirror-failure handling for *genuine* faults may change.
+- [ ] **Step 5:** Expose it - `unique` on `CreateIndexRequest`, `uniqueFields` in `CollectionCfg`, refusal with examples when duplicates already exist, `unique` reported by `ListIndexes`, and a CLI flag. Commit.
+
+---
+
+### Task 12: `cascade` and `set_null`, WAL-first
+
+**Files:** `service/src/database_grpc_impl.cpp`, `service/src/persistence/persistence_manager.*`, `tests/test_relation_enforcement.cpp`
+
+**Step 3 of the design's delete sequence is mandatory and is the part most likely to be skipped:** MemoryStore is rebuilt at boot from snapshot + WAL replay, **not** from LMDB. A cascade that wrote only LMDB would have its child deletions resurrected on the next restart.
+
+- [ ] **Step 1:** Failing test: cascade delete of a parent with three children; assert children gone, index entries gone, and - the important one - that **WAL entries exist for every child mutation** before the LMDB commit.
+- [ ] **Step 2:** Failing test: array-valued reference under `cascade` **pulls the id and keeps the document**. `cascade` and `set_null` collapse for arrays; deleting a node because one of its three credentials went away would be worse than useless. This is the one place a MySQL mental model actively misleads.
+- [ ] **Step 3:** Implement: resolve children → check policy → WAL parent + every child mutation, fsync → one `WriteTxn` (Task 10) mutating children, index and parent → commit → apply to MemoryStore taking each collection's lock in turn.
+- [ ] **Step 4:** Crash test: simulate a failure between WAL and commit, restart, assert recovery converges (replaying an already-applied delete is a no-op).
+- [ ] **Step 5:** Commit.
+
+---
+
+### Task 13: `validate_on_write`
+
+**Files:** `service/src/storage/document_store_lmdb.cpp`, `tests/test_relation_enforcement.cpp`
+
+Ships with Phase B rather than later because it is the documented remedy for the restrict race: a child insert racing a `restrict` check can still commit after the parent is gone, since LMDB serialises the two transactions but the loser simply commits second. Shipping the limitation without its mitigation would leave operators no way to close the gap.
+
+- [ ] **Step 1:** Failing test: with `validateOnWrite = true`, inserting a child whose parent does not exist is rejected; with it false, the same insert succeeds and `relations check` reports it as dangling.
+- [ ] **Step 2:** Implement as an `mdb_get` on the parent sub-db inside the child's own transaction - which is what closes the race.
+- [ ] **Step 3:** Run - passes.
+- [ ] **Step 4:** Commit, bump `VERSION` to 2.11.0, update `CLAUDE.md` and `docs/ROADMAP.md`.
+
+---
+
+## Out of scope
+
+- **A join or read-time lookup API.** The reverse index makes "children of X" O(children), but exposing that as a query is a separate, cheaper feature. This plan is about integrity.
+- **`ON UPDATE`.** `MemoryStore::update` forces `updated.id = id`, so an id cannot change; the equivalent is delete + insert, governed by `on_delete`.
+- **Cross-project relations.** No transaction spans two project envs.
+- **Repairing existing dangling references.** `relations check` reports them; a repair command is later work. Do **not** call that "Phase C" - that name is taken by the abandoned storage-engine phase and reusing it has already caused one wrong answer.
+
+## Self-review
+
+Run against the re-validated design:
+
+**Spec coverage** - data model (T1), keying and project scope (T1, T6), all four policies (T4 restrict/no_action, T12 cascade/set_null), field resolution including dot-paths and arrays (T3), reverse index (T2), write-path change (T10), WAL-first delete sequence (T12), child re-pointing (T3), restrict race and its remedy (T13), bootstrap scan (T7), RPC surface and `DescribeDelete` (T5, T6), error content (T4), mandatory e2e (T9), per-collection switches (T8), uniqueness activation (T11). No requirement is unassigned.
+
+**Placeholders** - none: every code step carries real code, and every signature used in a later task is defined in an earlier one's Interfaces block.
+
+**Type consistency** - `RelationInfo`/`OnDelete` defined in T1 and used unchanged after; `RelationRef` defined in T3; `relation_index_*` signatures fixed in T2 and reused in T3, T4, T5, T7; client `RelationDefinition` defined in T6.
+
+**Known gap, deliberate:** Task 4's unit test refers to an `enforcer.canDelete()` helper whose home (a free function in `relations/` versus a method on `DatabaseGrpcImpl`) is left to the implementer, because it depends on how much of the gate they choose to make testable without a gRPC context. The behaviour it must have is fully specified; only its address is not.

+ 5 - 4
docs/superpowers/specs/2026-08-03-relations-design.md

@@ -62,10 +62,11 @@
 > **One design choice is now beaten - see "Reverse index" below for the
 > replacement.**
 >
-> The companion plan (`docs/superpowers/plans/2026-08-04-relations-v2.5.0.md`,
-> 3278 lines) is **superseded**: it was written against the pre-v2.8 write path
-> and its task list embeds the wrong version and the stale facts above. Treat it
-> as a design input and re-plan rather than executing it.
+> **The plan to execute is `docs/superpowers/plans/2026-08-09-relations-v2.11.0.md`**,
+> re-planned from this corrected design. The older companion plan
+> (`2026-08-04-relations-v2.5.0.md`, 3278 lines) is superseded: it was written
+> against the pre-v2.8 write path and embeds the wrong version and the stale facts
+> above. Keep it only as raw material.
 
 Status: approved design, not yet implemented
 Target: v2.5.0 (Phases A + B together)