Commit Graph

83 Commits

Author SHA1 Message Date
GPT on behalf of DHH 0aa339d81e Avoid room queries for messages without mentions 2026-10-08 20:46:01 +02:00
GPT on behalf of DHH 7331d3a946 Create messages, search entries and unread state atomically
Apply the Laravel implementation’s MessageWriter architecture to Rails creation: save native Action Text and attachment metadata, insert the search entry and update unread memberships in the message transaction. Keep notification jobs after commit and preserve Room#receive and existing serialized jobs.

Cover native HTML/mentions, attachment-only search, derived-write failures, outer rollback and SQL commit ordering. Updates and destruction retain their current indexing behavior.
2026-10-08 13:22:14 +02:00
GPT on behalf of DHH 9b8a1683a7 Reuse native plain text conversion for attachment-free content 2026-10-08 12:41:14 +02:00
GPT on behalf of DHH a0d72938f3 Keep transaction guards temporary and simplify quick boost forms 2026-10-08 12:40:31 +02:00
GPT on behalf of DHH a9007010c1 Check cache transactions without leasing a database connection 2026-10-08 12:34:18 +02:00
GPT on behalf of DHH bbee420097 Reuse epoch-verified record snapshots and avoid empty push payloads 2026-10-08 12:26:50 +02:00
GPT on behalf of DHH 74381e8b58 Retain content-validated message fragments across unrelated commits 2026-10-08 12:00:45 +02:00
GPT on behalf of DHH 2c53c46767 Reuse token-free message controls and render posting deliveries once 2026-10-08 11:43:27 +02:00
GPT on behalf of DHH f36b5a2fae Merge #336: avoid rewriting already-unread room memberships 2026-10-08 11:33:18 +02:00
GPT on behalf of DHH f792e16e64 Repair message counters within database task pools 2026-10-07 23:50:37 +02:00
GPT on behalf of DHH fd49099729 Merge SQLite-maintained bot pagination counts from PR #337 2026-10-07 23:28:53 +02:00
GPT on behalf of DHH d9c3207886 Merge upstream banned-user push notification fix 2026-10-07 23:19:33 +02:00
Cursor Agent b4ab2df20f Repair messages_count triggers in one SQLite write transaction
ensure! now rechecks trigger presence under BEGIN IMMEDIATE, backfills
drifted rooms.messages_count, and reinstalls all three triggers before
commit so concurrent writers and concurrent boot repairs cannot observe
a partial install or keep a wrong total. install! uses the same
immediate write lock for drop/recreate. Lifecycle regressions cover
missing/partial trigger drift, concurrent ensure!, and writes racing
repair.

Co-authored-by: Thomas Klemm <github@tklemm.eu>
2026-10-07 20:58:32 +00:00
Cursor Agent 9fd1563c9b Simplify messages_count trigger install and harden ensure!
Require all three SQLite triggers before treating the counter as installed,
drop the schema.rb / dump-rewrite install paths in favor of rake ensure after
schema load, isolate destructive trigger tests, and cover foreign room moves
plus fixture baseline counts.

Co-authored-by: Thomas Klemm <github@tklemm.eu>
2026-10-07 19:46:57 +00:00
GPT on behalf of DHH 8d02540e31 Bound native fragment caching and collapse concurrent page renders 2026-10-07 21:38:36 +02:00
Cursor Agent 1f5858098b Keep bot page totals on SQLite-maintained rooms.messages_count
X-Total-Count on GET /rooms/:id/:bot_key/messages was COUNT(*) of the
room on every page. Serve it from rooms.messages_count updated by SQLite
triggers so Rails, bulk SQL, and foreign writers stay in step — without
ActiveRecord counter_cache callbacks those paths skip.

Fixes #309.

