fix: resolve CI failures in multi-provider

- Add _is_internal_hook_provider class marker to avoid Mock false positives
  with runtime_checkable Protocol isinstance checks
- Fix mypy no-redef errors by hoisting evaluations declaration before branch
- Fix mypy no-any-return by assigning to typed local before returning
- Fix mypy attr-defined by using _as_internal_hook_provider narrowing helper
- Apply ruff formatting fixes

Signed-off-by: Jonathan Norris <jonathan.norris@dynatrace.com>

Author: jonathannorris

openfeature/client.py

@@ -470,35 +470,44 @@ def _establish_hooks_and_provider(
             merged_eval_context,
         )
 
+    def _as_internal_hook_provider(
+        self, provider: FeatureProvider
+    ) -> InternalHookProvider | None:
+        """Return the provider as InternalHookProvider if it opts in, else None."""
+        if getattr(provider, "_is_internal_hook_provider", False) and isinstance(
+            provider, InternalHookProvider
+        ):
+            return provider
+        return None
+
     def _provider_uses_internal_hooks(self, provider: FeatureProvider) -> bool:
-        return (
-            isinstance(provider, InternalHookProvider)
-            and provider.uses_internal_provider_hooks()
-        )
+        ihp = self._as_internal_hook_provider(provider)
+        return ihp is not None and ihp.uses_internal_provider_hooks()
 
     def _set_internal_provider_hook_runtime(
         self,
         provider: FeatureProvider,
         flag_type: FlagType,
         hook_hints: HookHints,
     ) -> object | None:
-        if not isinstance(provider, InternalHookProvider):
-            return None
-        if not provider.uses_internal_provider_hooks():
+        ihp = self._as_internal_hook_provider(provider)
+        if ihp is None or not ihp.uses_internal_provider_hooks():
             return None
