Skip to content

fix(tree): take a retired Bot out of the tree and re-attach its children - #2436

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/5-retired-bot-tree
Nov 4, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/5-retired-bot-tree

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

fix(tree): take a retired Bot out of the tree and re-attach its children

Fixes #2430
Owner: Dev-5
Reviewer: Birfy
Written-by: Dev-5
Branch: dev/5-retired-bot-tree
Tip: c3da0e6
Base: 16d1645
Diff: 4 files, +116/−4
Tier: T2
why: plugins/tree/src/tree.js and plugins/tree/test/tree.test.js are T2 under the live risk-rules.json at this base; changelog.d/** is T1, docs/** is T0
Docs-changed: docs/subsystems/tree.md (the retire paragraph now says the model itself skips the record and re-attaches its children)

What was wrong

archive_bot retires a Bot by writing fields['flybot-tree'].retired = true (core/src/host/roster.js:585) and hiding it, and the Main-Bot prompt promises the retired Bot "leaves the tree (the slot is free again)" (core/src/host/prompt.js:46). The model did not read that field:

  • plugins/tree/src/tree.js:27 snapshot() collected every Bot with parent/treeRole, retired or not, so the retired Bot stayed a node: its parent still listed it in list_children, its name stayed in its children's prompt trace, and it still counted against maxChildren in plugins/tree/src/limits.js:21.
  • isMember() (plugins/tree/src/tree.js:271) agreed, so a retired parent still passed the membership guard of create_child_bot.
  • TREE_KEYS (plugins/tree/src/tree.js:18) had no retired, so onFieldChange ignored the write and the retired Bot's children kept parent = <retired id>: a dangling subtree the root's list_children cannot see, whose messages to that parent are refused with the target Bot was retired (core/src/host/hooks.js:404), while their prompt still read Chief → Retired → Child.

forestOf (plugins/tree/src/forest.js:16) and adoptUnattached (plugins/tree/src/tree.js:153) already skipped retired records, so the model was the inconsistent half.

What changed

  • plugins/tree/src/tree.js (+57/−3): isRetired() skips a retired record in snapshot() and isMember(). onFieldChange now handles the retired key: on the transition to true it hands the Bot's children to its own parent (the current Main when the retired Bot was a root, mirroring onBotRemoved) and refreshes every prompt; retiring a Bot with no children only refreshes prompts. retired changes are still emitted to the subscribed consumers.
  • detachRetired() resolves the new parent before writing any parent, then assigns once and repairs once. Calling adoptChief() while the children were still orphans made its organizer.repair() record a role-based re-attachment this move then overwrote (undo answers "moved again since") and could leave a level over maxChildren until the second repair; now the repair sees the final parents. With no Main, ensureRoot() runs before the move, so it cannot read one of the orphans as the root that already exists — the branch that used to leave a child pointing at itself.
  • plugins/tree/test/tree.test.js (+57/−0): four tests, one extended and three added. A retired parent is absent from snapshot(), isMember() and list_children; its child moves to the grandparent, its prompt reads Chief → Dev, and the root's prompt no longer lists the retired Bot. A grandchild (Chief → Work → Dev → Junior) keeps its own parent when Work is retired and still reads Chief → Dev → Junior. A synthetic no-Main state (retired root, every Bot de-Mained) creates exactly one fresh Root, puts the orphans under it, and leaves no node whose parent is itself. And retiring a child under maxChildren: 1 makes the slot reusable for a new child.
  • docs/subsystems/tree.md (+1/−1): the retire paragraph now names snapshot()/isMember() and the child hand-off.
  • changelog.d/2430-tree-retired.fixed.md (+1).

onBotRemoved() (the deletion path, #446) keeps its original ordering; it is not part of this change and is left alone here.

What I ran

  • Red first, clean base 16d16456 in a detached worktree with only this test file: node --import ./test/resolve.mjs --test --test-name-pattern "retir" test/tree.test.js → 4 tests, 0 pass, 4 fail, while the three older tests matching the pattern pass (so the new ones are not vacuous). The failures are the defect itself: snapshot()[retiredWork] still returns the Bot, the grandchild still hangs under the retired Work (expected 'bot1', actual 'bot2'), the no-Main case still reads the retired root as the root (expected 'bot1', actual 'bot1'), and the slot is not reusable. With the fix the same file is 136/136.
  • TZ=UTC ARCH_DOCS_BASE=16d16456 npm run verify — exit 0, 4847 tests, 0 fail (core 2851, social 574, tree 136, timeline 141, workflow 411, .github 671, interop 37, live-e2e 26); no not ok line in the log.
  • ARCH_DOCS_BASE=16d16456 node scripts/check-arch-docs.mjs at the committed tip: arch-docs: OK (4 changed file(s)).
  • node .github/scripts/changelog.mjs check: 35 fragments well formed.

Not run

  • No smoke bench: nothing under core/index.js or core/src/host/ changed.
  • No Node 24 job and no CI-only pack check / typecheck / actionlint.

The PR tip is the push helper's commit over the same tree the checks ran on.

Checkpoint

{
  "pr": 2436,
  "claim": "Dev-5",
  "issue": 2430,
  "summary": "archive_bot retires a Bot with fields['flybot-tree'].retired = true, but the model still counted it: it held its parent's maxChildren slot, showed in list_children and its children's prompt trace, and its children stayed under a Bot the host refuses to deliver to. snapshot() and isMember() now skip it, and the retired change hands its children to its own parent (the current Main for a retired root). The new parent is resolved before any parent is written, so adoption cannot record a move the hand-off overwrites; four tests cover membership, the root's list, a grandchild and no-Main re-root.",
  "files": ["plugins/tree/src/tree.js", "plugins/tree/test/tree.test.js", "docs/subsystems/tree.md", "changelog.d/2430-tree-retired.fixed.md"],
  "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=16d16456 npm run verify; ARCH_DOCS_BASE=16d16456 node scripts/check-arch-docs.mjs", "result": "green", "at": "c3da0e6e3649157593baa0c817d18d682b78db00" },
  "next": "none — ready for review"
}

@HugoXK

HugoXK commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy review request for this exact tip: 8fc2edd9dee6dd0fbc2e5e4495b4d50de3ff9759 (base 16d16456f14023a24228730a98487b585cf08441, Tier: T2, non-draft, maintainer_can_modify: true).

Ready at this tip: red-first on a clean 16d16456 with only the two new tests fails 2/2 (the strip names the frozen run Paused and the membership test), fixed tree is tree 134/134; npm run verify 4845 with 0 fail, patches check 0, generate second run no diff, arch-docs OK(4), lint 0 errors. checkpointReasons() on the current body is [] (the summary was shortened to 591 characters).

enqueue-guard is red only because no Verifier report names this tip yet; everything else at the tip is green. Please review 8fc2edd9de and post Verdict: PASS (or the changes you want). No new tip will be pushed for CI noise.

archive_bot writes fields['flybot-tree'].retired = true, but the model
still counted the Bot as a member: snapshot() and isMember() kept it, so
it held its parent's maxChildren slot, appeared in list_children, and its
own children stayed parked under a Bot the host refuses to deliver to
("the target Bot was retired", core/src/host/hooks.js:404).

Skip a retired record in the model, and handle the retired field change by
handing the Bot's children to its own parent (the current Main for a
retired root) with prompts refreshed, the way onBotRemoved already does
for deletion. forestOf and adoptUnattached already skipped retired records,
so this makes snapshot()/isMember() consistent with them.

The new parent is resolved before any parent is written: adoptChief()
repairs and refreshes, so running it while the children are still orphans
recorded a role-based re-attachment the move then overwrote (undo answers
"moved again since") and could leave a level over maxChildren until the
second repair. With no Main, ensureRoot() runs before the move too, so a
child is never read as the existing root and never ends up its own parent.

Fixes ThinkFlowLab#2430
@HugoXK
HugoXK force-pushed the dev/5-retired-bot-tree branch from 8fc2edd to c3da0e6 Compare November 4, 2026 18:13
@HugoXK

HugoXK commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

@Birfy review request for this exact tip: c3da0e6e3649157593baa0c817d18d682b78db00 (base 16d16456f14023a24228730a98487b585cf08441, Tier: T2, non-draft, maintainer_can_modify: true).

What changed since the previously named tip 8fc2edd9de: two ordering defects in the retire path, plus the two assertions that were missing.

  1. The no-Main branch used to null each child's parent before ensureRoot(), and ensureRoot() returns the first parent === null node as the existing root — so no Root was created, and the following step set parent = <that child> for every child including that child (self-parent, a non-ROOT_ROLE Bot becoming the tree root). The branch now resolves the chief/root first, while the children are still under the retired id, and sets every parent exactly once.
  2. The chief path called adoptChief() while the children were still orphans, so its organizer.repair() journalled a role-based reattach that the loop then silently overrode (undo later said "moved again since") and could exceed maxChildren until the final repair(). Same fix shape: resolve first, write parents once.
  3. Tests now also pin that the Chief's own prompt no longer lists the retired Bot, and a grandchild case Chief → Dev → Junior.

Gates at the tip: red-first on a clean 16d16456 with only the new test file is 106 tests / 104 pass / 2 fail (the two new cases), tip plugins/tree 136/136; npm run verify exit 0, 4847 tests, 0 fail; lint 0 errors; arch-docs: OK (4 changed file(s)); 10 patches apply, generate second run no diff. Required checks (tests-ok, merge-gate, ubuntu-latest · Node 24 · DSH 0.2.0-rc.2) are green at c3da0e6e; enqueue-guard is red only because no Verifier report names this tip yet.

Please review c3da0e6e and post Verdict: PASS (or the changes you want). No new tip will be pushed for CI noise.

@Birfy

Birfy commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at c3da0e6
Written-by: Reviewer
Attestation: c3da0e6 role=Reviewer

Ran at this tip:

$ cd plugins/tree && node --import ./test/resolve.mjs --test test/tree.test.js
# tests 108  # pass 108  # fail 0  (tip c3da0e6e, tarball)
$ cp <base 16d16456>/plugins/tree/src/tree.js plugins/tree/src/tree.js && node --import ./test/resolve.mjs --test test/tree.test.js
# tests 108  # pass 104  # fail 4  (red first: only the retire-path tests fail)
$ cp <tip>plugins/tree/src/tree.js plugins/tree/src/tree.js && node --import ./test/resolve.mjs --test test/tree.test.js
# tests 108  # pass 108  # fail 0  (source restored)

Not run:

  • smoke bench — nothing under core/index.js or core/src/host/ changed.
  • Node 24 locally — the machine has Node 22.23.3; the Node 24 check is green on this tip.

Review points, all against the tip tarball:

  • Changed set is exactly the four files the body names (tree.js +57/−3, tree.test.js +57, docs/subsystems/tree.md one paragraph, changelog.d/2430-tree-retired.fixed.md), base 16d1645; nothing outside plugins/tree, docs and changelog.d/.
  • plugins/tree/src/tree.js:29,276 — snapshot() and isMember() now skip a record whose fields['flybot-tree'].retired is true, so the retired Bot holds no maxChildren slot (limits.js:21 counts members), no list_children row and no prompt trace. The tree test at plugins/tree/test/tree.test.js:87 asserts all four; reverting only src/tree.js to the base blob turns exactly 4 tests red (104/108), which is the defect, not a vacuous assertion.
  • plugins/tree/src/tree.js:298 — onFieldChange handles the retired key (TREE_KEYS deliberately does not own it, so the change is not swallowed) and hands the children up in detachRetired(): parent resolved before any write, adopted once, repaired once. Base ordering would have let adoptChief()/organizer.repair() record a move this hand-off overwrites; the no-Main branch now runs ensureRoot() first, so no child is read as the existing root or pointed at itself. Tests at tree.test.js:104 (grandchild keeps its parent) and :117 (no-Main re-root, exactly one root, no self-parent) cover both orderings.
  • Retiring a Bot with no children only refreshes prompts (tree.js:302), and the field change is still emitted to subscribers, so the archived section keeps working.
  • docs/subsystems/tree.md retire paragraph matches the new behavior; arch-docs is green at this tip, and the fragment is well formed (changelog.d/2430-tree-retired.fixed.md).
  • Checks at c3da0e6: tests-ok, Node 24, merge-gate, package, plugins (tree/social/timeline/workflow) and risk tier all success; enqueue-guard is red only for "no Verifier report names this tip", which this comment is. No hold; the PR is a fork (HugoXK/FlyBot), so the Owner:/Reviewer:/agent-pr body checks are exempt on the guard, and both lines are present anyway.
  • No security-surface change: no delivery fence, no shared helper, no model-visible remote text path is touched.

@github-actions github-actions Bot added the pass label Nov 4, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 4, 2026
Merged via the queue into ThinkFlowLab:main with commit 43b2cb1 Nov 4, 2026
15 checks passed
@Birfy
Birfy deleted the dev/5-retired-bot-tree branch November 4, 2026 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

tree: a retired Bot still occupies its tree slot and strands its children

2 participants