chore: Merge 4.73.1 into single-server#7391
Merged
Merged
Conversation
* fix: emoji picker scroll does not reach the end * feat: add test to pick the last emoji in emoji picker * fix: e2e test * fix: tests * tests * remove unused pod * rename emoji category * fix: new test flow * rollback space
* feat: alt text * action: organized translations * fix: move alt text accessibility to button level and fix gallery height * chore: format code and fix lint issues * ix: stable gallery keys and forward alt text accessibility to gallery items * fix: i18n fallbacks, alt text a11y label, stale altText on remove, and 8.4.0 boundary test * action: organized translations * fix: i18n * action: organized translations * fix: preserve caption on send, correct version gate for share extension, and translated a11y labels in gallery * fix: alt text label * feat: queue composer attachments inline * feat: open attachment alt text in action sheet * fix: remove image gallery * action: organized translations * feat: edit message alt text * chore: format code and fix lint issues * feat: backward compatibilities * fix: edit * remove image gallery * fix: conflicts * code improvements * fix: composer * code improvements * fix: tests * fix(a11y): improve image attachment accessibility labels and order * action: organized translations * fix: unit tests * fix: focus * fix: a11y labels * chore: format code and fix lint issues * test * fix: merge conflicts * feat: improve a11y experience * fix: unlabelled on android * fix: action sheet input keyboard * fix: announce GIFs as interactive in ImageViewer for screen readers * fix: ignore whitespace-only alt text in message a11y label * refactor: extract sendAttachments helper from composer and share view * action: organized translations * feat: code improvements * code improvements * refactor: extract useImageDescriptionLabel hook and inline useAltTextSupported in Image * fix: i18n * action: organized translations * refactor: stabilize FlatList renderers and harden altText checks * fix(a11y): omit empty segments in message accessibilityLabel * refactor: extract normalizeAttachment and preserve handler order in useChooseMedia * refactor: extract normalizeAttachment and preserve handler order in useChooseMedia * rollback prettier changes * feat: attachment action sheet stories and test * feat: attachment actionsheet stories and tests * feat: announce images without description to screen readers * action: organized translations * fix: render Attachment file name in Thread Message preview when body is empty (#7323) * fix: preference value changes causing reset to other option (#7313) * fix: do not encrypt messages when workspace E2E is disabled (#7324) * fix: snapshot test * fix: snapshot test * feat: unify thumbs * efactor: rename handlePickedAttachments to handleSelectedAttachments * fix: pass altText and isAnimated to ImageViewer in ShareView Preview * refactor: make AltTextLabel altText optional with early return * fix: stop leaking attachment.altText into caption rendering * fix: if no alt text its rendering an empty absolute view * test: cover useMessageAccessibilityLabel and keep suffix on translated * refactor: prefer title_link/message_link over index for Reply attachment key * refactor: unify ShareView Thumbs and ComposerAttachments under shared AttachmentThumbs * feat: unify Thumbs * remove memo of useImageDescriptionLabel * fix: test and lint * fix: test * fix: i18n missing translation * fix: fallback accessibility label when alt text is empty * fix: remove unused code * rollback prettier changes * refactor: colocate useImageDescriptionLabel with message hooks * refactor: simplify message accessibility label composition * feat: add missing keys * feat: use thumb as children instead of add it again * refactor: tighten types and stable keys in message/share views * fix: useTheme on AltTextInput * action: organized translations * chore: code organization * chore: type improvement * chore: code organization * fix: image improvement * code improvements * fix: avoid undefined a11y label * fix: altText trim and improvements * fix: tests * fix: use memo composer attachments * chore: remove editAltText (not available) * feat: standardize shareView composer --------- Co-authored-by: OtavioStasiak <OtavioStasiak@users.noreply.github.com> Co-authored-by: Diego Mello <diegolmello@gmail.com> Co-authored-by: Rohit Bansal <40559587+Rohit3523@users.noreply.github.com>
* feat: announcement and focus * action: organized translations * fix: lint * fix: iOS announcement * fix: focus switch * fix: unit tests * fix: unit tests * chore: migrate last focus to hooks * fix: en translation * fix: back focus to message * code improvements * fix: message actions announcement * fix: tests * fix: touch * feat: a11y improvements * action: organized translations * fix: Touch * i18n: translations * useMessageAccessibilityActions and Label as hooks * action: organized translations * feat: control back focus on hook * feat(a11y): message a11y label just on Touchable and add "press to view thread" hint to message accessibility label * fix: snapshot * fix: test snapshot * test: fix iOS Maestro thread test by tapping message content instead of replied-on-thread indicator * fix: encryption test ios * code improvements * code improvements * fix: snapshot test * fix: i18n * action: organized translations * remove duplicated label * remove duplicated props --------- Co-authored-by: OtavioStasiak <OtavioStasiak@users.noreply.github.com> Co-authored-by: Diego Mello <diegolmello@gmail.com>
* fix: acrionSheet height considering handle * feat: e2e tests * fix: e2e tests
…7336) * chore:keyboard navigation focus on header instead of composer * fix: focus change * chore: code improvements * fix: room test flow * fix: test * fix: e2e tests * fix: test * fix: tests * fix: e2e test * fix: e2e test * fix: e2e test * rollback * improve autoFocus condition * rollback unused change * rollback unused change
…urvives backgrounding (#7346)
…es (#7305) * fix: fetch enough history when system messages are hidden * trying to load messages * load code improvements * feat: improve test useMessages * fix: dedupe concurrent room history fetches on cold open * refactor: simplify loadMessagesForRoom and document history loader terms * fix: tests
# Conflicts: # android/app/build.gradle # ios/RocketChatRN.xcodeproj/project.pbxproj # ios/RocketChatRN/Info.plist # ios/ShareRocketChatRN/Info.plist # package.json
…piler memoization (#7348)
* chore: unify search announcement results for a11y * chore: create useDirectorySearch hook * feat: migrate directoryView to hooks * code improvements * fix: tests
Brings the 4.73.1 release into the single-server line. Conflict resolution (mirrors the 4.73.0 single-server merge, NATIVE-1205): - Version files (package.json, build.gradle, Info.plist x2, pbxproj) -> 4.73.1 - android/gradle.properties -> single-server values (APPLICATION_ID=chat.rocket.reactnative, VERSIONCODE=1, keystore block) - Firebase configs kept deleted (google-services.json, GoogleService-Info.plist, debug strings.xml) - connect.ts / connect.test.ts -> 4.73.1 (single-server merely lacked the unsubscribeRooms() close-listener fix #7362) - pnpm-lock.yaml / Podfile.lock -> 4.73.1 (only delta was picker 2.11.1 -> 2.11.4) - Single-server product customizations intact (app.json hardcoded server, disabled NewServerView, disabled ServersList switcher)
diegolmello
had a problem deploying
to
approve_e2e_testing
GitHub Actions
Error
Contributor
WalkthroughRelease 4.73.1 updates app/native versions, adds history UI loader lifecycle and batched message loading, introduces markdown parse caching and block rendering, refactors DirectoryView/search to hooks with accessibility announcements, optimizes deep-link connection gating, hardens SDK socket liveness/reopen, and adjusts video-call permission and settings handling. ChangesRelease 4.73.1: Multi-Feature Bundle
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
app/lib/methods/helpers/normalizeDeepLinkingServerHost.ts (1)
10-17:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAnchor the localhost exception to the full host token.
These regexes match any host that merely starts with
localhost, so values likelocalhost.example.comandhttp://localhost.evil.testare treated as localhost and skip the HTTPS upgrade. In a deep-link flow that can preserve cleartext transport for a remote host.Suggested fix
- if (!/^(http|https)/.test(host)) { - if (/^localhost(:\d+)?/.test(host)) { + if (!/^(http|https):\/\//.test(host)) { + if (/^localhost(?::\d+)?(?:\/|$)/.test(host)) { host = `http://${host}`; } else { host = `https://${host}`; } - } else if (!/^http:\/\/localhost(:\d+)?/.test(host)) { + } else if (!/^http:\/\/localhost(?::\d+)?(?:\/|$)/.test(host)) { host = host.replace('http://', 'https://'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/lib/methods/helpers/normalizeDeepLinkingServerHost.ts` around lines 10 - 17, The localhost checks in normalizeDeepLinkingServerHost incorrectly match hosts that merely start with "localhost" (e.g., "localhost.example.com"); update the regexes used in the function so they anchor the full host token—replace /^localhost(:\d+)?/ with /^localhost(:\d+)?$/ and /^http:\/\/localhost(:\d+)?/ with /^http:\/\/localhost(:\d+)?$/ (so only exact "localhost" with optional port is treated as localhost) and keep the rest of the logic unchanged.app/sagas/deepLinking.js (1)
334-344:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMirror the
hostAlreadyConnectedshort-circuit inhandleClickCallPush.This branch still emits
NewServerand waits forSERVER.SELECT_SUCCESSeven whensdk.current?.client?.host === host. In that case the select-server flow short-circuits instead of dispatchingSERVER.SELECT_SUCCESS, so the video-call deep-link path can hang beforeloginRequestandhandleNavigateCallRoom.Suggested fix
- yield put(appStart({ root: RootEnum.ROOT_OUTSIDE })); - yield put(serverInitAdd(server)); - yield delay(1000); - EventEmitter.emit('NewServer', { server: host }); + const hostAlreadyConnected = sdk.current?.client?.host === host; + if (!hostAlreadyConnected) { + yield put(appStart({ root: RootEnum.ROOT_OUTSIDE })); + yield put(serverInitAdd(server)); + yield delay(1000); + EventEmitter.emit('NewServer', { server: host }); + } if (params.token) { - yield take(types.SERVER.SELECT_SUCCESS); - // SERVER.SELECT_SUCCESS doesn't mean 'connected'; skip the take if it already is. - const connected = yield select(state => state.meteor.connected); - if (!connected) { - yield take(types.METEOR.SUCCESS); + if (!hostAlreadyConnected) { + yield take(types.SERVER.SELECT_SUCCESS); + // SERVER.SELECT_SUCCESS doesn't mean 'connected'; skip the take if it already is. + const connected = yield select(state => state.meteor.connected); + if (!connected) { + yield take(types.METEOR.SUCCESS); + } } yield put(loginRequest({ resume: params.token }, true));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/sagas/deepLinking.js` around lines 334 - 344, The branch emitting EventEmitter.emit('NewServer', { server: host }) and yielding take(types.SERVER.SELECT_SUCCESS) should short-circuit exactly like handleClickCallPush when the SDK is already pointed at the same host; detect the same condition (e.g. sdk.current?.client?.host === host or the existing hostAlreadyConnected check) before emitting NewServer and before yielding take(types.SERVER.SELECT_SUCCESS), and if true skip those steps so the saga proceeds directly to the token-handling / loginRequest and handleNavigateCallRoom flow (still keep the connected check and possible take(types.METEOR.SUCCESS)). Use the same unique symbols to locate code: EventEmitter.emit('NewServer', ...), types.SERVER.SELECT_SUCCESS, sdk.current?.client?.host (or hostAlreadyConnected), loginRequest, and handleNavigateCallRoom.
🧹 Nitpick comments (2)
app/containers/markdown/index.tsx (1)
44-61: ⚡ Quick winConsider upgrading cache to LRU eviction.
The current implementation uses FIFO (First-In-First-Out) eviction—when the cache reaches 200 entries, it removes the oldest inserted key. This means frequently accessed messages will still be evicted after 200 newer messages are parsed, even if they're re-rendered often (e.g., when users scroll up and down).
Upgrading to LRU (Least Recently Used) would improve cache hit rates by keeping frequently accessed messages longer.
♻️ Suggested LRU implementation
const parseMessage = (msg: string): Root => { const cached = parseCache.get(msg); if (cached) { + // Move to end for LRU behavior + parseCache.delete(msg); + parseCache.set(msg, cached); return cached; } const result = parse(msg); if (parseCache.size >= PARSE_CACHE_MAX) { const oldestKey = parseCache.keys().next().value; if (oldestKey !== undefined) { parseCache.delete(oldestKey); } } parseCache.set(msg, result); return result; };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/containers/markdown/index.tsx` around lines 44 - 61, The cache currently evicts in FIFO order; update parseMessage and the parseCache usage to implement an LRU policy so frequently accessed messages stay cached: on a cache hit in parseMessage (parseCache.get(msg)), move that entry to be most-recently-used (delete then set the key) before returning; on cache insertion (after parse(msg)), insert normally and, if parseCache.size > PARSE_CACHE_MAX, remove the least-recently-used entry by deleting parseCache.keys().next().value; ensure all mentions of parseCache, parseMessage, PARSE_CACHE_MAX and parse() are updated accordingly so the Map behaves as an LRU cache.app/views/DirectoryView/hooks/useDirectorySearch.ts (1)
9-9: ⚡ Quick winAdd an explicit return type for the exported hook.
This hook is exported and currently relies on inferred return shape, which weakens API contract clarity for consumers.
💡 Suggested refactor
-export const useDirectorySearch = (directoryDefaultView: string) => { +interface IUseDirectorySearchResult { + data: IServerRoom[]; + loading: boolean; + type: string; + globalUsers: boolean; + search: () => void; + loadMore: () => void; + onSearchChangeText: (newText: string) => void; + changeType: (newType: string) => void; + toggleWorkspace: () => void; +} + +export const useDirectorySearch = (directoryDefaultView: string): IUseDirectorySearchResult => {As per coding guidelines:
Use TypeScript for type safety; add explicit type annotations to function parameters and return types.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/views/DirectoryView/hooks/useDirectorySearch.ts` at line 9, The exported hook useDirectorySearch currently relies on an inferred return type; add an explicit TypeScript return annotation to its signature (e.g., export const useDirectorySearch = (directoryDefaultView: string): UseDirectorySearchReturn => ...) by defining and exporting a corresponding return type alias or interface (UseDirectorySearchReturn) that declares the exact shape (states, setters, handlers, and any derived values) the hook returns; update any callers if necessary to import the new type.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/lib/hooks/useVideoConf/useVideoConfCall.ts`:
- Around line 20-33: The effect that calls init() should depend on all reactive
inputs so callEnabled/disabledTooltip are recomputed when inputs change: update
the useEffect in useVideoConfCall.ts to include rid, server version check
output, the permissions result from usePermissions, user?.username, and all
settings fields (jitsiEnabled, jitsiEnableTeams, jitsiEnableChannels,
videoConfEnableDMs, videoConfEnableChannels, videoConfEnableTeams,
videoConfEnableGroups, omnichannelCallProvider) in its dependency array and call
init whenever any change; inside init (or immediately before calling it) reset
disabledTooltip back to false so it can be recalculated (disabledTooltip
currently only set true and never cleared), ensuring the effect references the
init function defined in this hook and updates callEnabled/disabledTooltip
derived state accordingly.
In `@app/views/DirectoryView/hooks/useDirectorySearch.ts`:
- Around line 20-54: The load handler can have older async responses overwrite
newer results; fix by adding a request sequencing guard: create a stable
ref/counter (e.g., latestRequestIdRef) and increment it each time load runs,
capture the current id in the async closure, then after the await from
getDirectory(...) verify the captured id === latestRequestIdRef.current before
calling setData, setTotal, announceSearchResultsForAccessibility or
setLoading(false) for successful branch; also check the id in the error/catch
branch so stale failures don't clear loading or log for newer requests. Ensure
the guard is applied around all state updates in load (including the newSearch
branch logic) so only the most recent getDirectory() result mutates state.
---
Outside diff comments:
In `@app/lib/methods/helpers/normalizeDeepLinkingServerHost.ts`:
- Around line 10-17: The localhost checks in normalizeDeepLinkingServerHost
incorrectly match hosts that merely start with "localhost" (e.g.,
"localhost.example.com"); update the regexes used in the function so they anchor
the full host token—replace /^localhost(:\d+)?/ with /^localhost(:\d+)?$/ and
/^http:\/\/localhost(:\d+)?/ with /^http:\/\/localhost(:\d+)?$/ (so only exact
"localhost" with optional port is treated as localhost) and keep the rest of the
logic unchanged.
In `@app/sagas/deepLinking.js`:
- Around line 334-344: The branch emitting EventEmitter.emit('NewServer', {
server: host }) and yielding take(types.SERVER.SELECT_SUCCESS) should
short-circuit exactly like handleClickCallPush when the SDK is already pointed
at the same host; detect the same condition (e.g. sdk.current?.client?.host ===
host or the existing hostAlreadyConnected check) before emitting NewServer and
before yielding take(types.SERVER.SELECT_SUCCESS), and if true skip those steps
so the saga proceeds directly to the token-handling / loginRequest and
handleNavigateCallRoom flow (still keep the connected check and possible
take(types.METEOR.SUCCESS)). Use the same unique symbols to locate code:
EventEmitter.emit('NewServer', ...), types.SERVER.SELECT_SUCCESS,
sdk.current?.client?.host (or hostAlreadyConnected), loginRequest, and
handleNavigateCallRoom.
---
Nitpick comments:
In `@app/containers/markdown/index.tsx`:
- Around line 44-61: The cache currently evicts in FIFO order; update
parseMessage and the parseCache usage to implement an LRU policy so frequently
accessed messages stay cached: on a cache hit in parseMessage
(parseCache.get(msg)), move that entry to be most-recently-used (delete then set
the key) before returning; on cache insertion (after parse(msg)), insert
normally and, if parseCache.size > PARSE_CACHE_MAX, remove the
least-recently-used entry by deleting parseCache.keys().next().value; ensure all
mentions of parseCache, parseMessage, PARSE_CACHE_MAX and parse() are updated
accordingly so the Map behaves as an LRU cache.
In `@app/views/DirectoryView/hooks/useDirectorySearch.ts`:
- Line 9: The exported hook useDirectorySearch currently relies on an inferred
return type; add an explicit TypeScript return annotation to its signature
(e.g., export const useDirectorySearch = (directoryDefaultView: string):
UseDirectorySearchReturn => ...) by defining and exporting a corresponding
return type alias or interface (UseDirectorySearchReturn) that declares the
exact shape (states, setters, handlers, and any derived values) the hook
returns; update any callers if necessary to import the new type.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c74827f8-92e1-46c2-aca3-741809543fc3
⛔ Files ignored due to path filters (2)
ios/Podfile.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (30)
README.mdandroid/app/build.gradleapp/actions/actionsTypes.tsapp/actions/room.tsapp/containers/markdown/index.tsxapp/containers/markdown/styles.tsapp/lib/hooks/useVideoConf/index.tsxapp/lib/hooks/useVideoConf/useVideoConfCall.tsapp/lib/methods/helpers/announceSearchResultsForAccessibility.tsapp/lib/methods/helpers/index.tsapp/lib/methods/helpers/normalizeDeepLinkingServerHost.tsapp/lib/methods/loadMessagesForRoom.test.tsapp/lib/methods/loadMessagesForRoom.tsapp/lib/services/connect.test.tsapp/lib/services/connect.tsapp/reducers/room.test.tsapp/reducers/room.tsapp/sagas/__tests__/deepLinking.test.tsapp/sagas/deepLinking.jsapp/views/DirectoryView/hooks/useDirectorySearch.tsapp/views/DirectoryView/index.tsxapp/views/RoomView/List/hooks/useMessages.test.tsxapp/views/RoomView/List/hooks/useMessages.tsapp/views/RoomView/components/HeaderCallButton.tsxapp/views/RoomsListView/hooks/useSearch.tsios/RocketChatRN.xcodeproj/project.pbxprojios/RocketChatRN/Info.plistios/ShareRocketChatRN/Info.plistpackage.jsonpatches/@rocket.chat+sdk+1.3.3-mobile.patch
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/lib/methods/helpers/index.tsapp/actions/actionsTypes.tsapp/containers/markdown/styles.tsapp/reducers/room.test.tsapp/lib/services/connect.test.tsapp/lib/methods/helpers/normalizeDeepLinkingServerHost.tsapp/lib/services/connect.tsapp/reducers/room.tsapp/lib/methods/helpers/announceSearchResultsForAccessibility.tsapp/actions/room.tsapp/lib/methods/loadMessagesForRoom.test.tsapp/containers/markdown/index.tsxapp/views/RoomView/components/HeaderCallButton.tsxapp/sagas/deepLinking.jsapp/views/DirectoryView/hooks/useDirectorySearch.tsapp/lib/hooks/useVideoConf/useVideoConfCall.tsapp/views/RoomView/List/hooks/useMessages.tsapp/views/RoomView/List/hooks/useMessages.test.tsxapp/lib/hooks/useVideoConf/index.tsxapp/views/DirectoryView/index.tsxapp/lib/methods/loadMessagesForRoom.tsapp/views/RoomsListView/hooks/useSearch.tsapp/sagas/__tests__/deepLinking.test.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/lib/methods/helpers/index.tsapp/actions/actionsTypes.tsapp/containers/markdown/styles.tsapp/reducers/room.test.tsapp/lib/services/connect.test.tsapp/lib/methods/helpers/normalizeDeepLinkingServerHost.tsapp/lib/services/connect.tsapp/reducers/room.tsapp/lib/methods/helpers/announceSearchResultsForAccessibility.tsapp/actions/room.tsapp/lib/methods/loadMessagesForRoom.test.tsapp/containers/markdown/index.tsxapp/views/RoomView/components/HeaderCallButton.tsxapp/views/DirectoryView/hooks/useDirectorySearch.tsapp/lib/hooks/useVideoConf/useVideoConfCall.tsapp/views/RoomView/List/hooks/useMessages.tsapp/views/RoomView/List/hooks/useMessages.test.tsxapp/lib/hooks/useVideoConf/index.tsxapp/views/DirectoryView/index.tsxapp/lib/methods/loadMessagesForRoom.tsapp/views/RoomsListView/hooks/useSearch.tsapp/sagas/__tests__/deepLinking.test.ts
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/lib/methods/helpers/index.tsapp/actions/actionsTypes.tsapp/containers/markdown/styles.tsapp/reducers/room.test.tsapp/lib/services/connect.test.tsapp/lib/methods/helpers/normalizeDeepLinkingServerHost.tsapp/lib/services/connect.tsapp/reducers/room.tsapp/lib/methods/helpers/announceSearchResultsForAccessibility.tsapp/actions/room.tspackage.jsonapp/lib/methods/loadMessagesForRoom.test.tsapp/containers/markdown/index.tsxapp/views/RoomView/components/HeaderCallButton.tsxapp/sagas/deepLinking.jsapp/views/DirectoryView/hooks/useDirectorySearch.tsapp/lib/hooks/useVideoConf/useVideoConfCall.tsapp/views/RoomView/List/hooks/useMessages.tsapp/views/RoomView/List/hooks/useMessages.test.tsxapp/lib/hooks/useVideoConf/index.tsxapp/views/DirectoryView/index.tsxapp/lib/methods/loadMessagesForRoom.tsapp/views/RoomsListView/hooks/useSearch.tsapp/sagas/__tests__/deepLinking.test.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/lib/methods/helpers/index.tsapp/actions/actionsTypes.tsapp/containers/markdown/styles.tsapp/reducers/room.test.tsapp/lib/services/connect.test.tsapp/lib/methods/helpers/normalizeDeepLinkingServerHost.tsapp/lib/services/connect.tsapp/reducers/room.tsapp/lib/methods/helpers/announceSearchResultsForAccessibility.tsapp/actions/room.tsapp/lib/methods/loadMessagesForRoom.test.tsapp/containers/markdown/index.tsxapp/views/RoomView/components/HeaderCallButton.tsxapp/sagas/deepLinking.jsapp/views/DirectoryView/hooks/useDirectorySearch.tsapp/lib/hooks/useVideoConf/useVideoConfCall.tsapp/views/RoomView/List/hooks/useMessages.tsapp/views/RoomView/List/hooks/useMessages.test.tsxapp/lib/hooks/useVideoConf/index.tsxapp/views/DirectoryView/index.tsxapp/lib/methods/loadMessagesForRoom.tsapp/views/RoomsListView/hooks/useSearch.tsapp/sagas/__tests__/deepLinking.test.ts
app/actions/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place action creators in 'app/actions/' directory
Files:
app/actions/actionsTypes.tsapp/actions/room.ts
app/containers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place reusable UI components in 'app/containers/' directory
Files:
app/containers/markdown/styles.tsapp/containers/markdown/index.tsx
app/reducers/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place reducers in 'app/reducers/' directory
Files:
app/reducers/room.test.tsapp/reducers/room.ts
app/lib/services/connect.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use 'app/lib/services/connect.ts' for server connection management
Files:
app/lib/services/connect.ts
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/RoomView/components/HeaderCallButton.tsxapp/views/DirectoryView/hooks/useDirectorySearch.tsapp/views/RoomView/List/hooks/useMessages.tsapp/views/RoomView/List/hooks/useMessages.test.tsxapp/views/DirectoryView/index.tsxapp/views/RoomsListView/hooks/useSearch.ts
app/sagas/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place sagas in 'app/sagas/' directory for handling side effects
Files:
app/sagas/__tests__/deepLinking.test.ts
🧠 Learnings (5)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/lib/methods/helpers/index.tsapp/actions/actionsTypes.tsapp/containers/markdown/styles.tsapp/reducers/room.test.tsapp/lib/services/connect.test.tsapp/lib/methods/helpers/normalizeDeepLinkingServerHost.tsapp/lib/services/connect.tsapp/reducers/room.tsapp/lib/methods/helpers/announceSearchResultsForAccessibility.tsapp/actions/room.tsapp/lib/methods/loadMessagesForRoom.test.tsapp/containers/markdown/index.tsxapp/views/RoomView/components/HeaderCallButton.tsxapp/views/DirectoryView/hooks/useDirectorySearch.tsapp/lib/hooks/useVideoConf/useVideoConfCall.tsapp/views/RoomView/List/hooks/useMessages.tsapp/views/RoomView/List/hooks/useMessages.test.tsxapp/lib/hooks/useVideoConf/index.tsxapp/views/DirectoryView/index.tsxapp/lib/methods/loadMessagesForRoom.tsapp/views/RoomsListView/hooks/useSearch.tsapp/sagas/__tests__/deepLinking.test.ts
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
📚 Learning: 2026-05-07T17:47:14.516Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7303
File: package.json:5-5
Timestamp: 2026-05-07T17:47:14.516Z
Learning: When reviewing pnpm `packageManager` version pins in any `package.json` (e.g., `"packageManager": "pnpm@<version>"`), don’t rely solely on web-search results to determine whether a version exists. For very recently published versions, cross-check the target version against the official pnpm release page (https://github.com/pnpm/pnpm/releases) and the npm registry page for pnpm (https://www.npmjs.com/package/pnpm) before flagging the pinned version as non-existent.
Applied to files:
package.json
📚 Learning: 2026-05-07T13:19:52.152Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7304
File: app/sagas/deepLinking.js:237-243
Timestamp: 2026-05-07T13:19:52.152Z
Learning: In this codebase’s Redux-Saga usage, remember that `yield put(action)` dispatches through the Redux store synchronously, and any saga(s) that synchronously react via action listeners (and synchronous `put` chains) will run to completion before the calling saga resumes at its next `yield`. As a result, within a single saga there is no scheduler interleaving between a `yield select(...)` and a subsequent `yield take(...)` at the next `yield` point, so a check-then-take pattern like `const state = yield select(...); if (state !== TARGET) { yield take(a => a.type === TARGET); }` is safe from TOCTOU races under the synchronous `put`/take model described above.
Applied to files:
app/sagas/deepLinking.js
📚 Learning: 2026-05-05T21:08:33.177Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7298
File: patches/@rocket.chat+sdk+1.3.3-mobile.patch:79-79
Timestamp: 2026-05-05T21:08:33.177Z
Learning: In the RocketChat/Rocket.Chat.ReactNative repo, for patches under patches/*.patch (especially those touching rocket.chat/sdk), remember that the patching sandbox's node_modules reflects the pre-patch state. Grepping node_modules for symbols (e.g., userDisconnectCloseCode = 4000 in node_modules/rocket.chat/sdk/lib/drivers/ddp.ts) can yield false positives. Review patches by inspecting the diff and applying it to a fresh copy of the SDK or diffing against the SDK source, rather than relying on node_modules. Ensure the patch actually introduces/updates symbols in the SDK source and run the test suite to validate behavior after applying the patch.
Applied to files:
patches/@rocket.chat+sdk+1.3.3-mobile.patch
🔇 Additional comments (31)
README.md (1)
29-29: LGTM!android/app/build.gradle (1)
93-93: LGTM!package.json (2)
3-3: LGTM!
44-44: LGTM!ios/RocketChatRN/Info.plist (1)
29-29: LGTM!ios/ShareRocketChatRN/Info.plist (1)
22-22: LGTM!ios/RocketChatRN.xcodeproj/project.pbxproj (1)
2092-2092: LGTM!Also applies to: 2145-2145
app/containers/markdown/index.tsx (5)
1-22: LGTM!
41-41: AI summary states incorrect cache size.The code defines
PARSE_CACHE_MAX = 200, but the AI-generated summary claims "max 10 entries". The implemented value of 200 is correct.
63-69: LGTM!
104-154: LGTM!
71-102: React Compiler is configured for'use memo'directives
babel-plugin-react-compileris enabled inbabel.config.js(compilationMode: 'annotation') and present inpackage.json, so the'use memo'directive should be processed rather than silently ignored.app/containers/markdown/styles.ts (1)
11-13: LGTM!app/actions/actionsTypes.ts (1)
25-27: LGTM!app/actions/room.ts (1)
62-68: LGTM!Also applies to: 77-79, 152-164
app/reducers/room.ts (1)
71-82: LGTM!app/reducers/room.test.ts (1)
8-9: LGTM!Also applies to: 75-84
app/lib/methods/loadMessagesForRoom.ts (1)
3-10: LGTM!Also applies to: 15-150
app/lib/methods/loadMessagesForRoom.test.ts (1)
1-236: LGTM!app/views/RoomView/List/hooks/useMessages.ts (1)
4-6: LGTM!Also applies to: 40-40, 166-174
app/views/RoomView/List/hooks/useMessages.test.tsx (1)
12-15: LGTM!Also applies to: 49-49, 68-70, 76-99, 376-522
app/lib/services/connect.ts (1)
135-141: LGTM!app/lib/services/connect.test.ts (1)
485-513: LGTM!patches/@rocket.chat+sdk+1.3.3-mobile.patch (2)
9-45: LGTM!Also applies to: 171-179
102-167: checkAndReopen promise handling is already covered at call sitesCallers that run in the app handle the returned Promise:
app/sagas/state.jsusescheckAndReopen().catch(...), andapp/lib/services/voip/MediaCallEvents.tsawaitscheckAndReopen()inside atry/catch. Remaining hits are unit tests and theconnect.tswrapper definition.app/lib/hooks/useVideoConf/index.tsx (1)
1-1: LGTM!Also applies to: 68-71
app/views/RoomView/components/HeaderCallButton.tsx (1)
68-68: LGTM!app/views/DirectoryView/index.tsx (1)
1-3: LGTM!Also applies to: 7-7, 17-17, 19-21, 23-23, 26-28, 36-214
app/lib/methods/helpers/announceSearchResultsForAccessibility.ts (1)
1-13: LGTM!app/lib/methods/helpers/index.ts (1)
21-21: LGTM!app/views/RoomsListView/hooks/useSearch.ts (1)
6-6: LGTM!
Comment on lines
+20
to
+33
| // Read all call-related settings in a single subscription instead of one useSetting per key. | ||
| const settings = useAppSelector( | ||
| state => ({ | ||
| jitsiEnabled: state.settings.Jitsi_Enabled, | ||
| jitsiEnableTeams: state.settings.Jitsi_Enable_Teams, | ||
| jitsiEnableChannels: state.settings.Jitsi_Enable_Channels, | ||
| videoConfEnableDMs: state.settings.VideoConf_Enable_DMs, | ||
| videoConfEnableChannels: state.settings.VideoConf_Enable_Channels, | ||
| videoConfEnableTeams: state.settings.VideoConf_Enable_Teams, | ||
| videoConfEnableGroups: state.settings.VideoConf_Enable_Groups, | ||
| omnichannelCallProvider: state.settings.Omnichannel_call_provider | ||
| }), | ||
| shallowEqual | ||
| ); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/lib/hooks/useVideoConf/useVideoConfCall.ts"
if [ ! -f "$FILE" ]; then
echo "Missing file: $FILE"
exit 1
fi
echo "=== File header context ==="
sed -n '1,120p' "$FILE" | nl -ba | sed -n '1,120p'
echo "=== Lines 15-120 (for init + effect + derived state) ==="
sed -n '15,120p' "$FILE" | nl -ba
echo "=== Lines 120-220 (if more logic exists) ==="
if [ "$(wc -l < "$FILE")" -ge 220 ]; then
sed -n '120,220p' "$FILE" | nl -ba
else
sed -n '120,$p' "$FILE" | nl -ba
fi
echo "=== Search for init/callEnabled/disabledTooltip/rid dependencies ==="
rg -n "init\s*=|callEnabled|disabledTooltip|rid|useEffect\(|useCallback\(" "$FILE"
echo "=== Show full init function if small enough ==="
# Print any init function block if present
rg -n "const init|function init" "$FILE" || trueRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 168
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="app/lib/hooks/useVideoConf/useVideoConfCall.ts"
echo "Checking file: $FILE"
ls -la "$FILE"
echo "=== Lines 1-140 ==="
sed -n '1,140p' "$FILE" | cat -n
echo "=== Lines 140-260 ==="
sed -n '140,260p' "$FILE" | cat -n
echo "=== Search for init/callEnabled/disabledTooltip/rid dependencies ==="
rg -n "const init|function init|callEnabled|disabledTooltip|rid|useEffect\(|useCallback\(" "$FILE" || true
echo "=== Extract init block candidates ==="
rg -n "init\s*=" "$FILE" || true
rg -n "const init\s*=\s*async|const init\s*=\s*\(" "$FILE" || true
rg -n "useEffect\s*\(\s*\(" "$FILE" || trueRepository: RocketChat/Rocket.Chat.ReactNative
Length of output: 5288
Recompute call state when selector inputs change.
useEffect(() => { init(); }, []) runs init only once, but init uses reactive values (rid, server version check, usePermissions result, user?.username, and settings-derived flags like enabledDMs/enabledChannel/enabledTeams/enabledGroups/enabledLiveChat + jitsiEnabled/*). If any of those change after mount, callEnabled and disabledTooltip can stay stale (and disabledTooltip is only ever set to true, never reset).
💡 Suggested fix
-import { useEffect, useState } from 'react';
+import { useCallback, useEffect, useState } from 'react';
...
- const init = async () => {
+ const init = useCallback(async () => {
const room = await getSubscriptionByRoomId(rid);
if (room) {
...
}
return setCallEnabled(false);
- };
+ }, [
+ rid,
+ isServer5OrNewer,
+ canStartCall,
+ user?.username,
+ enabledDMs,
+ enabledChannel,
+ enabledTeams,
+ enabledGroups,
+ enabledLiveChat,
+ jitsiEnabled,
+ jitsiEnableTeams,
+ jitsiEnableChannels
+ ]);
useEffect(() => {
- init();
- }, []);
+ init();
+ }, [init]);Also reset disabledTooltip (e.g., to false) when recomputing so it matches the latest inputs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/lib/hooks/useVideoConf/useVideoConfCall.ts` around lines 20 - 33, The
effect that calls init() should depend on all reactive inputs so
callEnabled/disabledTooltip are recomputed when inputs change: update the
useEffect in useVideoConfCall.ts to include rid, server version check output,
the permissions result from usePermissions, user?.username, and all settings
fields (jitsiEnabled, jitsiEnableTeams, jitsiEnableChannels, videoConfEnableDMs,
videoConfEnableChannels, videoConfEnableTeams, videoConfEnableGroups,
omnichannelCallProvider) in its dependency array and call init whenever any
change; inside init (or immediately before calling it) reset disabledTooltip
back to false so it can be recalculated (disabledTooltip currently only set true
and never cleared), ensuring the effect references the init function defined in
this hook and updates callEnabled/disabledTooltip derived state accordingly.
Comment on lines
+20
to
+54
| const load = useDebounce(async ({ newSearch = false }: { newSearch?: boolean } = {}) => { | ||
| if (!newSearch && (loading || data.length === total)) { | ||
| return; | ||
| } | ||
|
|
||
| if (newSearch) { | ||
| setData([]); | ||
| setTotal(-1); | ||
| } | ||
| setLoading(true); | ||
|
|
||
| try { | ||
| const directories = await getDirectory({ | ||
| text, | ||
| type, | ||
| workspace: globalUsers ? 'all' : 'local', | ||
| offset: newSearch ? 0 : data.length, | ||
| count: 50, | ||
| sort: type === 'users' ? { username: 1 } : { usersCount: -1 } | ||
| }); | ||
| if (directories.success) { | ||
| setData(prev => [...(newSearch ? [] : prev), ...(directories.result as IServerRoom[])]); | ||
| setTotal(directories.total); | ||
| setLoading(false); | ||
| // Announce the full total on a fresh search; loadMore pages shouldn't re-announce | ||
| if (newSearch) { | ||
| announceSearchResultsForAccessibility(directories.total); | ||
| } | ||
| } else { | ||
| setLoading(false); | ||
| } | ||
| } catch (e) { | ||
| log(e); | ||
| setLoading(false); | ||
| } |
Contributor
There was a problem hiding this comment.
Prevent stale responses from overwriting newer search results.
Concurrent requests are not sequenced. If an older request resolves after a newer one, it can overwrite/append stale data into the latest query results.
💡 Suggested fix
-import { useEffect, useState } from 'react';
+import { useEffect, useRef, useState } from 'react';
@@
const [globalUsers, setGlobalUsers] = useState(true);
const [type, setType] = useState(directoryDefaultView);
+ const requestIdRef = useRef(0);
@@
const load = useDebounce(async ({ newSearch = false }: { newSearch?: boolean } = {}) => {
if (!newSearch && (loading || data.length === total)) {
return;
}
+ const requestId = ++requestIdRef.current;
@@
- try {
+ try {
const directories = await getDirectory({
@@
- if (directories.success) {
- setData(prev => [...(newSearch ? [] : prev), ...(directories.result as IServerRoom[])]);
- setTotal(directories.total);
- setLoading(false);
+ if (requestId !== requestIdRef.current) {
+ return;
+ }
+ if (directories.success) {
+ setData(prev => [...(newSearch ? [] : prev), ...(directories.result as IServerRoom[])]);
+ setTotal(directories.total);
// Announce the full total on a fresh search; loadMore pages shouldn't re-announce
if (newSearch) {
announceSearchResultsForAccessibility(directories.total);
}
- } else {
- setLoading(false);
}
} catch (e) {
+ if (requestId !== requestIdRef.current) {
+ return;
+ }
log(e);
+ } finally {
+ if (requestId === requestIdRef.current) {
+ setLoading(false);
+ }
- setLoading(false);
}
}, 200);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const load = useDebounce(async ({ newSearch = false }: { newSearch?: boolean } = {}) => { | |
| if (!newSearch && (loading || data.length === total)) { | |
| return; | |
| } | |
| if (newSearch) { | |
| setData([]); | |
| setTotal(-1); | |
| } | |
| setLoading(true); | |
| try { | |
| const directories = await getDirectory({ | |
| text, | |
| type, | |
| workspace: globalUsers ? 'all' : 'local', | |
| offset: newSearch ? 0 : data.length, | |
| count: 50, | |
| sort: type === 'users' ? { username: 1 } : { usersCount: -1 } | |
| }); | |
| if (directories.success) { | |
| setData(prev => [...(newSearch ? [] : prev), ...(directories.result as IServerRoom[])]); | |
| setTotal(directories.total); | |
| setLoading(false); | |
| // Announce the full total on a fresh search; loadMore pages shouldn't re-announce | |
| if (newSearch) { | |
| announceSearchResultsForAccessibility(directories.total); | |
| } | |
| } else { | |
| setLoading(false); | |
| } | |
| } catch (e) { | |
| log(e); | |
| setLoading(false); | |
| } | |
| const load = useDebounce(async ({ newSearch = false }: { newSearch?: boolean } = {}) => { | |
| if (!newSearch && (loading || data.length === total)) { | |
| return; | |
| } | |
| if (newSearch) { | |
| setData([]); | |
| setTotal(-1); | |
| } | |
| setLoading(true); | |
| const requestId = ++requestIdRef.current; | |
| try { | |
| const directories = await getDirectory({ | |
| text, | |
| type, | |
| workspace: globalUsers ? 'all' : 'local', | |
| offset: newSearch ? 0 : data.length, | |
| count: 50, | |
| sort: type === 'users' ? { username: 1 } : { usersCount: -1 } | |
| }); | |
| if (requestId !== requestIdRef.current) { | |
| return; | |
| } | |
| if (directories.success) { | |
| setData(prev => [...(newSearch ? [] : prev), ...(directories.result as IServerRoom[])]); | |
| setTotal(directories.total); | |
| // Announce the full total on a fresh search; loadMore pages shouldn't re-announce | |
| if (newSearch) { | |
| announceSearchResultsForAccessibility(directories.total); | |
| } | |
| } | |
| } catch (e) { | |
| if (requestId !== requestIdRef.current) { | |
| return; | |
| } | |
| log(e); | |
| } finally { | |
| if (requestId === requestIdRef.current) { | |
| setLoading(false); | |
| } | |
| } | |
| }, 200); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/views/DirectoryView/hooks/useDirectorySearch.ts` around lines 20 - 54,
The load handler can have older async responses overwrite newer results; fix by
adding a request sequencing guard: create a stable ref/counter (e.g.,
latestRequestIdRef) and increment it each time load runs, capture the current id
in the async closure, then after the await from getDirectory(...) verify the
captured id === latestRequestIdRef.current before calling setData, setTotal,
announceSearchResultsForAccessibility or setLoading(false) for successful
branch; also check the id in the error/catch branch so stale failures don't
clear loading or log for newer requests. Ensure the guard is applied around all
state updates in load (including the newSearch branch logic) so only the most
recent getDirectory() result mutates state.
Take the released (develop) version of CLAUDE.md, NotificationIntentHandler.kt, and the expo-file-system patch — these were stale single-server drift, not product customizations, so the single-server release merge should adopt the released content. Leaves the diff vs develop as whitelabel + version only.
diegolmello
requested a deployment
to
approve_e2e_testing
GitHub Actions
Waiting
10 tasks
diegolmello
requested a deployment
to
android_build
GitHub Actions
Waiting
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
patches/expo-file-system+19.0.21.patch (1)
14-21:⚠️ Potential issue | 🟠 MajorHarden injected shared
clientvisibility to prevent stale/untuned OkHttp reads (Line 14-21).
patches/expo-file-system+19.0.21.patchmakesFileSystemLegacyModulestore a mutable process-widecompanion object var client: OkHttpClient? = null, andSSLPinningTurboModulewrites it viaFileSystemLegacyModule.setOkHttpClient(client);. Without@Volatile/synchronization, there’s no JVM happens-before edge, so other threads can observe stalenull/previous values—risking SSL pinning not being applied as intended.Suggested patch update
companion object { `@JvmStatic` - var client: OkHttpClient? = null + `@Volatile` + private var client: OkHttpClient? = null `@JvmStatic` fun setOkHttpClient(okHttpClient: OkHttpClient) { client = okHttpClient } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@patches/expo-file-system`+19.0.21.patch around lines 14 - 21, The companion object's mutable process-wide client field in FileSystemLegacyModule is not thread-safe; mark the var client: OkHttpClient? as `@Volatile` (or switch to an AtomicReference<OkHttpClient?>) and ensure setOkHttpClient writes the volatile/atomic reference so JVM happens-before guarantees apply; update references in FileSystemLegacyModule (companion object client and setOkHttpClient) and any writer call sites (e.g., SSLPinningTurboModule) to use the new volatile/atomic field so readers on other threads cannot observe stale/null values.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@patches/expo-file-system`+19.0.21.patch:
- Around line 14-21: The companion object's mutable process-wide client field in
FileSystemLegacyModule is not thread-safe; mark the var client: OkHttpClient? as
`@Volatile` (or switch to an AtomicReference<OkHttpClient?>) and ensure
setOkHttpClient writes the volatile/atomic reference so JVM happens-before
guarantees apply; update references in FileSystemLegacyModule (companion object
client and setOkHttpClient) and any writer call sites (e.g.,
SSLPinningTurboModule) to use the new volatile/atomic field so readers on other
threads cannot observe stale/null values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8c75b976-5525-4d60-85c2-46fd823756af
📒 Files selected for processing (3)
CLAUDE.mdandroid/app/src/main/java/chat/rocket/reactnative/notification/NotificationIntentHandler.ktpatches/expo-file-system+19.0.21.patch
💤 Files with no reviewable changes (1)
- CLAUDE.md
✅ Files skipped from review due to trivial changes (1)
- android/app/src/main/java/chat/rocket/reactnative/notification/NotificationIntentHandler.kt
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative
Timestamp: 2026-06-09T19:32:40.059Z
Learning: Run `pnpm prettier-lint` and `TZ=UTC pnpm test` for modified files before committing
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative
Timestamp: 2026-06-09T19:32:40.059Z
Learning: Pre-commit hooks enforce Prettier and test checks
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative
Timestamp: 2026-06-09T19:32:40.059Z
Learning: Use local-first approach: UI reads from WatermelonDB, sagas sync with server
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative
Timestamp: 2026-06-09T19:32:40.059Z
Learning: Do not conflate VoIP and VideoConf features — they are entirely separate systems
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative
Timestamp: 2026-06-09T19:32:40.059Z
Learning: Support iOS 13.4+ and Android 6.0+
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative
Timestamp: 2026-06-09T19:32:40.059Z
Learning: Use React 19.1, React Native 0.81, and Expo 54
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative
Timestamp: 2026-06-09T19:32:40.059Z
Learning: Use pnpm (version pinned in `package.json#packageManager`, activated via corepack) and Node >= 18
Learnt from: CR
Repo: RocketChat/Rocket.Chat.ReactNative
Timestamp: 2026-06-09T19:32:40.059Z
Learning: Read UBIQUITOUS_LANGUAGE.md for project terminology and domain language
📚 Learning: 2026-05-05T21:08:33.177Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7298
File: patches/@rocket.chat+sdk+1.3.3-mobile.patch:79-79
Timestamp: 2026-05-05T21:08:33.177Z
Learning: In the RocketChat/Rocket.Chat.ReactNative repo, for patches under patches/*.patch (especially those touching rocket.chat/sdk), remember that the patching sandbox's node_modules reflects the pre-patch state. Grepping node_modules for symbols (e.g., userDisconnectCloseCode = 4000 in node_modules/rocket.chat/sdk/lib/drivers/ddp.ts) can yield false positives. Review patches by inspecting the diff and applying it to a fresh copy of the SDK or diffing against the SDK source, rather than relying on node_modules. Ensure the patch actually introduces/updates symbols in the SDK source and run the test suite to validate behavior after applying the patch.
Applied to files:
patches/expo-file-system+19.0.21.patch
🔇 Additional comments (1)
patches/expo-file-system+19.0.21.patch (1)
26-26: LGTM!
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
5 participants
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.
Proposed changes
Merge the 4.73.1 patch release into
single-server. Companion to themastermerge (#7370); this brings the same release to the single-server product line. As a patch, 4.73.1 does not flow throughdevelop.Single-server's product customizations are preserved: hardcoded server in
app.json,NewServerViewdisabled (OutsideStack.tsx), the server switcher disabled (RoomsListView/components/Header.tsx), the forced single-server flows inapp/sagas/init.js/app/sagas/login.js, single-server Android config inandroid/gradle.properties, and no Firebase configs shipped.Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1216
How to test or reproduce
cat package.json | grep version->4.73.1grep versionName android/app/build.gradle->4.73.1grep CFBundleShortVersionString -A1 ios/RocketChatRN/Info.plist ios/ShareRocketChatRN/Info.plist->4.73.1grep MARKETING_VERSION ios/RocketChatRN.xcodeproj/project.pbxproj-> two4.73.1entries (RocketChatRN, NotificationService)grep APPLICATION_ID android/gradle.properties->chat.rocket.reactnativels android/app/google-services.json ios/GoogleService-Info.plist-> both absentScreenshots
N/A
Types of changes
Checklist
Further comments
N/A