Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/1962-room-settings-line.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A group settings change now leaves one line in the room: renaming the room, editing or clearing the notice, switching the reply mode, or naming a new admin or task lead posts a single quiet event that says what changed and who changed it, opens no Bot turn, and stays quiet when a patch writes back the values the room already holds or when an unnamed room's title follows its members ([#1962](https://github.com/ThinkFlowLab/FlyBot/issues/1962)).
35 changes: 33 additions & 2 deletions core/src/host/roster.js
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,10 @@ export function install(rt) {
const workDir = moving ? await rt.cleanWorkDir(patch.workDir ?? '') : undefined

const changes = []
// What the room itself says about this patch. A seat change already writes its own joined/left line,
// and a working folder is not something the others need a line about, so only the room's own settings
// are collected here: one patch draws at most one event.
const settings = []
const names = ids => ids.map(member => rt.botOf(member)?.name).filter(Boolean).join(', ')
const added = members.filter(member => !room.members.includes(member))
const removed = room.members.filter(member => !members.includes(member))
Expand All @@ -424,21 +428,30 @@ export function install(rt) {
}
room.members = members
}
if (admin !== (room.admin ?? null)) changes.push(admin ? `made ${rt.botOf(admin).name} the admin` : 'removed the admin')
if (admin !== (room.admin ?? null)) {
changes.push(admin ? `made ${rt.botOf(admin).name} the admin` : 'removed the admin')
// A seat change that empties the admin says "removed the admin" in `changes` alone: the room's line
// about it would repeat the joined/left line the seat change already writes.
if (patch.admin !== undefined) settings.push(admin ? `made ${rt.botOf(admin)?.name ?? 'someone'} the admin` : 'removed the admin')
}
room.admin = admin
if (taskLead !== (room.taskLead ?? null)) {
if (taskLead) room.taskLead = taskLead
else delete room.taskLead
changes.push(taskLead ? `made ${rt.botOf(taskLead).name} the task lead` : 'the admin leads the board again')
const lead = taskLead ? rt.botOf(taskLead).name : ''
changes.push(taskLead ? `made ${lead} the task lead` : 'the admin leads the board again')
if (patch.taskLead !== undefined) settings.push(taskLead ? `made ${lead} the task lead` : 'let the admin lead the board again')
}
if (patch.mode !== undefined && patch.mode !== room.mode) {
room.mode = patch.mode
changes.push(patch.mode === 'admin' ? 'plain messages now go to the admin first' : 'every member now replies in turn')
settings.push(patch.mode === 'admin' ? 'switched plain messages to the admin first' : 'switched the room to every member replying in turn')
}
const noticeChanged = notice !== undefined && notice !== (room.notice ?? '')
if (noticeChanged) {
room.notice = notice
changes.push(notice ? 'updated the notice' : 'cleared the notice')
settings.push(notice ? 'updated the group notice' : 'cleared the notice')
}
// Members' group conversations move on to the folder at their next turn there.
if (moving && workDir !== room.workDir) {
Expand All @@ -465,6 +478,9 @@ export function install(rt) {
const nameChanged = room.name !== before
if (nameChanged) {
changes.push(`renamed it "${room.name}"`)
// An unnamed room's title follows its members; that move is not a rename someone asked for, so only
// an explicit name in the patch draws a line.
if (patch.name !== undefined) settings.push(`renamed the room to “${room.name}”`)
await retitleRoom(room)
}
await rt.save()
Expand All @@ -473,9 +489,24 @@ export function install(rt) {
// (#1949), so a rename is not waiting on the next line and a save that changes both sends one envelope,
// not two. Sent after the save, like the join at creation; a save that changes neither sends nothing.
if (nameChanged || noticeChanged) void rt.notifyRoomShare?.(room, { op: 'join' })
// The same settings change is a line in the room itself: the envelope above tells the guests, this
// tells everyone already holding the room that something moved. It is the quiet room sender, not a
// Bot, so it draws as an event and opens no turn.
if (settings.length > 0) showSettings(room, settings, actor, actorId)
return { room, changes }
}

function showSettings(room, settings, actor, actorId) {
const who = actor?.displayName ?? rt.botOf(actorId)?.name ?? 'Someone'
const said = settings.length === 1 ? settings[0] : `${settings.slice(0, -1).join(', ')} and ${settings.at(-1)}`
try {
void Promise.resolve(rt.postToRoom?.(room, { id: 'face-notice', name: who }, `${who} ${said}`, { quiet: true, notice: true }))
.catch(error => rt.warn('ds-bot: the settings line for %s was not shown: %s', room.name, error))
} catch (error) {
rt.warn('ds-bot: the settings line for %s was not shown: %s', room.name, error)
}
}

async function deleteRoom(id) {
const room = rt.roomOf(id)
if (room === undefined) throw new Error('Unknown group chat')
Expand Down
67 changes: 67 additions & 0 deletions core/test/remote-room.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2436,6 +2436,73 @@ test('#1939: a notice the host edits reaches the guests already seated, once, an
assert.equal(applied.room.notice, '', 'the guest clears its copy too')
})

// P-303: a settings change wrote the state and the `changes` list and left the room itself silent, so
// nobody seated in it learned that the notice, the name, the reply mode, the admin or the task lead had
// moved. One patch is now at most one quiet notice line — the `face-notice` sender the client already
// draws as an event — and it opens no Bot turn. A patch that writes back the values already there says
// nothing, and a seat change keeps its own joined/left line instead of adding a settings echo.
test('a settings patch leaves one visible line in the room, and only when a value changed', async (t) => {
const { rt } = host(t, JSON.parse(JSON.stringify(TEAM)))
installRoster(rt)
await rt.load()
const invite = await rt.createInvitation({ createdByMemberId: 'owner-1' })
const alice = await rt.redeemInvitation(invite.token, { displayName: 'Alice' })
const actorA = await rt.authenticate(alice.sessionToken)
const events = []
rt.postToRoom = async (room, sender, text, extra) => { events.push({ room: room.id, sender, text, extra }) }
const count = () => events.length
const last = () => events.at(-1)

await rt.updateRoom('war', { name: 'Renamed war' }, undefined, actorA)
assert.equal(count(), 1, `one rename is one line: ${JSON.stringify(events)}`)
assert.deepEqual(last().sender, { id: 'face-notice', name: 'Alice' }, 'the line is the quiet room sender, not a Bot')
assert.equal(last().text, 'Alice renamed the room to “Renamed war”')
assert.deepEqual(last().extra, { quiet: true, notice: true }, 'quiet and a notice, so no Bot turn opens')

await rt.updateRoom('war', { name: 'Renamed war' }, undefined, actorA)
assert.equal(count(), 1, 'a name written back at its own value says nothing')

await rt.updateRoom('war', { notice: 'Stand up' }, undefined, actorA)
assert.equal(last().text, 'Alice updated the group notice')
await rt.updateRoom('war', { notice: '' }, undefined, actorA)
assert.equal(last().text, 'Alice cleared the notice')
await rt.updateRoom('war', { notice: '' }, undefined, actorA)
assert.equal(count(), 3, 'an empty notice written back at its own value says nothing')

await rt.updateRoom('war', { admin: 'chief-b' }, undefined, actorA)
assert.equal(last().text, 'Alice made Chief B the admin')
await rt.updateRoom('war', { admin: 'chief-b' }, undefined, actorA)
assert.equal(count(), 4, 'the admin set again to the same Bot says nothing')

await rt.updateRoom('war', { mode: 'admin' }, undefined, actorA)
assert.equal(last().text, 'Alice switched plain messages to the admin first')
await rt.updateRoom('war', { mode: 'everyone' }, undefined, actorA)
assert.equal(last().text, 'Alice switched the room to every member replying in turn')

await rt.updateRoom('war', { taskLead: 'chief-a' }, undefined, actorA)
assert.equal(last().text, 'Alice made Chief A the task lead')
await rt.updateRoom('war', { taskLead: null }, undefined, actorA)
assert.equal(last().text, 'Alice let the admin lead the board again')

// Two settings in one patch are still one line, both named, so a save never draws two events.
const before = count()
await rt.updateRoom('war', { name: 'Renamed again', notice: 'Sit down' }, undefined, actorA)
assert.equal(count(), before + 1, 'a rename and a notice change in one call are one line')
assert.equal(last().text, 'Alice updated the group notice and renamed the room to “Renamed again”')

// A seat change already has its joined/left line, and leaving with the admin seat empties it through the
// member list rather than through a settings field: neither draws a settings line.
const seats = count()
await rt.updateRoom('war', { add: ['spec-a'] }, undefined, actorA)
assert.equal(count(), seats, 'a seat change is not a settings line')
await rt.updateRoom('war', { remove: ['chief-b'] }, undefined, actorA)
assert.equal(count(), seats, 'a seat change that empties the admin is not a settings line either')

// The tool path passes no actor, only the Bot's own id: the line still says who did it.
await rt.updateRoom('war', { notice: 'From the tool' }, 'chief-a')
assert.equal(last().text, 'Chief A updated the group notice')
})

// #904: a roster is stranger-written text, so the envelope that carries one is gated: only the room's
// own inviter, only the room this mirror is bound to, and nothing it says widens reach by itself.
test('#904: a roster from another home or for another room changes no participant', async (t) => {
Expand Down
8 changes: 6 additions & 2 deletions core/test/task-drive.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,10 @@ test('only the task lead assigns, the card is claimed, and the assignee is woken
await app.rt.updateRoom('war', { taskLead: null }, 'a')
assert.equal(app.rt.roomOf('war').taskLead, undefined)
await app.rt.updateRoom('war', { taskLead: 's' }, 'a')
// The three settings patches above draw their own room lines; what this test guards is that the driver
// adds none of its own, so the count is taken after the setup and compared, not pinned to zero.
await new Promise(resolve => setImmediate(resolve))
const posted = app.calls('war').length

const room = app.rt.roomOf('war')
room.members = ['s', 'b']
Expand Down Expand Up @@ -415,7 +419,7 @@ test('only the task lead assigns, the card is claimed, and the assignee is woken
assert.match(text, /Task board \(assign, claim, and progress live here/)
assert.match(text, /Ship the note/)
assert.match(text, /Bob writes/)
assert.equal(app.calls('war').length, 0, 'the driver does not post the cue into the room')
assert.equal(app.calls('war').length, posted, 'the driver does not post the cue into the room')
assert.equal(app.calls('b').length, 1)

const linked = await app.track('b', { tasks: [{ title: 'Bob extra', parent: 'Ship the note', group: 'War' }] })
Expand All @@ -426,7 +430,7 @@ test('only the task lead assigns, the card is claimed, and the assignee is woken
app.emit('b', 'assistant/message', { message: { content: [{ type: 'text', text: 'Which file should the note use?' }] } })
app.emit('b', 'turn/end', { reason: { kind: 'completed' } })
await app.ctx.taskDrive.settled()
assert.equal(app.calls('war').length, 0, 'the own-chat reply is not posted into the group')
assert.equal(app.calls('war').length, posted, 'the own-chat reply is not posted into the group')
assert.equal(app.calls('b').length, 1, 'a wake that did not move the board does not start another')
})

Expand Down
Loading
Loading