[rohanesor]

Fixes #1158

Problem

EmbeddedChatApi.logout() only awaited this.auth.logout(), which clears the auth token. The DDP connection and its _activeSubscriptions were left running, so after a "logout" the client kept receiving live messages and events for a session that was no longer authenticated — a privacy leak, and inconsistent state if another user logged in without a full page refresh.

The class already has a close() method that stops _activeSubscriptions and calls sdk.connection.close(). logout() simply never called it.

Fix

logout() now tears the connection down via the existing close(), with two deliberate details:

• the teardown runs in a finally block, so the socket is also closed when the logout request itself fails (a failed logout previously left a fully live session behind);
• close() errors are logged rather than propagated, so they cannot mask the logout result. Other call sites already treat close() as rejectable (EmbeddedChat.js wraps it in .catch()).
• in-flight connects are invalidated after each awaited step, preventing subscriptions from being registered after logout;
• a pending DDP handshake is raced against an explicit cancellation promise, so logout releases _connectPromise even when connection.connect() never settles. Cleanup is identity-guarded so an abandoned handshake cannot clear a newer connection attempt.

Verification

Added packages/api/test/connection-lifecycle.test.mjs and an API test command. The regression test covers the reviewed race directly:

1. the first DDP handshake remains pending;
2. logout cancels and releases that connection attempt;
3. new credentials are installed;
4. the next connect() starts a fresh handshake and calls loginWithToken();
5. the abandoned handshake is released late and cannot disrupt a later connection.

Validated with Node 24:

• yarn workspace @embeddedchat/api test — 1/1 passed;
• yarn tsc --noEmit -p packages/api/tsconfig.json — clean;
• yarn workspace @embeddedchat/api build — passed;
• yarn lint — 0 errors (existing warnings only).

The original teardown harness also passes all seven logout assertions on this branch and fails five of them on unpatched develop, confirming the base logout regression remains covered.

Prior art

@Aryan-Verma-999's earlier #1171 identified the same root cause and was closed unmerged. This PR keeps that insight and adds the finally handling so a failed logout cannot leave a live socket behind.

[Aryan-Verma-999]

Thanks for picking this up :)
please address the comments. Thanks.

[rohanesor]

Thanks for the review @Aryan-Verma-999 — you're right, and the window was wider than just joinRoom().

_doConnect() awaits in four places before it subscribes (connection.connect(), getCurrentUser(), loginWithToken(), joinRoom()). A close() during any of them left the connect to resume and register the streams afterwards.

Fixed in dceb793 with a connection generation counter:

• close() increments _connectionGeneration and stops the active subscriptions.
• _doConnect() snapshots the generation after its initial close() and bails out if it no longer matches, checked after each await.
• A final check after the subscriptions are registered stops them if a close() landed while they were being set up, which closes the last gap.

I also pulled the teardown out into _stopSubscriptions() so close() and that late check share it.

Verification

packages/api has no test runner, so I verified behaviourally: bundled the real source with the transport stubbed, made joinRoom() block, called close() mid-connect, then released it — which is exactly the sequence you described.

Before:

subscriptions created after logout : 5
subscriptions still active (leaked) : 5
  ['room-messages', 'notify-room', 'notify-room', 'notify-user', 'notify-user']
FAIL: ghost subscriptions remain

After:

subscriptions created after logout : 0
subscriptions still active (leaked) : 0
PASS: no ghost subscriptions

I ran the harness against the previous commit first to confirm it actually reproduces the bug rather than passing vacuously.

Also run: tsc --noEmit on packages/api (clean), prettier --check (clean), yarn lint (0 errors, warning count unchanged from develop).

One thing still outside this PR's reach: CI hasn't run on any of my PRs yet — the workflow runs are sitting at action_required pending a maintainer approval for a first-time contributor. If you're able to approve those, it'd be good to get real CI signal on this.

[rohanesor]

Hi @Aryan-Verma-999 — a brief follow-up on the race fix: the generation-counter change is on the PR head (). The latest Build and Lint / Playwright runs are still and need maintainer approval before they can provide CI results. If you consider the change ready, could someone approve those workflows when convenient? Happy to address any further review feedback; no urgency.

[rohanesor]

Correction to my previous note: the fix is commit dceb793, and the required workflow runs currently show action_required. They need maintainer approval to start; could someone approve the Build and Lint and Playwright workflows when convenient? Thank you.

[Aryan-Verma-999]

cancellation resolves this promise, so callers treat a cancelled connection as successful. The React auth listener calls RCInstance.connect().then(...) and sets the UI authenticated in that callback; logout during the handshake can therefore cancel the connect, then let the stale .then() set the UI back to authenticated.

Cancel by rejecting with a dedicated cancellation error (or make callers verify the connection is still current), and assert the interrupted connect() does not resolve successfully.

[rohanesor]

Addressed in ae81fd26. Cancellation now rejects connect() with a dedicated ConnectionCancelledError instead of resolving the race, so caller success continuations cannot run after logout. The lifecycle regression now asserts that rejection and still verifies that the next login reconnects successfully. Verified with the API lifecycle test, API TypeScript check, build, and repository lint (0 errors; existing warnings only).