host: one settings-page seat change sends the guest already seated two identical joins

core/src/host/room-membership.js

@@ -669,6 +669,10 @@ export function install(rt) {
         throw forbidden('Only a room admin can add participants')
       }
       const kind = draft.kind
+      // #2011: `updateRoom` sends its own join of the final name after the save, so a seat it changed must
+      // not also broadcast from here — the guests already seated would hear the same join twice. That join
+      // is not the targeted one to the arriving seat, which is still sent.
+      const broadcast = opts?.broadcast !== false
       const viaCode = opts?.viaCode === true && draft.kind === 'bot' && !isRemoteDraft(draft)
       // #923: a seat one of this room's links admitted names the pair reach.js enumerates. The link's
       // ticket, and the Bot of this home whose handle the guest knocked on, are what make that pair
@@ -758,17 +762,20 @@ export function install(rt) {
         void rt.notifyRoomShare?.(room, { op: 'join', handle: record.handle })
         // #2004: the arrival moved the name and only the arriving seat heard it; the guests already in
         // the room get the same name once, and the arriving seat keeps its one targeted join.
-        if (moved) reshareRoster(room, { except: record.handle })
-      } else if (record?.kind === 'bot' && !record.remote) reshareRoster(room)
+        if (moved && broadcast) reshareRoster(room, { except: record.handle })
+      } else if (record?.kind === 'bot' && !record.remote && broadcast) reshareRoster(room)
       return room
     })
   }
 
-  rt.removeRoomParticipant = function removeRoomParticipant(roomId, ref, actor) {
+  rt.removeRoomParticipant = function removeRoomParticipant(roomId, ref, actor, opts = {}) {
     return withRoomLock(async () => {
       const room = rt.roomOf(roomId)
       if (room === undefined) throw invalid('Unknown group chat')
       materialize(room)
+      // #2011: same as an add — `updateRoom` sends the final name itself, so a seat it removed does not
+      // broadcast again from here.
+      const broadcast = opts?.broadcast !== false
       const kind = ref.kind
       const id = ref.id || ref.handle
       const me = participantOf(room, kind, id)
@@ -813,12 +820,12 @@ export function install(rt) {
         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) {
         await finishTicketLeave(room).catch(error => rt.warn?.('ds-bot: leaving %s was not settled: %s', room.name, error))
-      } else if (kind === 'bot' && !me.remote) {
+      } else if (kind === 'bot' && !me.remote && broadcast) {
         reshareRoster(room)
       }
       // #2004: the leave moved the name, so the guests still in the room get the copy the host holds. The
       // seat that left is already out of `currentParticipants`; a local Bot's leave sent its roster above.
-      if (me.remote && moved) reshareRoster(room)
+      if (me.remote && moved && broadcast) reshareRoster(room)
       return room
     })
   }

core/src/host/roster.js

@@ -477,11 +477,13 @@ export function install(rt) {
         delete room.named
       }
     }
+    // #2011: the join below carries the final name to every guest, so a seat change made here must not
+    // broadcast from the seat path as well — the guest already seated would hear the same join twice.
     if (Array.isArray(patch.addParticipants)) {
-      for (const draft of patch.addParticipants) await rt.addRoomParticipant(room.id, draft, actor)
+      for (const draft of patch.addParticipants) await rt.addRoomParticipant(room.id, draft, actor, { broadcast: false })
     }
     if (Array.isArray(patch.removeParticipants)) {
-      for (const ref of patch.removeParticipants) await rt.removeRoomParticipant(room.id, ref, actor)
+      for (const ref of patch.removeParticipants) await rt.removeRoomParticipant(room.id, ref, actor, { broadcast: false })
     }
     if (!room.named) room.name = autoRoomName(room.members, room)
     const nameChanged = room.name !== before

core/test/remote-room.test.js

@@ -3590,6 +3590,60 @@ test('#2004: a seat change that moves the name reaches the guest already seated,
   assert.equal(titles.length, 4, 'the Session followed each of the four seat changes, no more')
 })
 
