fix: [SDK-5014] omit local subscription IDs from the create user request

[sherwinski]

Description

1 Line Summary

Stop the SDK from sending local-* placeholder subscription IDs in the Create User request, which the backend rejects as invalid UUIDs.

Details

The login() and logout() flows enqueue a TransferSubscriptionOperation with the current push subscription model ID. That ID can still be a local-* placeholder when the create-subscription operation did not complete first (for example, after a network failure dropped the pending operations). _canStartExecute blocks such a transfer op from starting on its own, but OperationRepo._getGroupableOperations does not check _canStartExecute for grouped operations. A LoginUserOperation with the same comparison key groups the transfer op, and the transfer branch in LoginUserOperationExecutor copied the local ID into the payload as {"id": "local-..."}. The backend rejects the request with uuid: incorrect UUID length 42.

The failure then persists: the 400 maps to FailPauseOpRepo, so the operations re-queue, persist in IndexedDB, and fail again on every page load. Affected users stay unsubscribed until they clear site data.

The fix changes the transfer branch for a local subscription ID. A transfer grouped with a create op keeps the create data and adds no id. A lone transfer op rebuilds the subscription data from the model in SubscriptionModelStore, again with no id, so the backend creates the subscription and ID translation still runs. A model with no token is skipped, because the store listener never sends a token-less subscription as enabled and the model self-heals on the next subscribe. A create user request that ends with no identity and no subscriptions is dropped (FailNoretry) instead of sent, which matches the Android SDK. Users who are stuck today recover on their next page load because the persisted login operation now sends a valid payload.

The size limit for OneSignalSDK.page.es6.js moves from 42.7 kB to 42.78 kB; the new branch pushed the gzip size 67 bytes over the old limit.

The manual repro harness used during review is not part of the final diff. Commit 9a0f42e on this branch has the last version of it, and the PR comment "How to test this manually" has the steps.

Systems Affected

• WebSDK
• Backend
• Dashboard

Validation

Tests

Info

5 new unit tests in LoginUserOperationExecutor.test.ts. Before the fix, the first test reproduced the exact invalid payload from the customer report (subscriptions: [{"id": "local-..."}]). The other tests cover the create-plus-transfer group, the rebuild from the model on logout, the token-less model, and the dropped empty payload. All pass after the fix, along with the full suite, vp check, and vp run build:prod.

Checklist

• All the automated tests pass or I explained why that is not possible
• I have personally tested this on my machine or explained why that is not possible
• I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

• Don't use default export
• New interfaces are in model files

Functions:

• Don't use default export
• All function signatures have return types
• Helpers should not access any data but rather be given the data to operate on.

Typescript:

• No Typescript warnings
• Avoid silencing null/undefined warnings with the exclamation point

Other:

• Iteration: refrain from using elem of array syntax. Prefer forEach or use map
• Avoid using global OneSignal accessor for context if possible. Instead, we can pass it to function/constructor so that we don't call OneSignal.context

Screenshots

Info

Not needed: the change affects a network payload with no UI impact. The unit tests assert the payload contents.

Checklist

• I have included screenshots/recordings of the intended results or explained why they are not needed

---

Related Tickets

SDK-5014

---

[sherwinski]

@claude review once

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[sherwinski]

How to test this manually (updated for the logout path)

The branch includes a repro harness in index.html (dev server page). It has 4 ordered repro buttons, a state card that shows whether the push subscription ID is a local-* placeholder, a reset button, and a log that prints every Create User request and response.

Background: the bug needs a push subscription whose ID is still a local-* placeholder, with no pending create operation left to translate it. In production this happens when the SDK drops the queued operations (see the PR description). The "Seed stuck local ID" button creates the same state instantly: it rewrites the subscription model ID with the NoPropogate tag, so no operation is enqueued. The logout step then exercises Create User deterministically — a login after the seed can take the Set Alias path instead, because the OneSignal ID is already server-assigned.

Setup

1. Run vp install, then vp run dev.
2. Open the printed localhost URL with ?app_id=<your test app id> (or set VITE_APP_ID in .env). Use a test app that allows the localhost origin.

See the bug (without the fix)

1. From main, copy only the harness: git checkout main && git checkout sherwin/sdk-5014 -- index.html.
2. Click Reset SDK state.
3. Click 1. Subscribe and grant permission. Wait for the green "server-assigned" tag.
4. Click 2. Login.
5. Click 3. Seed stuck local ID. The tag turns red.
6. Click 4. Logout. The log shows the Create User request with "subscriptions": [{"id": "local-..."}] and the 400 response: uuid: incorrect UUID length 42.
7. Reload the page. The same 400 repeats with no clicks, because the failed operations persist in IndexedDB.

Confirm the fix

1. Check out this branch (discard the copied index.html first: git checkout -- index.html).
2. Repeat steps 2–6. At step 6 the request carries the full subscription data (token, type, and so on) with no id, and the response is 200. The state card then shows a new server-assigned subscription ID for the anonymous user.
3. Recovery check: reproduce the stuck loop on main, then switch to this branch and reload without a reset. The persisted operations now send a valid payload and succeed.

Note: an anonymous create user request with no identity and no subscriptions (the user-3 case from the earlier review) no longer goes to the network; the SDK drops the operation instead.

[fadi-george]

maybe explore a change like this in LoginManger:

if (pushOp) {
  const operation = IDManager._isLocalId(pushOp.id)
    ? new CreateSubscriptionOperation({
        appId,
        onesignalId: newOneSignalId,
        subscriptionId: pushOp.id,
        type: pushOp.type,
        enabled: pushOp.enabled,
        token: pushOp.token,
        notification_types: pushOp._notification_types,
      })
    : new TransferSubscriptionOperation(appId, newOneSignalId, pushOp.id);
  OneSignal._coreDirector._operationRepo._enqueue(operation);
}

[dombartenope]

@fadi-george @sherwinski is this something going live in an upcoming update?

[cursor]

Review

Approved — this is the right minimal fix for SDK-5014.

The root-cause analysis checks out: _canStartExecute blocks a lone TransferSubscriptionOperation with a local-* ID, but _getGroupableOperations does not apply that check when grouping with LoginUserOperation, so the transfer branch in LoginUserOperationExecutor was copying the placeholder into the Create User payload.

The executor guard is well placed:

• Transfer-only + local ID → subscriptions: [], so the backend never sees an invalid UUID and stuck users break out of the 400 / FailPauseOpRepo loop.
• Create + transfer grouped + local ID → subscription data is kept without id, so creation and ID translation still work.

The two new unit tests cover both paths, CI is green, and the manual repro steps in the PR comment are clear.

Non-blocking follow-ups (optional)

1. LoginManager enqueue logic — Fadi's suggestion to enqueue CreateSubscriptionOperation instead of TransferSubscriptionOperation when pushOp.id is local could give fuller auto-recovery after login, not just unblocking the 400 loop. The executor guard is still worth keeping as defense in depth.
2. OperationRepo._getGroupableOperations — could also skip ops where _canStartExecute === false to prevent blocked ops from being pulled into batches at all. Separate PR material.
3. index.html harness churn — swapping the Safari VAPID test page for this repro is fine for QA; consider restoring or splitting dev harnesses so they do not keep overwriting each other.

None of these block merge. Ship it.

[fadi-george]

I did this check for logout:

1. Reset SDK state and subscribe; wait for a real subscription ID.
2. Log in first:

await OneSignal.login('logout-repro-user');

3. Seed a local ID without creating an operation:

const model = await OneSignal._coreDirector._getPushSubscriptionModel();
model._setProperty('id', `local-${crypto.randomUUID()}`, 1);

4. Log out:

await OneSignal.logout();

5. Inspect POST /apps/{appId}/users. It sends:

{ "identity": {}, "subscriptions": [] }

6. The backend returns 400 user-3.

But at least reloading recovers from it.

[sherwinski]

Hey @dombartenope, this is now live in v160610 🚀