feat/lc-64-message-drafts #175
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/lc-64-message-drafts"
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 LC-64 server-persisted drafts. Mechanical opener: new migration, new db helper module, test-file include_str! drift sweep. No runtime behavior change yet; the PUT handler, render-on-load, and cascade hooks land in later commits. Migration 0047_message_drafts.sql adds a (user_id, room_id, body, updated_at) table keyed by (user_id, room_id) so a user has at most one draft per room. room_id is ON DELETE CASCADE so room deletion cleans up drafts via the FK with no explicit code in delete_room. No user_id FK because auth users live in a separate pool (chat.db convention); the account-delete path purges per-user transactionally instead. A secondary index on (user_id) covers the WHERE user_id = ? shape that purge_user_chat uses; the (user_id, room_id) PK already covers the room-load lookup. The schema is single-table for both regular rooms and DMs because DMs are room_type='dm' rows in the same rooms table (see 0003_dms.sql). The ticket proposed a separate dm_drafts table; that would duplicate the cascade rules and the cleanup paths for no benefit. Same finding as the LC-XXX retention work, applied here. server/src/db/drafts.rs exposes get / upsert / delete / delete_for_user (Executor-taking for transactional composition) / delete_for_user_in_room. The DraftRow type carries body + updated_at; the render site does its own staleness check against datetime('now', '-60 days') so the "render-empty" and "delete-the-row" actions are paired in the same code path rather than split across this module and the handler. Test-file drift sweep per the (updated) CLAUDE.md test-maintenance guidance: 17 array-pattern files + 1 verbose (db_private_rooms.rs) get the new migration appended. 40+ common-helper files using sqlx::migrate! pick it up automatically and need no edits. just check (server, server-saas, desktop, clippy x2, fmt) clean; just test and just test-saas pass modulo the documented routes_uploads concurrent-load flake (passes in isolation). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Commit 2 of LC-64 server-persisted drafts. Adds the destructive handler and wires the three clear hooks; no composer-side wiring yet (commit 3) and no tests yet (commit 4 per the plan, integration angle). server/src/routes/drafts.rs is the new module exposing one route: PUT /room/{room_id}/draft accepts Form<DraftForm { body: String }>. Four handler paths, each returning the right status: 1. Room not visible to the caller -> 403. Drafts piggyback on db::chat::is_room_accessible, the same predicate the send path (post_message at line 437) uses. A user cannot draft in a room they could not also post in; otherwise drafts become a probe for room existence across the access boundary. 2. Body trims to empty -> DELETE the row, 204. The empty case is "I changed my mind" per the acceptance criteria. 3. Body matches a recent messages row from this user in this room (race guard) -> no-op, 204. The trim alignment is one-sided: the draft body is trimmed BEFORE the race-guard SELECT, and the SELECT compares the trimmed draft against messages.body (which is already stored trimmed because post_message at line 397 does `form.body.trim()` before inserting). No trim happens inside the SQL; messages.body is the canonical already-trimmed form, not a column needing further normalization. The 5-second window is generous enough that any debounce-fires-after-send race lands inside the guard. Acceptable false-positive: re-typing the same short message within 5 seconds of sending it drops the new draft; rare, recoverable. 4. Otherwise -> UPSERT the row, 204. PUT always returns 204 (no body, no fragment); the textarea on the client already holds the latest text. Three clear hooks wired: - routes/room.rs::finalize_message_send (line 733 area): one-line db::drafts::delete just before the Ok(message) return. Single chokepoint for both direct sends and LC-62 scheduled deliveries (the dispatcher funnels through finalize_message_send). Best-effort. - routes/scheduled.rs::post_scheduled (after insert_scheduled): one-line db::drafts::delete. The composed text is now in scheduled_messages, the draft's job is done. Redundant with finalize_message_send's clear at delivery time; the redundancy is harmless and matches the "clear at every commit point" posture. - routes/account.rs::purge_user_chat (transaction): db::drafts::delete_for_user(&mut *tx, user_id) next to the existing reminders / scheduled cleanup. The Executor-taking signature composes inside the existing transaction; not a new transaction. The route is wired alphabetically into routes/mod.rs's module list and the /room/{room_id}/draft entry slots next to /room/{room_id}/messages; axum routing imports `put` alongside the existing `get`/`post`/`delete`. Tests for the destructive path land in commit 4 (per the plan, integration angle covers handler + render together). Carrying one open item: kick-cleanup decision (acceptance criterion literally says "kicking a user removes their drafts in that room"; the delete_for_user_in_room helper is already in db::drafts, gets wired in commit 4 conditionally on Nate's read of the criterion). just check (server, server-saas, desktop, clippy x2, fmt) clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Commit 3 of LC-64 server-persisted drafts. The end-to-end pull-on-load loop is now functional behind a human-verify gate on the HTMX trigger filter (see HUMAN-VERIFY section below). New helper db::drafts::get_fresh_or_purge(pool, user_id, room_id, max_age_days) does the lazy-cleanup-on-load in one call: SELECT body and is-stale-bit, return Some(body) when fresh, silently DELETE and return None when stale. Single SELECT roundtrip with the staleness check pushed into SQL so the call site reads as "give me a fresh draft body" without a separate timestamp comparison in Rust. The doc string flags the write-in-read-path as intentional and rules out the alternative (a spawn-task cleanup sweep) as overkill for kilobyte-scale row volume. Mirrors the LC-XXX retention "lazy on load, no background task" posture. RoomPage, DmPage, and ComposerFragment view structs each grow an initial_draft field. RoomPage / ComposerFragment use &'a str (matches their borrowed-everything convention); DmPage uses String (matches its owned-fields convention). The four standalone ComposerFragment construction sites (post-message success, link-block, two slash-command result paths) pass `""` because those are render-after-action paths that ran AFTER the user's text was either committed or rejected; no in-progress draft to restore there. Page handlers (get_room, get_dm) fetch the real value via get_fresh_or_purge with the 60-day threshold. composer.html grows two changes on the textarea: 1. The textarea's inner content is now {{ initial_draft }} (Askama default-escapes). The autofocus attribute places the cursor at the START of the pre-populated text; same behavior as a manual reload would produce for unsubmitted text in the box. 2. Four new htmx attributes wire the PUT path: - hx-put="/room/{{ room.id }}/draft" - hx-include="this" (only the textarea's name=value, not the whole form) - hx-swap="none" (PUT returns 204; nothing to render) - hx-trigger="keyup[event.key !== 'Enter' || event.shiftKey] changed delay:1000ms" The trigger filter reads as "fire on keyup, except when it's plain Enter without shift." Plain Enter is the send-now keystroke (the existing onkeydown calls preventDefault before requestSubmit); the filter excludes it so the keyup after the send does not fire a draft PUT that would carry the just-sent body. Shift+Enter (a newline character) passes the filter so multi-line drafts continue to sync. The `changed` modifier is belt-and-braces against arrow-key keyups inside the mention combobox (Phase 25): arrow keys do not change the textarea's value, so `changed` filters them out independently of the key-name filter. The two together cover the "send race" (Enter) and the "popover navigation" (arrows) cases without needing client-side debounce cancellation in hx-on::after-request. ==== HUMAN-VERIFY GATE: HTMX trigger filter ==== I cannot reliably exercise an HTMX trigger filter in a real browser from this environment. The htmx 2.0.4 docs say the bracket-filter syntax `keyup[<js-expr>]` is supported, but "the docs say it works" is not the same as "I watched it fire correctly in Chrome." The client-side filter is exactly the kind of thing that silently does not compose; the server-side race guard from commit 2 is the net if this filter has a gap. Manual test sequence (run with the dev server on localhost): 1. Type a few characters, pause for 1 second. Network tab: EXACTLY ONE PUT to /room/{id}/draft. Body contains the typed text. Status 204. 2. With text in the box, hit Enter. Network tab: ZERO PUTs to /draft from the Enter keyup. ONE POST to /room/{id}/messages (the send). After the send completes, NO trailing PUT carries the just-sent body (this is the resurrect-race; the server-side guard would catch a hit, but the filter should prevent it from happening client-side at all). 3. With text in the box, hit Shift+Enter (newline). After the 1-second debounce: ONE PUT with the multi-line body. Confirms the filter does not block legitimate newlines. 4. Type "@" to open the mention popover. Use arrow keys to navigate options. Network tab: ZERO PUTs (arrow keys do not change the textarea value; `changed` filters them out). Press Enter to insert a mention. After the textarea fires its input event (which the IIFE dispatches manually after insert), wait 1 second: ONE PUT with the body including the inserted "@username ". 5. Type "/" to open the slash popover. Network: ZERO PUTs while typing matches no command threshold? Actually one PUT after debounce with the slash-prefixed body. Confirms the slash popover and the draft sync coexist (different timers, different targets). 6. Open the same room in a second tab. Confirm the composer pre-populates with the draft from the server. Empty the box, wait 1 second, refresh the second tab: composer is empty (the empty PUT deleted the row). If any of those fire-zero-PUTs cases instead fires a PUT, the trigger filter is not composing as expected: switch the manual test report to "filter gap, race guard is the only defense" and either iterate the filter syntax or accept the server-side net. ==== end HUMAN-VERIFY GATE ==== just check (server, server-saas, desktop, clippy x2, fmt) clean; just test and just test-saas pass with no failures. Tests for the destructive + render paths land in commit 4 per the plan; the human-verify gate is for the one piece that cannot be confirmed from a test binary. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>Commit 4 of LC-64 server-persisted drafts. 19 integration tests covering the destructive PUT path, the lazy-cleanup-actually-purges contract, the three clear hooks, the race guard's window + trim alignment, the privacy invariant, and the render path. Plus an EXPLAIN QUERY PLAN test that pins the race-guard SELECT's index usage so future schema changes that drop an index this query depends on surface here rather than as production latency. FOR THE RECORD: trigger filter divergence from the plan. The HTMX trigger filter that landed in commit 3 (`keyup[event.key !== 'Enter' || event.shiftKey] changed delay:1000ms`) DIVERGES from the form the plan specified (`keyup[!event.shiftKey && event.key !== 'Enter'] ...`). The landed form is correct and the plan's form had a bug; flagging the divergence here rather than absorbing it silently. Truth table on the plan's form for Shift+Enter: `!true && ('Enter' !== 'Enter')` = `false && false` = FALSE, which would suppress draft PUTs on newline keystrokes. A multi-line draft would not sync lines added via Shift+Enter until some other keystroke fired. The landed form `key !== 'Enter' || shiftKey` evaluates to TRUE for Shift+Enter (key is 'Enter' so the first clause is false, but shiftKey is true so the OR is true), correctly firing on legitimate newlines. The plan's form was confirmed in commit 2's reply; the correction happened silently when commit 3 wrote the real filter. Same genre as the LC-62 false-claim catch surfaced earlier on this work: a divergence from the agreed plan, even a correct one, goes on the record. Step 3 of the manual-test sequence in commit 3 (Shift+Enter = 1 PUT) is the exact case that would have failed under the plan's form. EXPLAIN QUERY PLAN finding. The race-guard SELECT `WHERE user_id = ? AND room_id = ? AND body = ? AND created_at >= datetime('now', '-5 seconds')` walks via `SEARCH messages USING INDEX idx_messages_room (room_id=?)`. The planner picks the existing `idx_messages_room` index (on room_id), narrows to that room's messages, then applies the remaining filters (user_id, body, created_at) by post-index inspection. At self-hosted scale this is fine: the 5-second window bounds the candidate set tightly because few users send multiple messages per 5-second interval. At larger scale a composite index on (user_id, room_id, created_at) would be more selective; flagged as a possible follow-up if metrics ever surface latency on this query. The `race_guard_query_plan_uses_an_index` test asserts the plan contains "USING INDEX" so a future schema refactor that removes `idx_messages_room` (or replaces it with a less-selective index for this query) fails in CI rather than degrading production silently. Tests (server/tests/routes_drafts.rs, 19 in standalone + saas): Four PUT handler paths: - put_creates_draft_row_with_204 - put_updates_existing_row_lww (LWW: second PUT advances updated_at) - put_empty_body_deletes_row - put_whitespace_only_body_deletes_row (trim before empty check) - put_to_private_room_non_member_returns_403 (visibility gate) - put_writes_to_authenticated_user_slot_not_other (privacy: user_id from session) Race guard with 5-second window: - race_guard_blocks_within_5s_window - race_guard_allows_after_5s_window (message backdated 10s; PUT upserts) - race_guard_trim_aligns_against_stored_body (draft "hi\n" trimmed matches stored "hi") - race_guard_does_not_block_different_user (guard is user-scoped) - race_guard_does_not_block_different_room (guard is room-scoped) Lazy cleanup on load, PINNING THE PURGE not just the non-render: - stale_draft_is_purged_on_load_not_just_hidden (asserts row count is 0 after the call, not just the call's None return) - fresh_draft_returned_unchanged_by_get_fresh_or_purge Clear hooks: - send_clears_draft_via_finalize_message_send (real POST through the route) - account_delete_purges_drafts_across_rooms (delete_for_user inside a transaction) - room_delete_cascades_drafts_via_fk (DELETE rooms exercises ON DELETE CASCADE) Render path: - room_render_pre_populates_textarea_with_draft (GET /room/1 contains draft body) - room_render_with_stale_draft_renders_empty_and_purges_row (the load-bearing test for the lazy-cleanup contract: stale body NOT in HTML AND row gone) Query-plan pin: - race_guard_query_plan_uses_an_index (EXPLAIN QUERY PLAN contains "USING INDEX") Notes on what is NOT here: - Kick-cleanup test: still pending Nate's read of the acceptance criterion. The `delete_for_user_in_room` helper sits in `db::drafts` ready; commit-4-onwards either adds the wire (two lines + one test) or documents the lingering-row reading. - HTMX trigger filter behavior in a real browser: commit 3's human-verify gate. The 19 tests here exercise the server-side path; the client-side filter composition must be confirmed with the dev server + browser network tab per the six-step sequence in commit 3. just check (server, server-saas, desktop, clippy x2, fmt) clean; the 19 new tests pass in standalone and saas. The pre-existing `routes_uploads` flake under concurrent-binary load (documented in CLAUDE.md) surfaced once during this run; passes in isolation, out of scope for this commit. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>