Skip to content

syncUserEventChannels reconciles only the first 30 channel memberships, not 100+ #1145

Description

@teetangh

The stale-channel reconciliation pass inside syncUserEventChannels believes it is paginating through every channel a user belongs to. It is not. It reads at most thirty channels and then stops, so any membership past the thirtieth is never examined and never cleaned up.

The defect

The reconciliation loop lives in actions/stream/chat/event-channel.action.ts at lines 604-628 on dev (verified at 22da01b8). It reads as follows.

// --- Reconciliation pass: remove user from stale channels ---
// Query Stream for every channel this user currently belongs to.
// Paginate to handle users with 100+ channel memberships.
const PAGE_SIZE = 100;
let allStreamChannels: Awaited<ReturnType<typeof client.queryChannels>> = [];
let offset = 0;
let page;
do {
  page = await withStreamCircuitBreaker(
    () =>
      client.queryChannels(
        { members: { $in: [userId] } },
        {},
        { limit: PAGE_SIZE, offset },
      ),
    () => [],
  );
  allStreamChannels = allStreamChannels.concat(page);
  offset += PAGE_SIZE;
} while (page.length === PAGE_SIZE);

Stream caps queryChannels at thirty results per call regardless of the limit argument passed to it. A request asking for one hundred channels comes back with thirty at most. The loop condition therefore compares a page length that can never exceed thirty against a PAGE_SIZE of one hundred, which means the condition is false on the very first evaluation and the do…while exits after a single iteration. allStreamChannels ends up holding thirty channels at most, no matter how many the user actually belongs to.

The comment sitting directly above the loop claims that it "Paginate[s] to handle users with 100+ channel memberships", which is precisely the case it fails to handle.

A second bug is stacked underneath the first and is currently latent. The cursor advances with offset += PAGE_SIZE, so it moves forward by one hundred while each page returns at most thirty rows. If the loop ever did take a second iteration, it would jump straight past seventy channels it had never read. Fixing only the loop condition without also fixing the cursor would turn a truncation bug into a skipping bug.

What it actually causes

The consequence needs stating carefully, because the obvious reading of it is wrong. staleChannels is derived by filtering streamChannels at lines 632-637, so the only channels that can ever be classified as stale are the ones that were fetched in the first place. Removal at lines 649-661 then operates on that filtered set alone.

Channels beyond the first thirty are never fetched, are therefore never classified as stale, and are therefore never cleaned up. The failure mode is incomplete reconciliation, which is to say a membership leak. It is not wrongful removal, and it is not data loss. A user who should have been removed from an old event channel simply stays in it, keeps seeing it in their channel list, and keeps receiving its messages. The reconciler reports success while having reconciled a fraction of the user's memberships.

This matters most for the accounts that need reconciliation most. A consultee with a handful of bookings is unaffected because they never cross the thirty-channel line. A consultant who has hosted dozens of webinars and classes, accumulated direct-message channels with every consultee they have ever seen, and needs stale memberships pruned is exactly the account for which the pass silently does almost nothing.

The reconciliation pass runs on every call to syncUserEventChannels, not only on forced ones. The function's own docblock at lines 455-457 describes the stale cleanup as something force=true enables, but no force guard wraps the block at 604-628 — the only thing force gates is the session-level dedup guard at line 474. That drift is minor next to the pagination bug, but it means the truncated pass runs on the ordinary chat-mount path through providers/StreamProviderImpl.tsx and on the payment-webhook path through lib/payments/webhooks/handlers.ts, rather than only on explicit re-syncs.

The fix

Set PAGE_SIZE to thirty so it matches Stream's real cap, and advance the cursor by the number of rows actually received rather than by the requested page size.

const PAGE_SIZE = 30; // Stream's hard cap, regardless of `limit`
let offset = 0;
let page;
do {
  page = await withStreamCircuitBreaker(
    () =>
      client.queryChannels(
        { members: { $in: [userId] } },
        {},
        { limit: PAGE_SIZE, offset },
      ),
    () => [],
  );
  allStreamChannels = allStreamChannels.concat(page);
  offset += page.length;
} while (page.length === PAGE_SIZE);

The circuit-breaker fallback still returns an empty page, which continues to end the loop cleanly and skip the cleanup for that run, so the #473 degradation behaviour is preserved.

getUserEventChannels has the same ceiling and does not paginate at all

The sibling function getUserEventChannels at lines 413-448 of the same file issues a single queryChannels call with { limit: 100 } and no offset loop whatsoever. It is subject to the identical cap, so it returns thirty channels at most and there is no second request to pick up the rest. The docblock above it says "Get all event channels for a user", and the truncation is silent.

Because this one feeds a read path rather than a reconciler, the user-visible effect is different: a user with more than thirty channels sees an incomplete channel list, with the omission determined by whatever order Stream returns rows in for a query sorted by last_message_at descending. Whether that is worth a full pagination loop or is acceptable as a "most recent thirty" list is a product call, but the current state is neither — it asks for one hundred, silently receives thirty, and presents the result as complete.

The other queryChannels call sites in the tree already respect the cap and are not affected. The table below records what each one passes.

Call site Limit passed Status
actions/stream/chat/event-channel.action.ts:618 (reconciler) 100 Broken, this issue
actions/stream/chat/event-channel.action.ts:423 (getUserEventChannels) 100 Truncated, no pagination
app/api/organizations/[orgId]/stream/channels/route.ts:77 20 per page with a working offset Correct
app/api/stream/debug/route.ts:83 30 Correct
hooks/useChatUnreadCount.ts:49 30 Correct

Why it matters that this was missed

The trap was already written down before the audit train started. .claude/skills/stream-sdk/SKILL.md lists it at lines 85-87 under "Traps that have bitten before":

queryChannels is capped at 30 per call, not whatever limit you pass. A do…while (page.length === PAGE_SIZE) loop with PAGE_SIZE = 100 exits after one page and silently reconciles only the first 30 memberships.

The correct cap is documented a second time in the codebase itself, in the header comment of app/api/organizations/[orgId]/stream/channels/route.ts at line 12:

PAGINATION: Stream caps queryChannels at 30 per call; we ship 20/page with offset-based pagination to keep the URL simple.

Two accurate in-repo descriptions of the exact failing pattern were not enough for the eight-PR audit train (#1136-#1143) to catch the one live instance of it. Whatever else comes out of this issue, that is the part worth reflecting on: the skill file describes the bug in the same words the code commits it.

Ownership

Nothing currently open owns this file. Checked with:

for br in fix/stream-durability fix/stream-correctness fix/stream-recording-consent \
          fix/stream-scale fix/stream-known-open fix/stream-chat-ux fix/stream-video-quality; do
  git diff --name-only origin/dev...origin/$br \
    | grep -q '^actions/stream/chat/event-channel.action.ts$' && echo "$br touches it"
done

The loop prints nothing, and cross-checking each pull request's file list through gh pr view <n> --json files agrees: none of #1137, #1138, #1139, #1140, #1141, #1142 or #1143 modifies actions/stream/chat/event-channel.action.ts. #1136 is already merged and did not touch it either. The bug is live on dev today and is unowned, so it needs its own change rather than being folded into a pull request already in flight.

Part of #1134

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinglaunch: post-mvpFirst 90 days after launch — coverage, polish, operational maturitystreamStream Chat/Video integration

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions