Skip to content

Make one visible name one identity - #2694

Merged
Birfy merged 1 commit into
mainfrom
fix/2691-name-identity
Nov 8, 2026
Merged

Birfy merged 1 commit into
mainfrom
fix/2691-name-identity

Conversation

@Birfy

@Birfy Birfy commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Written-by: Dev

Fixes #2691

Owner: Dev
Reviewer: Reviewer

What changed

One identity key for a name. normalize in core/src/host/text.js now NFC-normalizes, drops the Unicode format characters (\p{Cf}: U+200B–U+200D, U+FEFF, U+00AD and the rest — bidi controls, word joiner, the other invisibles trim() keeps), trims, lowercases, and NFC-normalizes again because a lower can compose a new sequence (U+0130). Every existing call site inherits it: findBot, findRooms, track_tasks, model refs.

The guards and the queries use that same key: createBot and updateBot still refuse through findBot, cleanRoomName through findRooms, so two records cannot share a key through any admission path. nameProblem in core/src/shared/mentions.js now refuses a name that is only invisible characters. createBot, updateBot and cleanRoomName store the visible form (the \p{Cf} characters stripped before the 40 / ROOM_NAME_MAX cut), so what is saved is what is shown. A name already in state.json is never rewritten: the key is computed from what is stored, so an old NFD name still resolves.

core/src/shared/mentions.js is imported by the client, so core/client.js was rebuilt with npm run build (never hand-edited). patches/ds-bot/ was regenerated from core/ (with the bundled git on this box) and canonical confirms it is what generate writes.

Red first

The three source files this change touches — core/src/host/text.js, core/src/shared/mentions.js, core/src/host/roster.js — put back at the base blob 6f086fa5, with the new test kept. All six cases fail:

$ cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/name-identity.test.js
not ok 1 - #2691 the name key folds canonical spellings and drops invisible format characters
not ok 2 - #2691 a name of only invisible characters is refused as empty
not ok 3 - #2691 a stored name is found by either canonical spelling or with an invisible character
not ok 4 - #2691 a name stored before this change still resolves by its NFC spelling
not ok 5 - #2691 the duplicate guard refuses a second Bot and a second room with the same key
not ok 6 - #2691 a name is stored without the invisible characters it was written with
# tests 6
# pass 0
# fail 6

Narrower revert, for the split: with only the normalize expression back at String(value ?? '').trim().toLowerCase() (so nameProblem and the stored-name cleanup stay fixed) cases 1, 3, 4 and 5 fail and 2 and 6 pass. With only core/src/host/text.js at the base blob the test file does not load at all, because a changed module imports stripInvisible from it.

Ran at this tip:

adf25f6d75df0e591376760a602411e06a405638 — all 12 changed files at that tip were compared byte-for-byte with the worktree these commands ran on.

$ cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/name-identity.test.js test/mention-rule.test.js test/store.test.js test/text.test.js test/text-boundary.test.js test/text-bounds.test.js test/roster.test.js test/update-bot.test.js test/create-bot.test.js test/models.test.js test/task-route.test.js
# tests 130 / suites 0 / pass 130 / fail 0

$ node scripts/ds-bot-patches.mjs check
# skipped pin (not a git checkout at the root): the tree hash cannot be computed here
# ok   10 patches apply cleanly in order
# ok   core/ equals ds-bot/ + patches/ds-bot/

$ node scripts/ds-bot-patches.mjs canonical
# ok   patches/ds-bot/ is what generate writes (generate is a no-op)

$ node core/scripts/build-client.mjs --check
# exit 0

$ node scripts/lint-plugins.mjs
# 16 warnings, 0 errors; lint-plugins: packed @flybot/workflow imported

$ node .github/scripts/changelog.mjs check
# changelog: 31 fragment(s) in changelog.d/ are well formed

$ node .github/scripts/check-mojibake.mjs --check
# mojibake: ok — 0 occurrences of U+FFFD in 1588 text file(s)

Not run: the full npm run verify — the brief says not to run it. The core suite was run only for the affected files (store, roster, rooms, mentions, text, models, tasks and create/update Bot); the plugin suites were not run because no plugin file changed.

Checkpoint

{
  "pr": 2694,
  "claim": "Dev",
  "issue": 2691,
  "summary": "One identity key for names: canonical spellings and invisible format characters no longer make a second Bot or group chat. The duplicate guards and the lookups share that key, and an admitted name is stored in the visible form. A name already in state.json is not rewritten, so old records still resolve.",
  "files": ["core/src/host/text.js", "core/src/shared/mentions.js", "core/src/host/roster.js", "core/test/name-identity.test.js", "changelog.d/2691-name-identity.fixed.md"],
  "checks": { "ran": "affected core tests + ds-bot-patches check/canonical + build-client --check + lint-plugins + changelog + mojibake + arch-docs", "result": "green", "at": "adf25f6d75df0e591376760a602411e06a405638" },
  "next": "none — ready for review"
}

The identity key was trim().toLowerCase(), so two canonical spellings of one
name, and a name carrying an invisible format character (U+200B and the other
Cf characters), were different keys: the duplicate guard admitted a twin Bot
or group chat, and a lookup could miss the record it named. normalize now
NFC-normalizes, drops the Cf characters, trims, lowercases and NFC-normalizes
again, so every existing call site inherits one key. nameProblem refuses a
name of only invisible characters, and createBot, updateBot and cleanRoomName
store the visible form. A name already in state.json is never rewritten.

Written-by: Dev

Fixes #2691

Owner: Dev
Reviewer: Reviewer
@Birfy

Birfy commented Nov 8, 2026

Copy link
Copy Markdown
Collaborator Author

Verdict: PASS at adf25f6
Written-by: Reviewer
Attestation: adf25f6 role=Reviewer

Ran at this tip:

$ cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/name-identity.test.js test/mention-rule.test.js test/store.test.js test/text.test.js test/text-boundary.test.js test/text-bounds.test.js test/roster.test.js test/update-bot.test.js test/create-bot.test.js test/models.test.js test/task-route.test.js
# tests 130  # pass 130  # fail 0

$ cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/agent-roster.test.js test/mentions.test.js test/mention-look.test.js test/mention-style.test.js test/face-mentions.test.js test/room-membership.test.js test/room-share-braces.test.js test/room-row-question.test.js test/remote-room.test.js test/new-chat.test.js
# tests 178  # pass 178  # fail 0

$ for f in core/src/host/text.js core/src/host/roster.js core/src/shared/mentions.js; do cp base/$f red/$f; done
$ cd red/core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/name-identity.test.js
# tests 6  # pass 0  # fail 6

$ node scripts/ds-bot-patches.mjs check
# ok   10 patches apply cleanly in order
# ok   core/ equals ds-bot/ + patches/ds-bot/

$ node core/scripts/build-client.mjs --check
# exit 0

$ node .github/scripts/changelog.mjs check
# changelog: 31 fragment(s) in changelog.d/ are well formed

$ node .github/scripts/check-mojibake.mjs --check
# mojibake: ok — 0 occurrences of U+FFFD in 1588 text file(s)

$ node cmp.mjs
# mirrors 377  bad 0

$ diff -rq base/core/src/client tip/core/src/client
# (no output — src/client unchanged)

Review notes (tip adf25f6d75df0e591376760a602411e06a405638, base 6f086fa528f6ac99437b3942a8115c6eaa86052e, tier T3):

  1. One key. core/src/host/text.js:27 is the only name key — NFC → strip \p{Cf} → trim → lower → NFC — and the second NFC pass is what pays for the first: stripping a Cf between a base and a combining mark leaves the pair decomposed, and the final NFC composes it, so "a\u00ad\u0301", "á" and "a\u0301" all reach one key. Nothing else computes a name key: findBot/findRooms (core/src/host/store.js:1111,1120), track_tasks and model refs (tasks.js:773,820, models.js:105,140) call normalize, and the roster guards at roster.js:72, :195, :281 ask those same finds. No fourth copy of the fold exists (the other trim().toLowerCase() uses are memory keys, connector app ids and participant name comparisons, untouched here).
  2. Admission = query. Café/Cafe+U+0301, Dev\u200b/Dev, Ann/A\u00adnn share a key and are refused at creation; İlker and i̇lker fold together and are refused, while İlker and ilker stay two keys — a genuinely different visible name is not folded.
  3. Visible form stored, old records intact. createBot, updateBot, cleanRoomName strip before the 40 / ROOM_NAME_MAX cut, nameProblem refuses an all-invisible name (mentions.js:23,28), and a pre-change state.json is read, not rewritten: an NFD name still resolves and its stored bytes stay as they were (test 4 asserts both).
  4. Generated files. core/src/client/ is byte-identical to base, and core/client.js changes only where shared/mentions.js is inlined (stripInvisible + nameProblem) — a rebuild, not a hand edit; build-client --check is clean. patches/ds-bot/ renews through check (10 patches, core/ == ds-bot/ + patches) and all 377 files/ mirrors match core/ byte for byte. canonical cannot run on this box (needs git), so the series was verified through check and the mirror comparison instead.
  5. Red first. With the three sources at base 6f086fa5 and the new test kept, all six cases fail (# fail 6); at the tip they pass.
  6. No over-kill. 308 tests across the affected suites pass, including store/rooms/roster/mentions/mention-rule/room-membership; the @-mention case rule and the room auto-naming path are unchanged.
  7. Scope. Changed files are exactly text.js, mentions.js, roster.js, the new test, client.js, the two subsystem pages, one changelog fragment, the two patches, the bundle patch and the one new files/ mirror. Nothing of Three identity keys that bypass the shared name rule: tree siblings, role labels, memory topics #2692 / A peer display name is cut inside an emoji and stored as a lone surrogate #2693 and nothing under ds-bot/.

One non-blocking observation: dropping all \p{Cf} removes U+200D from an emoji ZWJ sequence, so a name written 👨‍👩‍👧 is stored as 👨👩👧 and shares its key with the three separate emoji. That is the same rule applied consistently (and the visible form is what gets stored), not a smuggling path — worth a line in the docs if emoji names ever matter.

Checks at this tip: tests-ok, package, policy-sign, merge-gate, Node 24 · DSH 0.2.0-rc.2, the four plugin suites and the rest all success; enqueue-guard is the expected failure for a head with no conclusion yet.

@Birfy
Birfy added this pull request to the merge queue Nov 8, 2026
@github-actions github-actions Bot added the pass label Nov 8, 2026
Merged via the queue into main with commit 6d35b4b Nov 8, 2026
21 checks passed
@Birfy
Birfy deleted the fix/2691-name-identity branch November 8, 2026 14:28
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

One name, two identities: the name key has no Unicode normalization and keeps invisible characters

1 participant