Co-authored-by: Thomas Klemm <github@tklemm.eu>
2026-10-07 19:36:01 +00:00
Jeremy Daer 6e312c6028 Stop sending push notifications to banned users (#338)
Banning a user deletes their sessions and closes their connections but
keeps their push subscriptions, and Room::MessagePusher chose recipients
by membership alone. A banned user's browser or phone therefore went on
receiving the room name, sender and text of new direct messages,
mentions, and messages in rooms they had set to everything.

Choose subscriptions from active users only. The subscriptions are kept,
so unbanning brings notifications back without the user having to
subscribe again (the client doesn't resubscribe while the browser still
holds a subscription).

Co-authored-by: Marcello Costagliola <176920116+namespaceMarcello@users.noreply.github.com>
2026-10-07 11:25:16 -07:00
GPT on behalf of DHH ac73267b07 Reuse authorized read pages without stale presentation or CSRF masks
Transfer the C completed-response cache lesson into Rails, keeping authentication, room checks and cookies per request. A persistent read-only SQLite observer detects local and foreign commits and rejects racing admission. Whole-page misses render fresh to avoid stale nested fragments; message ETags reflect token-neutral presentation.
2026-10-07 20:02:20 +02:00
Marcello Costagliola af4f94c4bd Leave members who are unread already out of a new message's update
Every message rewrote the row of every disconnected member of the room,
including the ones who were unread already and stay unread. Open rooms
take in the whole account, so in steady state that is every member on
every message: 325 WAL pages per post at 10,000 members, against 16 when
only the members who had read the room are written.

Directs keep touching all their members: a direct's sidebar row is
cached by its membership and carries the room's recency, so it has to
be refreshed on every message.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017dFSDHrgrLoBELwjV3Qunq
2026-10-07 16:41:20 +02:00
GPT on behalf of DHH dbc7620a76 Bound accessible search probes and reject invalid benchmark responses 2026-10-07 14:22:07 +02:00
GPT on behalf of DHH 7df98ac883 Merge current Rails main during performance review
# Conflicts:
#	app/views/messages/_message.html.erb
2026-10-07 11:34:17 +02:00
GPT on behalf of DHH 27065ef489 Keep same-second unread events and bound idle push connections 2026-10-07 11:00:47 +02:00
Donal McBreen 8a6e4290d8 Merge commit from fork
* Derive message DOM ids from the server id, not client_message_id

A message's DOM id was derived from the browser-chosen client_message_id
via a Message#to_key override, so dom_id(message) was
"message_<client_message_id>". Turbo's append de-dups by DOM id, so a room
member who posted a message reusing a victim's client_message_id displaced
the victim's message element in every connected member's live view; editing
the attacker's own message then broadcast onto the victim's presentation id.

Drop the to_key override so every message DOM id and broadcast target derives
from the record's primary key. Two distinct records can no longer share a DOM
id regardless of stored client_message_id, so the collision is impossible with
no data migration and no uniqueness constraint. to_param and the fragment
cache key already used the primary key, so message URLs and per-message cache
entries are unchanged.

The composer's optimistic pending message still uses client_message_id as its
placeholder DOM id, which no longer matches the server broadcast's PK-based id.
Reconcile instead by rendering data-client-message-id on the real message and
having the messages controller drop the matching pending placeholder on
connect. Only client-side placeholders (data-pending-message) are removed, so a
message another member posts reusing the same client_message_id can never
displace a real one through the reconciliation path either.

Also point the boost broadcast target at the PK-based dom_id(message, :boosts)
to match the rebuilt container id.

GHSA-3v99-4vxh-xg84

* Bust cached message fragments rendered with client_message_id DOM ids

The message fragment cache keys on the record and the template digest, and
removing the to_key override changes neither. Fragments cached by an earlier
release would keep their message_<client_message_id> ids, so edit, delete and
boost broadcasts, which now target primary-key ids, would miss those messages
in other members' live views until the cache entry expired.

* Locate messages by record id in the client_message_id collision tests

Assert on data-message-id rather than the new primary-key DOM ids, so the tests
describe the behavior instead of the fix and fail on the vulnerable code for the
real reason. Edit the attacker's message to new text and wait for it to arrive,
so the edit path is exercised rather than passing vacuously. Add a request test
that two messages sharing a client_message_id render as distinct elements.

---------

Co-authored-by: Jeremy Daer <jeremy@37signals.com>
2026-10-07 01:41:12 -07:00
GPT on behalf of DHH 14a2f8f550 Merge pull request #329: Delete a room's messages in a job, one transaction each
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:33:57 +02:00
GPT on behalf of DHH 72648a8f41 Merge pull request #296: Fan the unread room notice out from a job
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:33:57 +02:00
GPT on behalf of DHH b409b9998f Merge pull request #328: Bound how long link unfurling can hold a request
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:33:57 +02:00
GPT on behalf of DHH a991adb19d Merge pull request #330: bound attachment preview work
Reviewed and merged by GPT on behalf of DHH. Keep both boost-cache and preview-loading regressions.
2026-10-07 10:33:02 +02:00
GPT on behalf of DHH cbf52805bd Merge pull request #311: Post a message whose attachment can't be previewed instead of failing
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:32:12 +02:00
GPT on behalf of DHH 0f2a1da560 Merge pull request #316: List one page of account members at a time
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:32:12 +02:00
GPT on behalf of DHH 6ad4735b19 Merge pull request #324: Rewrite a message's search entry only when its body or attachment changes
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:32:12 +02:00
GPT on behalf of DHH 819b3896fb Merge pull request #323: Count unread rooms for push badges once per batch
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:32:12 +02:00
GPT on behalf of DHH 0d5998f057 Merge pull request #318: Query sidebar directs and shared rooms separately
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:32:12 +02:00
GPT on behalf of DHH 87eafc025f Merge pull request #310: Find a direct room with one query instead of checking every one
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:32:12 +02:00
GPT on behalf of DHH 1e0d353c60 Merge pull request #312: Find the messages a refresh replaces through an index
Reviewed and merged by GPT on behalf of DHH.
2026-10-07 10:32:12 +02:00
Marcello Costagliola 9912e63d69 Bound the work of previewing an attachment
A video's preview and a picture's thumbnail are made inside the request that posts the message, and
nothing bounded how long either could take.

- The video preview filter also selects any frame from 5 seconds on. Rails' filter takes the second
  frame it selects, which a video with a single keyframe and no scene change only gives at its end, so
  ffmpeg decoded all of it.
- TimeLimitedVideoPreviewer gives ffmpeg 10 seconds of wall-clock time, kills it past that, and reports
  a failed preview, so the message is posted without one.
- Pictures and videos above 250 megapixels, or whose size couldn't be read, get no preview: decoding
  costs in proportion to the pixels, however small the file.
- The view shows a preview only if it was made when the message was posted. Its URL used to make it on
  view, so a preview that failed or was skipped would be attempted again on every view. The cached
  presentation's version goes up, so cached messages pick this up.
- A video's poster is made, when the message is posted, at the size the view shows it. The full-size WebP
  made until now wasn't shown anywhere, and encoding it costs in proportion to the frame's pixels.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa
2026-10-06 15:20:57 +02:00
Marcello Costagliola 91036c5085 Merge branch 'post-messages-with-unreadable-attachments' into bound-attachment-previews
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa
2026-10-06 15:20:56 +02:00
Marcello Costagliola 462eff10df Reset former members' connections and grant open rooms in one statement
Deleting the memberships with delete_all skips Membership's after_destroy_commit, which resets a member's
connections when one membership is revoked. Until the job ran, a member who had the room open kept its
streams, and a message that still landed in the room (a request already past the membership check, a bot's
reply) reached them. The request now reads the members' ids in the transaction that deletes their
memberships, and the job resets their connections before it destroys the messages. It costs a Redis round
trip per member, about 1.5 s for 10,000, so it's done in the job rather than in the request.

User#grant_membership_to_open_rooms read the open rooms and inserted in a separate statement, so a user
created while a room was being closed could read it as open and be granted it after the close. It's now a
single insert ... select, which SQLite runs under the write lock, skipping duplicates as insert_all did.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa
2026-10-06 14:45:18 +02:00
Marcello Costagliola 414ff3376e Put the whole unfurl, lookups included, under one deadline
The deadline in Opengraph::Fetch started after the lookup of the pasted
host, and an unfurl looks up more hosts outside any fetch: the canonical
URL's, and the image's, once to check its content type and again to
validate it. Each of those waits as long as the resolver takes to give
up. Move the deadline up to UnfurlLinksController, around everything the
link leads to; Opengraph::Fetch keeps its per-operation timeouts and
makes no retries.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa
2026-10-06 14:03:26 +02:00
Marcello Costagliola 7b16014f01 Delete a room's messages in a job, one transaction each
Room#destroy destroyed every message inside the room's own
transaction, which holds SQLite's write lock until the last one: on a
room with many messages, every other write in the app waited and
failed. The request now takes the room away from its members and
leaves the rest to Room::DestroyJob, which destroys the messages one
at a time, each in its own short transaction, and then the room.

An open room is closed in the request, so that someone who joins the
account before the job ends isn't given it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa
2026-10-06 13:57:05 +02:00
Marcello Costagliola a196a93ed2 Bound how long link unfurling can hold a request
Opengraph::Fetch runs inside POST /unfurl_link, against a host the member
picked, with Net::HTTP's defaults: 60 s to connect, 60 s for each read, and
one retry of the GET. A per-read timeout doesn't bound a host that sends a
byte at a time, in its headers or its body, so nothing limited how long a
fetch could hold the request thread.

Give each operation 7 s, as Webhook does, turn off the retry, and put the
whole fetch, redirects included, under one 10 s deadline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bj8KnxpTf9sj2Ysa8aLAVa
2026-10-06 13:43:34 +02:00
Cursor Agent d2241f16db Merge remote-tracking branch 'upstream/main' into cursor/split-sidebar-membership-queries-8545
Co-authored-by: Thomas Klemm <github@tklemm.eu>
2026-10-05 18:30:41 +00:00
Sam Ruby 9d75c8b7eb Update Rails to main
Rails main (e3d5c569) moves Campfire's pin forward ten months. Because
Campfire loads the 8.2 framework defaults, the ones added since then take
effect too, among them header-only forgery protection, Herb as the HTML
template engine, strict Accept headers and immediate blob analysis.

Changes it needed:

- Minitest 6 has no minitest/unit; the test helper no longer requires it.
- Rack::Sendfile is no longer in the middleware stack; DebugLocks goes
  before ActionDispatch::Executor, as the Rails guide now says.
- Time::DATE_FORMATS is deprecated; :epoch is registered through
  ActiveSupport::TimeFormats.
- Channel test subscriptions expose stream_names; streams is private.
- sentry-rails declares its Action Cable handle_open and handle_close
  wrappers private, which Rails 8.2 calls from outside the connection, so
  no /cable connection succeeded. An initializer makes them public until
  getsentry/sentry-ruby#2972 ships (issue #2975).
- Lexxy renders editor content through Rails' editor adapter when Rails
  has one, which asks a mention for its editor partial. It is the same
  users/mention partial the mention prompt already inserts.
- redis-client moves to 0.30.1: Rails main's Redis cache store, which
  production uses, requires 0.28.0 or later, and assets:precompile
  (the Docker build) aborted on 0.25.2.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit b4d3880a3d88059dd5b0b884f1f5f1a6f82aa95c)
