diff --git a/changelog.d/2797-remote-room-one-row.fixed.md b/changelog.d/2797-remote-room-one-row.fixed.md new file mode 100644 index 000000000..20c339505 --- /dev/null +++ b/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)). diff --git a/core/src/host/remote-chat.js b/core/src/host/remote-chat.js index a9dd1d853..fdc30cbac 100644 --- a/core/src/host/remote-chat.js +++ b/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 } : {}) } diff --git a/core/test/remote-chat.test.js b/core/test/remote-chat.test.js index 6fd5ffc96..1f789bbf3 100644 --- a/core/test/remote-chat.test.js +++ b/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') +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index d31d1ec95..f2c789b50 100644 --- a/docs/subsystems/core-host.md +++ b/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`, ` ()`, for remote and local seats alike (`room-membership.js`, #384); `faceAliases` still answers to the older ` · 's Bot` / `'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). diff --git a/patches/ds-bot/files/src/host/remote-chat.js b/patches/ds-bot/files/src/host/remote-chat.js index a9dd1d853..fdc30cbac 100644 --- a/patches/ds-bot/files/src/host/remote-chat.js +++ b/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 } : {}) } diff --git a/patches/ds-bot/files/test/remote-chat.test.js b/patches/ds-bot/files/test/remote-chat.test.js index 6fd5ffc96..1f789bbf3 100644 --- a/patches/ds-bot/files/test/remote-chat.test.js +++ b/patches/ds-bot/files/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') +})