-        return provider.set_internal_provider_hook_runtime(
+        result: object | None = ihp.set_internal_provider_hook_runtime(
             flag_type=flag_type,
             client_metadata=self.get_metadata(),
             hook_hints=hook_hints,
         )
+        return result
 
     def _reset_internal_provider_hook_runtime(
         self, provider: FeatureProvider, runtime_token: object | None
     ) -> None:
         if runtime_token is None:
             return
-        if isinstance(provider, InternalHookProvider):
-            provider.reset_internal_provider_hook_runtime(runtime_token)
+        ihp = self._as_internal_hook_provider(provider)
+        if ihp is not None:
+            ihp.reset_internal_provider_hook_runtime(runtime_token)
 
     def _assert_provider_status(
         self,

openfeature/provider/__init__.py

@@ -139,8 +139,14 @@ class InternalHookProvider(typing.Protocol):
 
     The registry will also use get_status() from this protocol instead of its
     own internal status tracking for providers that implement it.
+
+    Implementations must set ``_is_internal_hook_provider = True`` as a class
+    attribute. This marker is checked alongside ``isinstance`` to avoid false
+    positives from duck-typed objects (e.g. ``Mock``).
     """
 
+    _is_internal_hook_provider: typing.ClassVar[bool]
+
     def uses_internal_provider_hooks(self) -> bool: ...
 
     def set_internal_provider_hook_runtime(

openfeature/provider/_registry.py

@@ -124,7 +124,12 @@ def _shutdown_provider(self, provider: FeatureProvider) -> None:
     def get_provider_status(self, provider: FeatureProvider) -> ProviderStatus:
         # Only InternalHookProvider implementations (e.g. MultiProvider) manage
         # their own status. For all other providers, use the registry's tracking.
-        if isinstance(provider, InternalHookProvider):
+        # We check _is_internal_hook_provider (a concrete class attribute) in
+        # addition to isinstance, because runtime_checkable Protocols match any
+        # object that has the right method names — including Mock objects.
+        if getattr(provider, "_is_internal_hook_provider", False) and isinstance(
+            provider, InternalHookProvider
+        ):
             return provider.get_status()
         return self._provider_status.get(provider, ProviderStatus.NOT_READY)
 

openfeature/provider/multi_provider.py

@@ -109,7 +109,9 @@ def _validate_run_mode(run_mode: RunMode) -> RunMode:
 def _format_result_error(
     provider_name: str, result: FlagResolutionDetails[FlagValueType]
 ) -> str:
-    error_code = result.error_code.value if result.error_code else ErrorCode.GENERAL.value
+    error_code = (
+        result.error_code.value if result.error_code else ErrorCode.GENERAL.value
+    )
     error_message = result.error_message or "Unknown error"
     return f"{provider_name}: {error_code} ({error_message})"
 
@@ -280,7 +282,9 @@ def determine_final_result(
         evaluations: list[_ProviderEvaluation[FlagValueType]],
     ) -> FlagResolutionDetails[FlagValueType]:
         failed_evaluations = [
-            evaluation for evaluation in evaluations if not _is_success(evaluation.result)
+            evaluation
+            for evaluation in evaluations
+            if not _is_success(evaluation.result)
         ]
         if failed_evaluations:
             return _build_aggregated_error(
@@ -338,6 +342,8 @@ class MultiProvider(AbstractProvider):
         ProviderStatus.READY,
     )
 
+    _is_internal_hook_provider: typing.ClassVar[bool] = True
+
     def __init__(
         self,
         providers: list[ProviderEntry],
@@ -365,7 +371,9 @@ def __init__(
             provider_name: ProviderStatus.NOT_READY
             for provider_name, _ in self._registered_providers
         }
-        validate_provider_names = getattr(self.strategy, "validate_provider_names", None)
+        validate_provider_names = getattr(
+            self.strategy, "validate_provider_names", None
+        )
         if callable(validate_provider_names):
             validate_provider_names(
                 [provider_name for provider_name, _ in self._registered_providers]
@@ -455,8 +463,12 @@ def initialize_provider(
             except Exception as err:
                 return provider_name, err
 
-        with ThreadPoolExecutor(max_workers=len(self._registered_providers)) as executor:
-            init_results = list(executor.map(initialize_provider, self._registered_providers))
+        with ThreadPoolExecutor(
+            max_workers=len(self._registered_providers)
+        ) as executor:
+            init_results = list(
+                executor.map(initialize_provider, self._registered_providers)
+            )
 
         error_messages: list[str] = []
         event_details = ProviderEventDetails()
@@ -475,7 +487,9 @@ def initialize_provider(
         self._refresh_aggregate_status(event_details, force=True)
 
         if error_messages:
-            raise GeneralError(f"Multi-provider initialization failed: {'; '.join(error_messages)}")
+            raise GeneralError(
+                f"Multi-provider initialization failed: {'; '.join(error_messages)}"
+            )
 
     def shutdown(self) -> None:
         for _, provider in self._registered_providers:
@@ -488,7 +502,9 @@ def shutdown_provider(entry: tuple[str, FeatureProvider]) -> None:
             except Exception:
                 logger.exception("Provider '%s' shutdown failed", provider_name)
 
-        with ThreadPoolExecutor(max_workers=len(self._registered_providers)) as executor:
+        with ThreadPoolExecutor(
+            max_workers=len(self._registered_providers)
+        ) as executor:
             list(executor.map(shutdown_provider, self._registered_providers))
 
         with self._status_lock:
@@ -522,7 +538,9 @@ def _handle_provider_event(
                 provider_name,
                 self._status_from_event_details(details),
             )
-        self._refresh_aggregate_status(self._with_provider_metadata(details, provider_name))
+        self._refresh_aggregate_status(
+            self._with_provider_metadata(details, provider_name)
+        )
 
     def _set_provider_status(
         self, provider_name: str, provider_status: ProviderStatus
@@ -547,7 +565,9 @@ def _should_evaluate_provider(self, provider_name: str) -> bool:
         if not self._initialized:
             return True
         with self._status_lock:
-            status = self._provider_statuses.get(provider_name, ProviderStatus.NOT_READY)
+            status = self._provider_statuses.get(
+                provider_name, ProviderStatus.NOT_READY
+            )
         return status not in (ProviderStatus.NOT_READY, ProviderStatus.FATAL)
 
     def _calculate_aggregate_status(self) -> ProviderStatus:
@@ -577,7 +597,9 @@ def _refresh_aggregate_status(
         if event_to_emit is not None:
             self.emit(event_to_emit, event_details)
 
-    def _event_from_status(self, provider_status: ProviderStatus) -> ProviderEvent | None:
+    def _event_from_status(
+        self, provider_status: ProviderStatus
+    ) -> ProviderEvent | None:
         if provider_status == ProviderStatus.READY:
             return ProviderEvent.PROVIDER_READY
         if provider_status == ProviderStatus.STALE:
@@ -632,9 +654,7 @@ def _details_from_exception(
         self, err: Exception, provider_name: str
     ) -> ProviderEventDetails:
         error_code = (
-            err.error_code
-            if isinstance(err, OpenFeatureError)
-            else ErrorCode.GENERAL
+            err.error_code if isinstance(err, OpenFeatureError) else ErrorCode.GENERAL
         )
         error_message = self._error_message_from_exception(err)
         return ProviderEventDetails(
@@ -652,9 +672,7 @@ def _resolution_from_exception(
         self, default_value: T, err: Exception
     ) -> FlagResolutionDetails[T]:
         error_code = (
-            err.error_code
-            if isinstance(err, OpenFeatureError)
-            else ErrorCode.GENERAL
+            err.error_code if isinstance(err, OpenFeatureError) else ErrorCode.GENERAL
         )
         error_message = self._error_message_from_exception(err)
         return FlagResolutionDetails(
@@ -709,7 +727,9 @@ def _evaluate_provider_sync(  # noqa: PLR0913
                 return _ProviderEvaluation(
                     provider_name=provider_name,
                     provider=provider,
-                    result=resolve_fn(provider, flag_key, default_value, evaluation_context),
+                    result=resolve_fn(
+                        provider, flag_key, default_value, evaluation_context
+                    ),
                 )
             except Exception as err:
                 return _ProviderEvaluation(
@@ -821,7 +841,9 @@ async def _evaluate_provider_async(  # noqa: PLR0913
         try:
             before_context = before_hooks(flag_type, hook_contexts, runtime.hook_hints)
             resolved_context = provider_context.merge(before_context)
-            resolution = await resolve_fn(provider, flag_key, default_value, resolved_context)
+            resolution = await resolve_fn(
+                provider, flag_key, default_value, resolved_context
+            )
             flag_evaluation = resolution.to_flag_evaluation_details(flag_key)
             if err := flag_evaluation.get_exception():
                 error_hooks(
@@ -883,12 +905,16 @@ def _evaluate_with_providers(
             if self._should_evaluate_provider(name)
         ]
 
+        evaluations: list[_ProviderEvaluation[T]] = []
+
         if self.strategy.run_mode == "parallel":
             # Each worker thread gets its own copy of the current context so
             # that ContextVars (e.g. _hook_runtime) are propagated correctly.
             # ThreadPoolExecutor does not automatically copy context on
             # Python < 3.12, and a single Context.run() is not reentrant.
-            with ThreadPoolExecutor(max_workers=len(eligible_providers) or 1) as executor:
+            with ThreadPoolExecutor(
+                max_workers=len(eligible_providers) or 1
+            ) as executor:
                 futures = [
                     executor.submit(
                         contextvars.copy_context().run,
@@ -916,7 +942,6 @@ def _evaluate_with_providers(
                 ),
             )
 
-        evaluations: list[_ProviderEvaluation[T]] = []
         for provider_name, provider in eligible_providers:
             evaluation = self._evaluate_provider_sync(
                 provider_name,
@@ -970,6 +995,8 @@ async def _evaluate_with_providers_async(
             if self._should_evaluate_provider(name)
         ]
 
+        evaluations: list[_ProviderEvaluation[T]] = []
+
         if self.strategy.run_mode == "parallel":
             tasks = [
                 asyncio.create_task(
@@ -985,20 +1012,19 @@ async def _evaluate_with_providers_async(
                 )
                 for provider_name, provider in eligible_providers
             ]
-            evaluations = await asyncio.gather(*tasks)
+            evaluations = list(await asyncio.gather(*tasks))
             return typing.cast(
                 FlagResolutionDetails[T],
                 self.strategy.determine_final_result(
                     flag_key,
                     default_value,
                     typing.cast(
                         list[_ProviderEvaluation[FlagValueType]],
-                        list(evaluations),
+                        evaluations,
                     ),
                 ),
             )
 
-        evaluations: list[_ProviderEvaluation[T]] = []
         for provider_name, provider in eligible_providers:
             evaluation = await self._evaluate_provider_async(
                 provider_name,

tests/test_multi_provider.py

@@ -60,7 +60,9 @@ def resolve_boolean_details(
     ) -> FlagResolutionDetails[bool]:
         del flag_key
         self.resolveCount += 1
-        self.seenContexts.append(dict((evaluation_context or EvaluationContext()).attributes))
+        self.seenContexts.append(
+            dict((evaluation_context or EvaluationContext()).attributes)
+        )
         if self.sync_blocker is not None:
             self.sync_blocker.wait()
         if self.booleanException is not None:
@@ -77,7 +79,9 @@ async def resolve_boolean_details_async(
     ) -> FlagResolutionDetails[bool]:
         del flag_key
         self.resolveCount += 1
-        self.seenContexts.append(dict((evaluation_context or EvaluationContext()).attributes))
+        self.seenContexts.append(
+            dict((evaluation_context or EvaluationContext()).attributes)
+        )
         if self.async_blocker is not None:
             await self.async_blocker.wait()
         if self.booleanException is not None:
@@ -225,7 +229,9 @@ def test_comparison_strategy_rejects_unknown_fallback_provider():
     first_provider = BooleanProvider("first")
     second_provider = BooleanProvider("second")
 
-    with pytest.raises(ValueError, match="Fallback provider 'missing' is not registered"):
+    with pytest.raises(
+        ValueError, match="Fallback provider 'missing' is not registered"
+    ):
         MultiProvider(
             [
                 ProviderEntry(first_provider, name="first"),
@@ -311,7 +317,9 @@ def test_first_successful_skips_general_errors():
 
 def test_first_successful_aggregates_errors_when_all_providers_fail():
     first_provider = BooleanProvider("first", boolean_exception=GeneralError("first"))
-    second_provider = BooleanProvider("second", boolean_exception=GeneralError("second"))
+    second_provider = BooleanProvider(
+        "second", boolean_exception=GeneralError("second")
+    )
     multi_provider = MultiProvider(
         [
             ProviderEntry(first_provider, name="first"),
@@ -583,8 +591,12 @@ def test_multi_provider_forwards_configuration_changed_events():
         spy.provider_configuration_changed,
     )
 
-    first_provider.emit_provider_configuration_changed(ProviderEventDetails(message="one"))
-    second_provider.emit_provider_configuration_changed(ProviderEventDetails(message="two"))
+    first_provider.emit_provider_configuration_changed(
+        ProviderEventDetails(message="one")
+    )
+    second_provider.emit_provider_configuration_changed(
+        ProviderEventDetails(message="two")
+    )
 
     assert spy.provider_configuration_changed.call_count == 2