fix(ops): make add_entity atomic — snapshot + rollback on validation gate failure

[coderabbitai]

Summary

GtsOps::add_entity (and transitively add_entities) is non-atomic: self.store.register(entity.clone()) runs first, and every subsequent validation gate (validate_schema_modifiers, validate_trait_placement, validate_schema, validate_schema_chain, validate_schema_traits) returns ok: false on failure without rolling back the registration. Rejected entities therefore remain in the store and are visible to later reads and validations.

This pattern predates PR #90 (v0.12 support) and is systemic — it was not introduced by the trait-placement check added in that PR.

Desired behaviour

• Snapshot + rollback: before running any validation gate, snapshot (or skip registering until all gates pass), and roll back / evict the entity from the store if any gate fails.
• Rollback-safe re-registration: a failed re-registration of an already-registered entity must not wipe the previously-registered valid state.
• Batch transactionality: add_entities should either commit all or none (or at minimum clearly document per-item rollback behaviour).

References

• PR: feat(gts): support gts-spec v0.12 — RFC 7396 trait merge, inherited closedness, trait placement #90
• Comment thread: feat(gts): support gts-spec v0.12 — RFC 7396 trait merge, inherited closedness, trait placement #90 (comment)

/cc @aviator5

[aviator5]

Related but distinct: validate_schema (/validate-type-schema) never runs the placement/modifier gates

While the non-atomic add_entity is the systemic root cause described here, there's a separate consistency gap worth tracking alongside it: the three schema entry points are not aligned on which validation gates they run before trait merge.

Endpoint	ops method	runs validate_trait_placement / validate_schema_modifiers?
POST /entities	add_entity (ops.rs:281)	yes (ops.rs:332, :348)
POST /validate-entity	validate_entity (ops.rs:677)	yes (ops.rs:682, :695)
POST /validate-type-schema	validate_schema (ops.rs:643)	no — goes straight to validate_schema_traits (ops.rs:663)

So validate_schema calls into the trait pipeline without ever checking keyword placement.

How it composes with this issue. The two together produce a silent-wrong-result path:

1. POST /entities with a derived schema that nests x-gts-traits inside an allOf entry → register() stores it (ops.rs:317), the placement gate fails (ops.rs:348) and returns 422 — but per this issue the rejected schema lingers in the store.
2. POST /validate-type-schema for that same id → validate_schema (no placement gate) → collect_traits_from_value recurses into allOf (schema_traits.rs:406-410) and silently merges the misplaced x-gts-traits instead of rejecting it (contradicting §9.7.1 fail-fast).

Implication for the fix here. Snapshot + rollback (the desired behaviour above) removes step 1's precondition, so the misplaced schema never persists and step 2 becomes practically unreachable. That closes the exploitable path. What remains afterwards is a softer, defense-in-depth alignment, which could be folded into this work or split off:

• Run the same placement/modifier gates in validate_schema that add_entity/validate_entity already run, so all three entry points behave identically regardless of how a schema entered the store.
• Optional cleanup: the allOf recursion in collect_trait_schema_from_value / collect_traits_from_value (schema_traits.rs:344-411) is vestigial post-v0.12 — under §9.7.1 these keywords are top-level-only, so the recursion can only ever pick up a spec-forbidden placement. Its comment ("e.g. a derived schema that is an allOf overlay") is now misleading.

Happy to split the validate_schema alignment into its own issue if it's out of scope for the atomicity fix.