Write-path transaction-layering seam (graphdb_mgr:transaction/1)

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:

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}, ...]}
 %%

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)).