feat: close spec compliance gaps for OpenFeature v0.8.0 (#237)

Signed-off-by: Jose Colella <jose.colella@gusto.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: josecolella

lib/open_feature/sdk/api.rb

@@ -34,7 +34,7 @@ class API
       include Singleton # Satisfies Flag Evaluation API Requirement 1.1.1
       extend Forwardable
 
-      def_delegators :configuration, :provider, :set_provider, :set_provider_and_wait, :hooks, :evaluation_context
+      def_delegators :configuration, :provider, :set_provider, :set_provider_and_wait, :hooks, :add_hooks, :evaluation_context
 
       def configuration
         @configuration ||= Configuration.new

lib/open_feature/sdk/client.rb

@@ -28,6 +28,10 @@ def initialize(provider:, domain: nil, evaluation_context: nil)
         @hooks = []
       end
 
+      def add_hooks(*new_hooks)
+        @hooks.concat(new_hooks.flatten)
+      end
+
       def provider_status
         OpenFeature::SDK.configuration.provider_state(@provider)
       end
@@ -70,26 +74,29 @@ def fetch_#{result_type}_#{suffix}(flag_key:, default_value:, evaluation_context
       def fetch_details(type:, flag_key:, default_value:, evaluation_context: nil, invocation_hooks: [], hook_hints: nil)
         validate_default_value_type(type, default_value)
 
-        if OpenFeature::SDK.configuration.provider_tracked?(@provider)
-          error_code = short_circuit_error_code(provider_status)
-          if error_code
-            resolution = Provider::ResolutionDetails.new(
-              value: default_value,
-              error_code: error_code,
-              reason: Provider::Reason::ERROR
-            )
-            return EvaluationDetails.new(flag_key: flag_key, resolution_details: resolution)
-          end
-        end
-
         built_context = build_evaluation_context(evaluation_context)
 
         # Assemble ordered hooks: API → Client → Invocation → Provider (spec 4.4.2)
         provider_hooks = @provider.respond_to?(:hooks) ? Array(@provider.hooks) : []
         ordered_hooks = [*OpenFeature::SDK.hooks, *@hooks, *invocation_hooks, *provider_hooks]
 
+        # Check for short-circuit conditions (spec 1.7.6 + 1.7.7)
+        short_circuit_code = nil
+        if OpenFeature::SDK.configuration.provider_tracked?(@provider)
+          short_circuit_code = short_circuit_error_code(provider_status)
+        end
+
         # Fast path: skip hook ceremony when no hooks are registered
         if ordered_hooks.empty?
+          if short_circuit_code
+            resolution = Provider::ResolutionDetails.new(
+              value: default_value,
+              error_code: short_circuit_code,
+              reason: Provider::Reason::ERROR
+            )
+            return EvaluationDetails.new(flag_key: flag_key, resolution_details: resolution)
+          end
+
           return evaluate_flag(type: type, flag_key: flag_key, default_value: default_value, evaluation_context: built_context)
         end
 
@@ -111,8 +118,21 @@ def fetch_details(type:, flag_key:, default_value:, evaluation_context: nil, inv
         end
 
         executor = Hooks::HookExecutor.new(logger: OpenFeature::SDK.configuration.logger)
-        executor.execute(ordered_hooks: ordered_hooks, hook_context: hook_context, hints: hints) do |hctx|
-          evaluate_flag(type: type, flag_key: flag_key, default_value: default_value, evaluation_context: hctx.evaluation_context)
+
+        if short_circuit_code
+          # Spec 1.7.6 + 1.7.7: short-circuit must still run error hooks and finally hooks
+          resolution = Provider::ResolutionDetails.new(
+            value: default_value,
+            error_code: short_circuit_code,
+            reason: Provider::Reason::ERROR
+          )
+          evaluation_details = EvaluationDetails.new(flag_key: flag_key, resolution_details: resolution)
+          executor.run_short_circuit(ordered_hooks: ordered_hooks, hook_context: hook_context, hints: hints, evaluation_details: evaluation_details)
+          evaluation_details
+        else
+          executor.execute(ordered_hooks: ordered_hooks, hook_context: hook_context, hints: hints) do |hctx|
+            evaluate_flag(type: type, flag_key: flag_key, default_value: default_value, evaluation_context: hctx.evaluation_context)
+          end
         end
       end
 

lib/open_feature/sdk/client_metadata.rb

@@ -2,6 +2,8 @@
 
 module OpenFeature
   module SDK
-    ClientMetadata = Struct.new(:domain, keyword_init: true)
+    ClientMetadata = Struct.new(:domain, keyword_init: true) do
+      alias_method :name, :domain
+    end
   end
 end

lib/open_feature/sdk/configuration.rb

@@ -31,6 +31,10 @@ def initialize
         @client_handlers_mutex = Mutex.new
       end
 
+      def add_hooks(*new_hooks)
+        @hooks.concat(new_hooks.flatten)
+      end
+
       def provider(domain: nil)
         @providers[domain] || @providers[nil]
       end
@@ -85,9 +89,12 @@ def provider_tracked?(provider)
       end
 
       def shutdown
-        providers_to_shutdown = @provider_mutex.synchronize { @providers.values.uniq }
+        providers_to_shutdown = @provider_mutex.synchronize { @providers.values.uniq(&:object_id) }
 
         providers_to_shutdown.each do |prov|
+          # Spec 1.7.9: Set provider state to NOT_READY before shutdown
+          @provider_state_registry.update_state_from_event(prov, ProviderEvent::PROVIDER_READY)
+          @provider_state_registry.set_initial_state(prov, ProviderState::NOT_READY)
           prov.shutdown if prov.respond_to?(:shutdown)
         rescue => e
           @logger&.warn("Error shutting down provider #{prov&.class&.name || "unknown"}: #{e.message}")
@@ -113,28 +120,37 @@ def set_provider_internal(provider, domain:, wait_for_init:)
         # Capture evaluation context before acquiring mutex to prevent race conditions
         context_for_init = @evaluation_context
 
-        old_provider, provider_to_init = nil
+        old_provider = nil
+        needs_init = false
+        needs_shutdown = false
 
         @provider_mutex.synchronize do
           old_provider = @providers[domain]
 
-          # Remove old provider state to prevent memory leaks
-          @provider_state_registry.remove_provider(old_provider)
-
           new_providers = @providers.dup
           new_providers[domain] = provider
           @providers = new_providers
 
-          @provider_state_registry.set_initial_state(provider)
+          # Spec 1.1.2.2: Only initialize if the provider is not already active
+          # (i.e., not already bound to another domain)
+          already_active = @providers.any? { |d, p| d != domain && p.equal?(provider) && @provider_state_registry.tracked?(p) }
+          needs_init = !already_active
 
-          provider.send(:attach, self) if provider.is_a?(Provider::EventEmitter)
+          if needs_init
+            @provider_state_registry.set_initial_state(provider)
+            provider.send(:attach, self) if provider.is_a?(Provider::EventEmitter)
+          end
 
-          provider_to_init = provider
+          # Spec 1.1.2.3: Only shutdown old provider if it's no longer bound to any domain
+          if old_provider && !old_provider.equal?(provider)
+            still_bound = @providers.any? { |_, p| p.equal?(old_provider) }
+            needs_shutdown = !still_bound
+            @provider_state_registry.remove_provider(old_provider) unless still_bound
+          end
         end
 
         # Shutdown old provider outside mutex to avoid blocking other operations
-        # Only shutdown if it's a different provider to prevent race condition
-        if old_provider && old_provider != provider
+        if needs_shutdown
           begin
             old_provider.shutdown if old_provider.respond_to?(:shutdown)
           rescue => e
@@ -143,12 +159,17 @@ def set_provider_internal(provider, domain:, wait_for_init:)
         end
 
         # Initialize provider outside the mutex to avoid blocking other operations
-        if wait_for_init
-          init_provider(provider_to_init, context_for_init, raise_on_error: true)
-        else
-          Thread.new do
-            init_provider(provider_to_init, context_for_init, raise_on_error: false)
+        if needs_init
+          if wait_for_init
+            init_provider(provider, context_for_init, raise_on_error: true)
+          else
+            Thread.new do
+              init_provider(provider, context_for_init, raise_on_error: false)
+            end
           end
+        elsif wait_for_init
+          # Provider already active; no init needed but still dispatch READY
+          dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY)
         end
       end
 
@@ -164,16 +185,22 @@ def init_provider(provider, context, raise_on_error: false)
 
         dispatch_provider_event(provider, ProviderEvent::PROVIDER_READY)
       rescue => e
+        # Spec 1.7.8: Propagate error code from provider if available
+        error_code = if e.respond_to?(:error_code) && e.error_code
+          e.error_code
+        else
+          Provider::ErrorCode::GENERAL
+        end
+
         dispatch_provider_event(provider, ProviderEvent::PROVIDER_ERROR,
-          error_code: Provider::ErrorCode::GENERAL,
+          error_code: error_code,
           message: e.message)
 
         if raise_on_error
-          # Re-raise as ProviderInitializationError for synchronous callers
           raise ProviderInitializationError.new(
             "Provider #{provider.class.name} initialization failed: #{e.message}",
             provider:,
-            error_code: Provider::ErrorCode::GENERAL,
+            error_code: error_code,
             original_error: e
           )
         end

lib/open_feature/sdk/hooks/hook_executor.rb

@@ -18,6 +18,20 @@ def initialize(logger: nil)
           @logger = logger
         end
 
+        # Runs error hooks and finally hooks for a short-circuit evaluation
+        # (spec 1.7.6 + 1.7.7). Before hooks and after hooks are NOT run.
+        #
+        # @param ordered_hooks [Array] hooks in before-order
+        # @param hook_context [HookContext] the hook context
+        # @param hints [Hints] hook hints
+        # @param evaluation_details [EvaluationDetails] the short-circuit result
+        def run_short_circuit(ordered_hooks:, hook_context:, hints:, evaluation_details:)
+          error = StandardError.new(evaluation_details.error_code)
+          run_error_hooks(ordered_hooks, hook_context, error, hints)
+        ensure
+          run_finally_hooks(ordered_hooks, hook_context, evaluation_details, hints)
+        end
+
         # Executes the full hook lifecycle around the flag evaluation block.
         #
         # @param ordered_hooks [Array] hooks in before-order (API, Client, Invocation, Provider)

lib/open_feature/sdk/provider/in_memory_provider.rb

@@ -29,8 +29,10 @@ def add_flag(flag_key:, value:)
         end
 
         def update_flags(new_flags)
+          old_keys = @flags.keys
           @flags = new_flags.dup
-          emit_provider_changed(new_flags.keys)
+          changed_keys = (old_keys | new_flags.keys)
+          emit_provider_changed(changed_keys)
         end
 
         def fetch_boolean_value(flag_key:, default_value:, evaluation_context: nil)

spec/open_feature/sdk/provider/in_memory_provider_spec.rb

@@ -72,7 +72,7 @@
     end
 
     context "when attached to configuration" do
-      it "emits PROVIDER_CONFIGURATION_CHANGED event with all flag keys" do
+      it "emits PROVIDER_CONFIGURATION_CHANGED event with union of old and new flag keys" do
         config = instance_double("OpenFeature::SDK::Configuration")
         allow(config).to receive(:dispatch_provider_event)
         provider.send(:attach, config)
@@ -82,7 +82,7 @@
         expect(config).to have_received(:dispatch_provider_event).with(
           provider,
           OpenFeature::SDK::ProviderEvent::PROVIDER_CONFIGURATION_CHANGED,
-          flags_changed: contain_exactly("flag_a", "flag_b")
+          flags_changed: contain_exactly("bool", "str", "int", "float", "struct", "flag_a", "flag_b")
         )
       end
     end