feat/room-retention #174
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/room-retention"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Commit 1 of per-room message retention. Adds three chat migrations and updates every test file that hand-rolls the chat migration list. No runtime code yet: the retention sweep lands in a later commit, gated behind LETS_CHAT_RETENTION_SWEEP_ENABLED (default off), because the strict-vs-loose retention semantics question is still pending with the ticket author. 0043 adds a nullable rooms.retention_days column with CHECK retention_days IS NULL OR retention_days >= 1 (rejects 0 = delete-everything). A partial index covers only retention-enabled rooms so the global sweep query stays cheap. 0044 rebuilds link_filter_quarantine with ON DELETE CASCADE on message_id. The original schema in 0032 left the FK without an action because the codebase only soft-deleted messages; retention is the first hard-delete path, and without this rebuild a sweep touching a quarantined row would fail with SQLITE_CONSTRAINT_FOREIGNKEY. SQLite cannot ALTER an existing FK action, hence the create-new + copy + drop + rename rebuild. 0045 adds an AFTER DELETE trigger on messages that issues the FTS5 'delete' command on messages_fts. Existing triggers in 0008 fire only on UPDATE OF deleted_at and UPDATE OF body; no trigger fired on a real DELETE because no hard-delete path existed. Without this trigger, retention-swept rows would leave orphan FTS entries that still match search queries against deleted content. Test-file drift sweep: 18 of the 78 integration binaries hand-roll a chat migration list via include_str!. All three new migrations are appended to those lists. Exception: migration_enclaves.rs skips 0032, so the 0044 quarantine rebuild is skipped there too. Files using sqlx::migrate!("./migrations/chat") (db_dm.rs, db_moderation.rs, message_editing.rs) auto-pick up new migrations. just check and just test pass in both standalone and saas modes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Commit 4 of per-room message retention. Adds the spawn function, the new ChatEvent variant for retention-driven hard-deletes, and the OOB fragment that removes the message node from connected clients' DOM. The sweep logic from commit 3 stays unchanged; this commit only wires its destructive path into the running server and the WS layer. ChatEvent::MessagePurged { message_id, room_id }: distinct from MessageDeleted (which is the moderation soft-delete tombstone). PurgedMessageFragment renders `<div id="msg-{id}" hx-swap-oob="delete"></div>`: no "[deleted]" placeholder, because a visible tombstone leaks the metadata that a message existed there and defeats the compliance posture retention is for. The render arm slots into ws_fragments::render_event next to MessageDeleted; the catchall in routes/ws.rs::dispatch routes new variants through render_event automatically so no per-recipient match update is needed. SweepStats grows a `purged: Vec<(i64, i64)>` field that carries the directly-deleted (message_id, room_id) pairs in deletion order. The spawn function iterates this list AFTER the transaction commits, outside the BEGIN IMMEDIATE critical section, so channel writes never block the writer lock (same shape principle the dry-run log followed: only essential statements live between SELECT and DELETE). Cascade-deleted descendants (thread replies, etc.) intentionally do NOT appear in purged: the client renders threads nested, so removing the parent's DOM node also removes its rendered children, and the cascade fragments would be redundant. spawn_message_retention_sweeper in main.rs: 1-hour tick, skip-first-tick, mirrors spawn_orphan_sweeper's shape (sibling slow-clock sweep). The env flag (LETS_CHAT_RETENTION_SWEEP_ENABLED) is checked at spawn time, not per-tick: if unset, the task is never spawned at all. Flipping the flag requires a server restart, which is the right shape for a destructive feature gate the operator is opting into deliberately. The run_retention_sweep wrapper from commit 3 stays in the API surface for callers that want the flag check inline (a future admin "purge now" handler). Broadcast skip-for-old-messages optimization (the brainstorm raised it as a thundering-herd guard for the first-run cliff): consciously deferred. SWEEP_LIMIT is 500 per hour and the fragment is ~50 bytes; per-room broadcast already targets only currently-subscribed clients via the hub's channel routing. 500 * 50B/hr from this source is negligible by chat-server standards. Revisit if metrics surface pressure on the hub, but inline-it-now would be optimizing without data. Documented in the spawn function's docstring. Tests: server/tests/retention_sweep.rs grows by 1 test and amends another: - message_past_cutoff_is_deleted now asserts stats.purged == vec![(m, room)] in addition to the count, pinning the broadcast contract. - purged_field_groups_messages_by_room_for_broadcast: 3 messages across 2 rooms + a stale thread reply that cascade-deletes. Asserts purged carries exactly the 3 directly-deleted message ids with their room ids, and the cascade-deleted reply is NOT in purged (the parent's DOM removal covers the reply visually when the client renders threads nested). 19 sweep tests + 16 cascade tests, all pass in standalone and saas. just check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Commit 5 of per-room message retention; PR-closing commit. Adds the admin-facing surface that lets an admin or enclave-owner set rooms.retention_days. Lives on the existing /room/{id}/moderators page (same gate, same audit log target, sits next to posting policy). Two new routes: - GET /room/{id}/retention/preview?days=N -> HTMX confirmation fragment showing the count of messages the next sweep would delete at that setting, plus the permanence warnings, plus an embedded POST form. The count is computed by retention::sweep::count_candidates_for_room, the same function the sweep itself uses via the shared candidate_predicate; the preview_count_equals_sweep_actual_delete test from commit 3 enforces the SQL-level invariant. Empty days renders a "disable retention" fragment with the irreversibility notice instead. - POST /room/{id}/retention -> validates (1-day floor, integer, empty = disable; 0 / negative / non-numeric all 400), rejects DM rooms (defense-in-depth on top of the sweep predicate's room_type != 'dm' filter), writes rooms.retention_days, audits to mod_actions with target_user='-' / action='retention_set' / metadata={"old_days":..,"new_days":..}, and redirects back to the moderators page. Both routes gate on require_can_manage (now pub(crate) so sibling route modules can share it; same enclave-owner / site-admin set that posting policy uses). Permanence warning copy is accurate to the loose-correct sweep that currently ships: "messages older than N days are deleted, except messages in threads with replies newer than N days (active threads are preserved as a unit)." Does not promise strict behavior the sweep does not currently do. Also surfaces the no-pinned-exemption rule and points users at the room wiki as the escape hatch for important content. Honest update to retention::sweep::SweepStats::purged docstring: thread replies in the codebase's UI render as flat siblings inside #thread-replies-{parent_id} in the thread side panel, NOT as DOM children of the root message bubble. hx-swap-oob="delete" against the root's id removes the root's node but does not touch the sibling reply nodes that may still be visible in any open thread panel. The replies are gone server-side; the stale DOM resolves on reload. Emitting MessagePurged for cascaded reply ids would require a recursive descendant SELECT before the DELETE; deferred to a follow-up if the cosmetic gap surfaces as a real complaint. New db helpers in db::chat: get_room_retention_days, set_room_retention_days. Don't extend the Room struct (which is wide and read everywhere) since these are only needed by retention-specific code paths. CLAUDE.md env-var table grows a row for LETS_CHAT_RETENTION_SWEEP_ENABLED that captures the default-off rationale (strict-vs-loose semantics pending with the ticket author) and the restart-required note. 10 new integration tests in routes_retention.rs: - admin_can_set_and_disable_retention_with_audit (happy path round-trip + audit row shape for both enable and disable) - post_days_zero / negative / non_numeric_days each 400 (1-day floor in three error shapes) - non_admin_member_cannot_set_retention (403, no state mutation) - dm_room_rejects_retention_post_with_400 (defense-in-depth on top of the sweep predicate) - preview_fragment_includes_count_and_warnings (count number AND permanence AND no-pinned-exemption AND loose-correct thread language all in fragment) - preview_with_days_zero_is_rejected_with_400 - preview_with_empty_days_renders_disable_fragment (disable UX path) - preview_on_dm_room_rejects_with_400 just check + just test (standalone, modulo the documented routes_uploads concurrent-load flake) + just test-saas all green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>