tree: bound each reorganization job so a hung host call cannot block the queue

changelog.d/2751-reorg-queue-bound.fixed.md

@@ -0,0 +1 @@
+A tree reorganization no longer waits for ever on a host call that never answers: after 10 minutes the job is abandoned with its id in the log (`flybot-tree: reorganization job <id> was abandoned after …`), later placements, repairs and the daily tidy still run, and `settled()` still resolves. An abandoned `exclusive()` job reports `abandoned: true` with its job id instead of hanging. See [#2751](https://github.com/ThinkFlowLab/FlyBot/issues/2751).

docs/subsystems/tree.md

@@ -76,6 +76,7 @@ The tree organizes itself by role instead of leaving Bots outside it or a parent
 - **Placement** of a Bot outside the tree: its namesake, else the tree Bot that leads a group it belongs to (most specific key first, then the shallower lead; an area only through a lead made for it, e.g. "Work Lead"), else the root. At the first of those with room it is placed. A full level is **regrouped**: the most specific group of two or more children (the newcomer counted) moves under a suitable existing member, or under a new lead. Inside a full lead of its own group with no finer group, the member closest to its role takes it (all members share the group). There is **no "least-loaded child" fallback**: a Bot that fits no group while every level it matches is full stays outside, a warning names it and the sidebar tags it "outside the tree"; the next attempt (any `bot.created`, startup) tries again.
 - **Leads.** A suitable existing lead is a member already marked as that group's lead, already coordinating that group (it has children and the key, project and domain only), or named for it ("Life", "FlyBot Lead") or as a lead ("Kitchen Manager", 负责人). It gets `lead { key, label }`. Only when none fits a new lead is created with a clear role (`<Label> Lead`, role "Lead of the <Label> group: coordinates …"); its members move under it before it joins the level. A lead's own group key, and broad areas below any lead, are ignored inside it, so there is no second "Dev Lead" under "Dev Lead".
 - **Repair**: a parent over `maxChildren` (after a deletion re-attached a deleted Bot's children) is regrouped the same way, at once when the lead exists, else in the queue.
+- **One queue, bounded per job (#2751):** placement, repair and tidy run one at a time on the reorganization queue. A job whose host call never answers is abandoned after 10 minutes (`flybot-tree: reorganization job <id> was abandoned after …`): the next job still runs and `settled()` still resolves, and an `exclusive()` job's promise rejects with `abandoned: true` and its job id instead of hanging. The abandoned work cannot be cancelled, so it may still write afterwards.
 - **Daily tidy** (checked hourly, runs once a day; the first check after install only records the time; `dailyTidy: false` turns it off): (1) leaf Bots with no activity for `tidy.idleDays` (14) get `idle` (any turn or tool call clears it) and are **archived** after `tidy.archiveIdleDays` (30) more days; Bots with children, granted Bots and the root are never idle; (2) duplicates: two Bots with essentially the same role (same name once generic words are dropped and no conflicting description, or near-identical descriptions in one domain), both at least `tidy.mergeGraceDays` (1) in the tree, are **merged**: the newer one's children move to the kept Bot, which lists it in `mergedFrom` and is told in its prompt; the old one is archived; (3) project and domain groups of three or more (or two with an existing lead) on every level, never a group of a whole level without a lead.
 - **Archive, never delete**: an archived Bot leaves the tree (`archived { at, reason, parent, treeRole, into? }`, tree fields and block removed) but keeps its chat, memory and grants. It is **hidden from the default Bot list** (`bot.hidden` / structure `include`; Search still finds it) (#452), and the sidebar's `flybot-tree-archived` section is the way back to its chat (#810): collapsed by default, header `Archived (N)`, drawn only while N > 0, one row per archived Bot with the same tag `statusOf()` gives (`archived`, `archived · merged into <name>`) opening the chat through `actions.openSession`. `include()` still returns false, so the list and `botTree()` never see it. Deleting it stays the owner's own click. That section is registered at `order: 88` and the reorganization notice below at `order: 89`; a slot renders ascending, so the archived Bots sit **above** the notice. On a merge (#450 item 1), the archived Bot's own memory is carried into the kept Bot through `flybot.memory` (at most 20 entries, tagged `source: merged`), so a merge loses nothing; without the host's memory seam the merge still stands.
 - **Retire from the core (#638):** the Main Bot's `archive_bot` writes `fields['flybot-tree'].retired = true` (never `archived`, which is the tidy's own record) and sets `bot.hidden`, so the Bot leaves the tree and the default list the way an archived one does while its history stays readable; adoption skips a retired record and `forestOf` no longer counts it, and since #2430 the model itself does too: `snapshot()` and `isMember()` leave the record out (so it occupies no `maxChildren` slot, no `list_children` row and no prompt trace), and the field change that retires it hands its children to its own parent — or to the current Main for a retired root — with every moved Bot's prompt refreshed, instead of stranding them under a Bot the host refuses to deliver to. The tree panel (`treeOf`) skips a retired Bot the same way (#1793), and the `flybot-tree-archived` section lists it with the `retired` tag so its chat stays reachable; a record that also carries `archived` keeps the archived tag.

plugins/tree/src/organize.js

@@ -17,6 +17,8 @@ import { profileOf, keysOf, kindOf, KIND_RANK, labelOf, leadNameOf, canLead, sam
 import { systemPrompt, withTreeBlock, BLOCK_START } from './prompt.js'
 
 const DAY = 24 * 60 * 60 * 1000
+// #2751: how long one reorganization job may wait on the host before the queue gives up on it.
+const DEFAULT_JOB_TIMEOUT_MS = 10 * 60 * 1000
 const DOMAIN_AREA = new Map(DOMAINS.map(d => [d.id, d.area]))
 export const DEFAULT_TIDY = Object.freeze({ idleDays: 14, archiveIdleDays: 30, mergeGraceDays: 1, pinDays: 14, keepRuns: 5 })
 /** Mis-archive rename prefix (Chief update_bot / manual): 归档-Name or Archive-Name. Reorg never writes this. */
@@ -28,7 +30,7 @@ const MAX_TOOLS = 8
 /** Own prompt text without the tree's block: part of a Bot's description. */
 const ownText = prompt => { const s = String(prompt ?? ''); const i = s.indexOf(BLOCK_START); return (i >= 0 ? s.slice(0, i) : s).trim() }
 
-export function createOrganizer({ fb, limits, snapshot, childrenOf, depthOf, warn, refreshAll, writeBlock, findRootId, now = Date.now, classify, tidy: tidyConfig = {} }) {
+export function createOrganizer({ fb, limits, snapshot, childrenOf, depthOf, warn, refreshAll, writeBlock, findRootId, now = Date.now, classify, tidy: tidyConfig = {}, jobTimeoutMs: timeoutConfig }) {
   const cfg = { ...DEFAULT_TIDY, ...Object.fromEntries(Object.entries(tidyConfig).filter(([, v]) => Number.isFinite(v) && v >= 0)) }
   const get = (id, key) => fb.getField(id, key)
   const set = (id, key, value) => fb.setField(id, key, value)
@@ -306,14 +308,44 @@ export function createOrganizer({ fb, limits, snapshot, childrenOf, depthOf, war
     return best
   }
 
+  // #2751: one host call that never settles used to hold `queue` for ever: every later
+  // reorganization waited on it and idle() never resolved. Each job now carries a deadline. When the
+  // host does not answer in time, the queue records the abandoned job, warns with its id and moves
+  // on. The work behind it cannot be cancelled, so it may still write later; the queue no longer
+  // waits for it.
+  const jobTimeoutMs = Number.isFinite(timeoutConfig) && timeoutConfig > 0 ? timeoutConfig : DEFAULT_JOB_TIMEOUT_MS
   let queue = Promise.resolve()
   let pending = 0
-  const enqueue = job => {
+  let jobSeq = 0
+  // Resolves { ok: true, value } / { ok: false, error } / { abandoned: true, job } and never rejects,
+  // so one bad job cannot stop the chain.
+  const bounded = (jobId, job, onAbandon) => new Promise(resolve => {
+    let done = false
+    const timer = setTimeout(() => {
+      if (done) return
+      done = true
+      warn(`flybot-tree: reorganization job ${jobId} was abandoned after ${jobTimeoutMs}ms: the host call never settled; later reorganizations continue`)
+      try { onAbandon?.() } catch (error) { warn(`flybot-tree: could not clean up after ${jobId}:`, error?.message ?? error) }
+      resolve({ abandoned: true, job: jobId })
+    }, jobTimeoutMs)
+    Promise.resolve().then(job).then(
+      value => { if (done) return; done = true; clearTimeout(timer); resolve({ ok: true, value }) },
+      error => { if (done) return; done = true; clearTimeout(timer); resolve({ ok: false, error }) }
+    )
+  })
+  const runInQueue = (job, onAbandon) => {
+    const jobId = `reorg-${++jobSeq}`
     pending++
-    const run = queue.then(job).catch(error => warn('flybot-tree: reorganization failed:', error?.message ?? error)).finally(() => { pending-- })
+    const run = queue.then(() => bounded(jobId, job, onAbandon)).finally(() => { pending-- })
     queue = run
-    return run
+    return { jobId, run }
   }
+  /** Runs a job in the queue: resolves its value, undefined when it failed or was abandoned. */
+  const enqueue = (job, onAbandon) => runInQueue(job, onAbandon).run.then(outcome => {
+    if (outcome.abandoned) return undefined
+    if (!outcome.ok) { warn('flybot-tree: reorganization failed:', outcome.error?.message ?? outcome.error); return undefined }
+    return outcome.value
+  })
   const placeAt = (id, parentId, role) => { set(id, 'treeRole', role); set(id, 'parent', parentId); set(id, 'seenAt', get(id, 'seenAt') ?? now()) }
 
   /**
@@ -374,6 +406,11 @@ export function createOrganizer({ fb, limits, snapshot, childrenOf, depthOf, war
         if (fb.getBot(bot.id)) set(bot.id, 'organizing', null)
         refreshAll()
       }
+    }, () => {
+      // An abandoned placement never reaches its own finally: clear the flag so the next pass can
+      // place the Bot instead of leaving it marked 'organizing' for ever.
+      if (fb.getBot(bot.id)) set(bot.id, 'organizing', null)
+      refreshAll()
     })
   }
 
@@ -632,7 +669,25 @@ export function createOrganizer({ fb, limits, snapshot, childrenOf, depthOf, war
    * Runs `job` in the reorganization queue (one at a time with placement, repair and tidy; a lead
    * being set up there is not re-placed by adoption). Resolves the job's value, rejects with its error.
    */
-  const exclusive = job => new Promise((resolve, reject) => { enqueue(async () => { try { resolve(await job()) } catch (error) { reject(error) } }) })
+  /**
+   * Runs `job` in the reorganization queue (one at a time with placement, repair and tidy; a lead
+   * being set up there is not re-placed by adoption). Resolves the job's value, rejects with its
+   * error — or, when the host never answered, with an Error carrying `abandoned: true` and the job
+   * id, so the caller can tell a give-up from a failure.
+   */
+  const exclusive = job => {
+    const { jobId, run } = runInQueue(job)
+    return run.then(outcome => {
+      if (outcome.abandoned) {
+        const error = new Error(`reorganization job ${jobId} was abandoned after ${jobTimeoutMs}ms: the host call never settled`)
+        error.abandoned = true
+        error.jobId = jobId
+        throw error
+      }
+      if (!outcome.ok) throw outcome.error
+      return outcome.value
+    })
+  }
   // The shared primitives explicit allocation (#447, src/allocate.js) is built on: the same role
   // profiles, homes, limit checks, lead creation, moves and journal as the self-organization.
   const shared = { profile, subtreeOf, heightOf, fits, homesFor, createLead, markLead, move, begin, commit, exclusive }

plugins/tree/test/organize-queue-bound.test.js

@@ -0,0 +1,84 @@
+// #2751: one reorganization job whose host call never settles used to hold the serial queue for
+// ever: the next `exclusive()` job never ran and `idle()` never resolved. Each job now carries a
+// deadline; a job that misses it is reported as abandoned (with its id) and the queue moves on.
+import assert from 'node:assert/strict'
+import { test } from 'node:test'
+import { createOrganizer } from '../src/organize.js'
+
+// The queue needs nothing from the tree itself: no job below reads a Bot or a snapshot, so the
+// organizer is built with the smallest host that satisfies its constructor.
+function organizerAt({ jobTimeoutMs = 30 } = {}) {
+  const warnings = []
+  const fields = new Map()
+  const fb = {
+    getField: (id, key) => fields.get(`${id}:${key}`),
+    setField: (id, key, value) => { if (value == null) fields.delete(`${id}:${key}`); else fields.set(`${id}:${key}`, value) },
+    getBot: id => ({ id, name: id }),
+    listBots: () => [],
+    setSystemPrompt: () => {},
+    createBot: () => new Promise(() => {}),
+    updateBot: async () => {},
+    memoryRead: () => [],
+    memoryAppend: async () => ({ ok: true }),
+    isRemote: () => false,
+    hasAnyGrant: () => false,
+  }
+  const organizer = createOrganizer({
+    fb,
+    limits: { maxChildren: 8, maxDepth: 4 },
+    snapshot: () => ({}),
+    childrenOf: () => [],
+    depthOf: () => 0,
+    warn: (...args) => warnings.push(args.map(String).join(' ')),
+    refreshAll: () => {},
+    writeBlock: () => {},
+    findRootId: () => 'root',
+    now: () => 1_700_000_000_000,
+    classify: () => ({}),
+    jobTimeoutMs,
+  })
+  return { organizer, warnings }
+}
+
+test('#2751 a job whose host call never settles is abandoned, and the queue moves on', async () => {
+  const { organizer, warnings } = organizerAt({ jobTimeoutMs: 30 })
+  const never = new Promise(() => {})
+  const stuck = organizer.shared.exclusive(async () => { await never; return 'done' })
+  const later = organizer.shared.exclusive(async () => 'later ran')
+
+  await assert.rejects(stuck, error => {
+    assert.equal(error.abandoned, true, 'the give-up is reported, not as a failure of the host call')
+    assert.match(error.message, /reorg-1/)
+    return true
+  })
+  assert.equal(await later, 'later ran')
+  await organizer.idle()
+  assert.equal(organizer.busy(), false)
+
+  const warned = warnings.join('\n')
+  assert.match(warned, /reorg-1/) // the abandoned job is named
+  assert.match(warned, /abandoned after 30ms/)
+})
+
+test('#2751 a failed job does not poison the queue', async () => {
+  const { organizer } = organizerAt()
+  const failed = organizer.shared.exclusive(async () => { throw new Error('host said no') })
+  const later = organizer.shared.exclusive(async () => 'later ran')
+
+  await assert.rejects(failed, /host said no/)
+  assert.equal(await later, 'later ran')
+  await organizer.idle()
+  assert.equal(organizer.busy(), false)
+})
+
+test('#2751 a job that settles keeps its value and its order', async () => {
+  const { organizer } = organizerAt()
+  const order = []
+  const first = organizer.shared.exclusive(async () => { order.push('first'); return 1 })
+  const second = organizer.shared.exclusive(async () => { order.push('second'); return 2 })
+
+  assert.equal(await first, 1)
+  assert.equal(await second, 2)
+  assert.deepEqual(order, ['first', 'second'])
+  await organizer.idle()
+})