Code Review: Leave & Stop-Forward (m9-leave-revocation)

Summary

Implementation is complete, correct, and production-ready. All 8 integration tests pass, 1299 lib tests pass, cargo clippy -- -D warnings is clean, cargo fmt --check is clean. No blockers.

Files Changed

File	Change
`tidal/src/storage/keys.rs`	Added `Tag::CommunityLeave = 0x11`; updated from_byte, tests, proptest range
`tidal/src/entities/community.rs`	Added `MembershipStatus`, `CommunityMembership`, `serialize_membership`, `deserialize_membership`, 6 unit tests
`tidal/src/entities/mod.rs`	Re-exported new types
`tidal/src/db/mod.rs`	Added `community_leave_status: DashMap<u64, CommunityMembership>` field; initialized in both constructors
`tidal/src/db/community.rs`	Added `leave_community_layer`, `rejoin_community_layer`, `community_layer_status`, `persist_leave_record`
`tidal/src/db/signals.rs`	Added leave gate to `try_cohort_attribution` AND `try_community_forwarding`
`tidal/src/db/state_rebuild.rs`	Added `rebuild_community_leave_status`; called from `from_parts`
`tidal/tests/m9_leave_revocation.rs`	8 integration tests (TC-01 through TC-08)

Correctness

Gate placement

The gate is placed at the top of BOTH try_cohort_attribution (cohort predicate fan-out) and try_community_forwarding (community:: key fan-out). This is correct — both paths must be suppressed when a user has left. Before this change, leaving was silent on the forwarding path.

// In try_cohort_attribution AND try_community_forwarding:
if self
    .community_leave_status
    .get(&user_id)
    .map_or(false, |m| m.status == crate::entities::MembershipStatus::Left)
{
    return;
}

TOCTOU window

The spec acknowledges a TOCTOU window: a concurrent signal_with_context call that passes the gate before leave_community_layer completes its DashMap insert may still fan out one signal. This is acceptable — the retroactive-purge path handles historical contributions. The window is bounded to one signal write.

Storage key correctness

Tag::CommunityLeave = 0x11 is a new allocation. The existing Tag::CommunityMembership = 0x0E was already in use by m9-community-profile-sync with a community-name suffix — that tag cannot be reused without suffix (would be a key collision). Using 0x11 correctly avoids the collision.

Startup restore

rebuild_community_leave_status scans all users_engine entries and matches on Tag::CommunityLeave. It correctly uses parse_key to decompose the key and extract the entity_id, then deserializes the 9-byte value. Malformed records are silently skipped with a debug log. The function is called after rebuild_community_memberships in from_parts, preserving existing rebuild order.

Serde correctness

Fixed 9-byte format: [status: 1 byte][left_at_ns: 8 bytes LE]. Zero in the timestamp bytes is the unambiguous encoding for None (a real timestamp of 0 ns would be Unix epoch, which is not a valid leave time in practice). The deserialize_membership correctly checks ts_raw == 0 to distinguish None from a real value.

Idempotency

leave_community_layer called twice: second call writes a new timestamp to both storage and DashMap. The spec states this is intentional — allows a fresh boundary for the retroactive purge. The left_at_ns >= previous property is tested in TC-03.

Rejoin history preservation

rejoin_community_layer reads the current left_at_ns from the DashMap before creating the Active record, so the leave timestamp survives. Tested in TC-05.

Performance

Gate check: DashMap::get(&user_id) — O(1) per-shard lock, zero storage I/O. Users with no leave record (the vast majority) take the fast map_or default path without any DashMap shard contention beyond a key miss.
leave_community_layer / rejoin_community_layer: one storage put + one DashMap insert. Both are off the hot signal write path.
rebuild_community_leave_status: full scan_prefix(&[]) on startup — O(N_users_keys). This is the same approach as rebuild_community_memberships and all other startup rebuild functions. Acceptable for the expected user count.

Test Coverage

Test	Coverage
`leave_stops_cohort_fanout`	Gate works; base signal ledger unaffected
`join_resumes_cohort_fanout`	Gate cleared on rejoin
`leave_is_idempotent`	Double-leave returns Ok; timestamp monotone
`status_query_lifecycle`	Full Active→Left→Active lifecycle
`left_at_ns_preserved_on_rejoin`	History intact after rejoin
`durability_across_reopen`	Tag::CommunityLeave persisted and restored
`unknown_user_defaults_to_active`	Default Active for absent users
`leave_stops_community_forwarding`	`community::` key fan-out gated

All 8 pass. The m6_cohort regression suite (9 tests) passes — no existing cohort behavior broken.

Issues

None. Implementation matches spec and design exactly.

Verdict

Approved. Ship as-is.

4.9 KiB Raw Blame History