Send a shared-room leave while the seat is still reachable

changelog.d/1167.fixed.md

@@ -0,0 +1 @@
+Removing someone's Bot from a shared group now drops that room on their home. The other home was still listing the room and could keep receiving and sending in it, because the leave was sent only after the seat had already gone and a later line could open the room again. ([#1167](https://github.com/ThinkFlowLab/FlyBot/issues/1167))

core/src/host/room-membership.js

@@ -752,6 +752,14 @@ export function install(rt) {
         || (kind === 'bot' && (state.bots?.[me.id]?.ownerId === actor.memberId || me.ownerId === actor.memberId || room.sharedFrom?.ownerId === actor.memberId))
       )
       if (kind === 'bot' && room.origin !== 'inbound-share' && room.origin !== 'face-to-face' && !me.remote && botsLeft.length < 2 && !ownerLeave) throw invalid('A group chat needs at least two Bots')
+      // #1167: reach and a room link's pairs only see who is still in the room. Tell the other home
+      // before leftSeq, or that leave is refused and their copy of the room stays open.
+      if (me.remote) {
+        await Promise.resolve(rt.notifyRoomShare?.(room, { op: 'leave', handle: me.handle || me.id }))
+          .catch(error => rt.warn?.('ds-bot: a seat\'s leave was not sent: %s', error))
+      } else if (kind === 'bot' && room.origin === 'inbound-share' && room.sharedFrom?.ticket) {
+        await sendTicketLeave(room, me).catch(error => rt.warn?.('ds-bot: leaving %s was not sent: %s', room.name, error))
+      }
       const line = append(room, {
         kind: 'left',
         who: me.displayName,
@@ -767,12 +775,10 @@ export function install(rt) {
       }
       syncMembers(room)
       await rt.save()
-      if (me.remote) {
-        void Promise.resolve(rt.notifyRoomShare?.(room, { op: 'leave', handle: me.handle || me.id }))
-          .then(() => (isTicketSeat(me) ? settleTicket(room, me.ticket, me) : undefined))
-          .catch(error => rt.warn?.('ds-bot: a seat\'s leave was not settled: %s', error))
+      if (me.remote && isTicketSeat(me)) {
+        await settleTicket(room, me.ticket, me).catch(error => rt.warn?.('ds-bot: a seat\'s leave was not settled: %s', error))
       } else if (kind === 'bot' && room.origin === 'inbound-share' && room.sharedFrom?.ticket) {
-        void leaveTicketRoom(room, me).catch(error => rt.warn?.('ds-bot: leaving %s was not sent: %s', room.name, error))
+        await finishTicketLeave(room).catch(error => rt.warn?.('ds-bot: leaving %s was not settled: %s', room.name, error))
       } else if (kind === 'bot' && !me.remote) {
         reshareRoster(room)
       }
@@ -967,7 +973,9 @@ export function install(rt) {
   // and is told about the room by that Bot.
   const isTicketSeat = p => p?.remote === true && p.admittedBy === 'ticket'
   function seatSender(room, seat) {
-    const botId = seat.hostBotId && rt.botOf?.(seat.hostBotId) && room.members?.includes(seat.hostBotId) ? seat.hostBotId : undefined
+    const named = seat.hostBotId && rt.botOf?.(seat.hostBotId) && room.members?.includes(seat.hostBotId) ? seat.hostBotId : undefined
+    // The same fallback ticketPeers uses: a seat written before hostBotId still leaves from the room lead.
+    const botId = named ?? (room.admin && rt.botOf?.(room.admin) && room.members?.includes(room.admin) ? room.admin : undefined)
     if (!botId) return undefined
     const member = state.bots?.[botId]?.fields?.social?.member
     return { botId, from: { ...shareInviter(room), inviterHandle: member?.handle, ...(member?.home ? { inviterHome: member.home } : {}) } }
@@ -2024,28 +2032,30 @@ export function install(rt) {
   }
 
   // The guest's side of a seat (#884): a Bot of mine that a ticket seated leaves the room it mirrors.
-  // The inviter's home hears it from that Bot, and when none of mine is left the claim is given back.
-  async function leaveTicketRoom(room, me) {
+  // The inviter's home hears it from that Bot. Sent while the Bot is still seated (#1167): after
+  // leftSeq the pair is gone and the host never hears the leave. The claim is given back only once
+  // that Bot is no longer current, so the "am I the last?" check stays after leftSeq.
+  async function sendTicketLeave(room, me) {
     const inviter = room.sharedFrom?.inviterHandle
     const scope = room.sharedFrom?.remoteRoomId
-    if (inviter && scope) {
-      await sendShareTo(room, { handle: inviter, id: inviter }, {
-        op: 'leave',
-        roomId: scope,
-        name: room.name,
-        invitedHandle: inviter,
-        seq: room.seq ?? 0,
-        ticket: room.sharedFrom.ticket,
-      }, 'room-share', me.id, scope)
-    }
+    if (!inviter || !scope) return
+    await sendShareTo(room, { handle: inviter, id: inviter }, {
+      op: 'leave',
+      roomId: scope,
+      name: room.name,
+      invitedHandle: inviter,
+      seq: room.seq ?? 0,
+      ticket: room.sharedFrom.ticket,
+    }, 'room-share', me.id, scope)
+  }
+  async function finishTicketLeave(room) {
     const stays = currentParticipants(room).some(p => p.kind === 'bot' && !p.remote)
-    if (!stays) {
-      if (state.roomClaims?.[room.sharedFrom.ticket]) {
-        delete state.roomClaims[room.sharedFrom.ticket]
-        await rt.save?.()
-      }
-      await endServerTicket(room.sharedFrom.ticket)
+    if (stays || !room.sharedFrom?.ticket) return
+    if (state.roomClaims?.[room.sharedFrom.ticket]) {
+      delete state.roomClaims[room.sharedFrom.ticket]
+      await rt.save?.()
     }
+    await endServerTicket(room.sharedFrom.ticket)
   }
 
   // A seat of this home's own room that the sender is, for a share addressed to that room.
@@ -2093,6 +2103,9 @@ export function install(rt) {
     // `op: 'line'` envelope to the friend who invited this Bot, under *their* room id, one hop on,
     // carrying the handles it calls there.
     if (room.origin === 'inbound-share' && !isFaceRoom(room)) {
+      // #1167: a room the host already took back does not keep sending, even from a Session that is
+      // still open. sharedFrom.localBotId would otherwise still name a sender after everyone left.
+      if (room.hidden || !currentParticipants(room).some(p => p.kind === 'bot' && !p.remote)) return
       const inviter = room.sharedFrom?.inviterHandle
       if (!inviter || !rt.flybotSend) return
       // A Bot of mine a ticket seated speaks for itself, under that room's scope (#884).
@@ -2305,14 +2318,18 @@ export function install(rt) {
     })
   }
 
-  function markInboundLeft(room) {
+  function markInboundLeft(room, removedSeq) {
     const at = Date.now()
     const line = append(room, { kind: 'left', who: room.sharedFrom?.inviterName || 'the room', authorKind: 'bot', text: '' })
     for (const p of currentParticipants(room)) {
       p.leftSeq = line.seq
       p.leftAt = at
     }
     room.hidden = true
+    // #1167: the host's seq at the leave. A join at or before it is a roster refresh already in
+    // flight, not a re-invite; a later join may open the room again.
+    const seq = Number(removedSeq)
+    room.sharedFrom = { ...(room.sharedFrom ?? {}), removedSeq: Number.isFinite(seq) ? seq : (room.seq ?? line.seq) }
     syncMembers(room)
     return room
   }
@@ -2397,11 +2414,19 @@ export function install(rt) {
         }
         syncMembers(room)
       } else {
-        markInboundLeft(room)
+        markInboundLeft(room, data.seq)
       }
       await rt.save?.()
       return { handled: true, room }
     }
+    // #1167: a line, or a join that was already on its way when the seat was removed, must not open
+    // a room a leave just hid. A newer join is the host adding this Bot again.
+    if (room?.hidden) {
+      const removed = Number(room.sharedFrom?.removedSeq)
+      const seq = Number(data.seq)
+      const newerJoin = data.op === 'join' && Number.isFinite(seq) && (!Number.isFinite(removed) || seq > removed)
+      if (!newerJoin) return { handled: true, refused: 'left', room }
+    }
     if (!room) {
       room = await rt.recordInboundSharedRoom({
         name: data.name,
@@ -2419,6 +2444,7 @@ export function install(rt) {
       }, actor || (ownerId ? { memberId: ownerId } : undefined))
     } else {
       delete room.hidden
+      if (room.sharedFrom) delete room.sharedFrom.removedSeq
       // A ticket seats each Bot of mine on its own (#878): the join names one, and only that one comes
       // back or is added. A friend's share keeps the room's one Bot.
       const seatedHere = ticket && data.op === 'join' && localBotId

core/test/remote-room.test.js

@@ -2205,3 +2205,122 @@ test('#904: a seat\'s line is attributed to the seat, not to whatever the envelo
   const invented = room.log.filter(entry => String(entry.text).includes('line 3')).at(-1)
   assert.equal(invented.authorId, '@chief.bob.home', 'an invented handle never becomes an author')
 })
+
+const BOB_HOME = {
+  version: 2,
+  revision: 1,
+  mainBotIds: ['chief-b'],
+  members: { 'owner-b': { id: 'owner-b', createdAt: 1, designatedChiefId: 'chief-b' } },
+  bots: {
+    'chief-b': {
+      id: 'chief-b', sessionId: 'chief-b', name: 'Chief', instructions: '', createdAt: 1, ownerId: 'owner-b',
+      fields: { social: { member: { handle: '@chief.bob.home', home: '@bob.home', ownerName: 'Bob' } } },
+    },
+  },
+  rooms: {},
+  exchanges: [],
+  exchangeTurns: {},
+  questions: {},
+}
+
+const settleShares = async () => {
+  for (let i = 0; i < 12; i += 1) await new Promise(resolve => setImmediate(resolve))
+}
+
+// #1167: adding a friend's Bot is allowed by the seat (an open room link, no person-grant).
+// Reach only treats current participants as reachable, so a leave sent after leftSeq never
+// arrives and the other home keeps the room.
+test('#1167: removing a friend\'s Bot drops that home\'s room, and later lines do not return', async (t) => {
+  const a = host(t, TEAM)
+  const b = host(t, BOB_HOME)
+  await a.rt.load()
+  await b.rt.load()
+  const invite = await a.rt.createInvitation({ createdByMemberId: 'owner-1' })
+  const alice = await a.rt.redeemInvitation(invite.token, { displayName: 'Alice' })
+  const actorA = await a.rt.authenticate(alice.sessionToken)
+  const inviteB = await b.rt.createInvitation({ createdByMemberId: 'owner-b' })
+  const bob = await b.rt.redeemInvitation(inviteB.token, { displayName: 'Bob' })
+  const actorB = await b.rt.authenticate(bob.sessionToken)
+  a.rt.state.bots['chief-a'].fields = { social: { member: { handle: '@chief.alice.home', home: '@alice.home', ownerName: 'Alice' } } }
+  a.rt.state.bots['chief-b'].fields = { social: { member: { handle: '@helper.alice.home', home: '@alice.home', ownerName: 'Alice' } } }
+  publish(a.rt, [{ handle: '@chief.bob.home', name: 'Chief', home: '@bob.home', homeName: 'Bob', owner: 'Bob', plugin: 'social', remote: true }])
+  // No person-grant. An open link is the consent that lets this home's Bots speak in the room.
+  a.rt.roomInviteServer = { mint: async () => ({ ticket: 'Tkt0001AbCdEfGhIjKlMnO', expiresAt: null }) }
+  await a.rt.mintRoomInvite('war', actorA)
+  const traffic = []
+  const wire = (from, to, home) => {
+    from.rt.flybotSend = async (msg) => {
+      const verdict = from.rt.reach(msg)
+      traffic.push({ op: parseRoomShare(msg.text)?.op, action: verdict.action, to: msg.to })
+      if (verdict.action !== 'allow') return { action: 'deny', reason: verdict.reason }
+      const localBotId = Object.keys(to.rt.state.bots).find(id => to.rt.state.bots[id]?.fields?.social?.member?.handle === msg.to)
+      const sender = from.rt.state.bots[msg.from]
+      await to.rt.applyInboundRoomShare({
+        text: msg.text,
+        fromHandle: sender?.fields?.social?.member?.handle,
+        fromHome: home,
+        fromName: sender?.fields?.social?.member?.ownerName || 'Alice',
+        localBotId,
+      })
+      return { action: 'sent' }
+    }
+  }
+  wire(a, b, '@alice.home')
+  wire(b, a, '@bob.home')
+  await a.rt.addRoomParticipant('war', { kind: 'bot', remote: true, handle: '@chief.bob.home' }, actorA)
+  await settleShares()
+  const guest = () => Object.values(b.rt.state.rooms).find(room => room.origin === 'inbound-share')
+  assert.ok(guest(), 'the add projected the room on the friend\'s home')
+  assert.equal(traffic.some(item => item.op === 'join' && item.action === 'allow'), true, 'the join left while the seat was live')
+  const hello = await a.rt.postRoomMessage('war', { text: '测1' }, actorA)
+  await a.rt.fanoutRoomLine(a.rt.state.rooms.war, hello)
+  await settleShares()
+  assert.ok(guest().log.some(line => String(line.text).includes('测1')), 'a line while seated arrives')
+
+  await a.rt.removeRoomParticipant('war', { kind: 'bot', id: '@chief.bob.home' }, actorA)
+  await settleShares()
+  const leave = traffic.findLast(item => item.op === 'leave')
+  assert.equal(leave?.action, 'allow', 'the leave is sent while the seat can still be reached')
+  assert.equal(guest().hidden, true, 'the friend\'s home hid the room')
+  assert.equal(roomVisibleTo(b.rt.state, actorB, guest().id), false, 'the room drops out of that home\'s list')
+
+  const after = await a.rt.postRoomMessage('war', { text: '踢后测4' }, actorA)
+  await a.rt.fanoutRoomLine(a.rt.state.rooms.war, after)
+  await settleShares()
+  assert.equal(guest().log.some(line => String(line.text).includes('踢后测4')), false, 'a line after the remove does not arrive')
+  const before = a.rt.state.rooms.war.log.length
+  await b.rt.fanoutRoomLine(guest(), { who: 'Bob', authorId: 'owner-b', authorKind: 'human', text: 'B踢后测5', seq: (guest().seq ?? 0) + 1 })
+  await settleShares()
+  assert.equal(a.rt.state.rooms.war.log.length, before, 'the removed home cannot send into the room')
+  assert.equal(traffic.findLast(item => item.op === 'line' && String(item.to).includes('alice'))?.action, undefined)
+})
+
+test('#1167: a hidden shared room stays hidden for a later line or a stale join, and a newer join reopens it', async (t) => {
+  const { rt } = host(t, TEAM)
+  await rt.load()
+  const { bob } = await signAliceAndBob(rt)
+  const actorB = await rt.authenticate(bob.sessionToken)
+  const share = (op, seq, extra = {}) => encodeRoomShare({
+    op, roomId: 'alice-war', name: 'War room', inviterName: 'Alice',
+    inviterHome: '@alice.home', inviterHandle: '@chief.alice.home', invitedHandle: '@chief.bob.home', seq, ...extra,
+  })
+  const deliver = text => rt.applyInboundRoomShare({
+    fromHandle: '@chief.alice.home', fromHome: '@alice.home', text, localBotId: 'chief-b',
+  }, actorB)
+  const joined = await deliver(share('join', 4))
+  const room = joined.room
+  assert.equal(room.hidden, undefined)
+  await deliver(share('leave', 8))
+  assert.equal(room.hidden, true, 'the leave hid the mirror')
+  const line = await deliver(share('line', 9, { who: 'Alice', authorKind: 'human', text: '踢后测4' }))
+  assert.equal(line.refused, 'left', 'a line after the leave is refused')
+  assert.equal(room.hidden, true, 'the line does not bring the room back')
+  assert.equal(room.log.some(entry => String(entry.text).includes('踢后测4')), false)
+  const stale = await deliver(share('join', 8, { roster: [{ handle: '@chief.alice.home', name: 'Chief', ownerName: 'Alice' }] }))
+  assert.equal(stale.refused, 'left', 'a join from before the remove is not a re-invite')
+  assert.equal(room.hidden, true)
+  const again = await deliver(share('join', 12))
+  assert.equal(again.refused, undefined, 'a later join is a new invite')
+  assert.equal(room.hidden, undefined, 'the room opens again')
+  assert.equal(roomVisibleTo(rt.state, actorB, room.id), true)
+})

core/test/room-ticket-seat.test.js

@@ -351,8 +351,9 @@ test('#892: withdrawing the link stops new joins and keeps the seat; removing th
   assert.equal(mirror.hidden, true, 'the guest heard the leave')
   assert.equal(a.rt.state.rooms.war.localInvites.length, 0, 'a withdrawn link that seats nobody is dropped')
   assert.equal(mayShareRoom(a.rt.state, a.rt.state.rooms.war, 'writer-a'), false, 'with no link left, Writer is private again')
+  const beforeSend = log.length
   await b.rt.fanoutRoomLine(mirror, { who: 'Chief', botId: 'chief-b', authorId: 'chief-b', authorKind: 'bot', text: 'still there?', seq: 100 })
-  assert.equal(log.at(-1).delivered, false)
+  assert.equal(log.length, beforeSend, 'the guest sends nothing once the host has removed them')
   const before = Object.keys(a.rt.state.rooms).length
   const stray = await a.rt.applyInboundRoomShare({ text: `FLYBOT-ROOM-SHARE\n${JSON.stringify({ v: 1, op: 'line', roomId: 'war', text: 'hi', seq: 1 })}`, fromHandle: '@chief.bob.home', fromHome: '@bob.home' })
   assert.equal(stray.refused, 'not-seated')