* fix(store): make item field + parent-link update atomic (BUG-2013)
handleUpdateItem committed the field write, then ran SetParentLink/
ClearParentLink as a SEPARATE store transaction. A failure there
(cycle discovered late, DB error) returned 500 with half the patch
already applied — a code-acknowledged partial-commit window.
Fold the parent-link mutation into the SAME transaction as the field
write:
- store: extract setParentLinkTx / clearParentLinkTx from the public
SetParentLink / ClearParentLink (which keep their own tx). Add
UpdateItemWithParentLink(id, input, precheck, *ParentLinkUpdate) —
UpdateItemWithPreCheck now delegates to it with a nil link. The
link write runs after the field UPDATE but before COMMIT, so a
failing link write rolls the field write back too.
- lock ordering: the NEW parent's advisory key is folded into the
update's initial sorted AcquireParentChildrenLocks batch (extraKeys
on acquireParentChildrenLocksForUpdate), so setParentLinkTx's later
re-lock is an idempotent no-op and the combined update stays
deadlock-free. checkParentCycle is parameterized over the queryer
so the cycle walk reads inside the tx.
- handler: restructured into validate / atomic-write / post-commit
stages. The parent-link directive is built once and threaded through
all three UpdateItemWithParentLink call sites; the post-commit
SetParentLink/ClearParentLink block is removed.
Works on both SQLite and Postgres (advisory locks are pg-only; SQLite
gets atomicity from BEGIN IMMEDIATE). Adds store tests proving a
failing parent-link write rolls back the field change (no partial
state) and that the happy path commits both together.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
* fix(store): check parent cycle under lock in setParentLinkTx (Codex #868)
Codex review flagged a cycle-check TOCTOU: setParentLinkTx walked the
ancestor chain BEFORE acquiring the parent-children advisory locks, so
two concurrent reparents could each pass on a stale snapshot, block on
the lock, then both insert — forming a cycle (A→B→C→A). Move the cycle
check to AFTER lock acquisition; under the lock the tx-scoped walk sees
the edge the just-unblocked peer committed and rejects the cycle.
Pre-existing behavior (the old SetParentLink checked cycles on s.db
before even beginning its tx), hardened here since this function was
already being refactored. Residual: cycles closed via an edge on an
item neither endpoint locks remain possible — a limitation of the
per-endpoint lock scheme, tracked separately.
Claude-Session: https://claude.ai/code/session_019knGmnHcx5rrgWXQ8V8DZS
xarmian
a7994fc374