Skip to content

Keep the files an MLS message carries (GRYT-1523) - #246

Merged
sivert-io merged 1 commit into
mainfrom
claude/GRYT-1523-mls-attachments
Sep 28, 2026
Merged

sivert-io merged 1 commit into
mainfrom
claude/GRYT-1523-mls-attachments

Conversation

@sivert-io

@sivert-io sivert-io commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

What to look at

  • The new mls_attachments table in src/db/sqlite/connection.ts: (file_id, group_id, seq), with a foreign key to mls_log and ON DELETE CASCADE. So a ref goes whenever its entry goes, whether retention drops it or dropMlsGroupForConversation does. It's the same shape message_attachments has against messages. The table is new, so it's only a CREATE TABLE IF NOT EXISTS with no migration.
  • getFileOwnership in messages.ts now counts a file as sent to a conversation when an MLS entry there holds it. That's what lets the other person fetch it, and it's the one change here that widens who can read a file. getAllReferencedAttachmentIds and isFileReferencedByMessage read the new table too.
  • sweepMls now also returns fileIds, the files held by the entries it's about to drop, including a group whose conversation is gone. runMlsRetention hands them to deleteUnreferencedFiles, which is what deleting a message does. That only deletes what nothing else holds, and it needs S3. Anything it leaves, the media sweep gets on its next run.
  • The checks in attachmentRefusal repeat chat:send's rules for a sealed DM rather than sharing code with it, since chat:send answers some of them with a bare string. The rules are attach_files, ten at most, each file one the sender can already read, and under the upload cap.
  • The task asked for files "uploaded for this conversation", but the server doesn't record a conversation on an upload, and chat:send doesn't check one either. A sender can attach any file they can already read, which is how forwarding works in a sealed DM too.

What's in it

mls:send takes an optional attachmentIds: string[]. Duplicates are folded. An id that isn't a non-empty string gets invalid_payload, and so does a proposal that carries any. placeholder: false messages can carry files. The count feeds the spam filter's attachments, which was hard-coded to 0.

Tests

In the handler tests: a file Alice uploaded, sent with placeholder: false, is referenced, readable by Bob, and recorded against that seq. Bob's file, a missing id, eleven ids and a non-list are refused. After the entry ages past retention, the sweep returns the file, the media sweep would take it, and Bob can't read it any more. In the db tests: an aged entry's files come back from the sweep and a newer one's stay, then a group dropped with its conversation hands back the rest. Full suite: 1783 pass.

Task: GRYT-1523. Docs: Gryt-chat/docs#143, which merges after this one.

🤖 Generated with Claude Code

An MLS message never goes through chat:send, so no message row pointed at
the files it carried, and the media sweep deleted them 30 minutes after
upload. mls:send now takes attachmentIds. They're checked by chat:send's
rules for a sealed DM: attach_files, at most ten, each one a file the sender
can already read, and under the upload cap. Then they're recorded against
the log entry in a new mls_attachments table.

That table counts as a reference for the media sweep, and as somewhere a
file was sent, so the other person can fetch it. Its rows go with the log
entry, by a cascade. When retention drops an entry, its files are deleted
straight away if nothing else holds them, the way a deleted message's are.
Otherwise the media sweep takes them.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@sivert-io
sivert-io marked this pull request as ready for review September 28, 2026 08:46
@sivert-io
sivert-io merged commit 2dca6ad into main Sep 28, 2026
7 checks passed
@sivert-io
sivert-io deleted the claude/GRYT-1523-mls-attachments branch September 28, 2026 08:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant