diff --git a/crates/temper-runtime/src/persistence/mod.rs b/crates/temper-runtime/src/persistence/mod.rs index 80c236adc..0a164523d 100644 --- a/crates/temper-runtime/src/persistence/mod.rs +++ b/crates/temper-runtime/src/persistence/mod.rs @@ -92,6 +92,11 @@ pub struct EntityKeyRow { /// The declared key's identifier (the `[[key]]` block's `name`). pub key_name: String, /// The canonical, type-tagged hash of the key's values. + /// + /// An EMPTY hash is the RELEASE marker (ARN-238 / ADR-0172): the store drops the + /// entity's existing row for `key_name` and inserts nothing, so a + /// tombstoned entity or a fully-nulled key stops owning the value in the + /// same transaction as the journal append. pub key_hash: String, } diff --git a/crates/temper-server/src/entity_actor/actor.rs b/crates/temper-server/src/entity_actor/actor.rs index cbf46c9a7..2619c83a1 100644 --- a/crates/temper-server/src/entity_actor/actor.rs +++ b/crates/temper-server/src/entity_actor/actor.rs @@ -363,17 +363,32 @@ impl EntityActor { let reconcile_vectors = !table.vectors.is_empty(); let mut key_rows = Vec::new(); let mut vector_rows = Vec::new(); - if let Some(field_map) = state.fields.as_object() { - for key in &table.keys { - if let Some(hash) = + // ARN-238 / ADR-0172: a tombstoning write must RELEASE the entity's + // declared keys, not re-claim them from the still-populated fields — + // otherwise the dead entity holds the key values forever (keyed + // reads resolve to it, and new claimants are rejected as + // duplicates). The Delete arm persists before mutating status, so + // the event's to_status carries the tombstone signal. Release does + // not need the fields at all, so it is not gated on them. + let tombstoned = state.status == "Deleted" || event.to_status == "Deleted"; + for key in &table.keys { + let key_hash = if tombstoned { + None + } else { + state.fields.as_object().and_then(|field_map| { crate::key_index::canonical_key_hash(&key.name, &key.properties, field_map) - { - key_rows.push(temper_runtime::persistence::EntityKeyRow { - key_name: key.name.clone(), - key_hash: hash, - }); - } - } + }) + }; + // An empty hash is the RELEASE marker (see EntityKeyRow): + // the store drops the entity's row for this key_name and + // inserts nothing, so tombstoned or fully-nulled keys are + // purged in the same transaction as the journal append. + key_rows.push(temper_runtime::persistence::EntityKeyRow { + key_name: key.name.clone(), + key_hash: key_hash.unwrap_or_default(), + }); + } + if let Some(field_map) = state.fields.as_object() { // A soft-deleted (tombstone) entity is never indexed — it emits no // vector rows, so the reconcile below PURGES any it had, even though // its embedding field may still be present. Mirrors how the field-index diff --git a/crates/temper-server/src/state/projection_backfill.rs b/crates/temper-server/src/state/projection_backfill.rs index 72f511c83..e471227dd 100644 --- a/crates/temper-server/src/state/projection_backfill.rs +++ b/crates/temper-server/src/state/projection_backfill.rs @@ -41,9 +41,13 @@ pub(super) fn transition_table_for( pub(super) enum EntityLoadOutcome { /// Loaded — index it from these fields. Fields(serde_json::Value), - /// Definitively skippable: deleted, or a phantom with no events. Correctly NOT + /// Definitively skippable: a phantom with no events. Correctly NOT /// indexed, and NOT a failure (it must not block the watermark). Skip, + /// Tombstoned (status == "Deleted"). Not indexed — and the key backfill + /// RELEASES any rows the dead entity still holds (ARN-238 / ADR-0172 healing + /// pass for deletes that predate release-on-delete). + Tombstoned, /// The entity exists (it was enumerated from the durable store) but its current /// state could not be loaded — no transition table to replay with, an unreadable /// snapshot, or a replay error. Indexing it is impossible, so the type must NOT be @@ -82,7 +86,7 @@ pub(super) async fn load_entity_current_fields( .await { Err(_) => EntityLoadOutcome::LoadFailed, - Ok(state) if state.status == "Deleted" => EntityLoadOutcome::Skip, + Ok(state) if state.status == "Deleted" => EntityLoadOutcome::Tombstoned, Ok(state) if state.total_event_count == 0 => EntityLoadOutcome::Skip, Ok(state) => EntityLoadOutcome::Fields(state.fields), } diff --git a/crates/temper-server/src/state/projection_backfill/key_index.rs b/crates/temper-server/src/state/projection_backfill/key_index.rs index 22b47a405..4a5d77b0b 100644 --- a/crates/temper-server/src/state/projection_backfill/key_index.rs +++ b/crates/temper-server/src/state/projection_backfill/key_index.rs @@ -18,9 +18,12 @@ use super::{EntityLoadOutcome, load_entity_current_fields, transition_table_for} /// boot — the original bug that left ~0 of N entities keyed. /// /// Robustness at scale (tenants hold 10k–100k+ entities of a keyed type): -/// - **Resumable**: already-keyed entities are skipped (the costly step is loading -/// each entity's state), so a re-run after a partial pass only processes the -/// remainder instead of re-loading all N. +/// - **Resumable, with a healing exception (ARN-238 / ADR-0172)**: every entity is +/// loaded (pre-watermark only — the watermark still ends all re-runs), because an +/// already-keyed entity may hold STALE ownership: a tombstone, or a key whose +/// components were nulled before release-on-delete/null existed. Living, +/// fully-resolvable already-keyed entities skip the index write, so a resumed +/// pass re-loads but does not re-write the finished remainder. /// - **Sound**: a type is watermarked only if EVERY existing entity was either keyed /// or is definitively skippable (deleted/phantom). One entity that exists but /// cannot be loaded fails the type — it is not watermarked, and keyed misses keep @@ -105,15 +108,21 @@ pub(in crate::state) async fn populate_key_index_from_snapshots( // Resumability: on a FIRST-TIME backfill, skip entities already keyed (avoids // re-loading their state). On a key-set change we must re-key already-keyed // entities with the new key, so process all. - let already_keyed: BTreeSet = if force_full_rekey { - BTreeSet::new() + // `membership_known` tracks whether the empty/filled set is AUTHORITATIVE. + // On force_full_rekey and on a membership fetch error the set is empty by + // construction, which must mean "process everything" — including releasing + // stale rows of entities whose keys are all unresolvable — not "known to + // be unindexed" (ARN-238: skipping them here would let the watermark end + // their healing silently). + let (already_keyed, membership_known): (BTreeSet, bool) = if force_full_rekey { + (BTreeSet::new(), false) } else { match store .keyed_entity_ids_for_type(tenant.as_str(), entity_type) .await { - Ok(ids) => ids.into_iter().collect(), - Err(_) => BTreeSet::new(), // cannot resume → process all (correct, slower) + Ok(ids) => (ids.into_iter().collect(), true), + Err(_) => (BTreeSet::new(), false), // cannot resume → process all } }; @@ -121,15 +130,18 @@ pub(in crate::state) async fn populate_key_index_from_snapshots( let blob_store = state.blob_store_for_tenant(tenant).ok(); let total = entity_ids.len(); let mut newly_keyed = 0usize; + let mut healed = 0usize; let mut already = 0usize; let mut skipped = 0usize; let mut failed = 0usize; for entity_id in &entity_ids { - if already_keyed.contains(entity_id) { - already += 1; - continue; - } + // ARN-238: an already-keyed entity cannot be fast-skipped without + // loading it — its rows may be STALE ownership from a delete that + // predates release-on-delete, and healing requires seeing the + // tombstone. Living already-keyed entities still skip the upsert + // below; only the cheap membership shortcut moved. + let was_already_keyed = already_keyed.contains(entity_id); match load_entity_current_fields( tenant, entity_type, @@ -146,22 +158,49 @@ pub(in crate::state) async fn populate_key_index_from_snapshots( skipped += 1; continue; }; - let mut key_rows = Vec::new(); - for key in keys { - if let Some(hash) = crate::key_index::canonical_key_hash( - &key.name, - &key.properties, - field_map, - ) { - key_rows.push(temper_runtime::persistence::EntityKeyRow { - key_name: key.name.clone(), - key_hash: hash, - }); - } + // One row per declared key: a real hash when the key resolves, + // a RELEASE marker when it does not (ARN-238 — a key whose + // components were nulled must stop owning its old value; the + // stale row is exactly why the entity looks already-keyed). + let mut any_release = false; + let mut any_hash = false; + let key_rows: Vec = keys + .iter() + .map(|key| { + let hash = crate::key_index::canonical_key_hash( + &key.name, + &key.properties, + field_map, + ); + match hash { + Some(hash) => { + any_hash = true; + temper_runtime::persistence::EntityKeyRow { + key_name: key.name.clone(), + key_hash: hash, + } + } + None => { + any_release = true; + temper_runtime::persistence::EntityKeyRow { + key_name: key.name.clone(), + key_hash: String::new(), + } + } + } + }) + .collect(); + if was_already_keyed && !any_release { + // Alive, fully resolvable, already indexed — nothing to write. + already += 1; + continue; } - if key_rows.is_empty() { - // No resolvable key (all key components absent/null) — the - // entity is not addressable by this key, so skipping is sound. + if !any_hash && !was_already_keyed && membership_known { + // Authoritatively not indexed and nothing resolvable — the + // entity is not addressable by any declared key. Without + // membership authority we fall through and write the + // release markers (bounded no-op deletes), because the + // entity may hold stale rows we cannot see from here. skipped += 1; continue; } @@ -169,7 +208,12 @@ pub(in crate::state) async fn populate_key_index_from_snapshots( .backfill_entity_keys(tenant.as_str(), entity_type, entity_id, &key_rows) .await { - Ok(()) => newly_keyed += 1, + // A write with at least one real hash is a claim; a + // release-only write is a heal — kept separate so the + // completion log's newly_keyed stays an honest claim count. + Ok(()) if any_hash && !was_already_keyed => newly_keyed += 1, + Ok(()) if any_hash => already += 1, + Ok(()) => healed += 1, Err(e) => { failed += 1; tracing::warn!( @@ -180,6 +224,36 @@ pub(in crate::state) async fn populate_key_index_from_snapshots( } } EntityLoadOutcome::Skip => skipped += 1, + EntityLoadOutcome::Tombstoned => { + // ARN-238 healing pass: a deleted entity must not keep its + // declared keys. Emit a release marker per declared key so + // rows written before release-on-delete are purged. + let release_rows: Vec = keys + .iter() + .map(|key| temper_runtime::persistence::EntityKeyRow { + key_name: key.name.clone(), + key_hash: String::new(), + }) + .collect(); + match store + .backfill_entity_keys( + tenant.as_str(), + entity_type, + entity_id, + &release_rows, + ) + .await + { + Ok(()) => healed += 1, + Err(e) => { + failed += 1; + tracing::warn!( + error = %e, entity_type = %entity_type, entity_id = %entity_id, + "key index backfill: tombstone release failed" + ); + } + } + } EntityLoadOutcome::LoadFailed => { failed += 1; tracing::warn!( @@ -200,13 +274,13 @@ pub(in crate::state) async fn populate_key_index_from_snapshots( .await; tracing::info!( tenant = %tenant, entity_type = %entity_type, key_set = %current_key_set, - total, newly_keyed, already, skipped, + total, newly_keyed, healed, already, skipped, "entity_key_index backfill complete; type watermarked" ); } else { tracing::warn!( tenant = %tenant, entity_type = %entity_type, - total, newly_keyed, already, skipped, failed, + total, newly_keyed, healed, already, skipped, failed, "key index backfill: {failed} entities unresolved; type NOT watermarked (keyed misses keep scanning; will resume next run)" ); } diff --git a/crates/temper-server/src/state/projection_backfill/vector_index.rs b/crates/temper-server/src/state/projection_backfill/vector_index.rs index 44d0de6fe..e0238d508 100644 --- a/crates/temper-server/src/state/projection_backfill/vector_index.rs +++ b/crates/temper-server/src/state/projection_backfill/vector_index.rs @@ -179,7 +179,7 @@ pub(in crate::state) async fn populate_vector_index_from_snapshots( } } } - EntityLoadOutcome::Skip => { + EntityLoadOutcome::Skip | EntityLoadOutcome::Tombstoned => { // A deleted (or phantom) entity must hold no vector rows — purge // any it still has so a soft-deleted entity is never ranked // (reconcile with an empty row set). Harmless when there is nothing diff --git a/crates/temper-server/tests/dst_entity_key_index.rs b/crates/temper-server/tests/dst_entity_key_index.rs index 07dc0a66c..2e9787c1e 100644 --- a/crates/temper-server/tests/dst_entity_key_index.rs +++ b/crates/temper-server/tests/dst_entity_key_index.rs @@ -240,3 +240,274 @@ async fn dst_co_commit_atomic_on_uniqueness_reject() { ); } } + +/// ARN-238: deleting an entity must RELEASE its declared-key ownership. +/// +/// The Deleted event's co-commit recomputes key rows from the entity's fields +/// (which still hold the key values), so the tombstoned entity keeps its +/// `entity_key_index` rows: keyed reads resolve to a dead entity, and — the +/// durable damage — any NEW entity claiming the same key value is rejected +/// with a uniqueness violation forever. +#[tokio::test] +async fn dst_deleted_entity_releases_declared_key() { + for seed in 0..NUM_SEEDS { + let (_guard, _clock, _id) = install_deterministic_context(seed); + let store: BoxedEventStore = BoxedEventStore::new(SimEventStore::no_faults(seed)); + let table = doc_table(); + let system = ActorSystem::new("dst-keyed-delete"); + + // Doc A claims the key, then is deleted. + let id_a = format!("doc-a-{seed}"); + let actor_a = EntityActor::with_persistence( + "Doc", + &id_a, + table.clone(), + serde_json::json!({}), + store.clone(), + BackendLabel::Sim, + ) + .with_tenant("default"); + let ref_a = system.spawn(actor_a, &id_a); + let r = dispatch( + &ref_a, + "Create", + serde_json::json!({ "WorkspaceId": "ws1", "Path": "/a.md" }), + ) + .await; + assert!(r.success, "seed {seed}: Create A failed: {:?}", r.error); + + let deleted: EntityResponse = ref_a + .ask(EntityMsg::Delete, Duration::from_secs(5)) + .await + .expect("delete responds"); + assert!( + deleted.success, + "seed {seed}: Delete failed: {:?}", + deleted.error + ); + + // 1) The dead entity must no longer own the key. + let owner = store + .lookup_by_key("default", "Doc", "path", &doc_key_hash("ws1", "/a.md")) + .await + .expect("lookup ok"); + assert_eq!( + owner, None, + "seed {seed}: a deleted entity must release its declared key, got {owner:?}" + ); + + // 2) A new entity must be able to claim the released key value. + let id_b = format!("doc-b-{seed}"); + let actor_b = EntityActor::with_persistence( + "Doc", + &id_b, + table.clone(), + serde_json::json!({}), + store.clone(), + BackendLabel::Sim, + ) + .with_tenant("default"); + let ref_b = system.spawn(actor_b, &id_b); + let r = dispatch( + &ref_b, + "Create", + serde_json::json!({ "WorkspaceId": "ws1", "Path": "/a.md" }), + ) + .await; + assert!( + r.success, + "seed {seed}: a new entity must be able to claim a key released by deletion, got: {:?}", + r.error + ); + + // 3) And the key resolves to the new owner. + let owner = store + .lookup_by_key("default", "Doc", "path", &doc_key_hash("ws1", "/a.md")) + .await + .expect("lookup ok"); + assert_eq!( + owner, + Some(id_b.clone()), + "seed {seed}: the key must resolve to its new living owner" + ); + } +} + +/// ARN-238 healing pass: deletes that happened BEFORE release-on-delete left +/// stale `entity_key_index` rows behind. The key backfill must release a +/// tombstoned entity's rows instead of skipping them, so legacy stale +/// ownership heals on the next boot backfill. +#[tokio::test] +async fn dst_key_backfill_releases_stale_rows_of_deleted_entities() { + use temper_server::registry::SpecRegistry; + use temper_server::state::ServerState; + use temper_spec::csdl::parse_csdl; + + const CSDL_XML: &str = include_str!("../../../test-fixtures/specs/model.csdl.xml"); + + for seed in [7u64, 42u64] { + let (_guard, _clock, _id) = install_deterministic_context(seed); + let sim = SimEventStore::no_faults(seed); + let store: BoxedEventStore = BoxedEventStore::new(sim.clone()); + let entity_id = format!("legacy-doc-{seed}"); + let pid = format!("default:Doc:{entity_id}"); + + // Simulate the LEGACY sequence directly against the store (bypassing + // the fixed actor): a Create that claims the key, then a Deleted + // event whose co-commit re-claims it (the pre-fix behavior). + let stale_key = EntityKeyRow { + key_name: "path".to_string(), + key_hash: doc_key_hash("ws1", "/legacy.md"), + }; + let mk_env = |seq: u64, event_type: &str, payload: serde_json::Value| PersistenceEnvelope { + sequence_nr: seq, + event_type: event_type.to_string(), + payload, + metadata: EventMetadata { + event_id: sim_uuid(), + causation_id: sim_uuid(), + correlation_id: sim_uuid(), + timestamp: sim_now(), + actor_id: pid.clone(), + }, + }; + store + .append_with_keys( + &pid, + 0, + &[mk_env( + 1, + "Create", + serde_json::json!({ + "action": "Create", "from_status": "New", "to_status": "Ready", + "timestamp": sim_now(), "params": {"WorkspaceId": "ws1", "Path": "/legacy.md"} + }), + )], + std::slice::from_ref(&stale_key), + ) + .await + .expect("legacy create"); + store + .append_with_keys( + &pid, + 1, + &[mk_env( + 2, + "Deleted", + serde_json::json!({ + "action": "Deleted", "from_status": "Ready", "to_status": "Deleted", + "timestamp": sim_now(), "params": {} + }), + )], + std::slice::from_ref(&stale_key), // pre-fix: tombstone re-claims + ) + .await + .expect("legacy delete with stale re-claim"); + + // The stale row exists (the legacy bug's footprint). + let stale = store + .lookup_by_key("default", "Doc", "path", &stale_key.key_hash) + .await + .expect("lookup ok"); + assert_eq!( + stale, + Some(entity_id.clone()), + "seed {seed}: stale row seeded" + ); + + // Boot-style backfill over a ServerState sharing this store. + let csdl = parse_csdl(CSDL_XML).expect("CSDL parses"); + let mut registry = SpecRegistry::new(); + registry.register_tenant("default", csdl, CSDL_XML.to_string(), &[("Doc", DOC_IOA)]); + let mut server = ServerState::from_registry(ActorSystem::new("dst-key-heal"), registry); + server.set_storage_stack(temper_server::storage::StorageStack::from_sim(sim, None)); + let tenant = temper_runtime::tenant::TenantId::from("default".to_string()); + server.populate_index_from_store(&tenant).await; + server.populate_key_index_from_snapshots(&tenant).await; + + // The healing pass must have released the dead entity's key. + let healed = store + .lookup_by_key("default", "Doc", "path", &stale_key.key_hash) + .await + .expect("lookup ok"); + assert_eq!( + healed, None, + "seed {seed}: the key backfill must release stale rows held by deleted entities" + ); + } +} + +/// ARN-238 healing pass, nulled-key leg: a LIVING entity whose stale row +/// predates release-on-null (its declared key no longer resolves from its +/// current fields) must have that row released by the backfill. +#[tokio::test] +async fn dst_key_backfill_releases_stale_rows_of_living_unresolvable_keys() { + use temper_server::registry::SpecRegistry; + use temper_server::state::ServerState; + use temper_spec::csdl::parse_csdl; + + const CSDL_XML: &str = include_str!("../../../test-fixtures/specs/model.csdl.xml"); + + let seed = 11u64; + let (_guard, _clock, _id) = install_deterministic_context(seed); + let sim = SimEventStore::no_faults(seed); + let store: BoxedEventStore = BoxedEventStore::new(sim.clone()); + let entity_id = "legacy-null-doc".to_string(); + let pid = format!("default:Doc:{entity_id}"); + + // Legacy footprint: a Create whose event params DO NOT carry the key + // components (so the replayed current state cannot resolve the key), + // while a stale key row was still committed — the pre-fix behavior for + // a key later nulled. + let stale_key = EntityKeyRow { + key_name: "path".to_string(), + key_hash: doc_key_hash("ws1", "/nulled.md"), + }; + store + .append_with_keys( + &pid, + 0, + &[PersistenceEnvelope { + sequence_nr: 1, + event_type: "Create".to_string(), + payload: serde_json::json!({ + "action": "Create", "from_status": "New", "to_status": "Ready", + "timestamp": sim_now(), "params": {} + }), + metadata: EventMetadata { + event_id: sim_uuid(), + causation_id: sim_uuid(), + correlation_id: sim_uuid(), + timestamp: sim_now(), + actor_id: pid.clone(), + }, + }], + std::slice::from_ref(&stale_key), + ) + .await + .expect("legacy create with stale key row"); + + let stale = store + .lookup_by_key("default", "Doc", "path", &stale_key.key_hash) + .await + .expect("lookup ok"); + assert_eq!(stale, Some(entity_id.clone()), "stale row seeded"); + + let csdl = parse_csdl(CSDL_XML).expect("CSDL parses"); + let mut registry = SpecRegistry::new(); + registry.register_tenant("default", csdl, CSDL_XML.to_string(), &[("Doc", DOC_IOA)]); + let mut server = ServerState::from_registry(ActorSystem::new("dst-key-heal-null"), registry); + server.set_storage_stack(temper_server::storage::StorageStack::from_sim(sim, None)); + let tenant = temper_runtime::tenant::TenantId::from("default".to_string()); + server.populate_index_from_store(&tenant).await; + server.populate_key_index_from_snapshots(&tenant).await; + + let healed = store + .lookup_by_key("default", "Doc", "path", &stale_key.key_hash) + .await + .expect("lookup ok"); + assert_eq!( + healed, None, + "the key backfill must release a living entity's stale row when its key no longer resolves" + ); +} diff --git a/crates/temper-store-postgres/src/store.rs b/crates/temper-store-postgres/src/store.rs index 21aa5c717..3ec451557 100644 --- a/crates/temper-store-postgres/src/store.rs +++ b/crates/temper-store-postgres/src/store.rs @@ -143,6 +143,10 @@ impl EventStore for PostgresEventStore { // a reject is atomic (the journal does not advance). A different entity // already holding the key is the violation (reject + surface). for key in key_rows { + // Release markers (empty hash, ARN-238 / ADR-0172) claim nothing. + if key.key_hash.is_empty() { + continue; + } let holder: Option<(String,)> = crate::dbm::postgres_query_as!( "SELECT entity_id FROM entity_key_index \ WHERE tenant = $1 AND entity_type = $2 AND key_name = $3 AND key_hash = $4", @@ -229,6 +233,10 @@ impl EventStore for PostgresEventStore { .execute(&mut *tx) .await .map_err(|e| PersistenceError::Storage(e.to_string()))?; + // A release marker (ARN-238 / ADR-0172) only drops the prior row. + if key.key_hash.is_empty() { + continue; + } crate::dbm::postgres_query!( "INSERT INTO entity_key_index \ (tenant, entity_type, key_name, key_hash, entity_id, sequence_nr) \ @@ -316,6 +324,21 @@ impl EventStore for PostgresEventStore { .await .map_err(|e| PersistenceError::Storage(e.to_string()))?; for key in key_rows { + // A release marker (empty hash, ARN-238 / ADR-0172) only drops the entity's row. + if key.key_hash.is_empty() { + crate::dbm::postgres_query!( + "DELETE FROM entity_key_index \ + WHERE tenant = $1 AND entity_type = $2 AND key_name = $3 AND entity_id = $4", + ) + .bind(tenant) + .bind(entity_type) + .bind(&key.key_name) + .bind(entity_id) + .execute(&mut *tx) + .await + .map_err(|e| PersistenceError::Storage(e.to_string()))?; + continue; + } // A different entity already holding this key is a pre-existing data // conflict — log and skip (don't fail the whole backfill on one row; // the conflict surfaces via the metric and a keyed read still resolves diff --git a/crates/temper-store-sim/src/lib.rs b/crates/temper-store-sim/src/lib.rs index d52a96643..99aca3e2f 100644 --- a/crates/temper-store-sim/src/lib.rs +++ b/crates/temper-store-sim/src/lib.rs @@ -459,6 +459,10 @@ impl EventStore for SimEventStore { let entity_type = parts.next().unwrap_or(""); let entity_id = parts.next().unwrap_or(""); for row in key_rows { + // Release markers (empty hash, ARN-238 / ADR-0172) claim nothing. + if row.key_hash.is_empty() { + continue; + } if let Some(existing) = inner.key_index.get(&( tenant.to_string(), entity_type.to_string(), @@ -542,6 +546,10 @@ impl EventStore for SimEventStore { && kn.as_str() == row.key_name.as_str() && eid.as_str() == entity_id) }); + // A release marker (ARN-238 / ADR-0172) only drops the prior row. + if row.key_hash.is_empty() { + continue; + } inner.key_index.insert( ( tenant.to_string(), @@ -600,6 +608,16 @@ impl EventStore for SimEventStore { row.key_name.clone(), row.key_hash.clone(), ); + // A release marker (empty hash, ARN-238 / ADR-0172) only drops the prior row. + if row.key_hash.is_empty() { + inner.key_index.retain(|(t, et, kn, _), eid| { + !(t.as_str() == tenant + && et.as_str() == entity_type + && kn.as_str() == row.key_name.as_str() + && eid.as_str() == entity_id) + }); + continue; + } match inner.key_index.get(&slot) { // A different entity holds it — pre-existing conflict; skip (don't // clobber, don't fail the backfill). diff --git a/docs/adrs/0172-declared-key-release.md b/docs/adrs/0172-declared-key-release.md new file mode 100644 index 000000000..b32a3aca9 --- /dev/null +++ b/docs/adrs/0172-declared-key-release.md @@ -0,0 +1,122 @@ +# ADR-0172: Declared-Key Release on Delete and Null + +- Status: Accepted +- Date: 2026-07-14 +- Deciders: Temper core maintainers +- Related: + - ADR-0153: Declared composite key index + - ADR-0155: Declared vector access path + - `crates/temper-server/src/entity_actor/actor.rs` + - `crates/temper-server/src/state/projection_backfill/key_index.rs` + - `crates/temper-store-postgres/src/store.rs` + - `crates/temper-store-sim/src/lib.rs` + - Linear: ARN-238 + +## Context + +The ADR-0153 `entity_key_index` co-commit derives an entity's key rows from +its post-transition fields on every journal append. A tombstoning write +(`Deleted`) still has the key values in its fields, so the co-commit +re-claimed them: the dead entity kept owning its declared keys (ARN-238). +Keyed reads resolved to a tombstone, and — the durable damage — any new +entity claiming the same key value was rejected with a uniqueness violation +forever, because the uniqueness pre-check found the dead holder. + +The same failure mode applies when every component of a declared key becomes +null or absent: the actor emits no row for that key, and the store only +deletes prior rows for key names that appear in the emitted set — so the +old hash stays owned by an entity that can no longer resolve it. + +Vectors already handled tombstones (`index_vectors = status != "Deleted"` +plus delete-then-insert reconcile). Keys had no equivalent release path. + +## Decision + +### 1. Release markers + +`EntityKeyRow.key_hash == ""` means RELEASE: the store drops the entity's +existing row for that `key_name` and inserts nothing, in the same +transaction as the journal append. + +`persist_event` emits one row per declared key on every keyed write: + +- a real hash when the entity is living and the key resolves; +- a release marker when the write tombstones the entity + (`state.status == "Deleted"` or `event.to_status == "Deleted"` — the + Delete arm persists before mutating status) or when the key's components + are all null/absent. + +### 2. Stores skip markers on claim paths + +The uniqueness pre-check and the insert skip empty hashes; the per-key-name +delete always runs. Applied to the postgres and sim stores (Turso does not +maintain the key index live; its reads route through the same store impls +where the index is active). + +### 3. Backfill heals legacy stale rows + +Tombstoned entities emit release markers for every declared key, and living +entities emit a release marker for any declared key that no longer resolves +(nulled components) alongside real hashes for keys that do — computed +before the already-keyed resume shortcut, since a stale row is exactly what +makes such an entity look already-keyed. + +`EntityLoadOutcome` gains a `Tombstoned` variant for the delete leg. The +already-keyed resume shortcut loads the entity before deciding +(pre-watermark only); living, fully-resolvable already-keyed entities still +skip the write. + +## Consequences + +### Positive + +- Deleting an entity frees its declared key values for reuse immediately and + atomically with the tombstone event. +- Fully nulled keys stop owning their prior hash on the same write path. +- Legacy stale rows heal on the next boot backfill for types that are not + yet watermarked. + +### Negative / residual + +- Types already watermarked do not re-run the backfill, so their pre-existing + stale rows persist until a manual re-backfill. Live release prevents any + new staleness after this change. +- The backfill's resume path loads already-keyed entities once per run + (pre-watermark only) instead of fast-skipping on membership; the watermark + still prevents any post-completion cost. +- **Composite-write residual:** composite sub-writes append through + `append_batch`, whose `PersistenceAppend` carries no key rows — composites + never claimed declared keys, and after this change they do not release + them either. An actor-keyed entity tombstoned via a composite therefore + still leaves a stale row (healable by the backfill pre-watermark). This is + a pre-existing ADR-0153 gap on the composite path, tracked as a follow-up. + +### DST Compliance + +- No new wall-clock, OS random, or non-deterministic collections on the + write path. Sim and actor paths continue to use `BTreeMap`/`BTreeSet` and + simulated time. Empty-hash release markers are pure data. + +## Non-Goals + +- Re-running backfill for already-watermarked types on every boot. +- Extending composite `append_batch` to carry key rows (separate follow-up). +- Filtering tombstones only at read time (would leave uniqueness rejects). + +## Alternatives Considered + +1. **`reconcile_keys` flag mirroring `reconcile_vectors`** — delete ALL of + the entity's key rows per write, then insert emitted rows. Changes the + `EventStore` trait signature across every implementor for the same + outcome the release marker expresses per key, and loses per-key + granularity on partial null of one of several declared keys. +2. **Filtering tombstones at read time** (`lookup_by_key` joining status) — + leaves the uniqueness pre-check rejecting new claimants (the durable + damage) and spreads tombstone knowledge into every reader instead of + fixing ownership at the write. + +## Rollback Policy + +Revert the actor emission, store release handling, and backfill heal. Any +releases already applied leave the index correct (keys free); a rollback +only reintroduces the stale-ownership bug for subsequent deletes/nulls.