[Xauele]

Serialize message insertion by conversation before allocating sequence IDs. Apply the same lock order to pings, org events and membership mutations. Include a deterministic two-process PostgreSQL regression that fails before this change and passes afterwards.

Summary by CodeRabbit

• Bug Fixes
  • Improved consistency when multiple messages or conversation updates happen at the same time. Messages are now less likely to appear out of order, and conversation activity and read status remain more reliable during concurrent changes.
  • Improved reliability of organization membership changes when they overlap with other conversation activity.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6ea7be4d-8575-4b4c-b131-fafaaaaa45c1

Commits

Reviewing files that changed from the base of the PR and between 7d38bf2 and b571efe.

Files selected for processing: 8

• app/Actions/CreateConversationEvent.php
• app/Actions/CreateConversationPing.php
• app/Actions/CreateMessage.php
• app/Actions/CreateOrgMembers.php
• app/Actions/DeleteOrgMember.php
• app/Actions/UpdateOrgMemberRole.php
• tests/Feature/RemotesCursorConcurrencyTest.php
• tests/Support/RemotesCursorWorker.php

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

Conversation write actions now lock the conversation row during message, event, ping, and membership operations. A PostgreSQL feature test uses concurrent workers to verify message ordering and full and incremental cursor results.

Changes: Conversation Write Serialization

Layer / File(s)	Summary
Serialize message and event writes app/Actions/CreateMessage.php, app/Actions/CreateConversationPing.php, app/Actions/CreateConversationEvent.php	Message and ping creation lock the conversation row. Event creation now runs message creation, conversation advancement, and the actor’s read-through update in a transaction, after requiring the conversation row to exist.
Serialize membership changes app/Actions/CreateOrgMembers.php, app/Actions/DeleteOrgMember.php, app/Actions/UpdateOrgMemberRole.php	These actions lock the conversation row and require it to exist before changing membership or a member’s role.
Verify concurrent message ordering tests/Feature/RemotesCursorConcurrencyTest.php, tests/Support/RemotesCursorWorker.php	The PostgreSQL test starts independent message writers, checks that the second writer waits for the lock, and verifies message IDs and cursor results. The worker can pause after inserting a message until the test releases it.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant RemotesCursorConcurrencyTest
  participant RemotesCursorWorker
  participant CreateMessage
  participant PostgreSQL
  RemotesCursorConcurrencyTest->>RemotesCursorWorker: Start first worker in hold mode
  RemotesCursorWorker->>CreateMessage: Create message
  CreateMessage->>PostgreSQL: Lock conversation and insert message
  RemotesCursorWorker-->>RemotesCursorConcurrencyTest: Signal insert and wait for release
  RemotesCursorConcurrencyTest->>RemotesCursorWorker: Start second writer
  RemotesCursorWorker->>CreateMessage: Create second message
  CreateMessage->>PostgreSQL: Request conversation lock
  RemotesCursorConcurrencyTest->>RemotesCursorWorker: Release first worker
  PostgreSQL-->>CreateMessage: Grant lock to second writer
  RemotesCursorConcurrencyTest->>PostgreSQL: Check message IDs and cursor results

Loading

Possibly related PRs

• dev-null-sh/dev-null.sh#1101: Locks the conversation row when updating organization pins. This PR adds conversation-row locks to other write actions.

Suggested reviewers: xheaven

Merge Risk: Minimal · up to b571e

The change is mergeable after normal checks. The investigated lock-duration concern does not establish a material workflow impact.

Security architecture risk: Low · up to b571e

The change strengthens conversation ordering without an identified new authorization bypass. Remaining uncertainty concerns lock contention and concurrent membership transitions; the added regression covers message ordering rather than those broader behaviors.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The direct serialization domain is the selected conversation. For an organization, that shared row coordinates all members' message writes and membership administration. Broader connection-pool or service availability effects were not quantified.

Trust Boundaries and Controls

• observed — The inspected direct-message and ping requests retain canMessage and push-access checks. Organization message and ping requests require enabled Remotes and membership in the supplied organization. Member-removal requests check actor and target membership, while the deletion query independently scopes the mutation by conversation and target user.

Resilience and Maintainability Implications

• inferred — The new conversation lock is held during existing synchronous attachment processing and storage. Slow media work can therefore delay other writes, including membership removal, within that conversation. Retry and rollback cleanup provide important counterevidence against simply moving file creation outside the transaction; production latency and timeout evidence were unavailable.

Hardening Proposals

• proposed — Measure conversation lock wait and hold times for media-bearing messages and establish suitable processing and storage bounds. Any transaction-shortening change should preserve single-file retry behavior and rollback cleanup.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly and concisely describes the main change: preserving Remotes cursor order during concurrent writes.
Docstring Coverage		Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 6 functions across 8 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}