Repository navigation
[CLNP-8677][CLNP-8678][CLNP-8679][chore]: add Playwright E2E tests against a real Sendbird backend and run them in CI - #1477
Open
sf-tyler-jeong wants to merge 59 commits into
Open
sf-tyler-jeong wants to merge 59 commits into
sf-tyler-jeong wants to merge 59 commits into
Conversation
Add a Playwright harness that runs the testing app against a live Sendbird backend. The config gives E2E its own project and reporters, fixtures seed and tear down users, channels and messages through the Platform API, and a per-run tag keeps concurrent runs from colliding. Tier-0 specs cover connection, the channel list, sending, editing and deleting messages, reactions, quoted replies, channel settings, open channel, profile editing and copy-to-clipboard. Credentials come from .env.e2e; without them every spec skips, so the default test run and CI pipeline are unaffected. A separate workflow runs the suite on manual dispatch.
Cover the single-user flows the seeded fixtures unlock: channel list rendering, selection and leave; message pagination; message search with its empty and paging states; thread panel open, reply count and parent navigation; deep-link entry by channelUrl; channel freeze and quoted-message scroll; and open-channel history, send, delete, switch and settings.
Cover what needs a second user or a non-default environment: real-time delivery, unread badges, channel reordering, typing indicators, invites and mentions across two browser contexts; moderation, operator changes and cover-image upload in channel settings; file, voice and multi-file messages; read receipts and URL previews; mobile viewport, dark theme, RTL and feature-flag variants; and offline/online transitions.
connect() discarded the result of updateCurrentUserInfo, so whenever nickname or profileUrl was passed to the provider the user store and onConnected kept the values from before the update. Apply the returned user to the store and hand it to onConnected. A failed profile update no longer tears the connection down. The error is held until the remaining init steps succeed and then reported once through onFailed, immediately before onConnected, so a single failure cannot fire onFailed twice. InviteUsers ran its user-list query and enabled the create button before the SDK was initialized. Both now wait for the initialized flag, a generation counter drops the results of a query a newer run has replaced, and a rejected query leaves the existing list alone instead of surfacing an unhandled rejection.
Scenarios gated themselves on whether a piece of UI happened to be on screen and skipped when it was not. That cannot tell an app which never had a feature from one where the feature broke, so a regression in voice messages, open-channel moderation or message editing left a grey run instead of a red one. Where a test establishes its own precondition — a message it just sent, a channel it seeded, a flag it passed in the URL — the probe is now an assertion. Where the target app decides, the scenario is gone: message search, super channel and the thread search prompt need an application that has them, and the suite has no way to turn them on. Nine scenarios stay off behind a reason recorded beside them, each confirmed to fail on all of three runs: open-channel settings shows the participant view for a channel opened from the list, a mention seeded through the Platform API arrives with no mentioned users, the unread floating button does not mount after mark-as-unread, and creating an open channel does not move the view into it. Several locators pointed at nothing and are corrected — open-channel file bubbles were matched with group-channel classes, suggested replies with a button that is a div, the OG preview with its empty cover overlay — and a message now has to leave the pending state before its context menu is opened. The Platform API client backs off and retries a 429 or 5xx rather than failing the setup, and channel and message seeding wait for the write to be readable instead of for a fixed delay. Bring e2e/ under lint and typecheck, which neither covered before.
The case asserted only that the query ran and filled the list, which the previous implementation did as well — it ran on mount whatever the SDK was doing. Assert instead that nothing is queried while the SDK is uninitialized and that the rows arrive once it is, so removing the guard turns the case red.
The two throwaway users were created once per worker, so whatever a test did to them stayed for the rest of that worker's run. Editing a profile renames a user for good, and a member row renders the nickname — falling back to the id only when the nickname is empty — so every later test that looked a member up by id found nothing. The failures moved around between runs because which worker had already run the profile cases decided which cases broke, and lowering the worker count never helped: the contention was in the shared user, not in running things at once. Scope both users to the test, create them on entry and delete them on exit. Six consecutive runs: one failure on the first, none on the rest, and two cases that the renaming had been masking now pass.
Opening an accordion by clicking its panel item assumes it was closed; the item toggles, so a click can just as well collapse it. Read the state off the chevron the item already carries instead, and click only while it says closed. Open a channel by its URL rather than taking whichever one sits at the top of the list, and wait for a mute, ban or operator change to be readable through the Platform API before asserting on the list that shows it — those lists load when their accordion opens and do not load again. Raise the per-test timeout to 60s. Playwright's 30s default assumes a local app; here a case seeds through the Platform API, boots the app and connects the SDK before it touches the UI.
The testing app falls back to VITE_USER_ID for the nickname when the URL carries none, and the provider writes that nickname to the user on connect — so everyone who connected ended up sharing one name. Member rows and mention labels render the nickname, falling back to the id only when there is none, which left every lookup by id finding nothing. Pass the nickname alongside the user id so a user keeps its own name. With that, a mention renders again: send it from the composer, where the suggestion list supplies the mentioned user, rather than through the Platform API, which delivers the message with an empty mentioned_users list and nothing for UIKit to mark up. Both mention cases come back, and open-channel seeding now waits for its messages the way the group-channel side already did.
The case clicked the toggle again as soon as the frozen notification appeared, but the toggle only reports frozen once the channel change comes back — so the second click repeated the first request instead of reversing it and the channel stayed frozen. Wait for the toggle to say checked before clicking it off. Entering an open channel now also waits for the message list container. The header paints before the history arrives, which left callers reading a list that had not loaded.
…icks The deep-link case passed a channelUrl the app never reads, so it only ever watched the app open whichever channel it auto-selects — with one channel in the list, that is true whatever the parameter says. AppProps carries no channel to open, so there is nothing here to deep-link with; remove the case rather than keep one that cannot fail. What it was really watching is already covered, so make that cover it properly: assert the newest channel is the active row, that the older one is not, and that the header names it. Asserting only that some row is active passes even when the app opens a channel the case never made.
openGroupChannel took a channelUrl the app never reads and then waited for whatever channel the app auto-selects — which is what openFirstGroupChannel already does, only it clicks the row rather than trusting the selection. With each test owning its user and its channels, there is nothing left for the second helper to guard against. Use openFirstGroupChannel throughout and drop the channel bindings that existed only to feed the parameter.
The file cases wrote straight to the hidden input and then clicked a send button, swallowing the click when it was not there. Behind that input the change handler stages files in composer mode and uploads them otherwise, and when neither branch is ready it drops them without a word — so the files often never left, the swallowed click hid it, and the case failed later on a bubble that was never coming. Two of five runs went that way. Open the picker from the attach button instead, which only appears once the composer is wired, and press send when the composer offers one. The image, multi-file and open-channel cases pass five runs each, and four full runs came back clean.
A .catch(() => {}) around a click turns a step that did not happen into a
failure somewhere later, pointing at the wrong thing. Two cases were
built on one: the ban case clicked a confirmation button that does not
exist — banning takes effect from the menu — and the add-operator case
waited for a button matching /add/i to hide, which also matches the "Add
operator" button behind the modal that stays on screen. Both only looked
fine because the click was thrown away.
Take the swallow off every action, drop the click that had nothing to
click, and scope the modal assertion to the modal.
Two cases asserted nothing at all. The member-count one compared
`Number(text) || 1` against 1, which no value can fail, and read a class
that is not in the source; the accordion one guarded its whole body on
locators that are not there either. Give them the counts they were meant
to check: two member rows and a panel that says 2, one operator row for
the creator, and no rows under Banned users.
The invite case picked the second user's row when it could find it and fell back to "whichever checkbox comes first" when it could not — which invites someone else, leaving the row it looks for afterwards never to arrive. Name the user and let a missing row fail. That row was never the members list to begin with. A user row carries either the plain class or the --small one, never both, and the members list uses --small; matching only the plain one found the invite modal still open behind the assertion. Wait for the modal to close, reopen the accordion the member change collapses, and match the row the members list actually renders — then check the panel count alongside it.
updateCurrentUserInfo answers with the user it just wrote, and connect() dropped that answer. The store kept the name the user had before the one the app passed as a prop, and so did onConnected — while the server and sdk.currentUser held the new one. Everything reading the store saw the stale name: the channel-list header, the profile editor, the checks that decide which messages are yours. Take the answer and put it in the store. A failure there stays what it was: the outer catch resets the SDK and reports onFailed, and onConnected does not run. That is the contract consumers retry from, and a case now pins it.
The open channel list API leaves operators out unless the application asks for them — that is what its include_operators setting is for — and the SDK keeps what it got, so a channel first seen through the list carries an empty operator list and the settings panel offers the participant view. Seeding a channel through the Platform API and opening it from the list put every operator case on that path, where the view they are about to drive never appears. Create those channels through the app instead. A channel made in session comes back complete, which is the session an operator is in when they open its settings. Editing the name, deleting the channel and changing a participant's operator role all run again. Muting and banning stay off: both read the participant list back after the change, that list only loads when its accordion opens, and reopening it flips it shut as often as open. Also: enter the open channel before creating one, since a create that lands while the SDK is still connecting is dropped without a word; wait for a participant to be visible server-side before reading the list; and stop clicking a ban confirmation that does not exist.
Three faults sat in the same fetch, none of them visible from outside: A user list query built before the SDK is ready threw on a method that was not there yet — guard it and let the caller see nothing to query rather than a crash. StrictMode runs the effect twice on mount, so two fetches overlap. The first could resolve last and write its answer over the newer one; each run now carries a number and only writes while that number is current. Scrolling appended a page to the list as it stood when the handler was built, so two pages arriving together lost one of them. Append to the list as it stands instead, and handle a rejected page rather than letting it escape unhandled. Nothing about what the component shows or when it fetches changes.
… list Registering an operator turned out to share what already held muting and banning back, and failed two runs in five on its own: the participant list loads when its accordion opens and never loads again, so a second user who joins around that moment can simply be missing from it. Asking for the list again does not work either. Escape leaves the panel up, and the settings trigger and the accordion header both toggle, so reopening shuts them as often as it opens them — and closing the panel costs the operator view the case came for. Two attempts at a reopen helper each made it worse, from three passes in five to none. Hold the case with the others and say what it would take to run it.
The reason recorded beside them claimed the list never reloads after a change and that reopening the panel costs the operator view. Neither holds: the panel calls refreshList after an operator, mute or ban change, and closing and reopening it keeps the operator view — both checked against the running app. What broke those runs was a helper of mine that pressed Escape, which the panel ignores, and then toggled the panel shut. What does hold is narrower. The list is read once when the accordion opens, and the panel's handlers cover operator, mute, ban and channel changes but not onUserEntered — so a user who joins moments before the panel opens can be absent with nothing to bring them in. Every case here starts by finding that user's row.
The three open channel moderation cases were held back as a product limitation. It was not one — the helpers reaching them did not do what they claimed. Nothing listens for Escape, in the settings panel or anywhere above it, so pressing it left the panel open. The conversation header trigger only opens the panel; it is not a toggle. Each "reopen" therefore fell through to an unconditional click on the Participants header, which collapses an accordion that is already expanded. Retries alternated between open and collapsed, and a collapsed list has no rows to find. Read the chevron for accordion state instead, close the panel through its own close icon, and refresh a list by collapsing and expanding it — that unmounts the list and mounts one that runs its own query. Registering an operator, muting and banning all pass against an unchanged library. Ban now reads from the Banned users accordion. The server was still listing the banned user among the channel's participants 15s later, so an assertion on the participant roster times the backend rather than the panel. The old one asserted a count of zero against a list that was not on screen, and would have passed whatever the panel did.
The case seeded 12 messages and was held back as a product limitation: the New Messages separator appeared but the floating button never mounted. It was not a product limitation — the list only had 14px to scroll, so the separator never left the viewport, which is the one condition the button renders on. Seeding 30 messages gives the list enough height to carry the separator off screen on scroll, and the button appears. Server-side unread_message_count was 1 throughout, not 0 as the old comment assumed. The scroll-and-wait also raced paginating in older messages, which moves the scroll position out from under a single scrollTop write. Retrying the scroll alongside the assertion rides that out instead of guessing a fixed settle time.
Full-suite runs turned up an intermittent failure in the group channel ban case, and a rarer one in the open channel operator case — different specs, same shape. Each opens an accordion once, right after a Platform API call confirms a join/mute/ban/invite server-side, and asserts against whatever that single fetch returned. The panel's own list query is a separate read than the one the Platform API polled, and occasionally still lands ahead of it; nothing about the assertion made it try again. Collapsing and re-expanding an accordion unmounts its list and mounts a fresh one that runs its own query, so wrapping the reread in `toPass` gives a change on the backend a few more chances to show up before the assertion gives up. Applied everywhere that shape occurs: invite, mute, ban (both directions) in group channel settings, and the open channel participant register/cancel case, which now goes through the existing retry-aware row finder instead of a bare accordion-open. `refreshSettingsAccordion` factors out the collapse/expand pair for channel settings, mirroring `refreshOperatorAccordion` already added for open channel settings.
The register/cancel case still failed intermittently after the previous reread fix. Its remaining single-shot read was not the join check — that already went through the retry-aware row finder — but the operator badge itself. The row menu's onChange handler already triggers one automatic reread as part of registering, and that automatic read is a fresh query exactly like the others fixed here; it can just as easily land ahead of the write, and nothing was retrying past it. Six full-suite runs (82 tests each) came back clean after adding a forced second reread on top of the automatic one, both for registering and for cancelling.
…teUsers
The `.catch(() => {})` added to the initial fetch and to scroll
pagination had no regression test — the fifth of the five bug fixes in
this file, and the one gap flagged when counting them.
Asserting the absence of an unhandled rejection needed two corrections
from the first attempt: this worker treats unhandled rejections as
uncaught exceptions, so `uncaughtExceptionMonitor` is what observes one,
not the `unhandledRejection` event — that event never fired here, even
across several drained microtask queues. And the detection itself is not
microtask-timed: it took a measured ~250ms of real wall-clock time before
Vitest's own worker surfaced it, so the test waits 500ms rather than a
`setTimeout(0)` tick, which consistently saw nothing.
Confirmed both ways: green with the guards in place, and reverting either
`.catch` independently turns exactly its own test red (1 failed, 6
passed) while leaving the other six untouched.
The navigation spec covered two scenarios in one test: the list renders previews, and entering one renders the conversation. A single test that clicks a preview cannot report which of the two broke, and a list that never rendered fails at the click with no assertion of its own. Split into one test per scenario. The entry case also asserts what the scenario asks for and the old test did not — that the header names the channel, and that the composer is there to send into it.
E2E ran on manual dispatch only, so nothing checked it unless someone remembered to. Move it onto pull requests. Three shards split the 83 tests 28/28/27. Measured at the worker count CI uses, the test phase drops from 3.9min unsharded to 1.7min per shard; the fixed cost each shard still pays — install, browser, dev server — is why it is not a clean third. Shards multiply concurrent load on the one test application (shards x workers), so raise the count only after the backend is shown to tolerate it. Sharded runs report to a blob and a merge job assembles one HTML report; three jobs uploading the same artifact name would otherwise collide. The `E2E` summary job is what branch protection should require — the sharded jobs carry the shard index in their names and would need re-pointing every time the count changes. Retries go to 3, so a failing test gets the same four attempts the unit suite already gives it. Credentials are read from repo secrets, which a fork cannot see; there the suite skips and stays green as before. On an internal PR that same silence would report green while testing nothing, so a missing secret now fails the job outright. Also adds what the workflow was missing either way: a yarn cache, a Playwright browser cache, and a concurrency group so a new push cancels the run it superseded.
The guard only checked E2E_APP_ID, so a missing E2E_PLATFORM_API_TOKEN went past it. Fail when either secret is empty; fork pull requests are still exempt.
Ten specs skipped themselves in a beforeEach; the other fourteen failed in their fixtures instead, so a run without credentials went red rather than skipping. An auto fixture now skips any test when E2E_APP_ID or E2E_PLATFORM_API_TOKEN is empty, so the per-spec guards and the throwaway user's own token checks are gone.
Answer socket attempts with BUSY through Playwright's WebSocket routing and check the countdown modal, that every retry waits out the delay, and that the provider loads once the delayed connection lands. Covers a delayed first connection, a delay that repeats, and a connected user whose reconnect is delayed.
The thread test titled as navigating to the parent from the header link closes the panel with the header close button instead, and checks that the parent is still shown in the channel. Rename it to match. The jump from the header link is covered by the CLNP-7835 test.
…K is ready The invite list fetches its users in an effect that StrictMode runs twice. When the app's userListQuery hands back the same query instance each time, the second run called next() on a query that was still loading. The SDK refuses that call, and the first answer was then dropped as stale, so the list stayed empty. A run that is handed a query still loading now leaves it to finish. When the modal opened before the SDK had connected, there was no default query to build yet and the effect never ran again, so the list stayed empty after the connection landed. The effect now waits for the SDK when it builds the default query, and fetches once the SDK is ready. A query from userListQuery still runs once on mount: running it again would call next() on the same query and replace the first page with the second.
The test passed whenever the status icon existed, which it does for a sent, delivered or read message alike. It now expects the sent colour before the other member opens the channel and the read colour after.
The channel loads 30 messages when it opens and the test seeded 20, so the oldest one was on screen before any scroll. Seed 60, check that the oldest is not loaded on open, then scroll up until it is and compare the whole list in order.
E2E_PORT moved the URL Playwright waits on but not the port Vite listens on, so a run with a non-default port timed out waiting for a server that had started elsewhere. Pass the port to the dev server, and make Vite fail instead of moving to another port if it is taken.
The scrollbar layout test clicked the first row of the invite list, which leaves out the current user, so it passed only when other tests had already filled the app with users. On an app with no one else it failed before reaching the layout check. Create the second user and select that user's row, as the channel creation test does.
trackOpenChannel looked the channel up when it was called, after the UI had created it, so a test that failed while waiting for the new channel never registered it, and G11 never registered its channel at all. Channels created through the UI carry no custom_type, so the run sweep left them behind as well. trackOpenChannel now only records the name, and the tests call it before they create the channel; after the test, every open channel with that name is deleted. G10 also records the name it renames the channel to, and the tests that need the URL look it up after creating the channel. The run sweep now also deletes open channels whose name carries the run tag, which every channel the tests create through the UI has.
sendText and openMessageMenu chose between the group and open channel
paths with locator.isVisible({ timeout }), but Playwright ignores that
timeout and answers at once. A message still pending at that instant
fell through to the open channel branch, which only waited for the text,
so sendText could return before the server had confirmed the message.
Pick the path from the conversation on screen instead, and in a group
channel wait for the confirmed bubble.
These tests passed without checking the behaviour their titles name: - The new-message button and scroll-to-bottom tests clicked and then used toBeVisible, which ignores the scroll position, so they passed without any scrolling. They now check that the target is out of view before the click and in view after it, and the new-message test clicks that button rather than whichever button matched first. - The frozen channel test opened the channel as its operator, who can still type. It now opens it as a regular member and checks that the composer is disabled; the unfreeze test checks that the member's composer is enabled again afterwards. - The quoted-message test only checked that the parent was visible. It now seeds enough messages to push the parent out of view, then checks that clicking the quote highlights the parent and brings it into view. - The offline test never looked at the body class. UIKit adds sendbird__offline only when local caching is off, and it is on by default, so the test app gains a localCacheEnabled=false URL parameter for this test. The manual disconnectWebSocket() is gone: called right after setOffline, it closed the socket before the SDK noticed the browser going offline, so the reconnect that marks the app offline never started. - The reactions-off test hovered the list container, not a message. It now hovers a message, proves the hover through the message's menu button, and checks that no reaction trigger is rendered. Each test now fails when the behaviour it names is removed from the source, while its previous version still passes under the same change.
Throwaway users are created per test, but four comments still described them as per worker. Three comments next to message seeds blamed the per-user rate limit, which the Platform API client already absorbs by spacing its calls and retrying 429 responses.
The voice message test waited a fixed 2.5 s after starting a recording before stopping it. The recorder's clock only starts once getUserMedia resolves, and stopping before one second cancels the recording, so a slow microphone start under load turned the stop into a cancel. It now waits for the recording timer to pass one second. The new-message notification test waited a fixed 0.5 s for the scrolled-up state to register before the other user sent a message. It now waits for the scroll-to-bottom button, which appears once that state is set.
|
| CVE | Package | Version | Fix |
|---|---|---|---|
| CVE-2026-102278 | brace-expansion |
1.1.18 |
1.1.20 |
| CVE-2026-93749 | source-map-js |
1.2.1 |
1.2.2 |
| CVE-2026-112942 | launch-editor |
2.14.1 |
2.14.2 |
| CVE-2026-111755 | follow-redirects |
1.16.0 |
1.16.1 |
| CVE-2026-102422 | shell-quote |
1.10.0 |
1.11.0 |
| CVE-2026-84292 | fast-uri |
3.1.6 |
3.1.7 |
| CVE-2026-102278 | brace-expansion |
2.1.4 |
2.1.6 |
| CVE-2026-102276 | brace-expansion |
1.1.18 |
1.1.19 |
| CVE-2026-111181 | body-parser |
1.20.8 |
1.20.9 |
| CVE-2026-102276 | brace-expansion |
2.1.4 |
2.1.5 |
| CVE-2026-111756 | follow-redirects |
1.16.0 |
1.16.1 |
| CVE-2026-87776 | compression |
1.8.1 |
1.8.2 |
| CVE-2026-84394 | fast-uri |
3.1.6 |
3.1.7 |
🟡 Medium · 11 findings
| CVE | Package | Version | Fix |
|---|---|---|---|
| CVE-2026-110306 | fast-uri |
3.1.6 |
3.1.9 |
| CVE-2026-86472 | fast-uri |
3.1.6 |
3.1.8 |
| CVE-2026-102277 | brace-expansion |
2.1.4 |
2.1.7 |
| CVE-2026-110306 | fast-uri |
3.1.7 |
3.1.9 |
| CVE-2026-112944 | launch-editor |
2.14.1 |
2.14.2 |
| CVE-2026-110180 | fast-uri |
3.1.7 |
3.1.9 |
| CVE-2026-102277 | brace-expansion |
1.1.18 |
1.1.21 |
| CVE-2026-86472 | fast-uri |
3.1.7 |
3.1.8 |
| CVE-2026-110180 | fast-uri |
3.1.6 |
3.1.9 |
| CVE-2026-112943 | launch-editor |
2.14.1 |
2.14.2 |
| CVE-2026-104844 | postcss-selector-parser |
6.0.15 |
7.1.6 |
🟢 Low · 6 findings
| CVE | Package | Version | Fix |
|---|---|---|---|
| CVE-2026-110095 | fast-uri |
3.1.7 |
3.1.9 |
| CVE-2026-115897 | serialize-javascript |
4.0.0 |
7.1.3 |
| CVE-2026-115897 | serialize-javascript |
6.0.2 |
7.1.3 |
| CVE-2026-110725 | fast-uri |
3.1.6 |
3.1.9 |
| CVE-2026-110725 | fast-uri |
3.1.7 |
3.1.9 |
| CVE-2026-110095 | fast-uri |
3.1.6 |
3.1.9 |
✅ Resolved · 3 findings
| CVE | Package | Version |
|---|---|---|
| CVE-2026-97058 | sprintf-js |
1.0.3 |
| CVE-2026-93687 | braces |
3.0.3 |
| CVE-2026-104844 | postcss-selector-parser |
6.1.4 |
View full analysis in Upwind Console
Scan completed in 9s
Scan history (4 scans)
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
632f677 |
2026-10-19 02:29 UTC | +13 | -1 | +12 |
862190e |
2026-10-19 04:26 UTC | +14 | -1 | +13 |
ddb49b7 |
2026-10-19 05:12 UTC | +12 | -1 | +11 |
aa9fad2 < |
2026-10-26 08:02 UTC | +30 | -3 | +27 |
Last scanned: aa9fad2 · 2026-10-26 08:02 UTC
✅ Deploy Preview for sendbird-uikit-react ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
| Commit | Scanned at | New | Resolved | Net |
|---|---|---|---|---|
632f677 |
2026-10-19 02:29 UTC | 0 | 0 | 0 |
862190e |
2026-10-19 04:26 UTC | 0 | 0 | 0 |
ddb49b7 |
2026-10-19 05:12 UTC | 0 | 0 | 0 |
aa9fad2 < |
2026-10-26 08:02 UTC | 0 | 0 | 0 |
Last scanned: aa9fad2 · 2026-10-26 08:02 UTC
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 632f677c33
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…st app The test app's build type-checks src with `tsc --strict false`, which rejects casting window straight to Record<string, unknown> (TS2352). The repository's typecheck does not cover apps/testing, so only the deploy preview build caught it.
A connect whose user the provider had already switched away from could still finish. Its profile update resolved after the next user was initialized and wrote the previous user into the user store. The direct path never checked whether its connect was still current, so it also initialized the SDK store and called onConnected for that user. Write the profile only while the connect is current, and make the direct path follow the rule delayed-connection recovery already follows: a later connect, a disconnect or an unmount makes it stale.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 862190e849
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Both workflows grouped runs by the pull request's source branch name, and cancel-in-progress cancels any older run in the same group. Two pull requests from forks with the same branch name, or a fork's main and a push to main, shared a group, so one cancelled the other's run and its check reported failure. github.ref is refs/pull/<number>/merge for a pull request and the pushed ref otherwise, so each pull request now cancels only its own superseded runs.
A fixture that fails to delete its throwaway user swallows the error, and the backstop teardown only swept channels, so such a user stayed in the shared test application for good. The teardown now also deletes the users this run created, found by the run's user ID prefix rather than by nickname, which profile tests change, together with the group channels they belong to.
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddb49b71a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Author
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ddb49b71a5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Both setup-node steps asked for '24', which any 24.x in the runner's tool cache satisfies; the last run of this workflow used the cached 24.21.0. Node.js 24.22.1 updates the bundled root certificates to NSS 3.131.1 and rejects chains through the eMudhra (emSign) intermediates whose keys were reported compromised in October 2026. The E2E jobs install packages, download the browser and call the Platform API through that Node. Ask for '>=24.22.1 <25' in the shard and merge-report jobs, the same range #1478 sets for the other workflows.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an end-to-end test suite that drives
apps/testingin Chromium against a real Sendbird application, and a CI workflow that runs it on pull requests and onmain. The suite has 107 tests in 30 spec files: the core flows of each module (connection and profile, group channel list, messages, realtime updates, channel settings and creation, threads, open channels, theme and layout options), plus regression tests for previously reported bugs, each named after the fix it guards.Writing the tests surfaced two UIKit bugs, fixed here with unit tests: the connected user kept its previous nickname after
SendbirdProviderapplied thenickname/profileUrlprops, and the invite list in channel creation could crash or lose users.Root cause
completeConnectioninsrc/lib/Sendbird/context/hooks/useSendbird.tsxawaitedsdk.updateCurrentUserInfo(...)but discarded the user it returns. The user store was initialized with the user from before the update, andeventHandlers.connection.onConnectedreceived that same object, so the channel list header and the profile editor showed the old name until something else refreshed the store.InviteUsersfetched users from a mount-only effect. Opened before the connection finished,createDefaultUserListQuerycalledcreateApplicationUserListQueryon an undefinedsdkand threw. Under React StrictMode the effect ran twice and the slower fetch could overwrite the newer list; scroll pagination appended each page to theusersarray captured when the handler was created; failed fetches surfaced as unhandled rejections.Changes
E2E infrastructure
playwright.e2e.config.ts, separate from the visual-test config (yarn test:visual). It startsapps/testing(port fromE2E_PORT, orE2E_BASE_URLto target a deployed preview), allows 60 s per test, retries 3 times on CI and never locally, and keeps a trace of the first retry.e2e/fixtures.ts: every test gets its own throwaway users and channels, created and deleted through the Platform API, so tests share no state and can run in parallel, including concurrently across developers. Users live for one test rather than one worker, because a profile-editing test otherwise renamed the user for every later test in that worker. Without credentials every test skips. An uncaught page error or a React hook-rules warning fails the test even when its assertions pass; a test can allow a known error withallowedPageErrors.e2e/utils/platform.ts: a small Platform API client that spaces its calls, retries 429 and 5xx responses, and waits for a write to become readable before the test acts on it. The token stays in the Node process; the browser only receives the app ID, the user ID and feature flags through the URL.e2e/utils/run-tag.tsande2e/global-teardown.ts: everything a run creates carries a per-run tag, and a sweep at the end removes the channels and users a crashed test or a failed cleanup left behind. Users are found by the run's user ID prefix, since profile tests change nicknames.apps/testinggains no screens, only the URL parameters the tests need:theme,breakpoint,replyType,showSearchIconandlocalCacheEnabledon/group_channel, andgroupChannel_enableMarkAsUnread. It also exposes the SDK class aswindow.__SendbirdChatfor the offline tests. (localCacheEnabled=falseexists because UIKit marks the page offline only when local caching is off.)yarn lintandyarn typechecknow covere2e/(newtsconfig.node.json).CI:
.github/workflows/e2e.ymlruns when a pull request is opened or updated, on pushes tomain, and on manual dispatch. It splits the suite over 3 shards, merges their reports into one HTML artifact, and reports a singleE2Echeck intended for branch protection, since the shard job names change with the shard count. It reads the repository secretsE2E_APP_IDandE2E_PLATFORM_API_TOKEN. Pull requests from forks cannot read them, so the suite skips there; an internal pull request without them fails instead of reporting a green run that verified nothing. This workflow andbuild-and-test.ymlgroup concurrent runs bygithub.ref, so a run cancels only its own pull request's (or branch's) superseded runs; grouping by the source branch name let pull requests from forks with the same branch name, or a fork'smainand a push tomain, cancel each other. The shard and merge-report jobs ask setup-node for'>=24.22.1 <25', the same range #1478 sets for the other workflows, so they run on Node.js 24.22.1 or later (the release with the updated root certificate store) instead of whatever 24.x the runner has cached.Fixes
useSendbirdkeeps the user returned byupdateCurrentUserInfo, writes it to the user store, and passes it toonConnected, but only while that connect is still current. A connect counts as stale once a laterconnect, adisconnector an unmount has started; delayed-connection recovery already followed this rule, and the direct path now does too. Without it, a profile update for a user the provider had switched away from could resolve after the next user was initialized and write the previous user back into the store; the direct path would also initialize the SDK store and callonConnectedfor that user.InviteUsersre-runs its fetch once the SDK can create the default query, keyed on that readiness rather than onsdkitself: re-running on everysdkchange would callnext()again on a customuserListQueryand skip its first page, so a custom query is still requested once. Each fetch takes a number and only the latest one writes; scrolled pages are appended to the current list; failed fetches are caught. The internal helpercreateDefaultUserListQueryreturnsundefinedwhile the SDK is not ready.Backward compatibility
No changes to props, exports, types, CSS classes or StringSet keys.
One callback payload changes. When
nicknameorprofileUrlis passed toSendbirdProvider,eventHandlers.connection.onConnectednow receives the user as updated by those props instead of the user from before the update, anduseSendbirdStateContext().stores.userStore.userholds the same updated user. When it is called is unchanged (after the update completes), and so is the failure path: a failed update still ends inonFailedwithoutonConnected. Code that read the previous nickname fromonConnectednow sees the new one.A connect the provider abandons before it finishes (the user switched, or the provider disconnected or unmounted) no longer calls
onConnectedor writes the user and SDK stores; until now only a pending delayed-connection recovery was dropped this way.InviteUserskeeps its props, and a customuserListQueryis still called once when the modal opens.Testing
yarn lint,yarn typecheck,yarn build,yarn test(1344 passed),yarn test:e2e(107 passed) andyarn build:apps:testingpass.yarn api:snapshot: no change to the public API surface.yarn package:check: publint and attw pass.getUserMediadelayed by 2 s, the previous version of that test failed 2/2 and the fixed one passed 2/2.SendbirdProvider.sdkInit.integration.spec.tsxandSendbirdProvider.connectionDelayed.integration.spec.tsxeach switch users while the first user's profile update is still in flight, on the direct path and during delayed-connection recovery. Removing the staleness check from the profile write fails both; restoring the direct path's former always-current check fails the direct-path one.main,useSendbird.spec.tsxfails the store andonConnectedcase (its failure-path case passes either way, since that behaviour is unchanged and the test pins it), andInviteUsers/__tests__/index.spec.tsxfails 6 of its 11 cases.'>=24.22.1 <25'to Node.js 24.22.1 (Acquiring 24.22.1 - x64), and the shards passed 36 + 36 + 35 = 107 tests.Changelogs
SendbirdProviderapplied thenicknameorprofileUrlprop;eventHandlers.connection.onConnectednow receives the updated userSendbirdProviderswitched away from before it finished still callingeventHandlers.connection.onConnectedand overwriting the connected user