diff --git a/changelog.d/819-burst-reuse-seat.fixed.md b/changelog.d/819-burst-reuse-seat.fixed.md new file mode 100644 index 00000000..3b4bb945 --- /dev/null +++ b/changelog.d/819-burst-reuse-seat.fixed.md @@ -0,0 +1 @@ +A lead created for two simultaneous `create_child_bot` calls no longer claims a Bot it does not hold: a call that only reuses an existing Bot creates nothing, so it is no longer counted as a member of the group, and a burst of one new Bot plus one reused one makes no lead at all ([#819](https://github.com/ThinkFlowLab/FlyBot/issues/819)). diff --git a/docs/subsystems/tree.md b/docs/subsystems/tree.md index 2f92ad42..389237a9 100644 --- a/docs/subsystems/tree.md +++ b/docs/subsystems/tree.md @@ -62,7 +62,7 @@ Allocation puts a new Bot in a sensible place once; the self-organization below - **Reuse before create.** `find_bots(query?)` (read-only, every tree Bot) lists the Bots in the caller's branch (for Chief: the whole tree) with `id`, `name`, `role`, `parent`, `depth`, `path`, `children`, `lead` and `via` (the caller's direct child on the path, whom it delegates to). With a query, only matches, best first: same role (`sameRole`), the query in the name or role, shared role words (stems: "test" finds "Tester"), the same project, the same domain. Empty query: an overview (first 100). `create_child_bot` checks the whole branch first: a Bot with the same role at any depth is returned as `{ reused, note }` and nothing is created. A Bot outside the caller's branch is not looked at (the daily tidy merges duplicates across branches). - **Placement-aware creation.** `create_child_bot(name, role_description, parent?)`: `parent` (id or unique name) must be the caller or a descendant, else `parent must be you or one of your descendants`. Limits, depth and the sibling-name check apply at the target, under that parent's lock. Without `parent`, a **marked group lead** in the caller's branch whose group the new Bot belongs to takes it (most specific group first, then the shallower lead, when it has room): "FlyBot Docs" goes under "FlyBot Lead". A Bot that could lead that group itself, or a namesake, never goes under it; with no such lead the caller is the parent. The result names the parent and the tree path. - **Teams: lead plus children.** `create_team(team, members, parent?)` with `members` as `"Name: role"` strings (2 to `maxChildren`): `" Lead"` (role "Lead of the group: coordinates …", `lead { key, label }` from `roles.js`) under the caller or `parent`, and the members under it: `Chief → FlyBot Lead → {Dev, Test, Maintenance, Repo}`, one slot under Chief. An existing lead of that group in the caller's branch (marked for it, named "" / " Lead", or coordinating it) is reused and marked; a member with the same role already in the branch is reused, and moved under the lead when it sits flat next to it (that frees its slot, so a team fits at a full level when it reuses a flat member). Members that do not fit are listed as not created. A team is refused **before anything is written** when the lead's name or a member's name is already taken anywhere on the home — the host keeps names unique across the whole roster, not just under one parent — and the error names every clashing name with the path of the Bot that holds it; that is why a clash can no longer leave a childless ` Lead` behind ([#764](https://github.com/ThinkFlowLab/FlyBot/issues/764)). A team that moves or marks existing Bots is a journaled run (trigger `team`, one-click undo); a fresh team is a plain create. -- **Concurrent burst (#810).** Two `create_child_bot` calls from the same caller that are in flight at the same time — the second started before the first returned — and share the first group key become **one group**: one lead for that key, reused when `leadIn()` finds one and created once otherwise, with every member of the burst under it, so the second member does not fall into the broader pre-existing group the first one was heading for. The overlap is the definition: the seats are compared after one turn of the event loop, never after a timer or a settle window, and a call that starts after the previous one returned keeps the plain placement above. Members of a burst whose first keys differ are not forced together; an explicit `parent` keeps the caller's own placement; a role the vocabulary cannot place (`area:`) is not a group. `burstGrouping: false` turns it off. Limits are unchanged and each member is created through the lead's own per-parent lock, so a member that does not fit is refused with the existing message and one failure does not undo the others; no classifier and no model call takes part. +- **Concurrent burst (#810).** Two `create_child_bot` calls from the same caller that are in flight at the same time — the second started before the first returned — and share the first group key become **one group**: one lead for that key, reused when `leadIn()` finds one and created once otherwise, with every member of the burst under it, so the second member does not fall into the broader pre-existing group the first one was heading for. The overlap is the definition: the seats are compared after one turn of the event loop, never after a timer or a settle window, and a call that starts after the previous one returned keeps the plain placement above. Members of a burst whose first keys differ are not forced together; an explicit `parent` keeps the caller's own placement; a role the vocabulary cannot place (`area:`) is not a group. A call that reuses an existing Bot creates nothing, so it takes no seat for the roster: the lead of a burst holds exactly the members that were created in it, and a burst of one new member plus one reused makes no lead (the seat count decides, as it does for a serial call) (#819). `burstGrouping: false` turns it off. Limits are unchanged and each member is created through the lead's own per-parent lock, so a member that does not fit is refused with the existing message and one failure does not undo the others; no classifier and no model call takes part. - **Plan card**: not built. The codebase has no chat card for a model-proposed plan (approval cards belong to social's message holds), so forward allocation proceeds and names the resulting paths; deferral recorded on #447. Automatic reorganization never asks first. - **Prompts.** Chief (and every coordinator) is told: `find_bots` before creating and reuse what fits; a project with several roles is one lead with children (`create_team`); a new Bot of a group goes under its lead; at `maxChildren - 2` children or more, group related children under a lead instead of adding another flat one; name the tree path of every new Bot; a full level, orphans and duplicates are fixed by the automatic reorganization, without asking the owner first. - **Over capacity and orphans → #446.** A full target refuses with `limit reached` (pass a fitting `parent`, or let the tree regroup the level by role). A Bot made outside `create_child_bot` is placed by role (below). An **orphan** (its `parent` no longer in the tree) is a reparent candidate, never parked outside for good: each repair pass (startup, every `bot.created`, after a deletion) moves it with its subtree to the home its role points to (its group's lead, else the root) where it fits; a journaled, undoable move. One that fits nowhere yet is retried on the next pass. @@ -126,7 +126,7 @@ The plugin's discarded `.catch(() => {})` calls carry a one-line reason comment ## Tests -Covered by the plugin's own tests under `plugins/tree/` / core test suite as wired. The `#447` tests cover reuse of a same-role Bot at depth 3 and `find_bots` (scope, fields, ranking), `create_child_bot(parent)` refusing a non-descendant (sibling, ancestor, outsider, unknown) and checking limits at the target under its lock, `create_team` (Chief → FlyBot Lead → four members, one slot; reuse of the lead and members; a flat member's slot at a full level, journal and undo; a name that is already taken anywhere refusing the whole team with no Bot created, no journal entry and the tree byte-identical), later Bots joining their group's lead (only down leads of groups they belong to), and orphan reattachment; `test/host.test.js` runs `find_bots`, `create_team` and `create_child_bot(parent)` on the real host. The `#446` tests in `test/tree.test.js` cover role grouping (Life Lead, FlyBot Lead), existing leads, no unrelated fallback, the cap at every step, the daily tidy (merge, idle, archive), journal, undo and the client notice; `test/host.test.js` runs `create_bot` and the `tree/reorg` / `tree/undo` endpoints on the real host. `#16 Option B` in `test/tree.test.js` (and the matching host test) asserts a non-Main tree Bot can create a child under itself, that `maxChildren`/depth still refuse, and that the child is not auto-granted. Runtime load (#507) is covered by `test/load.test.js` (pure) and a `test/tree.test.js` case: a child's running turns show in the coordinator's stored prompt and `list_children`, and drop to zero at `turn.end`. The archived sidebar section and the concurrent burst (#810) are covered by `test/archived-section.test.js` (nothing at N=0, the collapsed header, a merged tag, a `归档-` name, a row opening the chat, and the structure entry still hiding all of them) and by `test/tree.test.js` against a fake `fb` that holds its creates open (one lead and two children for one burst, never a second lead, an existing lead reused, different keys and the `burstGrouping: false` control landing as before, a serial call unchanged, an explicit parent opting out). The age of an open turn (#624) is covered in both: `openFor` reads the oldest start and falls back to the newer turn once one ends, an end that names its turn closes that one and not the oldest (#696, unknown and missing ids included), the line names the age only past the threshold and never calls an idle child stalled, and `list_children` adds `openForMs`/`stalled` for an open turn while an idle child keeps the old shape. +Covered by the plugin's own tests under `plugins/tree/` / core test suite as wired. The `#447` tests cover reuse of a same-role Bot at depth 3 and `find_bots` (scope, fields, ranking), `create_child_bot(parent)` refusing a non-descendant (sibling, ancestor, outsider, unknown) and checking limits at the target under its lock, `create_team` (Chief → FlyBot Lead → four members, one slot; reuse of the lead and members; a flat member's slot at a full level, journal and undo; a name that is already taken anywhere refusing the whole team with no Bot created, no journal entry and the tree byte-identical), later Bots joining their group's lead (only down leads of groups they belong to), and orphan reattachment; `test/host.test.js` runs `find_bots`, `create_team` and `create_child_bot(parent)` on the real host. The `#446` tests in `test/tree.test.js` cover role grouping (Life Lead, FlyBot Lead), existing leads, no unrelated fallback, the cap at every step, the daily tidy (merge, idle, archive), journal, undo and the client notice; `test/host.test.js` runs `create_bot` and the `tree/reorg` / `tree/undo` endpoints on the real host. `#16 Option B` in `test/tree.test.js` (and the matching host test) asserts a non-Main tree Bot can create a child under itself, that `maxChildren`/depth still refuse, and that the child is not auto-granted. Runtime load (#507) is covered by `test/load.test.js` (pure) and a `test/tree.test.js` case: a child's running turns show in the coordinator's stored prompt and `list_children`, and drop to zero at `turn.end`. The archived sidebar section and the concurrent burst (#810) are covered by `test/archived-section.test.js` (nothing at N=0, the collapsed header, a merged tag, a `归档-` name, a row opening the chat, and the structure entry still hiding all of them) and by `test/tree.test.js` against a fake `fb` that holds its creates open (one lead and two children for one burst, never a second lead, an existing lead reused, different keys and the `burstGrouping: false` control landing as before, a serial call unchanged, an explicit parent opting out, and a burst whose member was reused — no lead for one new member, and no lead naming a Bot it does not hold (#819)). The age of an open turn (#624) is covered in both: `openFor` reads the oldest start and falls back to the newer turn once one ends, an end that names its turn closes that one and not the oldest (#696, unknown and missing ids included), the line names the age only past the threshold and never calls an idle child stalled, and `list_children` adds `openForMs`/`stalled` for an open turn while an idle child keeps the old shape. ## Common mistakes diff --git a/plugins/tree/README.md b/plugins/tree/README.md index c53b3d6a..9c0ddd39 100644 --- a/plugins/tree/README.md +++ b/plugins/tree/README.md @@ -14,7 +14,7 @@ Recursive bot tree plugin for FlyBot (built on ds-bot / DeepSeek Harness). - **Deleting a bot:** its children re-attach to its parent and every moved child (and its subtree) gets a fresh prompt/trace. No child is dropped. If that parent is now over `maxChildren`, it is regrouped by role automatically (#446), the same way as a full level, until it is within the limit. Each move is journaled and logged. - **`create_child_bot` is serialized per parent:** parallel calls never exceed `maxChildren` or create duplicate-name siblings. Two calls that are **in flight at the same time** and belong to the same project or domain become one group instead of each landing in whatever broader group it fits: one lead for that key (reused when it exists, created once otherwise) with both new Bots under it (#810). The overlap is the definition: one turn of the event loop, no timer, and a call that starts after the previous one returned keeps the ordinary placement. - Delegation is level by level: coordinators delegate to direct children whose scope fits and aggregate upward; a bot with no children does the work itself. A one-line trace like `Chief → Work → Dev` is added only to answers a tree bot sends to its parent tree bot; replies to the user or to a remote (friend's, Bot Social) bot never carry it, and the root (Chief) never adds one. -- Tools (visible to every tree bot, not only Main/root — #16 Option B): `list_children()`, `find_bots(query?)`, `create_child_bot(name, role_description, parent?)`, `create_team(team, members, parent?)` and — root only — `move_bot(bot, parent)` (#635), which moves an existing Bot with its subtree under another parent after checking for cycles, `maxChildren`, `maxDepth` and duplicate sibling names, then refreshes every prompt whose trace changed. Unlike `create_child_bot` it never creates a Bot; use it to reorganise a team, and note that deleting a Bot still re-attaches its children to its parent. A bot creates only in its own downstream: `parent` (id or name) must be the caller or one of its descendants, and limits are checked at that parent. **Forward allocation (#447, `src/allocate.js`):** `find_bots` (read-only) lists the bots in the caller's branch (Chief: the whole tree) with role, parent, depth and path, matches first; `create_child_bot` reuses a bot with the same role anywhere in the caller's branch instead of creating a duplicate (`{ reused, note }`), and without `parent` puts the new bot under a marked group lead in the branch that its role belongs to ("FlyBot Docs" under "FlyBot Lead"), else under the caller; `create_team` makes a project team as a lead plus children (`Chief → FlyBot Lead → {Dev, Test, …}`, `members` as `"Name: role"` strings), one slot under the parent, reusing an existing lead and same-role members (a flat one moves under the lead, journaled and undoable). Allocation uses the self-organization's primitives (role profiles, leads, moves, journal); a concurrent burst of two or more `create_child_bot` calls in one group is placed as one leader plus members (#810), and an explicit `parent` keeps the caller's own placement; a full level, orphans (a bot whose parent is gone is reattached by role on the next repair pass) and duplicates are left to the self-organization, which never asks first. There is no plan card. Limits: depth 5 (root = 1), 8 children per node; duplicate sibling names (case/space-insensitive) are rejected; names are 1–40 characters (ds-bot's roster limit). Every new child gets the same recursive system prompt template (`src/prompt.js`). A new child starts with no person-grant even if its parent is granted; sharing it still needs an independent Sharing → Copy invite link (default deny). +- Tools (visible to every tree bot, not only Main/root — #16 Option B): `list_children()`, `find_bots(query?)`, `create_child_bot(name, role_description, parent?)`, `create_team(team, members, parent?)` and — root only — `move_bot(bot, parent)` (#635), which moves an existing Bot with its subtree under another parent after checking for cycles, `maxChildren`, `maxDepth` and duplicate sibling names, then refreshes every prompt whose trace changed. Unlike `create_child_bot` it never creates a Bot; use it to reorganise a team, and note that deleting a Bot still re-attaches its children to its parent. A bot creates only in its own downstream: `parent` (id or name) must be the caller or one of its descendants, and limits are checked at that parent. **Forward allocation (#447, `src/allocate.js`):** `find_bots` (read-only) lists the bots in the caller's branch (Chief: the whole tree) with role, parent, depth and path, matches first; `create_child_bot` reuses a bot with the same role anywhere in the caller's branch instead of creating a duplicate (`{ reused, note }`), and without `parent` puts the new bot under a marked group lead in the branch that its role belongs to ("FlyBot Docs" under "FlyBot Lead"), else under the caller; `create_team` makes a project team as a lead plus children (`Chief → FlyBot Lead → {Dev, Test, …}`, `members` as `"Name: role"` strings), one slot under the parent, reusing an existing lead and same-role members (a flat one moves under the lead, journaled and undoable). Allocation uses the self-organization's primitives (role profiles, leads, moves, journal); a concurrent burst of two or more `create_child_bot` calls in one group is placed as one leader plus members (#810), a call that only reuses an existing Bot is not counted as a member of that group (#819), and an explicit `parent` keeps the caller's own placement; a full level, orphans (a bot whose parent is gone is reattached by role on the next repair pass) and duplicates are left to the self-organization, which never asks first. There is no plan card. Limits: depth 5 (root = 1), 8 children per node; duplicate sibling names (case/space-insensitive) are rejected; names are 1–40 characters (ds-bot's roster limit). Every new child gets the same recursive system prompt template (`src/prompt.js`). A new child starts with no person-grant even if its parent is granted; sharing it still needs an independent Sharing → Copy invite link (default deny). - Frontend (`client.js`, bundle id `@flybot/tree`, injects `flybot`): the tree is drawn by ds-bot's own Bot list, so this plugin's separate "Tree" sidebar section is **off by default** (`sidebarSection: true` in the client config brings it back). It always exposes the synchronous helper `ctx.flybot.tree.forest(bots)` (also `globalThis.__flybot__.tree.forest(bots)`), see **Tree data** below. It also registers `flybot.sidebar.structure` (`{ parentOf(botId) }`, parents from `forestOf` over `ctx.flybot.bots()`, refreshed on field changes and every 10 s); ds-bot's sidebar uses it to draw its Bot list as one expandable tree with Chief on top. Retired, archived and outside Bots and other Main Bots have no parent there, so ds-bot shows them directly under Chief; the entry's `nodeActions` draws a tag after such a row ("archived", "archived · merged into …", "outside the tree") and after an idle Bot ("idle"), so the sidebar and the tree model agree. A `flybot.sidebar.section` entry `flybot-tree-archived` lists the archived Bots the Bot list hides (#810): collapsed by default, header `Archived (N)`, nothing at all while N is 0, one row per Bot carrying the same tag (`archived`, or `archived · merged into …` after a merge) that opens that Bot's chat through `actions.openSession`; expanding is local state, so the 10 s refresh re-renders the rows without closing it (`archivedSection: false` hides it). It is registered with `order: 88` and the reorganization notice with `order: 89`: a slot renders in ascending order, so the archived list sits **above** the notice (a smaller number is higher) instead of the card pushing it down. The structure entry and the archived list read `ctx.flybot.bots()` through **one** shared feed (`rosterFeed`, `{ interval }`): the first subscriber starts the poll, later ones join it without a read of their own, a field change or a removal asks for an immediate read (`refresh()`), and a mounted sidebar asks the host once per 10 s tick — not once per consumer (#816). A `flybot.sidebar.section` entry `flybot-tree-reorg` shows the last reorganization (what moved) with Undo and Dismiss, through the host endpoints `tree/reorg`, `tree/undo`, `tree/dismiss`; it draws nothing when there is none in the last 7 days (`reorgNotice: false` in the client config hides it). The entry also has `menuItems({ bot })`, which ds-bot adds to each Bot's right-click menu: one item, `{ id: 'copy-invite', label: 'Copy invite link' }` (with `visible`, `run`); the invite with instructions is offered in the Bot's Sharing section only, so the menu does not list the same invite twice (#173). `visible` is `social.canInvite(bot.id) === true` (false without the social plugin); `run` calls `social.createInvite(botId)` (or `flybot.call('social.createInvite', { botId })`). **Copy invite link** copies only the bare URL (`link`); `copyInvite(flybot, botId, 'text')` copies the full message `text` for other callers; a missing field is an error, never swapped for the other. The clipboard write starts inside the click via `ClipboardItem` with the text as a promise, so it keeps the click's permission while the invite is created; `writeText` then a textarea are the fallbacks. It returns "Invite link for copied", which ds-bot shows as a toast; a failure throws and shows as the error toast. Right-clicking never opens the Bot's chat. ### Config diff --git a/plugins/tree/src/allocate.js b/plugins/tree/src/allocate.js index e54675b2..922ccac3 100644 --- a/plugins/tree/src/allocate.js +++ b/plugins/tree/src/allocate.js @@ -117,6 +117,11 @@ export function createAllocator({ fb, limits, snapshot, childrenOf, depthOf, tra const first = keysOf(profileFor(String(name ?? '').trim(), role))[0] const key = burst && kindOf(first) !== 'area' && !(typeof parentRef === 'string' && parentRef.trim()) ? first : undefined if (!key) return placeAlone(callerId, name, role, parentRef) + // A call that will reuse an existing Bot creates nothing, so it is not a member of the group: taking + // a seat for it made the lead's roster name a Bot it never held (#819). planChild is a read; + // placeAlone and joinGroup plan again under the lock they take. + const plan = planChild(callerId, name, role, parentRef) + if (plan.reused) return plan const seat = { key, name: String(name ?? '').trim(), group: undefined } const seats = burstSeats.get(callerId) ?? [] seats.push(seat) diff --git a/plugins/tree/test/tree.test.js b/plugins/tree/test/tree.test.js index ff6dbbba..306fcbf5 100644 --- a/plugins/tree/test/tree.test.js +++ b/plugins/tree/test/tree.test.js @@ -2046,3 +2046,35 @@ test('#816: one roster read per tick, shared by the structure entry and the arch stop() for (const stopEffect of cleanups.splice(0)) { try { stopEffect?.() } catch { /* already gone */ } } }) + +// #819: a call that reuses an existing Bot creates nothing, so it is not a member of the burst's +// group. It used to take a seat before the reuse was known, so the lead's roster named a Bot it never +// held: `FlyBot Lead` was made for "FlyBot Programmer, FlyBot Tester" with only the Tester under it. +test('#819: a burst whose member is reused does not seat it, so the lead never names a Bot it does not hold', async () => { + const f = fakeAdapter() + // The Bot the burst will reuse: already under Dev Lead, so 'FlyBot Programmer' has a namesake. + const reusedId = 'b90' + f.bots.set(reusedId, { id: reusedId, name: 'FlyBot Programmer', isMain: false, systemPrompt: '', fields: { parent: f.devLead, treeRole: 'writes the code' } }) + const tree = createTree(f.fb, { limits: { maxDepth: 5, maxChildren: 8 } }) + const releaseNew = f.hold('FlyBot Tester') + const a = tree.allocateChild(f.chief, 'FlyBot Programmer', 'writes code', undefined) + const b = tree.allocateChild(f.chief, 'FlyBot Tester', 'tests code', undefined) + await until('the new create to start', () => byName(f, 'FlyBot Tester')) + releaseNew() + const [reused, created] = await Promise.all([a, b]) + // The reuse is a reuse, not a create: its own parent and fields are untouched. + assert.ok(reused.reused, 'the existing Bot is reused') + assert.match(reused.note ?? '', /already exists/) + assert.equal(f.fb.getField(reusedId, 'parent'), f.devLead, 'the reused Bot keeps its parent') + assert.equal(f.bots.size, 5, 'only the Tester was created: Chief, Dev Lead, Existing Programmer, FlyBot Programmer, FlyBot Tester') + // Nothing in the tree claims a member it does not hold. + for (const bot of f.bots.values()) { + const names = bot.fields?.lead?.memberNames + if (!Array.isArray(names)) continue + const children = new Set(childrenIn(f, bot.id).map(x => x.name)) + for (const name of names) assert.ok(children.has(name), `${bot.name} names ${name}, which is not its child`) + } + // One new member is not a group: the lead is not made, and the Tester takes today's placement. + assert.equal(byName(f, 'FlyBot Lead'), undefined, 'a burst of one new member makes no lead') + assert.equal(created.created.parent.id, f.devLead) +})