feat/lc-62-scheduled-send-pr-b #157

Merged
longjacksonle merged 6 commits from feat/lc-62-scheduled-send-pr-b into main 2026-05-20 06:33:18 +02:00
No description provided.
spawn_scheduled_dispatcher mirrors spawn_idle_scanner / spawn_digest_sender / spawn_orphan_sweeper: tokio::spawn + tokio::time::interval with the skip-first-tick pattern. 30-second tick cadence matches the brainstorm decision (UI promises delivery within 30 seconds of scheduled_for; ~30s lateness is acceptable for chat, 10s would 6x the existing tick patterns for marginal user-visible gain). Logs only on non-empty ticks; warns on tick-level errors.

scheduled::dispatcher::run_dispatch_tick drains the due queue (LIMIT 100 per tick so a large startup backlog does not stampede push fan-out). For each due row, validate_delivery does the read-only re-validation: room exists, author not banned / muted / kicked / blocked-by-DM-peer, link filter still permits. Re-validation matches the live post_message gate set minus the rate limit (rate-limit was paid at schedule time per the ticket). On any deny, the row is claimed (delivered_at set) and mark_dropped writes failed_reason inside the same BEGIN IMMEDIATE so a crash between the two re-runs cleanly.

The atomic core: BEGIN IMMEDIATE + try_claim + INSERT INTO messages + COMMIT on one connection. On a lost claim (rows_affected == 0), ROLLBACK and return Retry. Mention reconciliation, link_upload_to_message, and finalize_message_send all run AFTER commit as best-effort side effects; matches the live-send semantics where the messages row commits before WS / push fire. A finalize_message_send error at this point is logged and the message remains in the DB; recipient sees it on the next page load even though the live broadcast missed.

Per-row internal errors bump attempt_count and push next_attempt_at forward by 2^min(count, 8) minutes (caps at ~4h). The bump_attempt UPDATE is WHERE delivered_at IS NULL so a late call after a delivery succeeded is a no-op. select_due's cooldown filter (next_attempt_at IS NULL OR next_attempt_at <= now) honors this without further dispatcher logic.

routes/mod.rs visibility change: `mod room` is now `pub(crate) mod room` so the dispatcher can call routes::room::finalize_message_send and routes::room::can_post_with_policy. Matches the precedent of pub(crate) mod email_verification and pub(crate) mod login_alerts already in this file. Both targets were already pub(crate); only the surrounding module was private.

Tests cover the load-bearing properties: past-due delivery + WS broadcast lands in the hub channel; future row stays pending; user kicked out of enclave gets dropped with the access-loss reason string; second tick after delivery is a no-op (idempotence + delivered_at filter); a cooldown-bumped row is skipped by select_due; mention target renamed between schedule and delivery drops the mention silently (Option B correctness check). The Option B test is the load-bearing one: it proves resolution happens at delivery against the current users table, body keeps the literal `@bob` token, and no mentions row is inserted for the renamed user.

Single-process invariant documented in dispatcher module's doc-comment, mirroring uploads/sweep.rs. A future move to a sidecar would make the BEGIN IMMEDIATE and try_claim atomicity load-bearing under cross-process concurrency, not just decorative within one process.
GET /scheduled lists the caller's pending rows sorted scheduled_for ASC, plus a "Recently dropped" section with failed_reason for surfacing why a delivery didn't ship. Each pending body is re-rendered through views::markdown::render on every view against the current users table; a @bob token whose target was renamed to robert between schedule and delivery shows as literal text with no profile-link chip, which is the Option B visible signal from the brainstorm. POST /scheduled validates lead-time bounds (30s..365d), enforces the message rate-limit (rate-limit is paid at schedule time per the ticket decision; delivery does NOT re-check), re-checks attachment ownership / enclave quota / quote validity, runs the shared delivery gates, and stores the row in SQLite's YYYY-MM-DD HH:MM:SS format. PATCH /scheduled/:id and DELETE /scheduled/:id are user-scoped row operations; both surface "already delivered or dropped" as 410 Gone via a direct Response so the management UI can update its row state precisely. Sidebar gains a /scheduled link next to /saved.

