Ann and Bob don't exist. They're two synthetic strangers in the test workspace of a CRM bridge I was about to publish. Ann is ann@x.example, Bob is bob@x.example, and both typed N/A into the phone field of a lead form, because that's what people type into a required phone field they'd rather not fill. My fake CRM merged them. Bob's upsert came back carrying Ann's contact id with Ann's conversation history hanging off it, and the audit ledger recorded contact_created twice. Two people created, one person stored, and the second creation line quietly naming the first one's id.
I caught it in a pre-publish review pass, not in production, which is the only comfortable part of this story. It's the third time in a few months I've hit the same family of bug, and this one stings in a specific way: it walked past a doctrine I had already written down twice.
The first incident was production. Python's casefold() merged two of my customers into one tenant: Unicode case folding is many-to-one, U+212A KELVIN SIGN folds to k, and two distinct identifiers became one entitlement key. The second was last week. My new repo argued against the exact bug it was carrying: the package preached ASCII-only folding while its own test fakes normalised emails with str.lower(). The doctrine existed; a caller just didn't call it.
This third one is worse in an instructive way, because the doctrine was right in both places you'd think to look.
The repo, for context
ghl-bridge is a policy-gated bridge between a GoHighLevel location and whatever generates reply drafts. Webhook in, dedupe, then a gate that only auto-sends a message when every policy passes, idempotency keyed on the event rather than the delivery, and a guard that raises if anything unapproved tries to leave. 379 tests, all offline, no account and no API key behind any of it.
Identity in this package follows two rules I've now paid for personally. Email folds ASCII case only, because str.lower() and str.casefold() apply Unicode tables that merge characters which were never case variants of each other. And phone is deterministic E.164 or an explicit refusal: a bare national number resolves only through the location's configured region, and when the rules can't determine the answer, the result is a PhoneNeedsReview value carrying the raw input and a named reason. A value, not an exception. The caller has to route it to a human queue on purpose, because guessing a country code merges strangers.
Both rules held. normalise_email was clean. normalise_phone was clean. Every caller I'd checked called them.
The door nobody was watching
The fake workspace models the platform's documented upsert semantics: within a location, an incoming contact that matches on email or phone updates the existing record instead of creating a new one. To do that it keeps an email index and a phone index, and before the review pass, its phone key helper had a fallback:
def _phone_key(self, state: _LocationState, raw: str) -> str:
result = normalise_phone(raw, default_region=state.location.default_region)
if isinstance(result, NormalisedPhone):
return result.e164
return raw.strip()
Look at how reasonable it is. The doctrine function is right there, called first. The fallback only fires for values the normaliser refused, and it just keeps them, tidied up, so the index stays total. Every visible key path in the package was doctrine-clean, and then the else branch of one private helper inside the test fake overruled the whole argument.
Run the sequence and the consequences arrive in order. Ann upserts with phone N/A; the normaliser refuses; the index files her under the literal string N/A. Bob arrives an hour later with the same placeholder; his email misses; his phone "matches". Merge. From then on, every lead whose phone field says N/A chains into that same contact record, which keeps getting fatter. Meanwhile the deduper, which computes its own doctrine-clean keys, saw nothing to match for Bob, so it logged contact_created while the store underneath merged him away. The audit trail and the data disagreed. In a real CRM this is one stranger reading another stranger's conversation history.
And the detail that made me laugh, the bitter kind: raw.strip() is the Unicode strip. The package's own identity.py defines a six-character ASCII whitespace constant precisely because bare str.strip() trims things like U+3000 IDEOGRAPHIC SPACE and merges identifiers that differ only there. So ext-12 and ext-12 followed by an ideographic space also folded onto one index key. The fallback didn't merely bypass the phone doctrine. It committed the exact sin from the email doctrine on its way past.
The fix is structural, and it's None
def _phone_key(self, state: _LocationState, raw: str) -> str | None:
"""The E.164 dedupe key, or None when the number does not determine
one. An unresolvable phone is stored as a field but never indexed:
indexing the raw string would merge two strangers who both typed
"N/A" into the form, and trimming it with ``str.strip`` would break
the ASCII-only doctrine everything else keys on. No key, no match,
no merge."""
result = normalise_phone(raw, default_region=state.location.default_region)
if isinstance(result, NormalisedPhone):
return result.e164
return None
The callers grew None checks: indexing skips it, phone search returns empty. The raw value still gets stored on the contact as a field, so a human reading the record sees exactly what arrived; it just never becomes a match key. Three tests now pin the behaviour: test_two_strangers_with_the_same_unparseable_phone_do_not_merge is the Ann and Bob story, test_an_unparseable_phone_is_stored_as_a_field_but_never_indexed pins the stored-but-never-keyed split, and test_the_ascii_trim_doctrine_holds_for_phone_indexing_too carries a literal U+3000 in its source. You can't see the character, which is exactly the point.
Two lessons worth carrying out of the repo, because both apply anywhere identity gets deduped.
A normalisation failure is not an identity. When the normaliser says "I don't know", indexing the raw value turns every placeholder into a master key: every N/A equals every other N/A, every none, every -, every xxx. This isn't a CRM quirk. Any dedupe, ETL join, or entity-resolution pass keyed on a column that contains placeholders has this failure mode, and the placeholder density of a phone column is never zero. The irresolvable key has to index as nothing at all.
One rule, and it's only as strong as its least visible door. Three incidents, one shape: a many-to-one transformation over identity is a merge, and a merge is a grant. In the tenant incident the grant was entitlements. In the on-behalf repo it would have been document access. Here it's two strangers becoming one contact with one shared history. What moved between the incidents is where the door was. In incident two, the doctrine existed and a caller didn't call it. In incident three, the callers all called it, and the leak was the fallback path inside one of them, in a test fake, the component everyone extends and nobody audits because "it's just the fake". The fake is the thing your entire suite treats as ground truth. A doctrine that stops at the fake's front door is a lobby sign.
What the fix does not fix
A placeholder that happens to parse walks straight through the front door: 5555555555 under a US-region location resolves to a legitimate-looking E.164 and mints a real key, so two strangers typing it will still merge. Refusing the unparseable stops junk from acting as a wildcard; it does not detect junk. Catching parseable junk needs a known-junk list or an alarm on keys that match suspiciously often, and this repo ships neither yet. Second, the fake is my model of the documented upsert semantics; whether the live platform's own server-side matching indexes raw strings is exactly the kind of thing the RUNBOOK says to verify against a real workspace before trusting anything. Third, the fix has a stated cost: an N/A lead with an unknown email now creates a fresh contact every time. That's deliberate. It fails toward duplicates a human can merge instead of merges nobody can split, but your review queue will feel the difference.
The doctrine, the fake, the deduper and the three anchor tests are all in github.com/vinimabreu/ghl-bridge if you want to run the Ann and Bob case yourself.
A key you couldn't compute is not a key to tidy up. It's no key at all.
Top comments (0)