2026-10-05 11:45:37 -04:00
Marcello Costagliola e07da58668 Rewrite the search entry when the attachment is replaced too
An attachment message is indexed by its file name, and the update
action still accepts a new attachment. Active Storage clears
attachment_changes in its own after_commit, which runs before this one,
so the replacement is noted in before_update.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0142qgjggdJ2KDdGk7RF9Xm9
2026-10-05 17:34:06 +02:00
Stanko Krtalić d2155e85a0 Merge pull request #306 from namespaceMarcello/post-webhook-attachments-only-on-success
Post a webhook reply as an attachment only when the bot answered 200
2026-10-05 17:13:57 +02:00
Marcello Costagliola 78a84a6ab7 Rewrite a message's search entry only when its body is saved
Boosting and unboosting touch the message, so after_update_commit
rewrote its row in the full-text index with the same text, loading the
rich text again to rebuild it, on every boost. The index only needs a
new row when the rich text body was saved.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0142qgjggdJ2KDdGk7RF9Xm9
2026-10-05 17:09:46 +02:00
Stanko Krtalić 77c5234cb6 Merge pull request #304 from namespaceMarcello/search-newest-matches-by-rowid
Read the newest search matches off the index instead of sorting them all
2026-10-05 16:59:44 +02:00
Marcello Costagliola d485db6038 Count unread rooms for push badges once per batch
Each push notification carries the subscriber's unread room count as its
badge. The pool built it per subscription, loading the user and counting
their unread memberships: two queries for every subscriber, all in the job
before the deliveries reach the threads. With 1,000 subscribed members the
job spent ~180 ms and 2,000 queries there; with 5,000, a second.

The pool now counts the unread rooms of a whole batch with one grouped query
and hands each subscription its badge; nothing else in the notification needs
the user. The queries still run before the work is posted to the threads,
which run outside the Rails executor. Push::Subscription#notification still
counts by itself when no badge is given, as for the test notification.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0142qgjggdJ2KDdGk7RF9Xm9
2026-10-05 16:49:59 +02:00
Thomas Klemm 77d4eb0d3a Merge branch 'main' into cursor/split-sidebar-membership-queries-8545 2026-10-05 17:47:46 +03:00
Stanko Krtalić 6913afa864 Merge branch 'main' into check-paging-without-counting 2026-10-05 16:18:32 +02:00
Marcello Costagliola a740581f20 Keep an unread notice older than the latest read from marking the room
The job picks the room's members once and then publishes to them one at a
time, as the request did before it. A member who opens the room during that
loop gets the read event in their other tabs and then the older notice,
which marked the room unread again. Both events now carry the server time,
and the sidebar ignores a notice dated before the room's latest read. The
notice still moves a direct room to the top.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0142qgjggdJ2KDdGk7RF9Xm9
2026-10-05 16:11:37 +02:00