Flaky CI Failure: node-sdk Preferences 'should stream preferences' — expected 4 updates but got 3

[github-actions]

Flaky CI Failure: node-sdk Preferences 'should stream preferences' — expected 4 updates but got 3

Workflow: Test
Failed run: https://github.com/xmtp/libxmtp/actions/runs/29026036425
Commit: f46d623
Failed jobs: test-node-sdk / Test (node-sdk) shard 1

Summary

The Node SDK integration test Preferences > should stream preferences in test/Preferences.test.ts failed because the preference stream delivered only 3 updates instead of the expected 4. The test ran for 3337ms before the assertion fired. All other 182 tests in the shard passed. This is consistent with a race condition or timing issue in the streaming path where one preference update event was not delivered before the assertion ran.

Error Details

FAIL test/Preferences.test.ts > Preferences > should stream preferences
AssertionError: expected 3 to be 4 // Object.is equality

- Expected
+ Received

- 4
+ 3

 ❯ test/Preferences.test.ts:216:32
    214|       preferences.push(...update);
    215|     }
    216|     expect(preferences.length).toBe(4);
       |                                ^

 Test Files  1 failed | 14 passed (15)
       Tests  1 failed | 182 passed (183)
   Start at  14:42:56
   Duration  21.89s

error: recipe `test-node-sdk-ci` failed on line 37 with exit code 1

Analysis

The test collects preference update events via a stream and asserts that exactly 4 are received. Only 3 arrived, meaning one event was either dropped, delayed beyond the collection window, or the stream was closed before the final event was emitted. This is a test/code failure — likely a race condition in the streaming or preference-update delivery path in the Node SDK bindings. The pattern (N-1 events received) is typical of async stream tests that don't wait long enough for the final event.

Related area: sdks/js/node-sdk/test/Preferences.test.ts, Node SDK preference streaming.

---

Reported by Flaky Failure Watcher

[xmtp-coder-tasks]

Task is running: https://sandbox.xmtp.team/tasks/insipx/7153523c-aea2-4058-8b1f-98e60ca3d9e8

[insipx]

Bugfix Spec — Issue #3831: Flaky should stream preferences test

Issue: #3831
Area: sdks/js/node-sdk/test/Preferences.test.ts — Node SDK preference streaming test

Current Behavior

The integration test Preferences > should stream preferences sets up 4 expected
preference updates on client1's stream:

1. ConsentUpdate — group consent set to Denied
2. ConsentUpdate — clientB inbox consent set to Denied
3. HmacKeyUpdate — from registering a new installation (client2)
4. HmacKeyUpdate — from registering a new installation (client3)

After triggering the updates and running a fixed sequence of syncAll() calls
interleaved with sleep(1000), the test ends the stream on a fixed 100 ms timer:

setTimeout(() => {
  void stream.end();
}, 100);

const preferences: UserPreferenceUpdate[] = [];
for await (const update of stream) {
  preferences.push(...update);
}
expect(preferences.length).toBe(4);

The HmacKeyUpdate events depend on network propagation of the new installations'
key packages. When the 4th event has not yet been delivered into client1's stream
within the 100 ms window, stream.end() flushes the iterator and the loop exits
early with only 3 updates collected, failing on expect(preferences.length).toBe(4)
(observed CI failure: expected 3 to be 4). Every other streaming test in this SDK
uses a 2000 ms window; this test's 100 ms window is far too short for the
network-propagated HmacKeyUpdate events, making the test flaky.

Expected Behavior

The test collects preference updates until it has received the expected number
(4), then ends the stream — rather than ending on an arbitrary fixed timer. A
generous safety-net timeout (well under vitest's 120 s testTimeout) still ends the
stream if fewer than 4 updates ever arrive, so a genuine regression fails with the
same clear expected N to be 4 assertion instead of hanging.

Concretely:

const preferences: UserPreferenceUpdate[] = [];

// Safety net: end the stream if the expected updates never arrive, so the
// assertion below fails clearly instead of the test hanging until timeout.
const endTimeout = setTimeout(() => {
  void stream.end();
}, 10000);

for await (const update of stream) {
  preferences.push(...update);
  if (preferences.length >= 4) {
    break;
  }
}
clearTimeout(endTimeout);
await stream.end();

expect(preferences.length).toBe(4);

With this change:

• When all 4 events arrive (the normal case) the loop breaks immediately and the
  test finishes fast — it no longer waits on wall-clock timing to decide it is done.
• When events are slow, the test waits up to the safety-net window instead of racing
  a 100 ms deadline, eliminating the flake.
• When a real regression drops an event, the safety net ends the stream and the
  existing expect(preferences.length).toBe(4) assertion fails clearly.

Unchanged Behavior

• The set of triggered updates and the syncAll() / sleep() setup sequence are
  unchanged — the test still exercises 2 consent updates + 2 HMAC key updates.
• All existing assertions on update ordering, types, and payloads
  (consentUpdate1/2, hmacKeyUpdate1/2) remain exactly as-is.
• No production/library code changes — this is a test-only reliability fix.
• Other tests (including should stream consent updates) are not modified.

[insipx]

Thanks — this matches the bugfix spec already posted, and no EARS requirements changed. That exact spec is already implemented in draft PR #3833 (#3833):

• Collects preference updates until all 4 arrive, then ends the stream (breaks the loop), instead of the fixed 100 ms timer.
• Keeps a 10 s safety-net setTimeout(stream.end, …) so a genuine regression fails clearly on expect(preferences.length).toBe(4) instead of hanging.
• Test-only change; all existing ordering/type/payload assertions and the syncAll()/sleep() setup are unchanged.

No code changes needed in response to this comment — the PR is up to date with this spec.