diff --git a/changelog.d/tree-removed-root-reparent.fixed.md b/changelog.d/tree-removed-root-reparent.fixed.md new file mode 100644 index 000000000..b78da0531 --- /dev/null +++ b/changelog.d/tree-removed-root-reparent.fixed.md @@ -0,0 +1 @@ +Deleting a former Main Bot (after handing the role to another Bot) no longer flattens the team: its children move to the current root (or a fresh Root when there is none), so exactly one root remains and the new Chief's `list_children` and replies keep working ([#725](https://github.com/ThinkFlowLab/FlyBot/issues/725)). diff --git a/docs/subsystems/tree.md b/docs/subsystems/tree.md index 2732368cf..69f2af53b 100644 --- a/docs/subsystems/tree.md +++ b/docs/subsystems/tree.md @@ -49,7 +49,7 @@ Everything in `plugins/tree/` (entry `plugins/tree/index.js` and any helpers). ### Reorganising the tree: `move_bot` and adoption after a deletion (#635) - **`move_bot(bot, parent)`** (root only) moves an existing Bot with its subtree under another parent, found by id or by unique name, and refreshes the prompt of every Bot whose trace changed. It refuses a cycle (`cannot move X under its own descendant Y`), a no-op (`already a child of`), a duplicate sibling name, `maxChildren` at the target, and a subtree that would end deeper than `maxDepth` — the same invariants `create_child_bot` keeps, applied to a move. It never creates a Bot: deletion already re-attaches a removed Bot's children to its parent, and this is the sideways counterpart. -- **A deletion re-runs adoption.** `onBotRemoved` re-attaches the deleted Bot's children first, then calls `adoptUnattached` on the current root: a Bot that stayed outside the tree only because a level was full joins as soon as the deletion frees a slot, instead of waiting for the next startup. A Bot that still fits nowhere stays outside, logged, and is retried on the next pass. +- **A deletion re-runs adoption.** `onBotRemoved` re-attaches the deleted Bot's children first, then calls `adoptUnattached` on the current root: a Bot that stayed outside the tree only because a level was full joins as soon as the deletion frees a slot, instead of waiting for the next startup. A Bot that still fits nowhere stays outside, logged, and is retried on the next pass. Removing the **root** hands its children to the team's current Main (adopted as the root now), or to a fresh Root when there is no Main, so the team stays one tree instead of children left with `parent: null` that `isRoot` reads as extra roots. - Tests: `test/tree.test.js` — `#635: move_bot moves a bot with its subtree and refreshes the prompts`, `#635: move_bot refuses cycles, overflow, duplicates and a too deep subtree`, `#635: move_bot refuses an overflowing parent and refreshes the subtree`, and `#635: a Bot left outside because the tree was full joins when a deletion frees a slot`. ### Placement rules: forward allocation (#447, owner decisions 2026-10-19 ~19:45 and 21:27) diff --git a/plugins/tree/src/tree.js b/plugins/tree/src/tree.js index edaf0e9f6..a67f79b3b 100644 --- a/plugins/tree/src/tree.js +++ b/plugins/tree/src/tree.js @@ -292,8 +292,27 @@ export function createTree(fb, { limits, rootName = 'Root', rootBotId, adoptExis */ function onBotRemoved({ botId, parent }) { if (parent === undefined) return // not a tree bot - const up = parent ?? null - for (const child of childrenOf(snapshot(), botId)) fb.setField(child.id, 'parent', up) + const kids = childrenOf(snapshot(), botId) + // Removing a root must not leave its children with `parent: null`, which isRoot reads as a + // root: the team flattened into a forest, its list_children was empty, and a restart did not + // repair it. Hand them to the team's current Main instead, adopted as the root now. + let up = parent ?? null + if (parent === null) { + const chief = findChief() + if (chief) { up = chief.id; adoptChief() } + } + for (const child of kids) fb.setField(child.id, 'parent', up) + if (parent === null && up === null && kids.length > 0) { + // No Main to adopt (the core keeps at least one, so this is an imported/edge state): + // create a Root and move the children under it once it exists. + for (const child of kids) fb.setField(child.id, 'parent', undefined) + void ensureRoot().then((root) => { + if (typeof root !== 'string') return + for (const child of kids) fb.setField(child.id, 'parent', root) + organizer.repair() + refreshAll() + }).catch(error => warn('flybot-tree: could not re-root after %s was removed: %s', botId, error?.message ?? error)) + } organizer.repair() refreshAll() emit({ botId, key: 'removed' }) diff --git a/plugins/tree/test/tree.test.js b/plugins/tree/test/tree.test.js index b95e085bd..3e10e4dc2 100644 --- a/plugins/tree/test/tree.test.js +++ b/plugins/tree/test/tree.test.js @@ -65,6 +65,22 @@ test('tools: create under caller only, list, duplicates, depth 5, 8 children', a assert.match(sp(m, work), /COORDINATOR[\s\S]*- Dev: Dev stuff/) }) +test('removing a former root re-attaches its children to the current root', async () => { + const m = await setup() + const work = await make(m, m.root, 'Work') + const life = await make(m, m.root, 'Life') + // A second Main takes over; the old Chief is demoted and then deleted (the UI allows both). + const chief2 = m.addBot({ name: 'Chief2', isMain: true }) + m.bots.get(m.root).isMain = false + m.removeBot(m.root) + await new Promise(resolve => setTimeout(resolve, 0)) + assert.equal(f(m, work, 'parent'), chief2) + assert.equal(f(m, life, 'parent'), chief2) + const parentless = [...m.bots.values()].filter(bot => bot.fields['flybot-tree']?.parent == null) + assert.deepEqual(parentless.map(bot => bot.name), ['Chief2'], 'one root, not a forest') + assert.deepEqual((await m.call(chief2, 'list_children')).children.map(c => c.name).sort(), ['Life', 'Work']) +}) + test('tools: configurable limits and hidden from non-tree bots', async () => { const m = await setup({ maxDepth: 2, maxChildren: 1 }) const a = await make(m, m.root, 'A')