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/1995-unnamed-title-follows-seats.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The title of a group without a chosen name follows its seats now ([#1995](https://github.com/ThinkFlowLab/FlyBot/issues/1995)): a friend's Bot invited to such a group appears in its name and in the group's Session title the moment the seat exists, and a seat that leaves drops out of both instead of staying in `state.json` until a restart. Before this, only a settings save or a renamed Bot recomputed that name, so inviting or removing a guest left the title reading the old set of people while the roster had already changed. A group created together with a guest was opened under the narrow name — `You, Chief A, Chief B`, with the guest missing — and was corrected in `state.json` without ever reaching the Session the user reads. A group with a chosen name keeps it.
21 changes: 21 additions & 0 deletions core/src/host/room-membership.js
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,22 @@ export function install(rt) {
else if (room.admin && !room.members.includes(room.admin)) room.admin = room.members[0] ?? null
}

// #1987: a group without a chosen name is named after its seats, so a seat that joins or leaves moves
// the name with it — and the group's own Session, the title the user reads, moves with the name. `room`
// is passed for both the comparison and the write: `autoRoomName` reads the people and the remote guests
// from it, so a call without one names the owner `You` and drops the guests, and that name would never
// match again (#1976). The name is set before the returned promise is awaited, so a path that cannot
// await still saves the right one; only the Session rename is deferred, and only when the name moved.
// A mirror of another home's room takes its name from that home's envelope and a face-to-face room has
// its own title, so neither is recomputed here.
const settleRoomName = (room) => {
if (room.named || room.origin === 'inbound-share' || isFaceRoom(room)) return Promise.resolve()
const next = rt.autoRoomName(room.members, room)
if (next === room.name) return Promise.resolve()
room.name = next
return rt.retitleRoom(room)
}

const materialize = (room) => {
if (!Array.isArray(room.participants)) room.participants = deriveParticipants(room, state)
return room
Expand Down Expand Up @@ -733,6 +749,7 @@ export function install(rt) {
}
}
syncMembers(room)
await settleRoomName(room)
await rt.save()
if (record?.remote && record.handle) void rt.notifyRoomShare?.(room, { op: 'join', handle: record.handle })
else if (record?.kind === 'bot' && !record.remote) reshareRoster(room)
Expand Down Expand Up @@ -783,6 +800,7 @@ export function install(rt) {
rt.dropGroupSession?.(room, id)
}
syncMembers(room)
await settleRoomName(room)
await rt.save()
if (me.remote && isTicketSeat(me)) {
await settleTicket(room, me.ticket, me).catch(error => rt.warn?.('ds-bot: a seat\'s leave was not settled: %s', error))
Expand Down Expand Up @@ -1769,6 +1787,7 @@ export function install(rt) {
})
}
syncMembers(room)
await settleRoomName(room)
// A poll reads this again every few seconds: a claim that is already seated must not rewrite the team.
if (fresh.length > 0 || legacy || homesBefore !== JSON.stringify(homesOf(invite))) await rt.save?.()
return { fresh }
Expand Down Expand Up @@ -1973,6 +1992,7 @@ export function install(rt) {
await withRoomLock(async () => {
markSeatLeft(room, seat)
syncMembers(room)
await settleRoomName(room)
})
await rt.save?.()
if (isTicketSeat(seat)) await settleTicket(room, seat.ticket, seat)
Expand Down Expand Up @@ -2250,6 +2270,7 @@ export function install(rt) {
if (room.seen) delete room.seen[me.id]
rt.dropGroupSession?.(room, me.id)
syncMembers(room)
await settleRoomName(room)
changed = true
})
}
Expand Down
4 changes: 4 additions & 0 deletions core/src/host/roster.js
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,10 @@ export function install(rt) {
rt.applyParticipants(state.rooms[id], drafts, actor)
if (!custom) {
state.rooms[id].name = autoRoomName(state.rooms[id].members, state.rooms[id])
// #1987: the Session was opened with the narrow name — the seats were not known yet, so a group
// made with a guest read `You, …` and no guest. The name above is the one that sees the room, so
// the Session takes it; the comparison is what keeps a room without guests at one rename.
if (state.rooms[id].name !== title) await rt.retitleRoom(state.rooms[id])
}
}
await rt.save()
Expand Down
139 changes: 136 additions & 3 deletions core/test/remote-room.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3304,9 +3304,10 @@ test('a rename that leaves the computed name alone sends nothing', async (t) =>
await rt.addRoomParticipant('war', { kind: 'bot', remote: true, handle: '@coder.bob.home' }, actorA)
sent.length = 0

const before = room.name
await rt.updateBot('spec-a', { name: 'Writer C' })

assert.equal(room.name, 'Alice, Chief A, Chief B', 'a Bot outside the group does not rename it')
assert.equal(room.name, before, 'a Bot outside the group does not rename it')
assert.equal(sent.filter(msg => msg.via === 'room-share').length, 0, 'the computed name did not move')
})

