Merge branch 'main' into save-gradle-profile-report-from-adhoc-builds

Author: mhawryluk

.claude/agents/code-inline-reviewer.md

@@ -410,7 +410,7 @@ function UserProfile({ userId }) {
 
 ---
 
-### [PERFORMANCE-12] Prevent memory leaks in components and plugins
+### [PERF-12] Prevent memory leaks in components and plugins
 
 - **Search patterns**: `setInterval`, `setTimeout`, `addEventListener`, `subscribe`, `useEffect` with missing cleanup
 
@@ -770,7 +770,7 @@ useEffect(() => {
 
 ---
 
-### [CONSISTENCY-5] Ensure proper error handling
+### [CONSISTENCY-6] Ensure proper error handling
 
 - **Search patterns**: `try`, `catch`, `async`, `await`, `Promise`, `.then(`, `.catch(`
 
@@ -1189,6 +1189,261 @@ function SaveButton({ getSiblingFormData }: { getSiblingFormData: () => FormData
 
 ---
 
+### [CLEAN-REACT-PATTERNS-4] Avoid side-effect spaghetti
+
+- **Search patterns**: Multiple `useEffect` in single component, large component bodies mixing data access/navigation/UI state/lifecycle, hooks or utilities handling several unrelated responsibilities
+
+- **Condition**: Flag when a component, hook, or utility aggregates multiple unrelated responsibilities in a single unit, making it difficult to modify one concern without touching others.
+
+  **Signs of violation:**
+  - Component has several `useEffect` hooks handling unrelated concerns (e.g., telemetry, deep linking, audio, session management all in one component)
+  - A single `useEffect` or hook handles multiple distinct responsibilities
+  - Unrelated state variables are interdependent or updated together
+  - Logic mixes data fetching, navigation, UI state, and lifecycle behavior in one place
+  - Removing one piece of functionality requires careful untangling from others
+
+  **What counts as "unrelated":**
+  - Group by responsibility (what the code does), NOT by timing (when it runs)
+  - Data fetching and analytics are NOT related — they serve different purposes even if both run on mount
+  - Session management and audio configuration are NOT related — different domains entirely
+
+  **DO NOT flag if:**
+  - Component is a thin orchestration layer that ONLY composes child components (no business logic, no effects beyond rendering)
+  - Effects are extracted into focused custom hooks with single responsibilities (e.g., `useDebugShortcut`, `usePriorityMode`) — inline `useEffect` calls are a code smell and should be named hooks
+
+- **Reasoning**: When multiple unrelated responsibilities are grouped into a single component, hook, or utility, if any one concern changes, then unrelated logic must be touched as well, increasing coupling, regression risk, and cognitive load. This is the single responsibility principle for React: extract small units that do very little, very well. A component with several unrelated effects is a code smell - even a single effect can benefit from extraction to something with a good name, proper description, and isolated tests.
+
+  **Bucketing questions for refactoring:**
+  1. Does this logic need the React render loop? YES → Extract to a focused custom hook. NO → Extract out of React entirely (e.g., Onyx migration, global initialization).
+  2. Does this logic need to be in this component? YES → Keep it, but use a focused hook. NO → Extract to a separate component that owns this concern.
+
+  **Hook granularity guidance:**
+  - Group effects that serve the same purpose into one hook (e.g., all telemetry setup in `useTelemetry`)
+  - Group effects that can be reused together across components
+  - Don't create 15 separate single-effect hooks if 5 well-named grouped hooks make more sense
+
+Good (separated concerns):
+
+- Each piece of logic is extracted to a focused hook or component
+- Parent component only orchestrates what to render
+- State subscriptions in smaller components don't cause re-renders in parent
+- Component-scoped hooks can be co-located in the same directory for maintainability
+
+```tsx
+function DebugMenu() {
+    useDebugShortcut();
+
+    return (
+        // Debug menu UI
+    );
+}
+
+function ParentComponent({ reportID }: { reportID: string }) {
+    return (
+        <View>
+            {/* Each child owns its own concerns */}
+            <ReportView reportID={reportID} />
+            <DebugMenu />
+        </View>
+    );
+}
+```
+
+```tsx
+// Focused hook that does one thing well
+function useDebugShortcut() {
+    useEffect(() => {
+        const debugShortcutConfig = CONST.KEYBOARD_SHORTCUTS.DEBUG;
+        const unsubscribeDebugShortcut = KeyboardShortcut.subscribe(
+            debugShortcutConfig.shortcutKey,
+            () => toggleTestToolsModal(),
+            debugShortcutConfig.descriptionKey,
+            debugShortcutConfig.modifiers,
+            true,
+        );
+
+        return () => {
+            unsubscribeDebugShortcut();
+        };
+    }, []);
+}
+```
+
+Bad (side-effect spaghetti):
+
+- Component mixes session management, deep linking, telemetry, navigation, splash screen, audio, and other startup logic
+- Several unrelated `useOnyx` calls and `useEffect` hooks in a single component
+- Changing one concern risks breaking others
+
+```tsx
+function Expensify() {
+    // Session & auth
+    const [account] = useOnyx(ONYXKEYS.ACCOUNT);
+    const [session] = useOnyx(ONYXKEYS.SESSION);
+    
+    // Navigation & routing
+    const [lastRoute] = useOnyx(ONYXKEYS.LAST_ROUTE);
+    const [lastVisitedPath] = useOnyx(ONYXKEYS.LAST_VISITED_PATH);
+    const [isNavigationReady, setIsNavigationReady] = useState(false);
+    
+    // App state
+    const [updateAvailable] = useOnyx(ONYXKEYS.UPDATE_AVAILABLE);
+    const [updateRequired] = useOnyx(ONYXKEYS.UPDATE_REQUIRED);
+    const [isSidebarLoaded] = useOnyx(ONYXKEYS.IS_SIDEBAR_LOADED);
+    
+    // Splash screen
+    const {splashScreenState, setSplashScreenState} = useContext(SplashScreenStateContext);
+    
+    // ... 10+ more useOnyx calls for unrelated concerns ...
+
+    // Telemetry effect
+    useEffect(() => {
+        bootsplashSpan.current = startSpan(CONST.TELEMETRY.SPAN_BOOTSPLASH.ROOT, {...});
+        // ...
+    }, []);
+
+    // Public room checking effect
+    useEffect(() => {
+        if (isCheckingPublicRoom) return;
+        endSpan(CONST.TELEMETRY.SPAN_BOOTSPLASH.ONYX);
+        // ...
+    }, [isCheckingPublicRoom]);
+
+    // Splash screen effect
+    useEffect(() => {
+        if (!shouldHideSplash) return;
+        startSpan(CONST.TELEMETRY.SPAN_BOOTSPLASH.SPLASH_HIDER, {...});
+    }, [shouldHideSplash]);
+
+    // Deep linking effect
+    useEffect(() => {
+        Linking.getInitialURL().then((url) => {
+            if (url) {
+                openReportFromDeepLink(url, ...);
+            }
+        });
+        // ...
+    }, []);
+
+    // Audio mode effect
+    useEffect(() => {
+        Audio.setAudioModeAsync({playsInSilentModeIOS: true});
+    }, []);
+
+    // ... 10+ more useEffects mixing concerns ...
+}
+```
+
+In this example:
+- The component handles telemetry, deep linking, audio, session, navigation, splash screen, and more
+- Each concern is interleaved with others, making it hard to modify one without risking regression in another
+- Effects could be extracted to focused hooks: `useTelemetrySpans`, `useDeepLinking`, `useAudioMode`, etc.
+- Entry points don't get special treatment — extracting effects into named hooks improves clarity and makes it possible to understand what each effect does and how to safely modify it
+
+---
+
+### [CLEAN-REACT-PATTERNS-5] Keep state and subscriptions narrow
+
+- **Search patterns**: Contexts/hooks/stores exposing large bundled objects, providers with many unrelated `useOnyx` calls, state structures mixing unrelated concerns
+
+- **Condition**: Flag when a state structure (context, hook, store, or subscription) bundles unrelated concerns together, causing consumers to re-render when data they don't use changes.
+
+  **Signs of violation:**
+  - State provider (context, hook, or store) that bundles unrelated data (e.g., navigation state + list data + cache utilities in one structure)
+  - State object where properties serve different purposes and change independently
+  - Multiple unrelated subscriptions (`useOnyx`, `useContext`, store selectors) aggregated into a single exposed value
+  - Consumers of a state source that only use a subset of the provided values
+
+  **DO NOT flag if:**
+  - State values are cohesive — they change together and serve the same purpose (e.g., `keyboardHeight` + `isKeyboardShown` both relate to keyboard state)
+  - The state structure is intentionally designed as an aggregation point and consumers use most/all values
+  - Individual `useOnyx` calls without selectors — this is covered by [PERF-11]
+
+- **Reasoning**: When unrelated pieces of data are grouped into a single state structure, if an unused part changes, then all consumers re-render unnecessarily. This silently expands render scope, increases coupling, and makes performance regressions hard to detect. Structuring state around cohesive concerns ensures render scope stays predictable and changes remain local.
+
+**Distinction from PERF-11**: PERF-11 addresses individual `useOnyx` selector usage. This rule addresses state structure — how multiple values are grouped and exposed to consumers via contexts, hooks, or stores.
+
+**Distinction from CLEAN-REACT-PATTERNS-2**: PATTERNS-2 addresses data flow direction — parent shouldn't fetch data just to pass to children. This rule addresses how state is structured and grouped within any state provider.
+
+Good (cohesive state — all values serve one purpose):
+
+- All state relates to one concern (keyboard)
+- Values change together — no wasted re-renders
+- Derived state computed inline, not stored separately
+
+```tsx
+type KeyboardStateContextValue = {
+    isKeyboardShown: boolean;
+    isKeyboardActive: boolean;
+    keyboardHeight: number;
+};
+
+function KeyboardStateProvider({children}: ChildrenProps) {
+    const [keyboardHeight, setKeyboardHeight] = useState(0);
+    const [isKeyboardActive, setIsKeyboardActive] = useState(false);
+
+    useEffect(() => {
+        const showListener = KeyboardEvents.addListener('keyboardDidShow', (e) => {
+            setKeyboardHeight(e.height);
+            setIsKeyboardActive(true);
+        });
+        const hideListener = KeyboardEvents.addListener('keyboardDidHide', () => {
+            setKeyboardHeight(0);
+            setIsKeyboardActive(false);
+        });
+        return () => {
+            showListener.remove();
+            hideListener.remove();
+        };
+    }, []);
+
+    const contextValue = useMemo(() => ({
+        keyboardHeight,
+        isKeyboardShown: keyboardHeight !== 0,  // Derived, not separate state
+        isKeyboardActive,
+    }), [keyboardHeight, isKeyboardActive]);
+
+    return <KeyboardStateContext.Provider value={contextValue}>{children}</KeyboardStateContext.Provider>;
+}
+```
+
+Bad (grab-bag state — bundles unrelated concerns):
+
+- State provider subscribes to many unrelated Onyx collections
+- Exposed value mixes navigation state, list data, membership data, and cache utilities
+- Any consumer re-renders when ANY subscribed value changes
+
+```tsx
+function SidebarOrderedReportsContextProvider({children}) {
+    // ❌ Many unrelated Onyx subscriptions bundled together
+    const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE);
+    const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
+    const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
+    const [transactions] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION);
+    const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
+    const [reportNameValuePairs] = useOnyx(ONYXKEYS.COLLECTION.REPORT_NAME_VALUE_PAIRS);
+    const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT);
+    const [betas] = useOnyx(ONYXKEYS.BETAS);
+    const [reportAttributes] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES);
+
+    // ❌ Context value mixes unrelated concerns
+    const contextValue = {
+        orderedReports,         // List data
+        orderedReportIDs,       // List data
+        currentReportID,        // Navigation state
+        policyMemberAccountIDs, // Policy membership
+        clearLHNCache,          // Cache management utility
+    };
+
+    return <Context.Provider value={contextValue}>{children}</Context.Provider>;
+}
+
+// A component needing only currentReportID re-renders when orderedReports changes
+// A component needing only policyMemberAccountIDs re-renders when navigation changes
+```
+
+---
+
 ## Instructions
 
 1. **First, get the list of changed files and their diffs:**

.claude/agents/deploy-blocker-investigator.md

@@ -207,6 +207,8 @@ Technical explanation of what went wrong in the code.
 
 **DO NOT:**
 - Remove `DeployBlockerCash` if there's an App PR that caused or contributed to the issue
+- Remove `DeployBlockerCash` if the issue is not reproducible on production
+- Remove any of the blocker labels if it has been added by an internal employee
 - Remove both blocker labels simultaneously
 - Make assumptions about code you haven't read
 - Recommend DEMOTE for bugs affecting core functionality (auth, payments, data loss)

.github/workflows/testBuild.yml

@@ -301,7 +301,7 @@ jobs:
 
       - name: Rock Remote Build - Android
         id: rock-remote-build-android
-        uses: callstackincubator/android@0bbc1b7c2e1a8be1ecb4d6c744c211869823fd65
+        uses: callstackincubator/android@2177ac62cabe662aa49a2bc6f3154070705dd8f5
         env:
           GITHUB_TOKEN: ${{ github.token }}
           SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
@@ -320,6 +320,7 @@ jobs:
           keystore-path: '../tools/buildtools/upload-key.keystore'
           comment-bot: false
           rock-build-extra-params: '--extra-params "-PreactNativeArchitectures=arm64-v8a,x86_64 --profile"'
+          custom-ref: ${{ needs.prep.outputs.APP_REF }}
 
       - name: Upload Gradle profile report
         if: always()
@@ -422,7 +423,7 @@ jobs:
 
       - name: Rock Remote Build - iOS
         id: rock-remote-build-ios
-        uses: callstackincubator/ios@8dcef6cc275e0cf3299f5a97cde5ebd635c887d7
+        uses: callstackincubator/ios@d638bd25c764655baeeced3232c692f1698cf72b
         env:
           GITHUB_TOKEN: ${{ github.token }}
           SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }}
@@ -450,6 +451,7 @@ jobs:
               }
             ]
           comment-bot: false
+          custom-ref: ${{ needs.prep.outputs.APP_REF }}
 
       - name: Set artifact URL output
         id: set-artifact-url

.github/workflows/testBuildOnPush.yml

@@ -5,6 +5,10 @@ on:
     branches: [main]
     paths-ignore: ['docs/**', 'contributingGuides/**', 'help/**', '.github/**', 'scripts/**', 'tests/**', 'jest/**', '.claude/**']
 
+concurrency:
+  group: 'testBuildOnPush'
+  cancel-in-progress: false
+
 jobs:
   prep:
     runs-on: ubuntu-latest
@@ -432,7 +436,6 @@ jobs:
               summary.addRaw(`\n**Mobile-Expensify Submodule SHA:** [${submoduleSHA}](${submoduleUrl})`);
             }
             
-
             const prUrl = '${{ needs.prep.outputs.PR_URL }}';
 
             if (prUrl) {
@@ -441,5 +444,4 @@ jobs:
               summary.addRaw(`\n**PR Link:** No PR associated with this commit.`);
             }
             
-            
             summary.write();

Mobile-Expensify

@@ -114,8 +114,8 @@ android {
         minSdkVersion rootProject.ext.minSdkVersion
         targetSdkVersion rootProject.ext.targetSdkVersion
         multiDexEnabled rootProject.ext.multiDexEnabled
-        versionCode 1009031153
-        versionName "9.3.11-53"
+        versionCode 1009031502
+        versionName "9.3.15-2"
         // Supported language variants must be declared here to avoid from being removed during the compilation.
         // This also helps us to not include unnecessary language variants in the APK.
         resConfigs "en", "es"