+// #2011: the settings page (`updateRoom`) sends one join of its own after the save, carrying the final
+// name; the seat paths it drives must not send a second one for the same change. Before this, a guest
+// already seated heard the same join twice on a settings-page add and on a settings-page remove.
+const settleEnvelopes = () => new Promise(resolve => setTimeout(resolve, 10))
+
+test('#2011: a settings-page seat change tells the guest already seated once', async (t) => {
+  const { rt, sent } = titledHost(t, TEAM)
+  await rt.load()
+  publish(rt, [CODER, WRITER])
+  await grantToFriend(rt, 'chief-a')
+  await grantToFriend(rt, 'chief-b')
+  const room = rt.state.rooms.war
+  delete room.named
+  room.name = rt.autoRoomName(room.members, room)
+  const guest = guestSide(sent)
+
+  await rt.addRoomParticipant('war', { kind: 'bot', remote: true, handle: CODER.handle }, ALICE)
+  guest.reads()
+
+  // A guest arrives through the settings page: the guest already seated ends on the host's name and is
+  // told once. `updateRoom` sends its own join without awaiting it, so let that envelope land first.
+  await rt.updateRoom('war', { addParticipants: [{ kind: 'bot', remote: true, handle: WRITER.handle }] }, ALICE.memberId, ALICE)
+  await settleEnvelopes()
+  let joins = guest.reads()
+  assert.equal(room.name, 'Alice, Chief A, Chief B, Coder, Writer')
+  assert.equal(guest.copies.get(CODER.handle), room.name, 'the seated guest ends on the name the host holds')
+  assert.equal(joins.get(CODER.handle), 1, 'a settings-page add costs the seated guest one join, not two')
+  assert.ok((joins.get(WRITER.handle) ?? 0) >= 1, 'the arriving seat still hears a join of its own')
+
+  // The same guest leaves through the settings page: still one join for the guest still seated.
+  await rt.updateRoom('war', { removeParticipants: [{ kind: 'bot', handle: WRITER.handle }] }, ALICE.memberId, ALICE)
+  await settleEnvelopes()
+  joins = guest.reads()
+  assert.equal(room.name, 'Alice, Chief A, Chief B, Coder')
+  assert.equal(guest.copies.get(CODER.handle), room.name, 'the guest still seated ends on the name the host holds')
+  assert.equal(joins.get(CODER.handle), 1, 'a settings-page remove costs the seated guest one join, not two')
+
+  // A local Bot's seat added the same way follows the same rule (shared with Bob, so it may sit with his Bot).
+  await grantToFriend(rt, 'spec-a')
+  await rt.updateRoom('war', { addParticipants: [{ kind: 'bot', id: 'spec-a' }] }, ALICE.memberId, ALICE)
+  await settleEnvelopes()
+  joins = guest.reads()
+  assert.equal(room.name, 'Alice, Chief A, Chief B, Writer A, Coder')
+  assert.equal(guest.copies.get(CODER.handle), room.name, 'a local seat moves the name for the seated guest too')
+  assert.equal(joins.get(CODER.handle), 1, 'a settings-page local add costs the seated guest one join, not two')
+
+  // #1949 still holds: a name asked for in the settings page is told to the guests, once.
+  await rt.updateRoom('war', { name: 'Renamed here' }, ALICE.memberId, ALICE)
+  await settleEnvelopes()
+  joins = guest.reads()
+  assert.equal(guest.copies.get(CODER.handle), 'Renamed here', 'an explicit rename still reaches the seated guest')
+  assert.equal(joins.get(CODER.handle), 1, 'and it is one join, not two')
+})
+
 test('#2004: a named group sends its seated guests nothing for a seat change', async (t) => {
   const { rt, titles, sent } = titledHost(t, TEAM)
   await rt.load()

patches/ds-bot/0002-flybot-api-host.patch

@@ -595,17 +595,19 @@ diff --git a/src/host/roster.js b/src/host/roster.js
      }
      // Members' group conversations move on to the folder at their next turn there.
      if (moving && workDir !== room.workDir) {
-@@ -371,20 +371,58 @@ export function install(rt) {
+@@ -371,20 +371,60 @@ export function install(rt) {
          delete room.named
        }
      }
 -    if (!room.named) room.name = autoRoomName(room.members)
 -    if (room.name !== before) {
++    // #2011: the join below carries the final name to every guest, so a seat change made here must not
++    // broadcast from the seat path as well — the guest already seated would hear the same join twice.
 +    if (Array.isArray(patch.addParticipants)) {
-+      for (const draft of patch.addParticipants) await rt.addRoomParticipant(room.id, draft, actor)
++      for (const draft of patch.addParticipants) await rt.addRoomParticipant(room.id, draft, actor, { broadcast: false })
 +    }
 +    if (Array.isArray(patch.removeParticipants)) {
-+      for (const ref of patch.removeParticipants) await rt.removeRoomParticipant(room.id, ref, actor)
++      for (const ref of patch.removeParticipants) await rt.removeRoomParticipant(room.id, ref, actor, { broadcast: false })
 +    }
 +    if (!room.named) room.name = autoRoomName(room.members, room)
 +    const nameChanged = room.name !== before

patches/ds-bot/files/src/host/room-membership.js

@@ -669,6 +669,10 @@ export function install(rt) {
         throw forbidden('Only a room admin can add participants')
       }
       const kind = draft.kind
+      // #2011: `updateRoom` sends its own join of the final name after the save, so a seat it changed must
+      // not also broadcast from here — the guests already seated would hear the same join twice. That join
+      // is not the targeted one to the arriving seat, which is still sent.
+      const broadcast = opts?.broadcast !== false
       const viaCode = opts?.viaCode === true && draft.kind === 'bot' && !isRemoteDraft(draft)
       // #923: a seat one of this room's links admitted names the pair reach.js enumerates. The link's
       // ticket, and the Bot of this home whose handle the guest knocked on, are what make that pair
@@ -758,17 +762,20 @@ export function install(rt) {
         void rt.notifyRoomShare?.(room, { op: 'join', handle: record.handle })
         // #2004: the arrival moved the name and only the arriving seat heard it; the guests already in
         // the room get the same name once, and the arriving seat keeps its one targeted join.
-        if (moved) reshareRoster(room, { except: record.handle })
-      } else if (record?.kind === 'bot' && !record.remote) reshareRoster(room)
+        if (moved && broadcast) reshareRoster(room, { except: record.handle })
+      } else if (record?.kind === 'bot' && !record.remote && broadcast) reshareRoster(room)
       return room
     })
   }
 
