Codex stop-gate caught a real correctness bug: `move_node`'s
apply path was:
1. validate source exists + cycle check
2. detach the source (vec.remove → owned Node)
3. find_node_mut_in_doc(target) — if None, return false
When target_parent_id was unknown, step 2 had already removed
the source from its parent. The owned Node fell off the end of
the function and was DROPPED — silently destroying the source.
The existing unknown-id test only covered the path where the
old `find_node_in_doc` on the cycle check incidentally caught
it; an LLM passing a bogus target_parent_id would lose data.
Fix: every validation (source exists, target exists, cycle
guard, page-root path's active page exists) runs upfront. Only
after all checks pass does the detach + reattach proceed. The
two halves of the mutation are no longer separable from each
other — match-statement style atomicity, no orphan path.
Test (1 added, 343 shell-core total):
- `apply_mcp_command_move_node_preserves_source_when_target_unknown`
builds the exact codex scenario: MoveNode { node_id: 11
(sample Title), target_parent_id: 99999 (doesn't exist) }.
Asserts:
1. apply returns false (no successful move),
2. Frame's child count is unchanged (Title still there),
3. Title is still in Frame's children vec,
4. Title appears EXACTLY ONCE in the doc (the dropped-
source bug would have left zero occurrences).
Pre-fix assertion #4 would have failed with count = 0.
The same pre-validate-then-mutate pattern is now consistent
across update_node (atomic on invalid geometry, 9e601573),
insert_node (id-space-exhausted guard, 62a2fb5b), and
move_node (this fix). DeleteNode is naturally atomic — single
remove_in_subtree call.
a39302b948