Expand All @@ -3330,9 +3331,10 @@ test('a restart migrates an unnamed room title with the formula that sees the ro
delete room.named
room.name = rt.autoRoomName(room.members, room)
assert.equal(room.name, 'Alice, Chief A, Chief B')
// The join path keeps the title it had; out of scope here and the reason the stored name is now stale.
await rt.addRoomParticipant('war', { kind: 'bot', remote: true, handle: '@coder.bob.home' }, actorA)
assert.equal(room.name, 'Alice, Chief A, Chief B', 'a join does not recompute the title')
// #1976's subject is the boot migration, not whichever path left the stored name stale, so the stale
// name is written here: a file from before a join recomputed the title (#1995 does now).
room.name = 'Alice, Chief A, Chief B'
await rt.save()
const saved = readFileSync(rt.statePath, 'utf8')

Expand Down Expand Up @@ -3378,3 +3380,134 @@ test('a restart migrates an unnamed room title with the formula that sees the ro
await fifth.rt.load()
assert.equal(readFileSync(fifth.rt.statePath, 'utf8'), healed, 'the healed file is left alone on the next boot')
})
// #1995: an unnamed group is named after its seats, so a seat that joins or leaves has to move the name
// with it — and the group's own Session, the title the user reads in the sidebar. `host()` above carries
// no session machinery, so the two module-level paths are added here: a workspace registry, a Session
// controller and a title recorder.
function titledHost(t, saved) {
const { rt } = host(t, saved)
const titles = []
const workspaces = new Map()
let sessions = 0
rt.config = {}
Object.assign(rt.ctx, {
workspaceRegistry: {
get: id => workspaces.get(id),
resolveByPath: async path => [...workspaces.values()].find(workspace => workspace.path === path),
create: async (path) => {
const workspace = { id: `w${workspaces.size + 1}`, path }
workspaces.set(workspace.id, workspace)
return workspace
},
archiveSession: async () => {},
},
sessionController: { create: async () => ({ sessionId: `s${++sessions}` }) },
sessionTitle: { rename: async (session, name) => { titles.push({ id: session?.id, name }) } },
})
installRoster(rt)
rt.resolveAgent = async id => ({ session: { id } })
return { rt, titles }
}

// The one owner of TEAM; authz is not enforced until a member session exists, so a plain actor is enough
// for the roster paths these cases drive.
const ALICE = { memberId: 'owner-1', displayName: 'Alice' }

test('#1995: a guest who joins an unnamed group moves the name and the Session title with it', async (t) => {
const { rt, titles } = titledHost(t, TEAM)
await rt.load()
publish(rt)
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)
titles.length = 0

await rt.addRoomParticipant('war', { kind: 'bot', remote: true, handle: '@coder.bob.home' }, ALICE)

assert.equal(room.name, 'Alice, Chief A, Chief B, Coder', 'the guest is in the name the moment the seat exists')
assert.deepEqual(titles.map(entry => entry.name), ['Alice, Chief A, Chief B, Coder'], 'the group Session follows, once')
})