scheduled::validation::check_delivery_gates is the shared helper. The dispatcher's validate_delivery (Task 3) now calls it, dropping ~40 lines of inline checks; routes/scheduled.rs::post_scheduled and patch_scheduled call the same helper. DenyReason carries the &'static str that lands in scheduled_messages.failed_reason at delivery and feeds AppError::Forbidden / BadRequest at schedule time (LinkBlocked maps to 400 to match the live post_message behavior; the rest map to 403). Reason strings are preserved bit-for-bit so the Task 3 dispatcher tests assert on the exact same surface.

The live post_message in routes/room.rs is intentionally NOT pulled into the shared helper. Task 1 explicitly scoped its refactor to the post-insert tail; reworking the pre-insert gate set is a larger blast radius than this PR should carry. Documented in validation.rs's module comment: if you change the gates, audit routes/room.rs::post_message for the live path.

Composer modal UI (Task 5) is intentionally not in this commit. POST /scheduled is the wire shape Task 5 will target; the route returns a small HTML status fragment with data-scheduled-id so the modal can fire an HX-Trigger-style success event without coupling the route shape to the eventual modal markup. The /scheduled page works today as a list/view/cancel surface; without the modal, the only way to create a row is the POST endpoint itself.

Tests (server/tests/routes_scheduled.rs, 12 cases): POST happy path; lead-time rejected on both bounds (now+10s -> 400, now+400d -> 400); access denied in a private room -> 403; attachment owned by another user -> 403; PATCH own row returns the scheduled_row.html fragment with the new body; PATCH other user -> 403; PATCH delivered -> 410; DELETE own -> 200; DELETE other -> 403; DELETE delivered -> 410; GET lists only the caller's pending rows in scheduled_for ASC; GET pre-rename includes the bob-profile chip, GET post-rename does NOT (the Option B UI signal). Uses the tests/common helper, so sqlx::migrate! picks up 0033 automatically.

Module visibility: routes/mod.rs adds `mod scheduled;` next to `saas_auth` / `search`; routes are registered after /saved. views/mod.rs adds `pub mod scheduled;` in the alphabetical slot. scheduled/mod.rs re-exports check_delivery_gates and DenyReason alongside the dispatcher surface.
The modal lives in layout.html's persistent shell so the open/close JS does not have to be re-bound on composer swaps. The trigger (the clock-icon button next to Send in composer.html) calls window.__lcOpenScheduledModal(), which reads the current composer's data-room-id + body + file_id at open time so a composer swap between page-load and trigger does not leak stale references. The composer form gains a data-room-id attribute (already had hx-post="/room/{id}/messages" so the value is colocated, just exposed for JS now).

Reuses Phase 25's window.__lcDialogTrap(rootElement) from layout.html for Tab cycling; ESC + backdrop click + Cancel + X + submit-success are each handled explicitly because __lcDialogTrap only wraps Tab (call.js's pattern, mirrored here). aria attributes: role="dialog" + aria-modal="true" + aria-labelledby on the dialog, aria-haspopup="dialog" on the trigger.

Native <input type="datetime-local"> per the ticket decision. On submit, JS converts the picker's local value to a Date, then to UTC via toISOString(), and POSTs the result as scheduled_for; the route in Task 4 accepts RFC3339 and stores in SQLite's YYYY-MM-DD HH:MM:SS format. Submit goes via fetch rather than HTMX so close-on-success + show-error-on-fail can branch on res.ok without binding a target.

Composer reset on success mirrors the live-send afterRequest path (clear textarea, disable send, clear attachment) so a scheduled send feels like a normal send from the composer's perspective. Best-effort: if the composer was swapped away mid-modal, the reset block is a no-op via document.contains check.

Close-path enumeration (all funnel through closeModal which disposes the trap exactly once, restores aria-hidden, and refocuses the opener if it still exists):

  1. Cancel button (data-lc-scheduled-modal-cancel)
  2. X button (data-lc-scheduled-modal-close)
  3. ESC keydown (keydown listener on the dialog root; __lcDialogTrap only wraps Tab)
  4. Backdrop click (mousedown on the dialog root where target === root; mousedown rather than click so a drag that starts inside the panel and releases on the backdrop does not dismiss)
  5. Submit success (fetch returns 2xx)

