registry.remove() prunes only one generation — orphaned grandchild subagents stay on the canvas

[jonioliveira]

Summary

registry.remove() deletes a node and its immediate children, but not deeper descendants. A subagent that spawned its own subagent leaves an orphaned grandchild in the registry — and because the UI treats a node with a missing parent as a root, the orphan is promoted to a top-level node on the canvas and stays there.

Root cause

remove(id: string) {
  this.agents.delete(id);
  for (const child of this.agents.values()) {
    if (child.parentId === id) this.agents.delete(child.id);   // one generation only
  }
  this.scheduleFlush();
}
// server/registry.ts:83-89

The loop matches only parentId === id. A node whose parentId is one of the just-deleted children is never visited.

The canvas then promotes the orphan, because "is a root" is defined as has no parent, or has a parent that no longer exists:

const roots = agents
  .filter((a) => !a.parentId || !agents.some((p) => p.id === a.parentId))   // lib/store.ts:262

Reachability

Not theoretical. In local-agent.ts:499 the nodeId that a new subagent is parented to is itself resolved through subagentByToolUse, so a subagent issuing its own Task call produces a genuine grandchild node:

const nodeId = options.agentID ? (subagentByToolUse.get(options.agentID) ?? agentId) : agentId;
// ...
registry.upsert({ id: childId, kind: "subagent", parentId: nodeId, ... });

Reproduction

Running the remove() logic verbatim on the tree root → kid → grand:

remaining after remove("root"): [ "grand" ]

grand survives with a dangling parentId, and per the root filter above it renders as a top-level node. In the app: start an agent, let it spawn a subagent that itself spawns a subagent, then Remove from canvas on the root — the grandchild stays behind as a stray node.

Suggested fix

Collect the full descendant set before deleting (array-as-queue, so newly discovered descendants are themselves expanded):

remove(id: string) {
  const doomed = [id];
  for (const target of doomed) {
    for (const child of this.agents.values()) {
      if (child.parentId === target) doomed.push(child.id);
    }
  }
  for (const target of doomed) this.agents.delete(target);
  this.scheduleFlush();
}

Low severity — stale nodes only, no hang or data loss. Grouping note: this is the same subagent-lifecycle area as the permission-routing bug, so the two could reasonably be fixed in one PR.

[pjbacelar]

Fixed in dff7bca, now on the mirror.

You were right: the loop only matched parentId === id, so a grandchild survived with a parentId pointing at a node that no longer existed, and lib/store.ts reads that as "is a root" and promotes it to a stray top-level node.

It now indexes children once into a Map<parentId, childIds[]> and walks breadth-first, emitting removed per node so trace.ts drops the deeper descendants' state too, which used to leak.

One note on your suggested patch, since it is worth knowing: collecting into a queue before deleting is not cycle-safe. With a -> b -> c -> a the queue regrows and it does not terminate (I measured past 100k iterations). The version that landed carries a seen set, the same idea you used in your permissions PR. Tested with a three-deep tree, a sibling branch, a cycle, a self-parent, and a 2000-deep chain.