Skip to content

Bug: remove() breaks tree/signal consistency when used with React observers — stale nodes, missed signals, zombie children #1

@rauhryan

Description

@rauhryan

Bug: remove() does not reliably emit tree change signals

Summary

remove() halts the spawned task that owns a node, which triggers cleanup in the task's finally block (removing from parent._children, deleting from tree.nodes, calling tree.markDirty()). However, task.halt() is asynchronous relative to the useTree eval loop's dirty-check. The finally block may run after the eval loop has already checked state.dirty and found it false, causing output.send() to never fire. Any subscriber listening via each(tree) will not be notified of the removal.

How remove() works today

append() spawns a task per child (freedom.ts)

*append(name, component): Operation<Node> {
  const parent = yield* NodeContext.expect();
  const tree = yield* TreeContext.expect();
  const child = new NodeImpl(tree.nextId(), name, parent);
  const ready = withResolvers<void>();

  const task = yield* spawn(function* () {
    parent._children.add(child);
    tree.nodes.set(child.id, child);
    yield* NodeContext.set(child);
    yield* spawnEvalLoop(child._channel);
    ready.resolve();

    try {
      yield* component();
      yield* suspend();
    } finally {
      parent._children.delete(child);   // cleanup on halt
      tree.nodes.delete(child.id);      // remove from global map
      tree.markDirty();                 // mark tree dirty
    }
  });

  child.data.set(Halt, task.halt);
  child.remove = () => FreedomApi.operations.remove(child);

  yield* ready.operation;
  return child;
}

remove() halts the task (freedom.ts)

*remove(node: Node): Operation<void> {
  const halt = node.data.expect(Halt);
  yield* halt();
}

The useTree notification pipeline (tree.ts)

const sub = yield* events;
yield* spawn(function* () {
  while (true) {
    const next = yield* sub.next();
    state.dirty = false;
    yield* rootNode.eval(() => DispatchApi.operations.dispatch(next.value));
    if (state.dirty) {
      output.send();               // ← subscribers see this
    }
  }
});

useFocus around-hook intercepts remove (focus.ts)

yield* FreedomApi.around({
  *remove([node], next) {
    if (node.props.focused === true) {
      yield* FocusApi.operations.advance();   // advance focus FIRST
    }
    yield* next(node);                         // then halt the node
  },
});

The race

The sequence of events when a dispatch triggers a remove():

  1. events.send(dispatchEvent) — the eval loop picks it up.
  2. state.dirty = false — reset at top of loop.
  3. rootNode.eval(() => dispatch(...)) — runs the dispatch handler, which calls remove().
  4. remove() calls task.halt()schedules the task's finally block (asynchronous).
  5. remove() returns — yield* halt() resolves.
  6. The dispatch handler returns. rootNode.eval() resolves.
  7. The eval loop checks if (state.dirty)may be false because the finally block hasn't run yet.
  8. The finally block runs (in a future microtask): parent._children.delete(child), tree.nodes.delete(child.id), tree.markDirty().

Result: markDirty() fires after the dirty-check. output.send() never fires. Subscribers are not notified of the removal.

The useFocus around-hook makes this worse: focus.advance() runs before next(node) (before halt()), mutating focused props on sibling nodes within the same dispatch. These mutations go through other nodes' eval channels, which can interleave with the dirty-check timing.

Expected behavior

After remove(), the tree should emit exactly one change notification so that any subscriber iterating via each(tree) is reliably informed that the tree structure has changed.

Specifically:

  • parent._children no longer contains the removed node.
  • tree.nodes no longer contains the removed node's id.
  • A subscriber awaiting each(tree) receives a notification.

Actual behavior

The notification is intermittently lost. The finally block runs asynchronously after the eval loop's dirty-check, so markDirty() has no effect on the current iteration. No output.send() fires. The subscriber never wakes up.

The tree's internal state is eventually consistent (the finally block does clean up), but the signal is lost. Subscribers see stale state.

Suggested fix

Option A: Synchronous remove — clean up before halting

Make remove() immediately clean up _children and tree.nodes before halting the task. This guarantees the dirty flag is set before the eval loop checks it:

*remove(node: Node) {
  const impl = node as NodeImpl;
  const parent = impl._parent;
  if (!parent) throw new Error("Cannot remove root node");

  // Synchronously clean up — dirty flag is guaranteed set before eval loop checks it
  parent._children.delete(impl);
  tree.nodes.delete(impl.id);
  tree.markDirty();

  // Then halt the task (finally block must become idempotent)
  const halt = impl.data.expect(Halt);
  yield* halt();
}

The finally block in append() must become idempotent:

finally {
  if (parent._children.has(child)) {
    parent._children.delete(child);
  }
  if (tree.nodes.has(child.id)) {
    tree.nodes.delete(child.id);
    tree.markDirty();
  }
}

Option B: Drain pending halts in the eval loop

Ensure the eval loop waits for all spawned-task finally blocks to settle before checking state.dirty:

// tree.ts
yield* rootNode.eval(() => DispatchApi.operations.dispatch(next.value));
// ← some kind of "drain" or "settle" yield here
if (state.dirty) {
  output.send();
}

This would require a mechanism to await completion of all in-flight halt cleanups, which may need support from Effection itself (e.g. a Scope.settle() operation).

Option C: Declarative reconcile() operation

Add a reconcile() that atomically updates the children set with a single markDirty(), avoiding the async-halt problem entirely:

*reconcile(desiredChildren: { name: string; key: string }[]) {
  // Remove children not in desiredChildren
  // Add children that are new
  // One atomic markDirty() at the end
}

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions