fix(app): fix recovery takeover state cleanup (#16694)

Closes RQA-3488

When a different app, app B, cancels the active recovery session for app A, some state cleanup should occur for app A. The exact condition for triggering this state clean up was a bit off: currently we are cleaning up the state for app A when app B enters Error Recovery. Instead, we should clean up app A's state when app B cancels app A's recovery session.

app/src/organisms/ErrorRecoveryFlows/hooks/__tests__/useCleanupRecoveryState.test.ts

@@ -11,7 +11,7 @@ describe('useCleanupRecoveryState', () => {
   beforeEach(() => {
     mockSetRM = vi.fn()
     props = {
-      isTakeover: false,
+      isActiveUser: false,
       stashedMapRef: {
         current: {
           route: RECOVERY_MAP.MANUAL_FILL_AND_SKIP.ROUTE,
@@ -22,7 +22,7 @@ describe('useCleanupRecoveryState', () => {
     }
   })
 
-  it('does not modify state when isTakeover is false', () => {
+  it('does not modify state when user was never active', () => {
     renderHook(() => useCleanupRecoveryState(props))
 
     expect(props.stashedMapRef.current).toEqual({
@@ -32,10 +32,26 @@ describe('useCleanupRecoveryState', () => {
     expect(mockSetRM).not.toHaveBeenCalled()
   })
 
-  it('resets state when isTakeover is true', () => {
-    props.isTakeover = true
+  it('does not modify state when user becomes active', () => {
+    props.isActiveUser = true
+
     renderHook(() => useCleanupRecoveryState(props))
 
+    expect(props.stashedMapRef.current).toEqual({
+      route: RECOVERY_MAP.MANUAL_FILL_AND_SKIP.ROUTE,
+      step: RECOVERY_MAP.MANUAL_FILL_AND_SKIP.STEPS.SKIP,
+    })
+    expect(mockSetRM).not.toHaveBeenCalled()
+  })
+
+  it('resets state when user becomes inactive after being active', () => {
+    const { rerender } = renderHook(
+      ({ isActiveUser }) => useCleanupRecoveryState({ ...props, isActiveUser }),
+      { initialProps: { isActiveUser: true } }
+    )
+
+    rerender({ isActiveUser: false })
+
     expect(props.stashedMapRef.current).toBeNull()
     expect(mockSetRM).toHaveBeenCalledWith({
       route: RECOVERY_MAP.OPTION_SELECTION.ROUTE,
@@ -44,9 +60,13 @@ describe('useCleanupRecoveryState', () => {
   })
 
   it('handles case when stashedMapRef.current is already null', () => {
-    props.isTakeover = true
+    const { rerender } = renderHook(
+      ({ isActiveUser }) => useCleanupRecoveryState({ ...props, isActiveUser }),
+      { initialProps: { isActiveUser: true } }
+    )
+
     props.stashedMapRef.current = null
-    renderHook(() => useCleanupRecoveryState(props))
+    rerender({ isActiveUser: false })
 
     expect(props.stashedMapRef.current).toBeNull()
     expect(mockSetRM).toHaveBeenCalledWith({
@@ -55,24 +75,47 @@ describe('useCleanupRecoveryState', () => {
     })
   })
 
-  it('does not reset state when isTakeover changes from true to false', () => {
+  it('does not reset state on subsequent inactive states', () => {
     const { rerender } = renderHook(
-      ({ isTakeover }) => useCleanupRecoveryState({ ...props, isTakeover }),
-      { initialProps: { isTakeover: true } }
+      ({ isActiveUser }) => useCleanupRecoveryState({ ...props, isActiveUser }),
+      { initialProps: { isActiveUser: true } }
     )
 
+    rerender({ isActiveUser: false })
     mockSetRM.mockClear()
+
     props.stashedMapRef.current = {
       route: RECOVERY_MAP.MANUAL_FILL_AND_SKIP.ROUTE,
       step: RECOVERY_MAP.MANUAL_FILL_AND_SKIP.STEPS.SKIP,
     }
 
-    rerender({ isTakeover: false })
+    rerender({ isActiveUser: false })
 
     expect(props.stashedMapRef.current).toEqual({
       route: RECOVERY_MAP.MANUAL_FILL_AND_SKIP.ROUTE,
       step: RECOVERY_MAP.MANUAL_FILL_AND_SKIP.STEPS.SKIP,
     })
     expect(mockSetRM).not.toHaveBeenCalled()
   })
+
+  it('resets state only after a full active->inactive cycle', () => {
+    const { rerender } = renderHook(
+      ({ isActiveUser }) => useCleanupRecoveryState({ ...props, isActiveUser }),
+      { initialProps: { isActiveUser: false } }
+    )
+
+    rerender({ isActiveUser: true })
+    expect(mockSetRM).not.toHaveBeenCalled()
+    expect(props.stashedMapRef.current).toEqual({
+      route: RECOVERY_MAP.MANUAL_FILL_AND_SKIP.ROUTE,
+      step: RECOVERY_MAP.MANUAL_FILL_AND_SKIP.STEPS.SKIP,
+    })
+
+    rerender({ isActiveUser: false })
+    expect(props.stashedMapRef.current).toBeNull()
+    expect(mockSetRM).toHaveBeenCalledWith({
+      route: RECOVERY_MAP.OPTION_SELECTION.ROUTE,
+      step: RECOVERY_MAP.OPTION_SELECTION.STEPS.SELECT,
+    })
+  })
 })

app/src/organisms/ErrorRecoveryFlows/hooks/useCleanupRecoveryState.ts

@@ -1,4 +1,4 @@
-import { useEffect } from 'react'
+import { useState } from 'react'
 
 import { RECOVERY_MAP } from '../constants'
 
@@ -9,25 +9,28 @@ import type {
 } from '../hooks'
 
 export interface UseCleanupProps {
-  isTakeover: ERUtilsProps['showTakeover']
+  isActiveUser: ERUtilsProps['isActiveUser']
   stashedMapRef: UseRouteUpdateActionsResult['stashedMapRef']
   setRM: UseRecoveryRoutingResult['setRM']
 }
 
-// When certain events (ex, a takeover) occur, reset state that needs to be reset.
+// When certain events (ex, someone terminates this app's recovery session) occur, reset state that needs to be reset.
 export function useCleanupRecoveryState({
-  isTakeover,
+  isActiveUser,
   stashedMapRef,
   setRM,
 }: UseCleanupProps): void {
-  useEffect(() => {
-    if (isTakeover) {
-      stashedMapRef.current = null
+  const [wasActiveUser, setWasActiveUser] = useState(false)
 
-      setRM({
-        route: RECOVERY_MAP.OPTION_SELECTION.ROUTE,
-        step: RECOVERY_MAP.OPTION_SELECTION.STEPS.SELECT,
-      })
-    }
-  }, [isTakeover])
+  if (isActiveUser && !wasActiveUser) {
+    setWasActiveUser(true)
+  } else if (!isActiveUser && wasActiveUser) {
+    setWasActiveUser(false)
+
+    stashedMapRef.current = null
+    setRM({
+      route: RECOVERY_MAP.OPTION_SELECTION.ROUTE,
+      step: RECOVERY_MAP.OPTION_SELECTION.STEPS.SELECT,
+    })
+  }
 }

app/src/organisms/ErrorRecoveryFlows/hooks/useERUtils.ts

@@ -50,7 +50,7 @@ export type ERUtilsProps = Omit<ErrorRecoveryFlowsProps, 'failedCommand'> & {
   isOnDevice: boolean
   robotType: RobotType
   failedCommand: ReturnType<typeof useRetainedFailedCommandBySource>
-  showTakeover: boolean
+  isActiveUser: UseRecoveryTakeoverResult['isActiveUser']
   allRunDefs: LabwareDefinition2[]
   labwareDefinitionsByUri: LabwareDefinitionsByUri | null
 }
@@ -85,7 +85,7 @@ export function useERUtils({
   isOnDevice,
   robotType,
   runStatus,
-  showTakeover,
+  isActiveUser,
   allRunDefs,
   unvalidatedFailedCommand,
   labwareDefinitionsByUri,
@@ -193,7 +193,7 @@ export function useERUtils({
   )
 
   useCleanupRecoveryState({
-    isTakeover: showTakeover,
+    isActiveUser,
     setRM,
     stashedMapRef: routeUpdateActions.stashedMapRef,
   })

app/src/organisms/ErrorRecoveryFlows/index.tsx

@@ -160,7 +160,7 @@ export function ErrorRecoveryFlows(
     toggleERWizAsActiveUser,
     isOnDevice,
     robotType,
-    showTakeover,
+    isActiveUser,
     failedCommand: failedCommandBySource,
     allRunDefs,
     labwareDefinitionsByUri,