diff --git a/changelog.d/2622.fixed.md b/changelog.d/2622.fixed.md new file mode 100644 index 000000000..1bad5b828 --- /dev/null +++ b/changelog.d/2622.fixed.md @@ -0,0 +1 @@ +Renaming a Bot now moves the DSH Session title before the stored record, so a refused title write leaves the Bot, its group chats and `state.json` under the old name instead of showing the new one until a restart silently reverts it ([#2622](https://github.com/ThinkFlowLab/FlyBot/issues/2622)). diff --git a/core/src/host/roster.js b/core/src/host/roster.js index cbcc8d086..cc1558c5a 100644 --- a/core/src/host/roster.js +++ b/core/src/host/roster.js @@ -196,9 +196,11 @@ export function install(rt) { // A name saved before the rule stays usable until it changes. const problem = next === bot.name ? undefined : nameProblem(next) if (problem !== undefined) throw new Error(problem) - bot.name = next + // The title write is the one step here that can refuse, so the record moves only after + // it has: a refused rename leaves the Bot, its rooms and the store as they were (#2622). const agent = await rt.resolveAgent(rt.chatOf(id)) await ctx.sessionTitle.rename(agent.session, next) + bot.name = next await followMemberNames(Object.values(state.rooms).filter(room => room.members.includes(id))) } if (patch.role !== undefined) bot.role = cut(String(patch.role).trim(), 24).text diff --git a/core/test/roster.test.js b/core/test/roster.test.js index dcb3dbb42..5d006046c 100644 --- a/core/test/roster.test.js +++ b/core/test/roster.test.js @@ -186,6 +186,41 @@ test('a name and a role stop their bound before a split surrogate pair', async ( bounded(saved.role, reRole, 24) }) +// #2622: the record must not move before the Session title write does. A refused rename used to +// leave the live team renamed while the store still held the old name, and a restart reverted it. +test('a refused Session rename leaves the Bot under its old name, in the team and on disk', async (t) => { + const { call, stop, home, ctx, rt } = start(undefined, { models: ['deepseek-v4-flash'] }) + t.after(stop) + await call('state', {}) + const writer = (await call('create-bot', { name: 'Writer' })).value + await rt.savesSettled() + const titles = [] + const working = ctx.sessionTitle.rename + let refuse = true + ctx.sessionTitle.rename = async (session, title) => { + titles.push([session, title]) + if (refuse) throw new Error('EACCES: session rename refused') + return working(session, title) + } + const answer = await call('update-bot', { id: writer.id, name: 'Renamed' }) + assert.equal(answer.ok, false) + await rt.savesSettled() + const live = (await call('state', {})).value.bots.find(bot => bot.id === writer.id) + assert.equal(live.name, 'Writer', 'the team still lists the old name after a refused rename') + const stored = JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')).bots[writer.id] + assert.equal(stored.name, 'Writer', 'the store still holds the old name after a refused rename') + + // The same rename goes through once the title write works, and the new name reaches the host. + refuse = false + titles.length = 0 + assert.equal((await call('update-bot', { id: writer.id, name: 'Renamed' })).ok, true) + await rt.savesSettled() + const renamed = (await call('state', {})).value.bots.find(bot => bot.id === writer.id) + assert.equal(renamed.name, 'Renamed') + assert.equal(JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')).bots[writer.id].name, 'Renamed') + assert.deepEqual(titles.map(([, title]) => title), ['Renamed'], 'the host was asked to title the Session with the new name') +}) + // Creation is silent (#645): no path that makes a Bot opens a turn, and only a brief from a // Bot that creates one (`create_bot`) becomes instructions. A line typed in the new-Bot card // is the user's first request, sent as a turn by the client, never a standing rule. diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index 83a61cc4b..b36bac15b 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -355,7 +355,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js }) } -@@ -166,7 +166,7 @@ export function install(rt) { +@@ -166,18 +166,20 @@ export function install(rt) { const moving = patch.workDir !== undefined const workDir = moving ? await rt.cleanWorkDir(patch.workDir ?? '') : undefined if (patch.name !== undefined) { @@ -364,8 +364,14 @@ diff --git a/src/host/roster.js b/src/host/roster.js const clash = rt.findBot(next) if (next === '' || (clash && clash.id !== id)) throw new Error(`Name ${next} is not available`) // A name saved before the rule stays usable until it changes. -@@ -177,7 +177,7 @@ export function install(rt) { + const problem = next === bot.name ? undefined : nameProblem(next) + if (problem !== undefined) throw new Error(problem) +- bot.name = next ++ // The title write is the one step here that can refuse, so the record moves only after ++ // it has: a refused rename leaves the Bot, its rooms and the store as they were (#2622). + const agent = await rt.resolveAgent(rt.chatOf(id)) await ctx.sessionTitle.rename(agent.session, next) ++ bot.name = next await followMemberNames(Object.values(state.rooms).filter(room => room.members.includes(id))) } - if (patch.role !== undefined) bot.role = String(patch.role).trim().slice(0, 24) @@ -688,7 +694,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 @@ -698,9 +704,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 @@ -731,13 +737,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,54 @@ 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/0009-identity-schema.patch b/patches/ds-bot/0009-identity-schema.patch index 983481b99..8b851b54d 100644 --- a/patches/ds-bot/0009-identity-schema.patch +++ b/patches/ds-bot/0009-identity-schema.patch @@ -135,7 +135,7 @@ diff --git a/test/roster.test.js b/test/roster.test.js test('a new name must be one "@" can call; a name saved before the rule still saves', async (t) => { const { call, stop } = start(undefined, { models: ['deepseek-v4-flash'] }) t.after(stop) -@@ -129,85 +129,59 @@ test('a new name must be one "@" can call; a name saved before the rule still sa +@@ -129,85 +129,94 @@ test('a new name must be one "@" can call; a name saved before the rule still sa assert.equal((await call('update-bot', { id: writer.id, name: '新 Writer' })).ok, true) }) @@ -175,20 +175,10 @@ diff --git a/test/roster.test.js b/test/roster.test.js - if (kind === 'made by a Main Bot') assert.match(text, /Why you exist: Review the team notes\./) - }) -} -- --test('a first message from the user is helped with, not replaced by an opening question', async (t) => { -- const { call, stop, sent } = start(undefined, { models: ['deepseek-v4-flash'] }) +test('a name and a role stop their bound before a split surrogate pair', async (t) => { + const { call, stop } = start(undefined, { models: ['deepseek-v4-flash'] }) - t.after(stop) - await call('state', {}) -- const brief = 'Review the team notes and list the next tasks.' -- const bot = (await call('create-bot', { name: 'Writer', brief })).value -- await until(() => sent.some(entry => entry.sessionId === bot.id)) -- const text = sent.find(entry => entry.sessionId === bot.id).message.content[0].text -- assert.ok(text.includes(brief)) -- assert.match(text, /then help with it\./) -- assert.doesNotMatch(text, /\b(?:ask_user(?:_question)?|allowCustom)\b/) ++ t.after(stop) ++ await call('state', {}) + const name = `${'a'.repeat(39)}\u{1F600}tail` + const role = `${'b'.repeat(23)}\u{1F600}tail` + const writer = (await call('create-bot', { name, role })).value @@ -207,6 +197,50 @@ diff --git a/test/roster.test.js b/test/roster.test.js + assert.equal(saved.role, 'd'.repeat(23)) + bounded(saved.role, reRole, 24) +}) + +-test('a first message from the user is helped with, not replaced by an opening question', async (t) => { +- const { call, stop, sent } = start(undefined, { models: ['deepseek-v4-flash'] }) ++// #2622: the record must not move before the Session title write does. A refused rename used to ++// leave the live team renamed while the store still held the old name, and a restart reverted it. ++test('a refused Session rename leaves the Bot under its old name, in the team and on disk', async (t) => { ++ const { call, stop, home, ctx, rt } = start(undefined, { models: ['deepseek-v4-flash'] }) + t.after(stop) + await call('state', {}) +- const brief = 'Review the team notes and list the next tasks.' +- const bot = (await call('create-bot', { name: 'Writer', brief })).value +- await until(() => sent.some(entry => entry.sessionId === bot.id)) +- const text = sent.find(entry => entry.sessionId === bot.id).message.content[0].text +- assert.ok(text.includes(brief)) +- assert.match(text, /then help with it\./) +- assert.doesNotMatch(text, /\b(?:ask_user(?:_question)?|allowCustom)\b/) ++ const writer = (await call('create-bot', { name: 'Writer' })).value ++ await rt.savesSettled() ++ const titles = [] ++ const working = ctx.sessionTitle.rename ++ let refuse = true ++ ctx.sessionTitle.rename = async (session, title) => { ++ titles.push([session, title]) ++ if (refuse) throw new Error('EACCES: session rename refused') ++ return working(session, title) ++ } ++ const answer = await call('update-bot', { id: writer.id, name: 'Renamed' }) ++ assert.equal(answer.ok, false) ++ await rt.savesSettled() ++ const live = (await call('state', {})).value.bots.find(bot => bot.id === writer.id) ++ assert.equal(live.name, 'Writer', 'the team still lists the old name after a refused rename') ++ const stored = JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')).bots[writer.id] ++ assert.equal(stored.name, 'Writer', 'the store still holds the old name after a refused rename') ++ ++ // The same rename goes through once the title write works, and the new name reaches the host. ++ refuse = false ++ titles.length = 0 ++ assert.equal((await call('update-bot', { id: writer.id, name: 'Renamed' })).ok, true) ++ await rt.savesSettled() ++ const renamed = (await call('state', {})).value.bots.find(bot => bot.id === writer.id) ++ assert.equal(renamed.name, 'Renamed') ++ assert.equal(JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')).bots[writer.id].name, 'Renamed') ++ assert.deepEqual(titles.map(([, title]) => title), ['Renamed'], 'the host was asked to title the Session with the new name') ++}) + +// Creation is silent (#645): no path that makes a Bot opens a turn, and only a brief from a +// Bot that creates one (`create_bot`) becomes instructions. A line typed in the new-Bot card