-  rt.removeRoomParticipant = function removeRoomParticipant(roomId, ref, actor) {
+  rt.removeRoomParticipant = function removeRoomParticipant(roomId, ref, actor, opts = {}) {
     return withRoomLock(async () => {
       const room = rt.roomOf(roomId)
       if (room === undefined) throw invalid('Unknown group chat')
       materialize(room)
+      // #2011: same as an add — `updateRoom` sends the final name itself, so a seat it removed does not
+      // broadcast again from here.
+      const broadcast = opts?.broadcast !== false
       const kind = ref.kind
       const id = ref.id || ref.handle
       const me = participantOf(room, kind, id)
@@ -813,12 +820,12 @@ export function install(rt) {
         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) {
         await finishTicketLeave(room).catch(error => rt.warn?.('ds-bot: leaving %s was not settled: %s', room.name, error))
-      } else if (kind === 'bot' && !me.remote) {
+      } else if (kind === 'bot' && !me.remote && broadcast) {
         reshareRoster(room)
       }
       // #2004: the leave moved the name, so the guests still in the room get the copy the host holds. The
       // seat that left is already out of `currentParticipants`; a local Bot's leave sent its roster above.
-      if (me.remote && moved) reshareRoster(room)
+      if (me.remote && moved && broadcast) reshareRoster(room)
       return room
     })
   }

patches/ds-bot/files/test/remote-room.test.js

@@ -3590,6 +3590,60 @@ test('#2004: a seat change that moves the name reaches the guest already seated,
   assert.equal(titles.length, 4, 'the Session followed each of the four seat changes, no more')
 })
 
+// #2011: the settings page (`updateRoom`) sends one join of its own after the save, carrying the final
+// name; the seat paths it drives must not send a second one for the same change. Before this, a guest
+// already seated heard the same join twice on a settings-page add and on a settings-page remove.
+const settleEnvelopes = () => new Promise(resolve => setTimeout(resolve, 10))
+
+test('#2011: a settings-page seat change tells the guest already seated once', async (t) => {
+  const { rt, sent } = titledHost(t, TEAM)
+  await rt.load()
+  publish(rt, [CODER, WRITER])
+  await grantToFriend(rt, 'chief-a')
+  await grantToFriend(rt, 'chief-b')
+  const room = rt.state.rooms.war
+  delete room.named
+  room.name = rt.autoRoomName(room.members, room)
+  const guest = guestSide(sent)
+
+  await rt.addRoomParticipant('war', { kind: 'bot', remote: true, handle: CODER.handle }, ALICE)
+  guest.reads()
+
+  // A guest arrives through the settings page: the guest already seated ends on the host's name and is
+  // told once. `updateRoom` sends its own join without awaiting it, so let that envelope land first.
+  await rt.updateRoom('war', { addParticipants: [{ kind: 'bot', remote: true, handle: WRITER.handle }] }, ALICE.memberId, ALICE)
+  await settleEnvelopes()
+  let joins = guest.reads()
+  assert.equal(room.name, 'Alice, Chief A, Chief B, Coder, Writer')
+  assert.equal(guest.copies.get(CODER.handle), room.name, 'the seated guest ends on the name the host holds')
+  assert.equal(joins.get(CODER.handle), 1, 'a settings-page add costs the seated guest one join, not two')
+  assert.ok((joins.get(WRITER.handle) ?? 0) >= 1, 'the arriving seat still hears a join of its own')
+
+  // The same guest leaves through the settings page: still one join for the guest still seated.
+  await rt.updateRoom('war', { removeParticipants: [{ kind: 'bot', handle: WRITER.handle }] }, ALICE.memberId, ALICE)
+  await settleEnvelopes()
+  joins = guest.reads()
+  assert.equal(room.name, 'Alice, Chief A, Chief B, Coder')
+  assert.equal(guest.copies.get(CODER.handle), room.name, 'the guest still seated ends on the name the host holds')
+  assert.equal(joins.get(CODER.handle), 1, 'a settings-page remove costs the seated guest one join, not two')
+
+  // A local Bot's seat added the same way follows the same rule (shared with Bob, so it may sit with his Bot).
+  await grantToFriend(rt, 'spec-a')
+  await rt.updateRoom('war', { addParticipants: [{ kind: 'bot', id: 'spec-a' }] }, ALICE.memberId, ALICE)
+  await settleEnvelopes()
+  joins = guest.reads()
+  assert.equal(room.name, 'Alice, Chief A, Chief B, Writer A, Coder')
+  assert.equal(guest.copies.get(CODER.handle), room.name, 'a local seat moves the name for the seated guest too')
+  assert.equal(joins.get(CODER.handle), 1, 'a settings-page local add costs the seated guest one join, not two')
+
+  // #1949 still holds: a name asked for in the settings page is told to the guests, once.
+  await rt.updateRoom('war', { name: 'Renamed here' }, ALICE.memberId, ALICE)
+  await settleEnvelopes()
+  joins = guest.reads()
+  assert.equal(guest.copies.get(CODER.handle), 'Renamed here', 'an explicit rename still reaches the seated guest')
+  assert.equal(joins.get(CODER.handle), 1, 'and it is one join, not two')
+})
+
 test('#2004: a named group sends its seated guests nothing for a seat change', async (t) => {
   const { rt, titles, sent } = titledHost(t, TEAM)
   await rt.load()