Skip to content

fix(mvc.Collection): id lookups can return the wrong model when a user id collides with a cid - #3487

Merged
kumilingus merged 1 commit into
clientIO:masterfrom
kumilingus:fix/collection-cid-id-collision
Aug 27, 2026
Merged

fix(mvc.Collection): id lookups can return the wrong model when a user id collides with a cid#3487
kumilingus merged 1 commit into
clientIO:masterfrom
kumilingus:fix/collection-cid-id-collision

Conversation

@kumilingus

Copy link
Copy Markdown
Contributor

Description

mvc.Collection stored models in a single _byId map under both their model id and their internal client id (cid). cids are generated as c<counter> with a counter global to the JS realm that never resets — so a user-supplied id matching /^c\d+$/ (e.g. "c856") is clobbered as soon as the session has created that many models and some other model's auto-generated cid collides with it.

Two silent failure modes:

  • graph.getCell('c856') returns the wrong cell — and everything downstream of it: link end resolution, findViewByModel, etc.
  • worse, set()/add() dedup runs through get(), so a genuinely new model whose cid collides with an existing model's id is silently merged away as a duplicate and never added.

Both are timing/volume dependent — a graph loads fine on a fresh page and breaks after enough models have been created in the session — which makes them look like heisenbugs.

Observed in the wild: demos/libavoid-standalone-link-routing example-2.json had an element with id "c856"; after loading a second large diagram, a link's cid collided, @joint/router-avoid's validateEnds() resolved the link's end to another link and permanently fallback-routed it.

Fix

Ids and cids now live in separate maps (_byId / _byCid):

  • get() checks real ids first — a real id always wins the string lookup;
  • cid string lookup (collection.get('c12'), Backbone compatibility) still works whenever no real id claims the string;
  • removing the id-holder no longer severs the other model's cid entry (the shared-map design also corrupted removal).

Inherited from Backbone's design; any graph with generated short ids is exposed.

Tests

  • New: get: a model id colliding with another model's cid — pins winning lookup, both models reachable, removal of the id-holder restores the cid string lookup. Fails on master (the colliding model is merged away as a duplicate).
  • Updated: the legacy _addReference binds all collection events & adds to the lookup hashes test asserts against _byCid for the cid entry (it inspects the internal maps directly).
  • Full client suite: 2113/2113.

🤖 Generated with Claude Code

A single _byId map stored models under both their id and their cid.
cids are generated as c<counter> with a realm-global counter, so a
user-supplied id matching /^c\d+$/ (e.g. 'c856') was clobbered as soon
as some other model's auto-generated cid reached the same value:
graph.getCell('c856') returned the wrong cell, and set() could silently
merge a genuinely new model away as a duplicate of the id-holder.

Ids and cids now live in separate maps and a real id always wins the
string lookup; cid string lookup still works when no id claims the
string.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@kumilingus
kumilingus merged commit f7455fd into clientIO:master Aug 27, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants