fix(docmodel): stop infinite loop hydrating documents with a block-tree cycle

[ericvicenti]

TL;DR

A single production document could pin a CPU core forever. isAncestor walks a block's parent chain in an unbounded loop with no cycle guard, so a document containing a block that is its own parent made hydration never terminate. Reading such a document (and the read path retries on timeout) burned one core per concurrent request — the real reason the gateway's CPU fell over at trivial request rates (~6 req/s).

Validated against the real affected production document: hydration went from non-terminating (>400s) to ~6ms.

Root cause

treeOpSet.State() rebuilds the block tree by replaying move ops. For each move it calls isAncestor(block, parent) to prevent cycles:

func (state *blockTreeState) isAncestor(a, b string) bool {
	n, ok := state.blocks.Get(b)
	for {                                   // <-- unbounded, no visited guard
		if !ok || n.Parent == "" || n.Parent == TrashNodeID { return false }
		if n.Parent == a { return true }
		n, ok = state.blocks.Get(n.Parent)  // X.Parent == X -> loops forever
	}
}

The cycle-prevention guard is correct for normal moves, but it inspects the pre-move state. For a move where block == parent, isAncestor(X, X) looks at X's current parent (not yet itself) and returns false, so the self-parent move is applied — X.Parent = X. Any later walk through X then loops forever.

Three documents in one production space (z6Mkjdkv…: research-roadmap, opportunity-backlog, content-roadmap) contain this corruption. The signed changes are immutable, so hydration must be robust to them, not merely prevent new ones.

Why this looked like everything else first

• "Slow" (18s): that was just the gateway's upstream timeout cutting off an infinite computation.
• Not O(n²): the op-log is tiny (2,236 moves, 1,558 shallow blocks) — size was never the issue.
• Cache didn't help: a computation that never finishes never populates the cache; clients time out and retry, re-triggering the loop (696 hung requests observed on one doc).

Fix

1. isAncestor bounds its walk by the number of blocks — a valid ancestor chain visits each block at most once, so exceeding that means a cycle. It stops and treats a cyclic path as "not an ancestor," guaranteeing termination. This un-bricks the existing documents.
2. State() marks a self-parent move (block == parent) invisible, so replaying the corrupt op yields a sane tree (the block keeps its last valid position).
3. Move() rejects self-parent moves, so new corruption can't be created.

Validation

• Replayed the real exported change history of the affected production doc: Hydrate went from >400s (killed, effectively infinite) to 6.2ms, producing a correct 31-block document.
• Full docmodel + documents suites pass with no regressions.

Tests added

• TestSelfParentDoesNotHang — injects a self-parent move into the op-log (as historical data has it) and asserts State() terminates (fails via timeout if the guard regresses) and the move is ignored.
• TestIsAncestorTerminatesOnCycle — a self-loop and a two-node cycle must both terminate as not-an-ancestor.
• TestMoveBlockRejectsSelfParent — the write path refuses to create a self-parent move.

Relationship to other PRs

• Supersedes perf(docmodel): O(1) block lookups in isAncestor to cut hydrate CPU #859 — this PR now incorporates perf(docmodel): O(1) block lookups in isAncestor to cut hydrate CPU #859's btree→map O(1) lookup optimization and adds the cycle-termination guard on top. perf(docmodel): O(1) block lookups in isAncestor to cut hydrate CPU #859 alone does not fix the incident (its isAncestor still loops forever on a self-parent block); see the comment below. perf(docmodel): O(1) block lookups in isAncestor to cut hydrate CPU #859 can be closed in favor of this.
• Independent of fix(documents): cache hydrated documents by version to end read-path CPU storm #858 (version-keyed hydrate cache). fix(documents): cache hydrated documents by version to end read-path CPU storm #858 reduces repeated hydration cost; this PR fixes the root cause of the CPU fallover (the infinite loop). Different concerns; can land in either order.

Follow-up (not in this PR)

Consider a one-off scan/repair to detect documents with cyclic/self-parent moves and record them, and possibly a migration that supersedes the bad moves. With this fix they render correctly regardless, so it's not urgent.

🤖 Generated with Claude Code

[ericvicenti]

This PR now supersedes #859 (incorporates it + fixes the actual bug)

I've merged #859 (fix/hydrate-miss-cost, @juligasa's btree→map O(1) lookup optimization) into this branch and combined it with the correctness fix. #860 is now a strict superset of #859 — you get the faster lookups and the guaranteed termination. #859 can be closed in favor of this.

Why #859 alone doesn't fix the incident

#859 and this PR both touch isAncestor, but they target different things:

• perf(docmodel): O(1) block lookups in isAncestor to cut hydrate CPU #859 makes each isAncestor lookup O(1) (map instead of btree). But its isAncestor keeps the unbounded for {} with no cycle guard. On a self-parented block (X.Parent == X) it still loops forever — just spins the infinite loop faster. The three poison documents (research-roadmap, opportunity-backlog, content-roadmap in z6Mkjdkv…) would keep pinning cores.
• This PR bounds the walk by the block count so it always terminates, marks self-parent moves invisible in State(), and rejects them in Move(). That's what actually un-bricks the documents.

The trap (why the profile was misleading)

The CPU profile shows isAncestor → btree ops (hintsearch/Less/cmpbody) dominating, which reads as "the lookups are the bottleneck, make them O(1)." But those ops were hot because of the infinite loop doing billions of lookups, not because lookups are slow. The profile pointed at the symptom. #859 optimizes the symptom; this PR fixes the cause. (I fell into the same read early on — it's a very natural misdiagnosis.)

Combined result, validated on real data

Replaying the actual exported change history of the affected production document:

• before: non-terminating (>400s, killed)
• perf(docmodel): O(1) block lookups in isAncestor to cut hydrate CPU #859 alone (map, no bound): still non-terminating
• this PR (map + bound + self-parent guard): 5.3ms, correct 31-block output

Full docmodel + documents suites pass under -race; regression tests cover the self-parent hang, a two-node cycle, and the write-path guard.