fix(remote-chat): key a remote chat by its room, so one published room is one row (#2797)

changelog.d/2797-remote-room-one-row.fixed.md

@@ -0,0 +1 @@
+A room a friend publishes once per member is one chat again: the friends' list keys a remote chat by the room — the same published name at the same home — instead of by the member handle it arrived on, so one group no longer fills the sidebar with a row per member and keeps its history when that handle changes ([#2797](https://github.com/ThinkFlowLab/FlyBot/issues/2797)).

core/src/host/remote-chat.js

@@ -139,11 +139,33 @@ export function remoteTurn(messages) {
   return { kind: 'none' }
 }
 
+// The chat a published record names (#2797). A room reaches this home once per member of that room:
+// every member record carries the same name — the room's own title — at the same home, and each one
+// used to open a chat of its own, so the friends' list showed one room as many rows. The chat belongs
+// to the room, not to the member handle it happened to arrive on. A record with no home, or none of
+// its own name, keeps the plain per-handle rule, and so does every older chat that recorded neither.
+const remoteNameKey = name => String(name ?? '').trim().replace(/\s+/g, ' ').toLowerCase()
+const roomKeyOf = ({ home, name }) => {
+  const at = normRemoteHandle(home)
+  // The name is cut where a chat's stored name is cut, so a long room title keys the same on both sides.
+  const label = remoteNameKey(cut(String(name ?? ''), 40).text)
+  return label && at.length > 1 ? `${at}|${label}` : ''
+}
+
 export function install(rt) {
   const { ctx, state } = rt
   const warn = (...args) => rt.warn?.(...args)
   const chats = () => Object.values(state.bots ?? {}).filter(isRemoteChat)
   const chatOfHandle = handle => chats().find(bot => normRemoteHandle(bot.remote.handle) === normRemoteHandle(handle))
+  // The chat already holding this room, if one is. Two plugins publishing the same title are still
+  // two chats, and a chat that recorded no plugin is not rewritten by one that does.
+  const chatOfRoom = ({ home, name, plugin }) => {
+    const key = roomKeyOf({ home, name })
+    if (!key) return undefined
+    return chats().find(bot => (plugin === undefined || bot.remote.plugin === undefined || bot.remote.plugin === plugin)
+      && roomKeyOf({ home: bot.remote.home, name: bot.name }) === key)
+  }
+  const chatRoomKey = bot => roomKeyOf({ home: bot?.remote?.home, name: bot?.name })
   const senders = new Map()
   const pending = new Map()
 
@@ -229,7 +251,7 @@ export function install(rt) {
     if (isHomeLevelRemote({ handle, home })) return Promise.resolve(undefined)
     const run = opening.then(async () => {
       await rt.load()
-      const existing = chatOfHandle(handle)
+      const existing = chatOfHandle(handle) ?? chatOfRoom({ home, name, plugin })
       if (existing !== undefined) {
         const next = { ...existing.remote, home, ...(homeName ? { homeName } : {}), plugin, gone: undefined }
         delete next.gone
@@ -261,15 +283,26 @@ export function install(rt) {
     const goneBefore = new Map()
     for (const bot of chats()) if (bot.remote.plugin === plugin) goneBefore.set(bot.id, Boolean(bot.remote.gone))
     const handles = new Set()
+    const rooms = new Set()
     for (const member of list) {
       if (isHomeLevelRemote(member)) continue
       handles.add(normRemoteHandle(member.handle))
+      // One room is published once per member: opening each member would give the room one row each
+      // (#2797), so the room is opened once and the rest of its members only keep it from going stale.
+      const room = roomKeyOf(member)
+      if (room) {
+        if (rooms.has(room)) continue
+        rooms.add(room)
+      }
       await open({ ...member, plugin }).catch(error => warn('ds-bot: chat for %s not opened: %s', member.handle, error))
     }
     let changed = false
     for (const bot of chats()) {
       if (bot.remote.plugin !== plugin) continue
-      const gone = isHomeLevelRemote(bot.remote) || !handles.has(normRemoteHandle(bot.remote.handle))
+      // A chat whose room is still published is not gone when the member handle that carried the room
+      // has changed: the publisher names a room by its members, and any of them speaks for it.
+      const room = chatRoomKey(bot)
+      const gone = isHomeLevelRemote(bot.remote) || (!handles.has(normRemoteHandle(bot.remote.handle)) && !(room && rooms.has(room)))
       const wasGone = goneBefore.get(bot.id) ?? Boolean(bot.remote.gone)
       if (wasGone !== gone) {
         bot.remote = { ...bot.remote, ...(gone ? { gone: true } : {}) }

core/test/remote-chat.test.js

@@ -366,3 +366,76 @@ test('P-321: a friend\'s Bot name from another home is cut at 40 code points on
   await rt.remoteChats.open(short)
   assert.equal(state.bots[sid].name, 'Bob')
 })
+
+// #2797: the publisher names one room once per member of that room — every member record carries
+// the same name (the room's own title) at the same home — so the friends' list showed one row per
+// member: the owner's screenshot has the same row eight times and reads it as one group. The chat
+// belongs to the room, not to the member handle it happens to arrive on.
+const ROOM = '123456 · 刘红升 2, flybot682b8a43, FlyBot'
+const roomHost = extra => {
+  const state = { bots: {} }
+  let created = 0
+  const rt = {
+    ...extra,
+    state, load: async () => {}, save: async () => {}, warn() {}, ctx: extra?.ctx ?? {},
+    // roster.createBot's own bound (roster.js:65), so the name kept is the one the app would keep.
+    createBot: async ({ name, model, remote }) => {
+      const bot = { id: `b${++created}`, name: cut(String(name ?? '').trim(), 40).text, model, remote }
+      state.bots[bot.id] = bot
+      return bot
+    },
+  }
+  install(rt)
+  return { state, rt, created: () => created }
+}
+const member = (n, name = ROOM, home = '@flybot682b8a43.home') => ({ handle: `@member${n}.flybot682b8a43.home`, name, home, homeName: 'FlyBot 682b8a43' })
+
+test('#2797: one room published once per member is one row in the list', async () => {
+  const { rt } = roomHost()
+  await rt.remoteChats.sync('social', [member(1), member(2), member(3), member(4)])
+  assert.equal(rt.remoteChats.list().length, 1, 'the same room is one chat, never one chat per member')
+  assert.equal(rt.remoteChats.list()[0].name, ROOM, 'and the row is the room, named the way the publisher names it')
+  // The second sync of the same room adds nothing either: same name, same home, same one row.
+  await rt.remoteChats.sync('social', [member(1), member(2)])
+  assert.equal(rt.remoteChats.list().length, 1, 'a repeated sync of one room is still one row')
+})
+
+test('#2797: a room whose member handle changes keeps its one chat instead of a vanished copy', async () => {
+  const { state, rt } = roomHost()
+  await rt.remoteChats.sync('social', [member(1)])
+  const id = rt.remoteChats.list()[0].id
+  // The publisher sends the same room through another member (another handle, the same room title).
+  await rt.remoteChats.sync('social', [member(7)])
+  assert.equal(rt.remoteChats.list().length, 1, 'the room is one row, not the old copy plus a new one')
+  assert.equal(rt.remoteChats.list()[0].id, id, 'and it is the same chat, so its history stays where it was')
+  assert.equal(state.bots[id].remote.gone, undefined, 'a room still published is never marked gone')
+})
+
+test('#2797: a line from another member of the room lands in the room\'s one chat', async () => {
+  const followups = []
+  let host
+  host = roomHost({
+    ctx: { sessions: { flush: async () => {} } },
+    botOf: id => host.state.bots[id],
+    chatOf: id => ({ id }),
+    resolveAgent: async () => ({ session: 's', followup: message => followups.push(message) }),
+  })
+  const rt = host.rt
+  await rt.remoteChats.sync('social', [member(1)])
+  const id = rt.remoteChats.list()[0].id
+  const landed = await rt.remoteChats.deliver({ ...member(2), plugin: 'social', text: 'hello room' })
+  assert.equal(landed, id, 'the member\'s line is the room\'s chat, not a second one')
+  assert.equal(rt.remoteChats.list().length, 1, 'so the list still holds one row')
+  assert.equal(followups.length, 1, 'and the line was delivered exactly once')
+})
+
+// The control: two friends, or two names, are still two chats — the room rule never folds a list.
+test('#2797: two names at one home, and one name at two homes, stay two rows', async () => {
+  const { rt } = roomHost()
+  await rt.remoteChats.sync('social', [
+    member(1, 'Helper'),
+    member(2, 'Coder'),
+    member(3, ROOM, '@flybot98b.home'),
+  ])
+  assert.equal(rt.remoteChats.list().length, 3, 'different names, and the same name at another home, are their own chats')
+})

docs/subsystems/core-host.md

@@ -485,7 +485,7 @@ has lines (#2790, `core/test/room-read-floor-clamp.test.js`).
 - A face-to-face room's presented Bot participants carry `label` = `remoteBotTitle`, `<Bot> (<owner>)`, for remote and local seats alike (`room-membership.js`, #384); `faceAliases` still answers to the older `<Bot> · <owner>'s Bot` / `<owner>'s Chief` labels.
 - A friend's DM that is waiting on a reply (`remoteAwaiting`, #443) is told when the other home's turn dies (#1068). `turns.js` `remoteFailureNotice` picks one of two fixed sentences — `Could not reply: this Bot has no model set up yet.` for `MISSING_CREDENTIAL`, `NO_KEY`, or a provider line that says the route has no API key, and `Could not reply: this Bot could not answer right now.` for every other `reason.kind === 'error'`. The provider's message is never copied (it may echo part of a key; `AUTH` is the generic sentence even when its text mentions a key). `flybotReplyRemote` carries `failure: true`. A stopped turn sends nothing. A line whose body is already one of those sentences does not earn another, so two homes that cannot run a model do not trade it. The sender's chat shows it as that Bot's reply and drops the weighing row, the same way any reply does (`remote-chat.js` `deliver`).
 - `remoteTurn` (`src/host/remote-chat.js`) finds what the user typed on a friend's remote chat by skipping host-appended `role:'user'` context: forms `snapshot`, `instructions` (the AGENTS.md baseline on a chat's first turn) and `notice`, and the time-context kinds. Match by form, not by a whitelist of senders, or the first message of a new chat is dropped (#66).
-- `remoteChats.sync(plugin, list)` (`src/host/remote-chat.js`) records that a chat's Bot left the published list as `bot.remote.gone` and never as a `bot.hidden` of its own (#956). The two are separate: `gone` is the platform's fact, `hidden` is the user's **Hide from sidebar** (`roster.setFlags`) and nothing else writes it. So a share that ends keeps its row in the sidebar (`isShownRemoteChat`, marked *no longer shared*) with its history and its way back, a hide the user set keeps the row out whether the Bot is published or not, and a Bot published again is an ordinary chat again with no leftover marker. It remembers which chats were `gone` before `open()` clears the marker, and takes back a `hidden` that a version before #956 wrote beside its `remote.hiddenByGone` marker on the next sync — and deletes the marker with it, because taking that hide back is its only use and one left behind would make a later **Hide from sidebar** read as ours and be deleted by the next sync — so an existing state file shows the row again without the user doing anything, and the user’s own hide works from then on. Covered by the `#956` cases in `core/test/remote-chat.test.js` and `core/test/new-chat.test.js`.
+- `remoteChats.sync(plugin, list)` (`src/host/remote-chat.js`) records that a chat's Bot left the published list as `bot.remote.gone` and never as a `bot.hidden` of its own (#956). The two are separate: `gone` is the platform's fact, `hidden` is the user's **Hide from sidebar** (`roster.setFlags`) and nothing else writes it. So a share that ends keeps its row in the sidebar (`isShownRemoteChat`, marked *no longer shared*) with its history and its way back, a hide the user set keeps the row out whether the Bot is published or not, and a Bot published again is an ordinary chat again with no leftover marker. It remembers which chats were `gone` before `open()` clears the marker, and takes back a `hidden` that a version before #956 wrote beside its `remote.hiddenByGone` marker on the next sync — and deletes the marker with it, because taking that hide back is its only use and one left behind would make a later **Hide from sidebar** read as ours and be deleted by the next sync — so an existing state file shows the row again without the user doing anything, and the user’s own hide works from then on. Covered by the `#956` cases in `core/test/remote-chat.test.js` and `core/test/new-chat.test.js`. The chat is the room's, not the member record's (#2797): a room reaches this home once per member of that room — every member record carries the same name, the room's own title, at the same home — so `open()` reuses the chat holding that name at that home and `sync()` opens the room once and keeps it live while any of its members is published; the stored `remote.handle` stays the one the chat was created with, because the chat''s model route names it. A record with no home, or no name of its own, and a chat that recorded neither, keep the plain per-handle rule, and two names at one home or one name at two homes stay two chats. Covered by the `#2797` cases in `core/test/remote-chat.test.js`.
 - `notSentLine` (`src/host/remote-chat.js`) is the one short line for a failed send to a friend's Bot. It names only steps a FlyBot owner can take (#404): a paused chat says to approve a waiting message or remove and re-add the friend (or, when the plugin's error says `resend`, to send the message again, which starts a new chat; there is no Extend and no turn limit), and the fallbacks end in *Try again.*; it never points to the Bot Social console, which a FlyBot owner cannot sign in to. A friend's Bot Bot Social no longer knows (`bot_not_found` / "no such remote bot") gets its own sentence — *the Bot is no longer shared with you, so messages to it cannot be delivered. Ask its owner for a new invite.* — which the social plugin also writes and the host reuses whole instead of translating again; the social plugin's "not shared with your Bots" sentence from a revoked share (`has not granted chat.public`, #971) is passed through whole the same way, and a raw `chat.public` refusal that arrives before that mapping still gets that one sentence rather than the server's words. A bare `capability_not_granted`, or the same code naming another capability, is not that fact (#988). No line carries *Try again.* twice (#954).
 - A friend chat waits on its reply (#443): a successful `forward` sets `state.remoteAwaiting[chatId]` (saved before the send turn ends), the friend's reply clears it as it arrives (`deliver`, before the reply is written into the chat) and again in the echo turn. A failed send never sets it. `channel.js` exposes it as `roster.remoteAwaiting`, and `authz.js` narrows it per owner like `remoteTypedTurns`.
 - Remote-chat `open` copies optional published `color` / `avatar` / `avatarUpdatedAt` onto the chat record (cached by `avatarUpdatedAt`) so headers, the picker, and group lists match Friends' Bots. A missing look stays handle-seeded (#137).

patches/ds-bot/files/src/host/remote-chat.js

@@ -139,11 +139,33 @@ export function remoteTurn(messages) {
   return { kind: 'none' }
 }
 
+// The chat a published record names (#2797). A room reaches this home once per member of that room:
+// every member record carries the same name — the room's own title — at the same home, and each one
+// used to open a chat of its own, so the friends' list showed one room as many rows. The chat belongs
+// to the room, not to the member handle it happened to arrive on. A record with no home, or none of
+// its own name, keeps the plain per-handle rule, and so does every older chat that recorded neither.
+const remoteNameKey = name => String(name ?? '').trim().replace(/\s+/g, ' ').toLowerCase()
+const roomKeyOf = ({ home, name }) => {
+  const at = normRemoteHandle(home)
+  // The name is cut where a chat's stored name is cut, so a long room title keys the same on both sides.
+  const label = remoteNameKey(cut(String(name ?? ''), 40).text)
+  return label && at.length > 1 ? `${at}|${label}` : ''
+}
+
 export function install(rt) {
   const { ctx, state } = rt
   const warn = (...args) => rt.warn?.(...args)
   const chats = () => Object.values(state.bots ?? {}).filter(isRemoteChat)
   const chatOfHandle = handle => chats().find(bot => normRemoteHandle(bot.remote.handle) === normRemoteHandle(handle))
+  // The chat already holding this room, if one is. Two plugins publishing the same title are still
+  // two chats, and a chat that recorded no plugin is not rewritten by one that does.
+  const chatOfRoom = ({ home, name, plugin }) => {
+    const key = roomKeyOf({ home, name })
+    if (!key) return undefined
+    return chats().find(bot => (plugin === undefined || bot.remote.plugin === undefined || bot.remote.plugin === plugin)
+      && roomKeyOf({ home: bot.remote.home, name: bot.name }) === key)
+  }
+  const chatRoomKey = bot => roomKeyOf({ home: bot?.remote?.home, name: bot?.name })
   const senders = new Map()
   const pending = new Map()
 
@@ -229,7 +251,7 @@ export function install(rt) {
     if (isHomeLevelRemote({ handle, home })) return Promise.resolve(undefined)
     const run = opening.then(async () => {
       await rt.load()
-      const existing = chatOfHandle(handle)
+      const existing = chatOfHandle(handle) ?? chatOfRoom({ home, name, plugin })
       if (existing !== undefined) {
         const next = { ...existing.remote, home, ...(homeName ? { homeName } : {}), plugin, gone: undefined }
         delete next.gone
@@ -261,15 +283,26 @@ export function install(rt) {
     const goneBefore = new Map()
     for (const bot of chats()) if (bot.remote.plugin === plugin) goneBefore.set(bot.id, Boolean(bot.remote.gone))
     const handles = new Set()
+    const rooms = new Set()
     for (const member of list) {
       if (isHomeLevelRemote(member)) continue
       handles.add(normRemoteHandle(member.handle))
+      // One room is published once per member: opening each member would give the room one row each
+      // (#2797), so the room is opened once and the rest of its members only keep it from going stale.
+      const room = roomKeyOf(member)
+      if (room) {
+        if (rooms.has(room)) continue
+        rooms.add(room)
+      }
       await open({ ...member, plugin }).catch(error => warn('ds-bot: chat for %s not opened: %s', member.handle, error))
     }
     let changed = false
     for (const bot of chats()) {
       if (bot.remote.plugin !== plugin) continue
-      const gone = isHomeLevelRemote(bot.remote) || !handles.has(normRemoteHandle(bot.remote.handle))
+      // A chat whose room is still published is not gone when the member handle that carried the room
+      // has changed: the publisher names a room by its members, and any of them speaks for it.
+      const room = chatRoomKey(bot)
+      const gone = isHomeLevelRemote(bot.remote) || (!handles.has(normRemoteHandle(bot.remote.handle)) && !(room && rooms.has(room)))
       const wasGone = goneBefore.get(bot.id) ?? Boolean(bot.remote.gone)
       if (wasGone !== gone) {
         bot.remote = { ...bot.remote, ...(gone ? { gone: true } : {}) }