fix(conversations): concurrent membership changes must not lose each other

[WhichPaths]

conversations.members is a jsonb array, and every one of the five places that changed it did the same thing: SELECT members, splice it in JavaScript, write the whole array back.

const next = [...c.members, target]
await pool.query(
  `UPDATE conversations SET members = $2::jsonb, updated_at = NOW() WHERE id = $1`,
  [convoId, JSON.stringify(next)],
)

Two of those overlapping is last-write-wins: the second write is computed from a snapshot taken before the first, so the first change is erased. No error, no log line.

Why this is a hot path, not a theoretical one

Concurrency is the normal case here. The scheduler wakes several agents for the same message, and invite / leave / kick are what those agents do in response.

And the dropped invite is worse than it first looks:

• the joined system row is posted regardless, so the transcript records a join that the members column does not agree with
• the mailbox query filters on members @> [agentId], so the agent who "joined" is never woken for that conversation again
• the CLI returned ok(...) with a member count computed from the stale array, so the acting agent was told it worked

The leave/invite overlap is the ugliest ordering: whoever writes last either resurrects an agent into a conversation they left, or erases one that was just added.

Proved before fixing

Straight SQL against Postgres 16, no app code — two "invites" with overlapping read→write windows:

read-modify-write → ["a","b","x"]      ← y is gone
atomic            → ["a","b","y","x"]

Then through the real code path, server/src/__integration__/membership-concurrency.test.ts on unfixed main:

not ok 1 - simultaneous invites do not lose an invitee
not ok 2 - a leave that overlaps an invite keeps both effects
not ok 3 - concurrent kicks of different agents both take effect
# pass 2  # fail 3

Reproduced on three consecutive runs; 5/5 on three consecutive runs after the fix.

A note on that first test: it invites eight agents at once rather than two. With a single overlapping pair the read→write window is short enough that a local socket often serialises them, and the test passed on the broken code — which would have made it worthless as a regression test. Eight loses several every run.

The fix

Postgres edits the array, not us:

-- add
UPDATE conversations
   SET members = members || to_jsonb(ARRAY[$2::text]), updated_at = NOW()
 WHERE id = $1 AND NOT (members @> to_jsonb(ARRAY[$2::text]))
RETURNING members

-- remove
UPDATE conversations SET members = members - $2::text, updated_at = NOW()
 WHERE id = $1
RETURNING members

The "already a member" guard moves into the WHERE rather than living only in JavaScript, because the JS guard reads a snapshot and two callers can both pass it. jsonb - text deletes every matching element, so removal is idempotent and also cleans up a duplicate left behind by the old path.

Callers now use the RETURNING value instead of the array they predicted, so the user-visible member count reflects the row rather than a read taken before the guards ran.

One implementation, not five

All five call sites — cumora leave / invite / kick, and POST /conversations/:id/members / :id/leave — now go through two helpers in agents/membership.ts. That file already exists for exactly this reason, and says so:

Putting both in one file keeps the CLI and HTTP paths from drifting.

Pasting the atomic SQL into five places would have left the same trap set. This is the same lesson as #102, where one of two identical blocks got a fix and the other did not.

Scope

computers.available_engines has the same read-modify-write shape, but it is written by one daemon per computer on a periodic heartbeat, so a lost merge self-heals on the next beat. Left alone deliberately rather than swept in.

Membership guards that read a snapshot (kick refusing to empty a group, invite rejecting a non-participant) can still race on their conditions — this fixes the data, not every TOCTOU on a policy check. Those failures are visible and recoverable; a silently dropped member was neither.

Checks

5/5 in the new integration test (×3 runs), the full integration suite green locally against Postgres 16, 79/79 across the related unit test files, typecheck / biome lint . / both guards clean.