From 3c887a51a1907dd16b8121af8ed164498e8bb5e8 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Thu, 5 Nov 2026 09:28:31 +0800 Subject: [PATCH] Keep a face room's Hide from sidebar through the poll Every face snapshot the social plugin applies re-upserts the room, and `upsertFaceRoom` deleted `hidden` on any open room, so turning the switch on (or back off) sprang back a poll later (#2482). `setFlags` now stamps `room.hiddenAt` when a person hides a room and drops it when they unhide, and the upsert only clears `hidden` when that stamp is absent -- which still clears the stale flag #1605 is about, and leaves a closed or left room alone. One optional number on `state.rooms[id]`: no `STATE_VERSION` bump, no migration, and the client-visible fields and the API are unchanged. Fixes #2482 --- .../2482-face-room-hide-persist.fixed.md | 1 + core/src/host/room-membership.js | 5 +- core/src/host/roster.js | 11 +- core/test/face-room-hidden.test.js | 120 ++++++++++++++++++ docs/subsystems/core-host.md | 1 + patches/ds-bot/0002-flybot-api-host.patch | 33 +++-- .../ds-bot/files/src/host/room-membership.js | 5 +- .../files/test/face-room-hidden.test.js | 120 ++++++++++++++++++ 8 files changed, 280 insertions(+), 16 deletions(-) create mode 100644 changelog.d/2482-face-room-hide-persist.fixed.md create mode 100644 core/test/face-room-hidden.test.js create mode 100644 patches/ds-bot/files/test/face-room-hidden.test.js diff --git a/changelog.d/2482-face-room-hide-persist.fixed.md b/changelog.d/2482-face-room-hide-persist.fixed.md new file mode 100644 index 000000000..fddd4f762 --- /dev/null +++ b/changelog.d/2482-face-room-hide-persist.fixed.md @@ -0,0 +1 @@ +A face-to-face room's **Hide from sidebar** switch now holds: the hide a person chose survives the poll that re-reads the room every few seconds (and turning it back off survives too), while a `hidden` an older state left behind is still cleared by the room's open snapshot, as #1605 asked ([#2482](https://github.com/ThinkFlowLab/FlyBot/issues/2482)). diff --git a/core/src/host/room-membership.js b/core/src/host/room-membership.js index 9608a27d4..e37c04f58 100644 --- a/core/src/host/room-membership.js +++ b/core/src/host/room-membership.js @@ -2941,8 +2941,9 @@ export function install(rt) { // #1605: a face room this home is a member of is a row in the sidebar, never a hidden room. // A `hidden` left by an earlier state (a share that ended here, a room made before its snapshot // named it open) must not outlive an open or waiting snapshot: the API already lists the room, - // so the row has to be there too. - if (room.face.status !== 'closed' && room.face.status !== 'left') delete room.hidden + // so the row has to be there too. A hide the person chose keeps its `hiddenAt` stamp + // (roster.js `setFlags`), and the poll must not overrule it: this runs on every snapshot. + if (room.face.status !== 'closed' && room.face.status !== 'left' && !room.hiddenAt) delete room.hidden if (spec.name || isAutoFaceTitle(room.name)) room.name = title if (Array.isArray(spec.members)) rt.syncFaceParticipants(room, spec.members, spec.localBots) // When the lead is one of my Bots it is this room's admin here, so the round asks it first (#476). diff --git a/core/src/host/roster.js b/core/src/host/roster.js index e71ed8f13..0c8c98499 100644 --- a/core/src/host/roster.js +++ b/core/src/host/roster.js @@ -245,7 +245,16 @@ export function install(rt) { const entry = rt.botOf(id) ?? rt.roomOf(id) ?? ownEntry(state.agents, id) if (entry === undefined) throw new Error('Unknown conversation') if (typeof pinned === 'boolean') entry.pinned = pinned - if (typeof hidden === 'boolean') entry.hidden = hidden + if (typeof hidden === 'boolean') { + entry.hidden = hidden + // A room's hide is this switch's, and a face room is re-upserted on every poll; the stamp + // is what tells that write apart from the `hidden` an older state left behind, which + // `upsertFaceRoom` still clears (#1605, room-membership.js). + if (rt.roomOf(id) === entry) { + if (hidden) entry.hiddenAt = Date.now() + else delete entry.hiddenAt + } + } await rt.save() return entry } diff --git a/core/test/face-room-hidden.test.js b/core/test/face-room-hidden.test.js new file mode 100644 index 000000000..b7e853c77 --- /dev/null +++ b/core/test/face-room-hidden.test.js @@ -0,0 +1,120 @@ +// A face-to-face room's "Hide from sidebar" switch has to survive the room's own +// poll: every snapshot the social plugin applies re-upserts the room (`upsertFace`), +// and that write used to delete `hidden` on any open room, so the switch sprang back +// the moment the next poll landed. The rule now belongs to the person who hid the +// room, while a `hidden` an older state left behind (never chosen here) is still +// cleared by an open snapshot, which is what #1605 asked for. +import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { fakeContext } from './fake-ctx.js' +import { install as installStore } from '../src/host/store.js' +import { install as installRooms } from '../src/host/room-membership.js' +import { install as installRoster } from '../src/host/roster.js' + +const TEAM = { + version: 2, + revision: 1, + mainBotIds: ['chief-a'], + members: { + 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'chief-a' }, + }, + bots: { + 'chief-a': { id: 'chief-a', sessionId: 'chief-a', name: 'Chief A', instructions: '', createdAt: 1, ownerId: 'owner-1' }, + 'spec-a': { id: 'spec-a', sessionId: 'spec-a', name: 'Writer A', instructions: '', createdAt: 3, ownerId: 'owner-1' }, + }, + rooms: {}, + exchanges: [], + exchangeTurns: {}, + questions: {}, +} + +function host(t, saved) { + const dir = mkdtempSync(join(tmpdir(), 'ds-bot-face-hidden-')) + const statePath = join(dir, 'state.json') + if (saved !== undefined) writeFileSync(statePath, JSON.stringify(saved)) + const warnings = [] + const disposers = [] + const ctx = fakeContext() + ctx.logger = { warn: (...args) => warnings.push(args), info() {} } + ctx.effect = (execute) => { + const dispose = execute() + if (typeof dispose === 'function') disposers.push(dispose) + } + const rt = { + ctx, + home: dir, + statePath, + autoRoomName: () => 'Room', + retitleRoom: async () => {}, + dropGroupSession: () => {}, + warn: (...args) => warnings.push(args), + flybotSend: async () => ({ action: 'hold' }), + } + installStore(rt) + installRooms(rt) + installRoster(rt) + t.after(async () => { + await Promise.allSettled(disposers.splice(0).map(cleanup => cleanup())) + rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) + }) + return { rt, warnings } +} + +// The snapshot the poll applies: the room is open and this home's own Bot is seated. +const openSnapshot = () => ({ + roomId: 'frm_hide', + localId: 'face-hide', + status: 'open', + startedAt: '2026-11-05T01:00:00.000Z', + localBots: [{ id: 'spec-a', handle: '@writer.alice.home', name: 'Writer A' }], + members: [ + { handle: '@writer.alice.home', displayName: 'Writer', ownerName: 'Alice', published: false }, + { handle: '@coder.bob.home', displayName: 'Coder', ownerName: 'Bob', published: false }, + ], +}) + +test('the hide a person chose outlives the next face-room poll', async (t) => { + const { rt } = host(t, TEAM) + await rt.load() + const room = await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.face.status, 'open') + assert.equal(room.hidden, undefined, 'an open room starts visible in the sidebar') + + await rt.setFlags(room.id, { hidden: true }) + assert.equal(room.hidden, true, 'the switch hides the row') + + await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.hidden, true, 'the poll that re-upserts the open room keeps the hide') + + // A second poll, the way the plugin keeps going every couple of seconds. + await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.hidden, true, 'and so does the one after it') +}) + +test('turning the switch back off survives the polls too', async (t) => { + const { rt } = host(t, TEAM) + await rt.load() + const room = await rt.upsertFaceRoom(openSnapshot()) + await rt.setFlags(room.id, { hidden: true }) + await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.hidden, true) + + await rt.setFlags(room.id, { hidden: false }) + assert.equal(room.hidden, false) + await rt.upsertFaceRoom(openSnapshot()) + assert.notEqual(room.hidden, true, 'the row stays in the sidebar after the switch is turned off') +}) + +test('a hidden an earlier state left behind is still cleared, as #1605 asked', async (t) => { + const { rt } = host(t, TEAM) + await rt.load() + const room = await rt.upsertFaceRoom(openSnapshot()) + // No switch ever chose this one: it is the stale shape the earlier rule exists for. + room.hidden = true + await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.hidden, undefined, 'an open face room a home is a member of is never left hidden by a stale flag') + assert.equal(room.face.status, 'open') +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 63ab615d2..26e3627a2 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -345,6 +345,7 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi - Channel `send` on a face-to-face room (`origin: 'face-to-face'`) always `postRoomMessage`s, then shows a quiet local notice (`postToRoom` `quiet`+`notice`). It does not call `sessionController.prompt` unless a local Bot is @mentioned. Unsigned-in cloud UI uses `implicitRoomOwner` (the local human) so the line still fans out over `onFaceLine` / `FLYBOT-FACE-HUMAN` and the other FlyBot instance shows it; that path must not open a bot round (#124). Covered by `core/test/remote-room.test.js` and `core/test/rounds.test.js`. - A face room's auto title (`faceRoomFallbackTitle`) is built from every member the room has had (the poll's snapshot plus stored participants) and the creation time (`faceRoomCreatedAt`: `spec.createdAt`, else the joining deadline minus the 10-minute window, else the start; #194), so every home shows the same title and a member who left is not dropped from it (#182). - `upsertFaceRoom` keeps the six-digit number a room was joined by (`spec.joinCode` / `spec.code`) on `room.face.joinCode`, and the auto title uses it whenever a later upsert (a poll, a leave) does not repeat it, so the title never falls back from the number to a date (#304). +- A face room's `hidden` belongs to the person who hid its row (**Hide from sidebar**, `set-flags` → `roster.js` `setFlags`, which stamps `room.hiddenAt`): `upsertFaceRoom` runs on every poll, so without that stamp the write would delete the hide and the switch would spring back a poll later. A `hidden` carrying no stamp — the stale shape #1605 is about — is still cleared by an open or waiting snapshot, and a closed or left room keeps whatever it holds. Covered by `core/test/face-room-hidden.test.js`. - Another member's own face-room line arrives through `flybot.rooms.postHuman` (#180): a quiet `postToRoom` post carrying `source.human { authorId, name }`. `group_relay` turns it into a round reply `{ kind: 'user', other: true, name, authorId }` (not the grey `notice`), so it draws as a left-hand bubble from that person; it asks no Bot to answer. `postNotice` stays for system events (joined, left, started). Covered by `core/test/rounds.test.js`. - **Lines from elsewhere mid-turn (#455):** a `postHuman` or `postRemote` post (`source.human` / `source.remote`) that arrives while the group Session is `running` is not `followup`ed, which would park it in DSH's composer queue (raw `[Group post from …]`, steerable). `postToRoom` holds it (`heldPosts`): the round that is running (`rt.roomProgress`) shows it at once, as `personReply` or a `{ kind: 'remote' }` reply (left out of round reports), and a Bot's line is logged then (a person's line is in the log already through `applyFaceLine`); after `agent.whenIdle()` it is `followup`ed with `shown: true`, so its own round routes it by the usual rules but `group_relay` neither shows nor logs it again. With no round running it is only held and goes as before. The same hold covers this home's **own** Bot's line (#595: a late reply or a `post_to_group` that lands mid-turn showed the raw header in the composer queue): an ordinary line is logged and drawn as the ordinary `{ botId, name, color, seq }` bubble its own reply is, and a Bot's round event (`failed`, `stopped`, `resumed`) or other quiet post is held without being drawn, so its header never reaches the composer either and `group_relay` draws it as that event once the turn is done. A room notice (the `face-notice` sender) keeps the queue path: it is not a Bot's line. Covered by `core/test/rounds.test.js`. - `flybot.chatApprovals.changed()` saves the team (moving the revision) so `bot.pendingCount` reaches the browser at once when a hold arrives or is decided (#226). Covered by `core/test/flybot.test.js`. diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index 7fec50f6e..5facf538c 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -376,7 +376,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js if (patch.model !== undefined) { if (patch.model === null || patch.model === '') delete bot.model else bot.model = await rt.resolveModel(patch.model) -@@ -212,17 +212,11 @@ export function install(rt) { +@@ -212,20 +212,23 @@ export function install(rt) { await rt.save() if (modelChanged) await rt.applyModel(bot, { force: true }) if (moved) void rt.followFolder(bot.id) @@ -394,7 +394,20 @@ diff --git a/src/host/roster.js b/src/host/roster.js + const entry = rt.botOf(id) ?? rt.roomOf(id) ?? ownEntry(state.agents, id) if (entry === undefined) throw new Error('Unknown conversation') if (typeof pinned === 'boolean') entry.pinned = pinned - if (typeof hidden === 'boolean') entry.hidden = hidden +- if (typeof hidden === 'boolean') entry.hidden = hidden ++ if (typeof hidden === 'boolean') { ++ entry.hidden = hidden ++ // A room's hide is this switch's, and a face room is re-upserted on every poll; the stamp ++ // is what tells that write apart from the `hidden` an older state left behind, which ++ // `upsertFaceRoom` still clears (#1605, room-membership.js). ++ if (rt.roomOf(id) === entry) { ++ if (hidden) entry.hiddenAt = Date.now() ++ else delete entry.hiddenAt ++ } ++ } + await rt.save() + return entry + } @@ -235,11 +235,18 @@ export function install(rt) { // admin may change its own group but not delete it. Only a member can be admin. @@ -669,7 +682,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js next = others.with(at, id) } else if (primary) { next = [id, ...others] -@@ -417,14 +417,45 @@ export function install(rt) { +@@ -417,12 +417,43 @@ export function install(rt) { } // The sidebar shows every Main Bot, so a hidden Bot would vanish again once demoted. if (main) delete bot.hidden @@ -679,9 +692,9 @@ diff --git a/src/host/roster.js b/src/host/roster.js - } state.mainBotIds = next + rt.syncDesignatedChief() - await rt.save() - } - ++ await rt.save() ++ } ++ + // #638: retire a Bot instead of deleting it. The tree already reads fields['flybot-tree'].retired: + // adoption skips the record and it no longer counts against maxChildren, while its chat, its parts + // and its memory stay where they are. `hidden` keeps it out of the default Bot list the way the @@ -712,13 +725,11 @@ diff --git a/src/host/roster.js b/src/host/roster.js + delete state.grants[id] + await rt.reachChanged?.(id) + } -+ await rt.save() + await rt.save() + return { bot, freed } -+ } -+ + } + async function deleteBot(id) { - const bot = rt.botOf(id) - if (bot === undefined) throw new Error('Unknown Bot') @@ -434,22 +434,42 @@ export function install(rt) { const current = rt.chatOf(id) // Questions and exchange turns are kept per Session, so every part of the chat is cleared. diff --git a/patches/ds-bot/files/src/host/room-membership.js b/patches/ds-bot/files/src/host/room-membership.js index 9608a27d4..e37c04f58 100644 --- a/patches/ds-bot/files/src/host/room-membership.js +++ b/patches/ds-bot/files/src/host/room-membership.js @@ -2941,8 +2941,9 @@ export function install(rt) { // #1605: a face room this home is a member of is a row in the sidebar, never a hidden room. // A `hidden` left by an earlier state (a share that ended here, a room made before its snapshot // named it open) must not outlive an open or waiting snapshot: the API already lists the room, - // so the row has to be there too. - if (room.face.status !== 'closed' && room.face.status !== 'left') delete room.hidden + // so the row has to be there too. A hide the person chose keeps its `hiddenAt` stamp + // (roster.js `setFlags`), and the poll must not overrule it: this runs on every snapshot. + if (room.face.status !== 'closed' && room.face.status !== 'left' && !room.hiddenAt) delete room.hidden if (spec.name || isAutoFaceTitle(room.name)) room.name = title if (Array.isArray(spec.members)) rt.syncFaceParticipants(room, spec.members, spec.localBots) // When the lead is one of my Bots it is this room's admin here, so the round asks it first (#476). diff --git a/patches/ds-bot/files/test/face-room-hidden.test.js b/patches/ds-bot/files/test/face-room-hidden.test.js new file mode 100644 index 000000000..b7e853c77 --- /dev/null +++ b/patches/ds-bot/files/test/face-room-hidden.test.js @@ -0,0 +1,120 @@ +// A face-to-face room's "Hide from sidebar" switch has to survive the room's own +// poll: every snapshot the social plugin applies re-upserts the room (`upsertFace`), +// and that write used to delete `hidden` on any open room, so the switch sprang back +// the moment the next poll landed. The rule now belongs to the person who hid the +// room, while a `hidden` an older state left behind (never chosen here) is still +// cleared by an open snapshot, which is what #1605 asked for. +import assert from 'node:assert/strict' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { fakeContext } from './fake-ctx.js' +import { install as installStore } from '../src/host/store.js' +import { install as installRooms } from '../src/host/room-membership.js' +import { install as installRoster } from '../src/host/roster.js' + +const TEAM = { + version: 2, + revision: 1, + mainBotIds: ['chief-a'], + members: { + 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'chief-a' }, + }, + bots: { + 'chief-a': { id: 'chief-a', sessionId: 'chief-a', name: 'Chief A', instructions: '', createdAt: 1, ownerId: 'owner-1' }, + 'spec-a': { id: 'spec-a', sessionId: 'spec-a', name: 'Writer A', instructions: '', createdAt: 3, ownerId: 'owner-1' }, + }, + rooms: {}, + exchanges: [], + exchangeTurns: {}, + questions: {}, +} + +function host(t, saved) { + const dir = mkdtempSync(join(tmpdir(), 'ds-bot-face-hidden-')) + const statePath = join(dir, 'state.json') + if (saved !== undefined) writeFileSync(statePath, JSON.stringify(saved)) + const warnings = [] + const disposers = [] + const ctx = fakeContext() + ctx.logger = { warn: (...args) => warnings.push(args), info() {} } + ctx.effect = (execute) => { + const dispose = execute() + if (typeof dispose === 'function') disposers.push(dispose) + } + const rt = { + ctx, + home: dir, + statePath, + autoRoomName: () => 'Room', + retitleRoom: async () => {}, + dropGroupSession: () => {}, + warn: (...args) => warnings.push(args), + flybotSend: async () => ({ action: 'hold' }), + } + installStore(rt) + installRooms(rt) + installRoster(rt) + t.after(async () => { + await Promise.allSettled(disposers.splice(0).map(cleanup => cleanup())) + rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) + }) + return { rt, warnings } +} + +// The snapshot the poll applies: the room is open and this home's own Bot is seated. +const openSnapshot = () => ({ + roomId: 'frm_hide', + localId: 'face-hide', + status: 'open', + startedAt: '2026-11-05T01:00:00.000Z', + localBots: [{ id: 'spec-a', handle: '@writer.alice.home', name: 'Writer A' }], + members: [ + { handle: '@writer.alice.home', displayName: 'Writer', ownerName: 'Alice', published: false }, + { handle: '@coder.bob.home', displayName: 'Coder', ownerName: 'Bob', published: false }, + ], +}) + +test('the hide a person chose outlives the next face-room poll', async (t) => { + const { rt } = host(t, TEAM) + await rt.load() + const room = await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.face.status, 'open') + assert.equal(room.hidden, undefined, 'an open room starts visible in the sidebar') + + await rt.setFlags(room.id, { hidden: true }) + assert.equal(room.hidden, true, 'the switch hides the row') + + await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.hidden, true, 'the poll that re-upserts the open room keeps the hide') + + // A second poll, the way the plugin keeps going every couple of seconds. + await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.hidden, true, 'and so does the one after it') +}) + +test('turning the switch back off survives the polls too', async (t) => { + const { rt } = host(t, TEAM) + await rt.load() + const room = await rt.upsertFaceRoom(openSnapshot()) + await rt.setFlags(room.id, { hidden: true }) + await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.hidden, true) + + await rt.setFlags(room.id, { hidden: false }) + assert.equal(room.hidden, false) + await rt.upsertFaceRoom(openSnapshot()) + assert.notEqual(room.hidden, true, 'the row stays in the sidebar after the switch is turned off') +}) + +test('a hidden an earlier state left behind is still cleared, as #1605 asked', async (t) => { + const { rt } = host(t, TEAM) + await rt.load() + const room = await rt.upsertFaceRoom(openSnapshot()) + // No switch ever chose this one: it is the stale shape the earlier rule exists for. + room.hidden = true + await rt.upsertFaceRoom(openSnapshot()) + assert.equal(room.hidden, undefined, 'an open face room a home is a member of is never left hidden by a stale flag') + assert.equal(room.face.status, 'open') +})