UI behavior is NOT browser-verified in this commit. The close paths, focus restoration, aria-hidden flipping, datetime conversion, and end-to-end "click Schedule, wait 30s, observe delivery" flow were not exercised against a running dev-web-local. The code matches the close-path enumeration above through review only; manual smoke verification is deferred to Task 8 (README + verification pass).

Excluded by design: no Submit-failure close path (errors show inline in the modal so the user can adjust and retry without losing the open dialog). No browser back-button close (modal does not push history state in v1).

No new test binary. Task 4's POST /scheduled wire-shape tests cover the route the modal targets; UI behavior is hard to integration-test and the alternative (headless browser harness) is overkill for one dialog.
routes/account.rs::purge_user_chat gains a db::scheduled::delete_for_user(&mut tx, user_id) call inside the existing user-purge transaction, placed after the mentions DELETE and before the file_uploads DELETE. Uses the helper added in PR-A's db::scheduled module, which takes &mut SqliteConnection so the call shares the surrounding transaction handle; auto-deref from &mut Transaction satisfies clippy's explicit_auto_deref rule (the surrounding raw-sqlx queries use &mut *tx because Executor lives on both Transaction and Connection, so the deref there is load-bearing).

Once this transaction commits, no scheduled rows remain for the deleted user in any state (pending, delivered, or dropped); the dispatcher's "author no longer exists" branch in validate_delivery (Task 3) becomes unreachable by construction. The defensive branch stays as a belt-and-suspenders guard: a row authored by a now-missing user is still treated as a drop with a reason string rather than an Err, so an out-of-band DB modification (manual SQL surgery, restore from backup with a stale snapshot, future user-delete path that bypasses purge_user_chat) does not loop the dispatcher forever on backoff.

Test (extended the existing delete_wipes_user_and_chat_rows in routes_account_delete.rs rather than adding a new test, matches the file's "seed all the things, delete, assert all the things gone" shape): seeds a pending scheduled_messages row authored by the user, runs the delete, asserts db::scheduled::get_scheduled returns None. The seed uses 2099-01-01 (far future) so it is genuinely pending - not relying on dispatcher timing for the assertion to be meaningful.
One bullet next to the email-digest entry, matching the existing concise style. No new env vars to add (LC-62 reuses LETS_CHAT_DATA_DIR and the existing chat pool; nothing in the env-var table needs updating). Scheduler-pattern note for CLAUDE.md is intentionally not added: spawn_scheduled_dispatcher is the third instance of the spawn_* + tokio::time::interval pattern (after spawn_idle_scanner, spawn_digest_sender, spawn_orphan_sweeper), and the pattern itself is already documented at those call sites.
fix(scheduled): label /scheduled timestamps as UTC (LC-62)
All checks were successful
Check / clippy + fmt + tests (pull_request) Successful in 11m29s
59ae590a4c
The /scheduled management page rendered scheduled_for + delivered_at as raw "YYYY-MM-DD HH:MM:SS" strings without indicating they are UTC. The composer's success fragment already labels "UTC" (added in Task 4); the row template and the Recently dropped section did not. A user who picked "2:30 PM" in their local picker would see "delivers 2026-05-20 12:30:00" and have to mentally subtract their offset to recognize it as their own scheduled time.

Append " UTC" to the three timestamp sites on the page, matching the existing convention used by templates/settings/page.html for session timestamps and templates/email/login_alert.* for alert timestamps. One word per site, no JS, no new pattern.

Client-side toLocaleString() rendering is intentionally NOT introduced here: client-side time localization is a cross-cutting decision (it should apply everywhere - message timestamps, edit-history timestamps, admin modlog, inbox, all of it - not bolted onto one page), and the codebase has no precedent for it today. Deferred as a follow-up consideration.
longjacksonle deleted branch feat/lc-62-scheduled-send-pr-b 2026-05-20 06:33:18 +02:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
psa-systems/lets-chat!157
No description provided.