diff --git a/TASKS.md b/TASKS.md index 4ae411c..b68ee0c 100644 --- a/TASKS.md +++ b/TASKS.md @@ -94,18 +94,70 @@ authoring: ## Write-path completion `graphdb_mgr` routes node and relationship creation to the workers. The -gaps that remain are node mutation, the template attribute list, and -wiring the multilingual write path. - -### Node deletion and attribute-value-pair update - -`graphdb_mgr:delete_node/1` and `update_node_avps/2` still return -`{error, not_implemented}`: no worker implements node deletion or general -AVP update. Both already pass through the category guard (rejecting the -scaffold category nodes) before returning. Wire them to a worker once one -provides the underlying functionality. - -### Template attribute list and instance-only enforcement +gaps that remain are node mutation, relationship mutation, the template +attribute list, and wiring the multilingual write path. They are +independent and broken into slices A–E below. `graphdb_mgr` owns the +generic low-level node/relationship CRUD; type-specific behaviour +delegates to the owning worker. + +### Transaction-layering seam (slice A prerequisite) — IN DESIGN + +The decided convention for all write-path mutation: separate the Mnesia +transaction boundary from the CRUD logic, so operations compose into one +atomic transaction without nesting. + +- **Tier 1 — in-transaction primitives.** Assume they already run inside + an Mnesia activity; do their reads + writes with bare Mnesia ops; signal + failure via `mnesia:abort/1`. They never open a transaction, so they + compose. +- **Tier 2 — single-op public API** (e.g. `graphdb_mgr:delete_node/1`). + Owns the transaction: static guards, then + `mnesia:transaction(fun() -> Primitive end)`, mapping `{atomic, R}` → + `{ok, R}` and `{aborted, Reason}` → `{error, Reason}`. +- **Tier 3 — batch / composite** (a future `mutate([Mutation])`, or + "delete an instance with its parts"). Wraps one transaction and calls + the tier-1 primitives directly — never the tier-2 wrappers; no nested + transactions. + +This slice delivers the **minimal seam only**: the convention plus a +shared transaction-runner helper in `graphdb_mgr`, with tests proven +against a sample primitive. No existing write op changes. `delete_node` +and `remove_relationship` adopt it as their first consumers. + +Tracked follow-ups (not in the seam spec): + +- **Retrofit existing write ops** (`create_instance`, `add_relationship`, + the membership `do_*` ops) onto the primitive/wrapper layering — uniform + convention, no behaviour change. +- **Batch `mutate([Mutation])`** — the tier-3 entry point. + +### Node deletion (slice A, after the seam) + +`graphdb_mgr:delete_node/1` still returns `{error, not_implemented}`. +Decided policy: **refuse-if-referenced**. The node's own upward attachment +(its parent↔child arc pair, its class-membership arc pair, its own AVPs) +is removed as part of the delete; deletion is refused when other things +depend on the node: + +- it is a compositional/taxonomic parent with children; +- it is a class with live instances; +- it is targeted by an incoming connection arc from a peer. + +Only runtime nodes (`nref >= ?NREF_START`) are deletable — the entire +permanent tier (categories, bootstrap attributes, arc labels, init seeds) +is refused, extending the current category-only guard. **Known non-goal:** +detecting a node referenced as an AVP *value* on another node or arc (no +index exists; not treated as a blocker). The blocker reads must run in the +same transaction as the deletes (TOCTOU). `delete_node` is the first +tier-1 primitive + tier-2 wrapper built on the seam. + +### Node AVP update (slice B) + +`graphdb_mgr:update_node_avps/2` still returns `{error, not_implemented}`: +basic AVP merge/replace + validation on a node row. Independent. Mirrors +the arc-AVP edit in slice E. + +### Template attribute list and instance-only enforcement (slice C, depends on slice B) A template currently carries only a name and its compositional arc into the owning class — there is no per-template list of which attributes the @@ -129,7 +181,17 @@ characteristic AVP shape (a declared-but-unbound attribute carries `value => undefined`) already accommodates an instance-only attribute naturally — it stays `undefined` at every class level. -### Multilingual write-path integration +### Relationship mutation (slice E) + +Only `add_relationship` (create) exists today — there is no remove or +update. `remove_relationship` deletes both directed rows of a logical edge +atomically and fixes the `parents`/`classes` caches on the referrers; it +shares the arc-removal primitive with `delete_node` (slice A). +`update_relationship` changes `characterization` / `target_nref` / +`reciprocal` / AVPs; the AVP-only edit mirrors `update_node_avps` +(slice B). Built on the transaction seam. + +### Multilingual write-path integration (slice D) Now unblocked — the `graphdb_mgr` write-side is wired. When an environment node is created, the write path must additionally: diff --git a/apps/graphdb/src/graphdb_mgr.erl b/apps/graphdb/src/graphdb_mgr.erl index 947cb5f..60be8ee 100644 --- a/apps/graphdb/src/graphdb_mgr.erl +++ b/apps/graphdb/src/graphdb_mgr.erl @@ -113,6 +113,8 @@ add_relationship/4, delete_node/1, update_node_avps/2, + %% Transaction helper (write-path seam) + transaction/1, %% Cache invariant audit / repair verify_caches/0, rebuild_caches/0 @@ -250,6 +252,30 @@ update_node_avps(Nref, AVPs) -> gen_server:call(?MODULE, {update_node_avps, Nref, AVPs}). +%%----------------------------------------------------------------------------- +%% transaction(Fun) -> {ok, Result} | {error, Reason} +%% +%% Runs Fun inside one Mnesia transaction and normalises the result: +%% {atomic, R} -> {ok, R}; {aborted, Reason} -> {error, Reason}. +%% +%% Fun is a tier-1 write-path primitive (or a composition of them): it +%% performs its reads/writes with bare mnesia ops, never opens its own +%% transaction, and signals a domain failure via mnesia:abort/1. This is +%% the single transaction boundary the write-path seam standardises on; +%% see docs/designs/write-path-transaction-seam-design.md. +%% +%% This is a plain function, not a gen_server:call -- mnesia:transaction/1 +%% runs in the calling process, so routing writes through the graphdb_mgr +%% server would needlessly serialise them. +%%----------------------------------------------------------------------------- +-spec transaction(fun(() -> Result)) -> {ok, Result} | {error, term()}. +transaction(Fun) -> + case mnesia:transaction(Fun) of + {atomic, Result} -> {ok, Result}; + {aborted, Reason} -> {error, Reason} + end. + + %%----------------------------------------------------------------------------- %% verify_caches() -> ok | {error, [{Nref, Field, Expected, Actual}, ...]} %% diff --git a/apps/graphdb/test/graphdb_mgr_SUITE.erl b/apps/graphdb/test/graphdb_mgr_SUITE.erl index 1740fcc..11a9ede 100644 --- a/apps/graphdb/test/graphdb_mgr_SUITE.erl +++ b/apps/graphdb/test/graphdb_mgr_SUITE.erl @@ -90,7 +90,12 @@ verify_caches_clean_after_bootstrap/1, verify_caches_detects_poisoned_parents/1, verify_caches_detects_poisoned_classes/1, - rebuild_caches_restores_after_poison/1 + rebuild_caches_restores_after_poison/1, + %% Transaction seam + transaction_commit_returns_ok/1, + transaction_abort_rolls_back/1, + transaction_composition_rolls_back/1, + transaction_crash_passes_through/1 ]). @@ -104,7 +109,7 @@ suite() -> all() -> [{group, init_tests}, {group, read_ops}, {group, category_guard}, {group, write_delegation}, - {group, cache_audit}]. + {group, cache_audit}, {group, transaction_seam}]. groups() -> [ @@ -145,6 +150,12 @@ groups() -> verify_caches_detects_poisoned_parents, verify_caches_detects_poisoned_classes, rebuild_caches_restores_after_poison + ]}, + {transaction_seam, [], [ + transaction_commit_returns_ok, + transaction_abort_rolls_back, + transaction_composition_rolls_back, + transaction_crash_passes_through ]} ]. @@ -713,3 +724,90 @@ do_delete_dir(Dir) -> false -> ok end. + + +%%============================================================================= +%% Transaction Seam Tests +%% +%% Sample throwaway primitives write bare #node rows at scratch nrefs in the +%% ?NREF_START + 500_000 band (well clear of bootstrap and other suites' +%% runtime allocations); each case runs in its own isolated Mnesia DB. +%%============================================================================= + +%%----------------------------------------------------------------------------- +%% transaction/1 commits a primitive's writes and returns {ok, Result}. +%%----------------------------------------------------------------------------- +transaction_commit_returns_ok(_Config) -> + {ok, _} = graphdb_mgr:start_link(), + Nref1 = ?NREF_START + 500001, + Nref2 = ?NREF_START + 500002, + Fun = fun() -> + ok = mnesia:write(nodes, + #node{nref = Nref1, kind = instance, + parents = [], classes = [], attribute_value_pairs = []}, + write), + ok = mnesia:write(nodes, + #node{nref = Nref2, kind = instance, + parents = [], classes = [], attribute_value_pairs = []}, + write), + {written, 2} + end, + ?assertEqual({ok, {written, 2}}, graphdb_mgr:transaction(Fun)), + ?assertMatch([#node{nref = Nref1}], mnesia:dirty_read(nodes, Nref1)), + ?assertMatch([#node{nref = Nref2}], mnesia:dirty_read(nodes, Nref2)). + +%%----------------------------------------------------------------------------- +%% transaction/1 maps mnesia:abort/1 to {error, Reason} and rolls back the +%% primitive's write (single-primitive atomicity). +%%----------------------------------------------------------------------------- +transaction_abort_rolls_back(_Config) -> + {ok, _} = graphdb_mgr:start_link(), + NrefA = ?NREF_START + 500010, + Fun = fun() -> + ok = mnesia:write(nodes, + #node{nref = NrefA, kind = instance, + parents = [], classes = [], attribute_value_pairs = []}, + write), + mnesia:abort(blocked) + end, + ?assertEqual({error, blocked}, graphdb_mgr:transaction(Fun)), + ?assertEqual([], mnesia:dirty_read(nodes, NrefA)). + +%%----------------------------------------------------------------------------- +%% transaction/1 over a composition of two primitives rolls back BOTH when +%% the second aborts -- the property tier-3 batch composition relies on. +%%----------------------------------------------------------------------------- +transaction_composition_rolls_back(_Config) -> + {ok, _} = graphdb_mgr:start_link(), + NrefP = ?NREF_START + 500020, + First = fun() -> + ok = mnesia:write(nodes, + #node{nref = NrefP, kind = instance, + parents = [], classes = [], attribute_value_pairs = []}, + write) + end, + Second = fun() -> mnesia:abort(second_failed) end, + Fun = fun() -> First(), Second() end, + ?assertEqual({error, second_failed}, graphdb_mgr:transaction(Fun)), + ?assertEqual([], mnesia:dirty_read(nodes, NrefP)). + +%%----------------------------------------------------------------------------- +%% An uncaught crash inside the fun surfaces as Mnesia's standard +%% {aborted, {Reason, Stacktrace}} shape, passed through unreshaped as +%% {error, {Reason, Stacktrace}} (design doc section 4), and rolls back the +%% primitive's write. +%%----------------------------------------------------------------------------- +transaction_crash_passes_through(_Config) -> + {ok, _} = graphdb_mgr:start_link(), + NrefC = ?NREF_START + 500030, + Fun = fun() -> + ok = mnesia:write(nodes, + #node{nref = NrefC, kind = instance, + parents = [], classes = [], attribute_value_pairs = []}, + write), + %% Uncaught crash -- not a deliberate mnesia:abort/1. + erlang:error(crash_in_txn) + end, + ?assertMatch({error, {crash_in_txn, _Stack}}, + graphdb_mgr:transaction(Fun)), + ?assertEqual([], mnesia:dirty_read(nodes, NrefC)). diff --git a/docs/designs/write-path-transaction-seam-design.md b/docs/designs/write-path-transaction-seam-design.md new file mode 100644 index 0000000..ba46867 --- /dev/null +++ b/docs/designs/write-path-transaction-seam-design.md @@ -0,0 +1,239 @@ + + +# Write-Path Transaction-Layering Seam — Design + +**Status:** Designed; not yet planned or implemented. + +**Context:** First piece of the **write-path completion** track in +`TASKS.md`. The track decomposes into five independent slices — +A `delete_node`, B `update_node_avps`, C template attribute list + +instance-only enforcement, D multilingual write-path wiring, and +E relationship mutation. Slice A is itself split: this document specifies +the **transaction-layering seam** that `delete_node` and +`remove_relationship` are built on; `delete_node` itself follows in a +later design once the seam exists. + +**Spec citation:** none. The knowledge-network spec +(`docs/TheKnowledgeNetwork.md`) is a data model and is silent on +transaction mechanics. This is an infrastructural design that records a +convention for how write-path mutations compose over Mnesia. + +--- + +## 1. Scope + +### 1.1 The problem + +Every existing write operation in the graphdb workers opens its own +`mnesia:transaction/1` (e.g. `graphdb_instance:do_add_relationship/7, +do_write_class_membership/2`). That is correct for a single operation but +does not compose: there is no way to run two mutations as one atomic unit +without nesting transactions, and the upcoming write-path work needs +exactly that — + +- `delete_node` (slice A) does blocker-check reads and row deletes that + must commit or roll back together; +- `remove_relationship` (slice E) removes both directed rows of a logical + edge atomically; +- a future batch `mutate([Mutation])` (tracked follow-up) runs an + arbitrary list of mutations in one transaction; +- a future "delete an instance with its parts" composes a generic delete + with type-specific cleanup. + +This design fixes **where the transaction boundary lives** so all of the +above share the same building blocks. + +The convention already exists in embryo. `graphdb_mgr` carries several +functions documented "Must run inside an active mnesia transaction" — +`expected_parents/1`, `expected_classes/1`, `verify_one/1`, +`rebuild_one/1` — which are tier-1 primitives in all but name. And the +`{atomic, R} -> {ok, R}; {aborted, Reason} -> {error, Reason}` mapping is +already hand-rolled inline in at least three places (`do_get_relationships/2` +and the `verify_caches/0` / `rebuild_caches/0` wrappers). This slice names +the convention and factors that repeated mapping into one helper. + +### 1.2 What this slice delivers + +The **minimal seam only**: + +1. a documented three-tier convention (§2); +2. one shared transaction-runner helper, `graphdb_mgr:transaction/1` + (§3); +3. tests proving the runner's atomicity and result normalisation against + sample primitives (§5). + +No existing write operation is changed. `delete_node` and +`remove_relationship` adopt the seam as their first real consumers, in +their own later slices. + +### 1.3 Out of scope (tracked elsewhere) + +| Item | Where it lives | +| ---------------------------------------------- | ----------------------------- | +| `delete_node` implementation + deletion policy | Slice A, later design | +| `remove_relationship` / `update_relationship` | Slice E | +| Retrofitting existing ops onto the seam | Tracked follow-up in TASKS.md | +| Batch `mutate([Mutation])` tier-3 entry point | Tracked follow-up in TASKS.md | + +The retrofit follow-up's first concrete targets are the inline-mapping +wrappers already in `graphdb_mgr` — `do_get_relationships/2`, +`verify_caches/0`, and `rebuild_caches/0` — which hand-roll the +`{atomic}`/`{aborted}` mapping that `transaction/1` replaces. + +--- + +## 2. The convention — three tiers + +The transaction boundary lives in **exactly one tier**. Mutations are +written as composable primitives that assume a surrounding transaction; +only wrappers open one. + +### Tier 1 — in-transaction primitives + +A *tier-1 primitive* is any function that: + +- assumes it is **already inside** an Mnesia activity — it uses + `mnesia:read/write/delete/index_read` directly and **never** calls + `mnesia:transaction/1`; +- signals a domain failure by calling `mnesia:abort(Reason)`; +- returns its success value normally. + +Because a primitive opens no transaction of its own, primitives compose: +several can run inside one transaction with a single atomic outcome. +`graphdb_mgr:expected_parents/1`, `expected_classes/1`, `verify_one/1`, +and `rebuild_one/1` already satisfy this contract today. + +### Tier 2 — single-op public API + +A *tier-2* function is the public entry point for one mutation (e.g. the +future `graphdb_mgr:delete_node/1`). It performs any static guards that +need no transaction (argument validation, the permanent-tier nref +boundary, kind lookups), then wraps **one** primitive: + +```erlang +delete_node(Nref) -> + case static_guards(Nref) of + ok -> graphdb_mgr:transaction(fun() -> delete_node_(Nref) end); + {error, _}=E -> E + end. +``` + +### Tier 3 — batch / composite + +A *tier-3* function composes several mutations into one atomic unit. It +wraps **one** transaction and calls the **tier-1 primitives directly** — +never the tier-2 wrappers — so there is no transaction nesting: + +```erlang +mutate(Mutations) -> + graphdb_mgr:transaction( + fun() -> [apply_mutation(M) || M <- Mutations] end). +``` + +A composite such as "delete an instance with its parts" follows the same +shape: inside one `transaction/1`, call the generic delete primitive plus +whatever type-specific cleanup the owning worker contributes (also as +tier-1 primitives). + +### Why not rely on nested transactions + +Mnesia *does* support nesting — an inner `mnesia:transaction/1` joins the +outer one and commits with it. The seam deliberately does not lean on +that: nesting muddies abort reasons and return shapes and makes the +atomicity boundary implicit. Standardising on "primitives assume a +transaction; only wrappers open one" keeps the boundary explicit and the +error contract uniform. + +--- + +## 3. The helper + +One new export on `graphdb_mgr`. It is a **plain exported function, not a +`gen_server:call`**: `mnesia:transaction/1` runs in the *calling* +process, so routing writes through the `graphdb_mgr` server process would +needlessly serialise every write and risk deadlock. The helper is +stateless and holds no relation to the gen_server's `State`. + +```erlang +%% transaction(Fun) -> {ok, Result} | {error, Reason} +%% +%% Runs Fun inside one Mnesia transaction and normalises the result. +%% Fun is a tier-1 primitive (or a composition of them): it does its +%% reads/writes with bare Mnesia ops and signals failure via +%% mnesia:abort/1. +-spec transaction(fun(() -> Result)) -> {ok, Result} | {error, term()}. +transaction(Fun) -> + case mnesia:transaction(Fun) of + {atomic, Result} -> {ok, Result}; + {aborted, Reason} -> {error, Reason} + end. +``` + +The helper is added to `graphdb_mgr`'s export list and documented with the +module's existing header-comment style. No supervision-tree change, no new +module. + +--- + +## 4. Error normalisation + +| Mnesia outcome | `transaction/1` returns | +| -------------------------------- | -------------------------- | +| `{atomic, R}` | `{ok, R}` | +| `{aborted, Reason}` (from abort) | `{error, Reason}` | +| `{aborted, {Reason, Stack}}` | `{error, {Reason, Stack}}` | + +- A primitive's `mnesia:abort(my_reason)` surfaces as `{error, + my_reason}` — this is the channel tier-1 primitives use to report a + domain failure (e.g. a future `{error, {has_children, Nref}}`). +- An uncaught Erlang exit/exception inside the transaction body surfaces + as Mnesia's standard `{aborted, {Reason, Stacktrace}}` shape; the + helper passes it through as `{error, {Reason, Stacktrace}}` without + reshaping. Callers that need clean domain errors must `mnesia:abort/1` + deliberately rather than relying on crashes. + +--- + +## 5. Testing + +No real consumer ships in this slice, so the runner is exercised with +**sample throwaway primitives** against the real `nodes` / `relationships` +tables in the Common Test scratch database. Three properties: + +1. **Success + result passthrough.** A primitive writes two rows and + returns a value → `transaction/1` returns `{ok, Value}`; both rows are + present after commit. +2. **Abort rolls back.** A primitive writes one row, then + `mnesia:abort(blocked)` → `transaction/1` returns `{error, blocked}`; + the row it wrote is **not** present (single-primitive atomic rollback). +3. **Composition rolls back.** A fold runs two primitives inside one + `transaction/1`; the *second* aborts → the *first* primitive's write is + also absent. This proves the atomicity property tier-3 relies on. + +Tests use a scratch nref well above `?NREF_START` and clean up after +themselves, consistent with the suite's per-case isolation. + +--- + +## 6. Files touched + +| File | Change | +| ----------------------------------------- | ----------------------------------- | +| `apps/graphdb/src/graphdb_mgr.erl` | Add exported `transaction/1` + doc | +| `apps/graphdb/test/graphdb_mgr_SUITE.erl` | Add the three runner cases (§5) | +| `TASKS.md` | Already updated — seam + follow-ups | + +No `docs/Architecture.md` change is required: the supervision tree, the +Mnesia schema, and the public worker contracts are unchanged. The seam is +an internal convention plus one helper. (Architecture.md may pick up a +one-line note when the first consumer — `delete_node` — lands.) + +--- + +## 7. Open items + +None. The convention, the helper signature, the error mapping, and the +test plan are fixed. The name `transaction/1` is confirmed. diff --git a/docs/superpowers/plans/2026-06-17-write-path-transaction-seam.md b/docs/superpowers/plans/2026-06-17-write-path-transaction-seam.md new file mode 100644 index 0000000..65c3fa7 --- /dev/null +++ b/docs/superpowers/plans/2026-06-17-write-path-transaction-seam.md @@ -0,0 +1,328 @@ + + +# Write-Path Transaction-Layering Seam Implementation Plan + +> **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:** Add `graphdb_mgr:transaction/1`, a shared Mnesia +transaction-runner that normalises results, and document the three-tier +primitive/wrapper/batch convention it anchors. + +**Architecture:** A single new exported function on `graphdb_mgr`. It +wraps `mnesia:transaction/1` and maps `{atomic, R}` → `{ok, R}` and +`{aborted, Reason}` → `{error, Reason}`. It is a plain function (not a +`gen_server:call`) because `mnesia:transaction/1` runs in the calling +process. Tier-1 primitives (functions that assume a surrounding +transaction and signal failure via `mnesia:abort/1`) compose under it. +No existing write op is changed in this slice. + +**Tech Stack:** Erlang/OTP 28, Mnesia, Common Test, rebar3 3.27. + +**Design:** `docs/designs/write-path-transaction-seam-design.md` + +## Global Constraints + +- **OTP 28 / rebar3 3.27.** Build with `./rebar3 compile` from the repo + root (kerl PATH is preset — no `source ~/.bashrc` prefix). +- **Zero compiler warnings.** The tree compiles clean; keep it clean. +- **Module header + macro conventions.** `graphdb_mgr.erl` already has its + copyright block, `-export` lists, and NYI/UEM macros; do not duplicate + or disturb them. Add the new export to the existing public-API + `-export` block, not a new one. +- **CT suite uses explicit per-group `-export` lists** (no + `-compile(export_all)`). Every new test function MUST be added to the + test-case `-export` block AND registered in `groups/0` AND reachable + from `all/0`, or Common Test fails with `undef` at runtime. +- **Commit message footer.** End commit messages with: + `Co-Authored-By: Claude Opus 4.8 ` + +--- + +## File Structure + +| File | Responsibility | +| ------------------------------------------ | ------------------------------------------------------ | +| `apps/graphdb/src/graphdb_mgr.erl` | Add exported `transaction/1` + its header comment | +| `apps/graphdb/test/graphdb_mgr_SUITE.erl` | New `transaction_seam` group: 3 CT cases proving the contract | + +No supervision-tree, schema, `docs/Architecture.md`, or +`docs/diagrams/ontology-tree.md` change is required: the seam is an +internal convention plus one stateless helper. + +--- + +## Task 1: `graphdb_mgr:transaction/1` runner + contract tests + +**Files:** +- Modify: `apps/graphdb/src/graphdb_mgr.erl` (export block at lines + 103–119; add the function implementation in the public-API section, + immediately before the `verify_caches/0` public function) +- Test: `apps/graphdb/test/graphdb_mgr_SUITE.erl` + +**Interfaces:** +- Produces: + `graphdb_mgr:transaction(fun(() -> Result)) -> {ok, Result} | {error, term()}` + — runs `Fun` inside one Mnesia transaction; `{atomic, R}` → `{ok, R}`, + `{aborted, Reason}` → `{error, Reason}`. `Fun` is a tier-1 primitive (or + a composition of them): it does bare-Mnesia reads/writes and signals + failure via `mnesia:abort/1`. + +### Why these three tests + +The runner's whole job is (a) result/error normalisation and (b) +inheriting Mnesia's atomicity. The three cases pin exactly that: success +passthrough, single-primitive rollback on abort, and multi-primitive +rollback (the property tier-3 batch composition relies on). The sample +primitives are throwaway funs that write bare `#node` rows at scratch +nrefs well above `?NREF_START`; each CT case already runs in an isolated +Mnesia database (`init_per_testcase` → `setup_isolated_env`), and the +temp DB is deleted in `end_per_testcase`, so no in-test cleanup is needed. +The rows written by the success case have empty `parents`/`classes` and no +arcs, so they satisfy the `verify_cache_invariant/1` audit that +`end_per_testcase` runs. + +- [ ] **Step 1: Register the new group and test-case exports** + +In `apps/graphdb/test/graphdb_mgr_SUITE.erl`, add the three case names to +the test-case `-export` block (the one ending at line 94, after +`rebuild_caches_restores_after_poison/1`). Add a trailing comma to the +current last entry: + +```erlang + rebuild_caches_restores_after_poison/1, + %% Transaction seam + transaction_commit_returns_ok/1, + transaction_abort_rolls_back/1, + transaction_composition_rolls_back/1 +``` + +Add the group to `all/0` (currently lines 104–107): + +```erlang +all() -> + [{group, init_tests}, {group, read_ops}, + {group, category_guard}, {group, write_delegation}, + {group, cache_audit}, {group, transaction_seam}]. +``` + +Add the group definition to `groups/0` (after the `cache_audit` group, +before the closing `]` at line 149 — add a comma after the `cache_audit` +group's closing brace): + +```erlang + {cache_audit, [], [ + verify_caches_clean_after_bootstrap, + verify_caches_detects_poisoned_parents, + verify_caches_detects_poisoned_classes, + rebuild_caches_restores_after_poison + ]}, + {transaction_seam, [], [ + transaction_commit_returns_ok, + transaction_abort_rolls_back, + transaction_composition_rolls_back + ]} +``` + +These cases need no special `init_per_testcase` clause: the default +clause (line 216) starts `nref` + `rel_id_server`, and each test body +starts `graphdb_mgr` itself (creating the `nodes`/`relationships` tables), +exactly as the read-ops cases do. + +- [ ] **Step 2: Write the three failing test cases** + +Append these case functions to `apps/graphdb/test/graphdb_mgr_SUITE.erl` +(at the end of the file, after the last existing case). The `#node{}` +record and `?NREF_START` are already available (record defined at lines +24–30; `graphdb_nrefs.hrl` included at line 18). + +```erlang +%%============================================================================= +%% Transaction Seam Tests +%%============================================================================= + +%%----------------------------------------------------------------------------- +%% transaction/1 commits a primitive's writes and returns {ok, Result}. +%%----------------------------------------------------------------------------- +transaction_commit_returns_ok(_Config) -> + {ok, _} = graphdb_mgr:start_link(), + Nref1 = ?NREF_START + 500001, + Nref2 = ?NREF_START + 500002, + Fun = fun() -> + ok = mnesia:write(nodes, + #node{nref = Nref1, kind = instance, + parents = [], classes = [], attribute_value_pairs = []}, + write), + ok = mnesia:write(nodes, + #node{nref = Nref2, kind = instance, + parents = [], classes = [], attribute_value_pairs = []}, + write), + {written, 2} + end, + ?assertEqual({ok, {written, 2}}, graphdb_mgr:transaction(Fun)), + ?assertMatch([#node{nref = Nref1}], mnesia:dirty_read(nodes, Nref1)), + ?assertMatch([#node{nref = Nref2}], mnesia:dirty_read(nodes, Nref2)). + +%%----------------------------------------------------------------------------- +%% transaction/1 maps mnesia:abort/1 to {error, Reason} and rolls back the +%% primitive's write (single-primitive atomicity). +%%----------------------------------------------------------------------------- +transaction_abort_rolls_back(_Config) -> + {ok, _} = graphdb_mgr:start_link(), + NrefA = ?NREF_START + 500010, + Fun = fun() -> + ok = mnesia:write(nodes, + #node{nref = NrefA, kind = instance, + parents = [], classes = [], attribute_value_pairs = []}, + write), + mnesia:abort(blocked) + end, + ?assertEqual({error, blocked}, graphdb_mgr:transaction(Fun)), + ?assertEqual([], mnesia:dirty_read(nodes, NrefA)). + +%%----------------------------------------------------------------------------- +%% transaction/1 over a composition of two primitives rolls back BOTH when +%% the second aborts -- the property tier-3 batch composition relies on. +%%----------------------------------------------------------------------------- +transaction_composition_rolls_back(_Config) -> + {ok, _} = graphdb_mgr:start_link(), + NrefP = ?NREF_START + 500020, + First = fun() -> + ok = mnesia:write(nodes, + #node{nref = NrefP, kind = instance, + parents = [], classes = [], attribute_value_pairs = []}, + write) + end, + Second = fun() -> mnesia:abort(second_failed) end, + Fun = fun() -> First(), Second() end, + ?assertEqual({error, second_failed}, graphdb_mgr:transaction(Fun)), + ?assertEqual([], mnesia:dirty_read(nodes, NrefP)). +``` + +- [ ] **Step 3: Run the new group to verify it fails** + +Run: + +```bash +./rebar3 ct --suite apps/graphdb/test/graphdb_mgr_SUITE --group transaction_seam +``` + +Expected: FAIL. The cases compile but fail at runtime because +`graphdb_mgr:transaction/1` is undefined — an `undef` / +`{badrpc, ...}`-style error or a function-clause/`error:undef` on +`graphdb_mgr:transaction/1`. (If instead the suite fails to *compile*, +re-check the Step 1 export/group edits.) + +- [ ] **Step 4: Add the `transaction/1` export** + +In `apps/graphdb/src/graphdb_mgr.erl`, extend the public-API `-export` +block (lines 103–119). Add a `transaction/1` entry under a new comment +group, before the cache-audit exports: + +```erlang + add_relationship/4, + delete_node/1, + update_node_avps/2, + %% Transaction helper (write-path seam) + transaction/1, + %% Cache invariant audit / repair + verify_caches/0, + rebuild_caches/0 + ]). +``` + +- [ ] **Step 5: Implement `transaction/1`** + +In `apps/graphdb/src/graphdb_mgr.erl`, add the function in the public-API +function section, immediately before the public `verify_caches/0` +definition. Match the module's existing banner-comment style: + +```erlang +%%----------------------------------------------------------------------------- +%% transaction(Fun) -> {ok, Result} | {error, Reason} +%% +%% Runs Fun inside one Mnesia transaction and normalises the result: +%% {atomic, R} -> {ok, R}; {aborted, Reason} -> {error, Reason}. +%% +%% Fun is a tier-1 write-path primitive (or a composition of them): it +%% performs its reads/writes with bare mnesia ops, never opens its own +%% transaction, and signals a domain failure via mnesia:abort/1. This is +%% the single transaction boundary the write-path seam standardises on; +%% see docs/designs/write-path-transaction-seam-design.md. +%% +%% This is a plain function, not a gen_server:call -- mnesia:transaction/1 +%% runs in the calling process, so routing writes through the graphdb_mgr +%% server would needlessly serialise them. +%%----------------------------------------------------------------------------- +-spec transaction(fun(() -> Result)) -> {ok, Result} | {error, term()}. +transaction(Fun) -> + case mnesia:transaction(Fun) of + {atomic, Result} -> {ok, Result}; + {aborted, Reason} -> {error, Reason} + end. +``` + +- [ ] **Step 6: Run the new group to verify it passes** + +Run: + +```bash +./rebar3 ct --suite apps/graphdb/test/graphdb_mgr_SUITE --group transaction_seam +``` + +Expected: PASS — 3 cases, 0 failures. + +- [ ] **Step 7: Compile clean and run the full mgr suite** + +Run: + +```bash +./rebar3 compile +./rebar3 ct --suite apps/graphdb/test/graphdb_mgr_SUITE +``` + +Expected: zero compiler warnings; the full `graphdb_mgr_SUITE` passes +(the pre-existing cases plus the 3 new ones). + +- [ ] **Step 8: Commit** + +```bash +git add apps/graphdb/src/graphdb_mgr.erl \ + apps/graphdb/test/graphdb_mgr_SUITE.erl +git commit -m "feat: add graphdb_mgr:transaction/1 write-path transaction seam + +Adds the shared Mnesia transaction-runner that normalises +{atomic,R}->{ok,R} / {aborted,R}->{error,R} and anchors the tier-1 +primitive / tier-2 wrapper / tier-3 batch convention. First consumers +(delete_node, remove_relationship) land in later slices. + +Co-Authored-By: Claude Opus 4.8 " +``` + +--- + +## Self-Review + +**1. Spec coverage.** The spec's three deliverables map to this plan: +(§1.2.1 the convention) — documented in the design doc and in the +`transaction/1` banner comment (Step 5); (§1.2.2 the helper) — Steps 4–5; +(§1.2.3 tests) — Steps 1–3 + 6. The spec's §5 three test properties +(success+passthrough, abort rollback, composition rollback) are the three +cases in Step 2. §4 error normalisation is exercised by the `{error, +blocked}` / `{error, second_failed}` assertions. Out-of-scope items (§1.3) +are correctly absent. + +**2. Placeholder scan.** No TBD/TODO/"handle errors"/"similar to" — every +step shows exact code and exact commands. + +**3. Type consistency.** `transaction/1` signature, the `{ok, Result}` / +`{error, Reason}` return shapes, and the `mnesia:abort/1` reasons +(`blocked`, `second_failed`) are consistent between the test assertions +(Step 2), the export (Step 4), and the implementation + spec (Step 5). +Scratch nrefs are distinct per case (`?NREF_START + 500001/500002`, +`+500010`, `+500020`).