fix(bindx): stop writing a child that its parent deletes - #102
Merged
matej21 merged 3 commits intoSep 9, 2026
Merged
Conversation
A has-many item that is dirty AND planned for removal with `delete` got a
standalone `update<Entity>` on top of the nested `{ delete: { id } }` in its
parent's update. With the default sequential adapter the parent runs first,
so the standalone update hit a row that was already gone: the save failed
with NotFoundOrDenied although the deletes had landed on the server. The
everyday trigger is a sortable Repeater, which renumbers the survivors before
removing the next item, leaving later-removed items dirty and removed at once.
HasManyStore now keeps a refcounted index of the children some relation plans
to remove with `delete`, maintained by diffing previous against next state in
the writeHasMany / deleteHasMany chokepoints, next to the live-edge index, and
exposed through RelationStore and SnapshotStore. BatchPersister.buildMutations
skips the standalone update of such an entity.
The skip lives in buildMutations rather than in DirtyTracker on purpose: the
entity:persisting interceptors are offered exactly the dirty entities, and a
veto there is what suppresses the parent-side delete (54bf2b0). Dropping the
item from getAllDirtyEntities() would make it unvetoable.
Second half of the same bug: after a confirmed nested delete the child's
snapshot survived in the store and stayed dirty, so once commitAllRelations
cleared the planned removal the next save updated a row that no longer
existed. reconcileConfirmedEntities now drops those children, mirroring what
the top-level delete path already does.
Untouched by design: `disconnect` removals keep their standalone update (the
row survives), and a created-then-removed item never reaches the index —
removeFromHasMany cancels its addition instead of planning a removal.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Euwkf2wtutqvtE4YRutU5R
The previous commit purged children deleted through a parent for BOTH relation
kinds, but suppressed the standalone update for has-many only. On the has-one
path that combination was worse than either half alone: the target was dropped
from the store while its standalone update was still sent, and processing that
update's result resurrected a clean snapshot whose data had lost its `id`
field — a state that did not exist before.
A has-one target marked `deleted` is removed by the nested `{ delete: true }`
in its parent's update, exactly like a has-many `delete` removal, so it belongs
in the same index. HasOneStore now maintains it in its own writeRelation /
deleteRelation chokepoint, and RelationStore.isPlannedForDeleteByParent unions
both sub-stores. The refcounted multiset moves into PlannedDeleteIndex, shared
by the two stores the way RelationEdgeIndex already is.
The indexed id is `serverId`, not `currentId`: MutationCollector deletes
serverId for a `deleted` relation, and a null serverId emits no delete at all,
so that target must keep its own update.
Vetoes are unaffected on both paths — the skip lives in buildMutations, so a
target cancelled by an entity:persisting interceptor is still offered, still
suppresses the parent-side delete, and keeps its snapshot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Euwkf2wtutqvtE4YRutU5R
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was broken
A child entity that is dirty and already scheduled to be removed by its parent got its own top-level
update<Entity>mutation on top of the nested delete in the parent's update. With the default sequential adapter (ContemberAdapterhas nopersistTransaction) the parent update runs first and the row is gone, so the standalone update answersNotFoundOrDeniedand the whole save is reported as failed — while the deletes have already landed on the server.This PR covers both relation kinds, which are the same bug:
plannedRemovalswith typedelete. The everyday trigger is a sortableRepeater/BlockRepeater:remove()renumbers the surviving items beforefield.remove(id), so removing two or more items in one gesture leaves every later-removed item dirty and removed at the same time.deleted, removed by the parent's{ delete: true }. Same failing shape, same cause.The fix — two halves, one set
1. No standalone update for a child its parent deletes.
A refcounted
PlannedDeleteIndex(shared the wayRelationEdgeIndexalready is) tracks the children some relation plans to delete through its parent. Each relation sub-store maintains it in its own single write chokepoint by diffing previous state against next —HasManyStore.writeHasMany/deleteHasManyforplannedRemovalsof typedelete,HasOneStore.writeRelation/deleteRelationfor statedeleted. No second write path, no scanning of relation state on read.RelationStore.isPlannedForDeleteByParentunions the two sub-stores,SnapshotStorere-exposes it, andBatchPersister.buildMutationsskips the standalone update of such an entity.The has-one side indexes
serverId, notcurrentId:MutationCollectordeletesserverIdfor adeletedrelation, and a nullserverIdemits no delete at all, so that target must keep its own update.The skip sits in
buildMutationsrather than inDirtyTracker.getAllDirtyEntities. That placement is load-bearing:entity:persistinginterceptors are offered exactly the dirty entities, and a veto there is what suppresses the parent-side delete (see 54bf2b0). Dropping the item from the dirty set makes it unvetoable — three existing tests invetoedDeleteStaysPending.test.tsandnestedDeleteOfDirtyEntity.test.tsfail that way. Keeping it in the dirty set and only withholding its mutation preserves both contracts, on both relation kinds.2. Purge the child after a confirmed nested delete — why this is needed.
Without it the first save succeeds and the bug comes straight back. After a confirmed nested delete the child's snapshot survived in the store and stayed dirty:
reconcileConfirmedEntitiescalledstore.removeEntity(...)only for entities whose own top-level operation wasdelete, while the relation loops folded the removal into the server baseline and never dropped the child. OncecommitAllRelationscleared the planned removal, the child was dirty again, the save button stayed dirty, and the next save emitted an update for a row that no longer exists — the sameNotFoundOrDenied.reconcileConfirmedEntitiesnow drops the children of every confirmed has-many removal of typedeleteand of every confirmed has-onedeletetransition, mirroring what the top-level delete path already does. The child's type comes from the collected relation fields, the only place a nested child's entity type is recorded.Both halves now cover exactly the same set. An earlier revision of this branch purged both relation kinds but suppressed the update for has-many only; on the has-one path that dropped the target from the store while still sending its update, and processing that update's result resurrected a snapshot whose data had lost its
idfield. That is fixed here, and pinned by a test.Deliberately untouched:
disconnectremovals and plain has-one disconnects keep their standalone update — the row survives.scheduleForDeletion-ed keep reportingdeleteand keep their own mutation alongside the parent-side delete (54bf2b0).removeFromHasManycancels theirplannedAdditionsentry instead of planning a removal.How it was verified
updateTag(tag-2, { order: 0 })was emitted after the parent's deletes.updateBlockwas emitted next tocover: { delete: true }, and the ghost snapshot came back asdata: { title: "Edited" }with noid.getAllDirtyEntities()is empty after a successful save (the regression that would otherwise re-open this on the second save), for both relation kinds;disconnectkeeps its standalone update and its snapshot; a created-then-removed item stays out of the persist entirely; a has-one target vetoed by anentity:persistinginterceptor is still skipped, still suppresses the parent-side delete, and keeps its snapshot.bun run test, everything excepttests/browser).bun run typecheckclean,bun run lintreports 0 errors (17 pre-existing warnings, none in the touched files).Fixes #91 — and the has-one variant of the same bug, which was never filed separately.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Euwkf2wtutqvtE4YRutU5R