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/1976-store-auto-name.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A restart no longer renames an unnamed group to `You, …` or drops its remote guests ([#1976](https://github.com/ThinkFlowLab/FlyBot/issues/1976)). On load the host compared the stored title with the name computed from the room's people and guests, then wrote a title computed from the member Bots alone: the owner became `You`, every remote guest fell out of the name, and because that name could never match again the file and the group's Session title were rewritten on every boot. Both the comparison and the write now use the one name that sees the room, so a restart migrates an old title once and then leaves it alone.
15 changes: 12 additions & 3 deletions core/src/host/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,9 +311,18 @@ export function install(rt) {
room.notice ??= ''
migrated = true
}
// Unnamed groups from before names followed their members may still list old names.
const stale = Object.values(state.rooms).filter(room => !room.named && room.name !== rt.autoRoomName(room.members, room))
for (const room of stale) room.name = rt.autoRoomName(room.members)
// Unnamed groups from before names followed their members may still list old names. The name
// passes `room` for both the comparison and the write: `autoRoomName` reads the people and the
// remote guests from `room`, so a call without it names the owner `You` and drops remote guests,
// and that name would never match again on the next boot (#1976).
const stale = []
for (const room of Object.values(state.rooms)) {
if (room.named) continue
const next = rt.autoRoomName(room.members, room)
if (room.name === next) continue
room.name = next
stale.push(room)
}
// Identity schema (#47). A version-1 team had no member records — ownership was
// implicit in the single human — so it gets one initial owner and every Bot binds
// to it. readFileState already refused a legacy file that carries ownership fields
Expand Down
71 changes: 70 additions & 1 deletion core/test/remote-room.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert/strict'
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { test } from 'node:test'
Expand Down Expand Up @@ -3290,3 +3290,72 @@ test('a rename that leaves the computed name alone sends nothing', async (t) =>
assert.equal(room.name, 'Alice, Chief A, Chief B', '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')
})

// #1976: `load()` compared an unnamed group's stored name with the live formula (people and remote
// guests from `room.participants`) and then assigned the narrow one (`['You']`, no guests). A stored
// title the join path had not recomputed was therefore replaced by the wrong name, kept being stale on
// every later boot, and each boot rewrote the file and the group Session title.
test('a restart migrates an unnamed room title with the formula that sees the room (#1976)', async (t) => {
const first = host(t, TEAM)
const rt = first.rt
installRoster(rt)
rt.resolveAgent = async id => ({ session: id })
rt.ctx.sessionTitle = { rename: async () => {} }
await rt.load()
const { alice } = await signAliceAndBob(rt)
const actorA = await rt.authenticate(alice.sessionToken)
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)
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')
await rt.save()
const saved = readFileSync(rt.statePath, 'utf8')

// A restart: the same file, read by a second host.
const second = host(t, saved)
installRoster(second.rt)
second.rt.resolveAgent = async id => ({ session: id })
const titles = []
second.rt.ctx.sessionTitle = { rename: async (session, name) => { titles.push(name) } }
await second.rt.load()
assert.equal(second.rt.state.rooms.war.name, 'Alice, Chief A, Chief B, Coder', 'load() writes the live name, not `You, …` with the guest gone')
const migrated = readFileSync(second.rt.statePath, 'utf8')
assert.notEqual(migrated, saved, 'the migration is written once')
assert.deepEqual(titles, ['Alice, Chief A, Chief B, Coder'], 'the group Session gets that same name')

// The next boot finds the file already matching, so it neither renames nor rewrites: the loop stops.
const third = host(t, migrated)
installRoster(third.rt)
third.rt.resolveAgent = async id => ({ session: id })
const again = []
third.rt.ctx.sessionTitle = { rename: async (session, name) => { again.push(name) } }
await third.rt.load()
assert.equal(third.rt.state.rooms.war.name, 'Alice, Chief A, Chief B, Coder', 'a name that already matches is kept')
assert.equal(readFileSync(third.rt.statePath, 'utf8'), migrated, 'and load() writes nothing back')
assert.deepEqual(again, [], 'and does not rename the group Session again')

// Control: a file carrying the old narrow string is migrated to the live name, once.
const narrow = JSON.parse(migrated)
narrow.rooms.war.name = 'You, Chief A, Chief B'
const fourth = host(t, narrow)
installRoster(fourth.rt)
fourth.rt.resolveAgent = async id => ({ session: id })
fourth.rt.ctx.sessionTitle = { rename: async () => {} }
await fourth.rt.load()
assert.equal(fourth.rt.state.rooms.war.name, 'Alice, Chief A, Chief B, Coder', 'the narrow name is replaced by the live one')
const healed = readFileSync(fourth.rt.statePath, 'utf8')
assert.notEqual(healed, migrated, 'and that replacement is written')

const fifth = host(t, healed)
installRoster(fifth.rt)
fifth.rt.resolveAgent = async id => ({ session: id })
fifth.rt.ctx.sessionTitle = { rename: async () => {} }
await fifth.rt.load()
assert.equal(readFileSync(fifth.rt.statePath, 'utf8'), healed, 'the healed file is left alone on the next boot')
})
20 changes: 16 additions & 4 deletions patches/ds-bot/0002-flybot-api-host.patch
Original file line number Diff line number Diff line change
Expand Up @@ -998,13 +998,25 @@ diff --git a/src/host/store.js b/src/host/store.js
// LEGACY: Bots saved with a lab ran on that lab's default model; the model they
// last ran on becomes their own, so dropping the lab changes nothing for them.
if ('lab' in bot) {
@@ -161,12 +161,479 @@ export function install(rt) {
@@ -160,13 +160,489 @@ export function install(rt) {
room.notice ??= ''
migrated = true
}
// Unnamed groups from before names followed their members may still list old names.
- // Unnamed groups from before names followed their members may still list old names.
- const stale = Object.values(state.rooms).filter(room => !room.named && room.name !== rt.autoRoomName(room.members))
+ const stale = Object.values(state.rooms).filter(room => !room.named && room.name !== rt.autoRoomName(room.members, room))
for (const room of stale) room.name = rt.autoRoomName(room.members)
- for (const room of stale) room.name = rt.autoRoomName(room.members)
+ // Unnamed groups from before names followed their members may still list old names. The name
+ // passes `room` for both the comparison and the write: `autoRoomName` reads the people and the
+ // remote guests from `room`, so a call without it names the owner `You` and drops remote guests,
+ // and that name would never match again on the next boot (#1976).
+ const stale = []
+ for (const room of Object.values(state.rooms)) {
+ if (room.named) continue
+ const next = rt.autoRoomName(room.members, room)
+ if (room.name === next) continue
+ room.name = next
+ stale.push(room)
+ }
+ // Identity schema (#47). A version-1 team had no member records — ownership was
+ // implicit in the single human — so it gets one initial owner and every Bot binds
+ // to it. readFileState already refused a legacy file that carries ownership fields
Expand Down
71 changes: 70 additions & 1 deletion patches/ds-bot/files/test/remote-room.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert/strict'
import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'
import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { test } from 'node:test'
Expand Down Expand Up @@ -3290,3 +3290,72 @@ test('a rename that leaves the computed name alone sends nothing', async (t) =>
assert.equal(room.name, 'Alice, Chief A, Chief B', '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')
})

// #1976: `load()` compared an unnamed group's stored name with the live formula (people and remote
// guests from `room.participants`) and then assigned the narrow one (`['You']`, no guests). A stored
// title the join path had not recomputed was therefore replaced by the wrong name, kept being stale on
// every later boot, and each boot rewrote the file and the group Session title.
test('a restart migrates an unnamed room title with the formula that sees the room (#1976)', async (t) => {
const first = host(t, TEAM)
const rt = first.rt
installRoster(rt)
rt.resolveAgent = async id => ({ session: id })
rt.ctx.sessionTitle = { rename: async () => {} }
await rt.load()
const { alice } = await signAliceAndBob(rt)
const actorA = await rt.authenticate(alice.sessionToken)
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)
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')
await rt.save()
const saved = readFileSync(rt.statePath, 'utf8')

// A restart: the same file, read by a second host.
const second = host(t, saved)
installRoster(second.rt)
second.rt.resolveAgent = async id => ({ session: id })
const titles = []
second.rt.ctx.sessionTitle = { rename: async (session, name) => { titles.push(name) } }
await second.rt.load()
assert.equal(second.rt.state.rooms.war.name, 'Alice, Chief A, Chief B, Coder', 'load() writes the live name, not `You, …` with the guest gone')
const migrated = readFileSync(second.rt.statePath, 'utf8')
assert.notEqual(migrated, saved, 'the migration is written once')
assert.deepEqual(titles, ['Alice, Chief A, Chief B, Coder'], 'the group Session gets that same name')

// The next boot finds the file already matching, so it neither renames nor rewrites: the loop stops.
const third = host(t, migrated)
installRoster(third.rt)
third.rt.resolveAgent = async id => ({ session: id })
const again = []
third.rt.ctx.sessionTitle = { rename: async (session, name) => { again.push(name) } }
await third.rt.load()
assert.equal(third.rt.state.rooms.war.name, 'Alice, Chief A, Chief B, Coder', 'a name that already matches is kept')
assert.equal(readFileSync(third.rt.statePath, 'utf8'), migrated, 'and load() writes nothing back')
assert.deepEqual(again, [], 'and does not rename the group Session again')

// Control: a file carrying the old narrow string is migrated to the live name, once.
const narrow = JSON.parse(migrated)
narrow.rooms.war.name = 'You, Chief A, Chief B'
const fourth = host(t, narrow)
installRoster(fourth.rt)
fourth.rt.resolveAgent = async id => ({ session: id })
fourth.rt.ctx.sessionTitle = { rename: async () => {} }
await fourth.rt.load()
assert.equal(fourth.rt.state.rooms.war.name, 'Alice, Chief A, Chief B, Coder', 'the narrow name is replaced by the live one')
const healed = readFileSync(fourth.rt.statePath, 'utf8')
assert.notEqual(healed, migrated, 'and that replacement is written')

const fifth = host(t, healed)
installRoster(fifth.rt)
fifth.rt.resolveAgent = async id => ({ session: id })
fifth.rt.ctx.sessionTitle = { rename: async () => {} }
await fifth.rt.load()
assert.equal(readFileSync(fifth.rt.statePath, 'utf8'), healed, 'the healed file is left alone on the next boot')
})
Loading