test('#1995: a guest who leaves an unnamed group drops out of its name and title', async (t) => {
const { rt, titles } = titledHost(t, TEAM)
await rt.load()
publish(rt)
await grantToFriend(rt, 'chief-a')
await grantToFriend(rt, 'chief-b')
const room = rt.state.rooms.war
delete room.named
// The room as the join leaves it: the guest is seated and already in the name. The leave is what this
// case tests, so the invite path above is not needed to set it up.
rt.applyParticipants(room, [
{ kind: 'human', id: 'owner-1', role: 'admin', displayName: 'Alice' },
{ kind: 'bot', id: 'chief-a', role: 'admin' },
{ kind: 'bot', id: 'chief-b', role: 'member' },
{ kind: 'bot', remote: true, handle: '@coder.bob.home' },
], ALICE)
room.name = rt.autoRoomName(room.members, room)
assert.equal(room.name, 'Alice, Chief A, Chief B, Coder', 'the fixture starts from a name that names the guest')
titles.length = 0

await rt.removeRoomParticipant('war', { kind: 'bot', handle: '@coder.bob.home' }, ALICE)

assert.equal(room.name, 'Alice, Chief A, Chief B', 'the seat is gone from the name, not only from the roster')
assert.deepEqual(titles.map(entry => entry.name), ['Alice, Chief A, Chief B'], 'and the group Session lets it go, once')
})

test('#1995: a group created with a guest is titled by the formula that sees the room', async (t) => {
const { rt, titles } = titledHost(t, TEAM)
await rt.load()
publish(rt)
await grantToFriend(rt, 'chief-a')
await grantToFriend(rt, 'chief-b')

const room = await rt.createRoom({
members: ['chief-a', 'chief-b'],
participants: [
{ kind: 'bot', id: 'chief-a', role: 'admin' },
{ kind: 'bot', id: 'chief-b' },
{ kind: 'bot', remote: true, handle: '@coder.bob.home' },
],
actor: ALICE,
})

assert.equal(room.name, 'Alice, Chief A, Chief B, Coder', 'the new group is named with the guest in it')
// The Session has to exist before the seats do, so it is opened under the narrow name and corrected
// once the room knows its guests; what the user is left with is the name that sees the room.
assert.equal(titles.filter(entry => entry.id === room.id).at(-1)?.name, 'Alice, Chief A, Chief B, Coder',
'the Session ends on the name that sees the guest')
})

test('#1995: a named group keeps the name its owner chose, and no title is sent for it', async (t) => {
const { rt, titles } = titledHost(t, TEAM)
await rt.load()
publish(rt)
await grantToFriend(rt, 'chief-a')
await grantToFriend(rt, 'chief-b')
const room = rt.state.rooms.war
assert.equal(room.named, true, 'the fixture room carries a chosen name')
titles.length = 0

await rt.addRoomParticipant('war', { kind: 'bot', remote: true, handle: '@coder.bob.home' }, ALICE)
await rt.removeRoomParticipant('war', { kind: 'bot', handle: '@coder.bob.home' }, ALICE)

assert.equal(room.name, 'War room', 'a chosen name does not follow the seats')
assert.deepEqual(titles, [], 'and the Session is not renamed for a seat change')
})

test('#1995: an unnamed group without guests is renamed once per seat change', async (t) => {
const { rt, titles } = titledHost(t, TEAM)
await rt.load()
const room = rt.state.rooms.war
delete room.named
room.name = rt.autoRoomName(room.members, room)
titles.length = 0

await rt.addRoomParticipant('war', { kind: 'bot', id: 'spec-a' }, ALICE)

assert.equal(room.name, 'Alice, Chief A, Chief B, Writer A', 'the added member is in the name')
assert.equal(titles.length, 1, 'one seat change is one rename, not a second from the settings save')
assert.deepEqual(titles.map(entry => entry.name), ['Alice, Chief A, Chief B, Writer A'])
})
Loading
Loading