Skip to content

[CONS-513] fix(sync): derive username collisions from an email hash instead of sequential probing#27

Merged
bordoni merged 3 commits into
mainfrom
fix/cons-513-username-generation-oom
Jun 11, 2026
Merged

[CONS-513] fix(sync): derive username collisions from an email hash instead of sequential probing#27
bordoni merged 3 commits into
mainfrom
fix/cons-513-username-generation-oom

Conversation

@redscar

@redscar redscar commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

Production has pages of WorkOS users — effectively all with info@ emails — who can log in via WorkOS but never get a WordPress account. The provisioning request dies before wp_insert_user() ever runs: generate_username() resolves collisions by probing info, info_1, info_2, … one username_exists() call at a time, and every call hydrates a WP_User into the object cache. info is the most common shared-mailbox local part, so its chain is thousands deep — the walk exhausts the 256M heap and the request fatals (fatals.log shows the matching exhaustion in class-wp-user.php:535 and object-cache-pro's client). Worse, it's self-reinforcing: once the chain is deep enough to OOM, every later info@ signup dies at the same wall, which is why the failures are all the same prefix.

The fix drops the probe entirely. Emails are unique, so a suffix derived from sha256( email ) is unique by construction — there's no need to look at the existing chain at all. The generator now does at most 2 exact-match lookups whether there are 16 or 16,000 info_* users, and the same email always derives the same username, so re-provisioning (crash retries, replayed webhooks) converges instead of minting new names.

What usernames look like now

Scenario Result
info@example.com, first info@ ever info (unchanged)
info@acme-widgets.com, info taken info_48f25
another user already owns info_48f25 (~1.5% at 16k users) info_48f257791ee0 (same hash, widened)
even that taken (~2⁻⁴⁸) salted re-derivation, sha256( email|attempt ), max 3
63-char local part base capped at 47 so the widest suffix lands exactly on user_login's varchar(60)

Those are real values — sha256('info@acme-widgets.com') starts 48f257791ee0…, which is also what the tests pin. Existing accounts keep their current names; only newly provisioned users get the new shape. Nothing keys on user_login (identity flows use email + WorkOS ID), so no downstream changes.

Two things worth calling out from review of my own first draft: strtok( $email, '@' ) silently skips a leading @, so @example.com dodged the workos_user fallback — replaced with explode(). And username_exists() in a for condition cost a redundant lookup on the common path; early returns keep it at exactly 2.

What changed

  • src/WorkOS/Sync/UserSync.phpgenerate_username() rewritten as above; behavior for a free base is unchanged.
  • tests/wpunit/UserSyncGenerateUsernameTest.php — new. A data provider walks the full collision ladder with precomputed hashes, plus standalone tests for idempotency, truncation not causing collisions, and the regression that matters: 150 seeded info_* users → exactly 2 lookups (the old code needs 152), counted via the username_exists filter so caching can't mask it.
  • tests/wpunit/UserSyncFindOrCreateTest.php — one end-to-end case pinning the exact created login (info_48f25) through find_or_create_wp_user().

Test plan

slic run tests/wpunit/UserSyncGenerateUsernameTest.php   # 11 tests
slic run tests/wpunit/UserSyncFindOrCreateTest.php       # 11 tests, incl. new e2e
slic run wpunit                                          # full suite

Full suite: 564 tests, 4 failures — the same 4 "when plugin disabled" tests fail on a pristine main checkout, so they're pre-existing and unrelated (verified by stashing this change and re-running).

After this ships

The stuck users need no manual repair — the bug never created anything, it just prevented creation. Their next login walks the now-working create path, links the account, and the downstream import follows. wp workos sync import can pre-provision the backlog in one pass if we don't want to wait for organic logins.

🤖 Generated with Claude Code

redscar and others added 3 commits June 11, 2026 12:33
…ial probing

generate_username() resolved collisions by probing info, info_1,
info_2, ... with no upper bound. Each username_exists() call hydrates
and caches a WP_User, so on deep shared-mailbox chains (info@ being the
deepest in production) the walk exhausted the 256M heap and fataled the
request before wp_insert_user() ran — WorkOS sessions existed but no WP
account was ever created (CONS-513).

The generator now appends a 5-hex sha256(email) suffix when the bare
local part is taken, widening to 12 hex and then salted re-derivations
on the astronomically rare collisions. Lookups are constant (2 on the
common path) regardless of chain depth, and the same email always
derives the same username, so re-provisioning is idempotent.

Also switches local-part extraction from strtok() to explode():
strtok skips leading delimiters, so @example.com bypassed the
workos_user fallback.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…te_wp_user

The generator's unit tests invoke it via reflection; this covers the
public provisioning path with a taken local part, pinning the exact
created login (info_48f25) rather than just a prefix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replaces the eight per-rung test methods with one provider-driven test
whose named cases read top to bottom as the ladder itself (bare base →
5-hex → 12-hex → salted retries), each arranging its collisions via a
seed closure. Inline FQCNs move to use imports, and every case picks up
a blanket never-exceeds-60-chars assertion.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@redscar redscar self-assigned this Jun 11, 2026
@bordoni bordoni added this to the 1.0.6 milestone Jun 11, 2026
@bordoni bordoni merged commit 58da18a into main Jun 11, 2026
7 checks passed
@bordoni bordoni deleted the fix/cons-513-username-generation-oom branch June 11, 2026 20:37
bordoni added a commit that referenced this pull request Jun 11, 2026
- CHANGELOG.md: Fixed entry for the email-hash collision suffix (#27)
- readme.txt: matching Fix line and Upgrade Notice mention
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