From 9eb0aaf5a33cba6a7848d80ee927a598d86d10c2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 05:55:11 +0000 Subject: [PATCH 1/7] Initial plan for issue From 1930f8ad9c4ab5c07818c1021a91398c30f383ae Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 05:57:53 +0000 Subject: [PATCH 2/7] Initial investigation complete - analyzing configuration loading issue Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- .bundle/config | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.bundle/config b/.bundle/config index 0146a1ce..f9263841 100644 --- a/.bundle/config +++ b/.bundle/config @@ -1,7 +1,8 @@ --- BUNDLE_BIN: "bin" -BUNDLE_PATH: "vendor/gems" +BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle" BUNDLE_CACHE_PATH: "vendor/cache" BUNDLE_CACHE_ALL: "true" BUNDLE_SPECIFIC_PLATFORM: "true" BUNDLE_NO_INSTALL: "true" +BUNDLE_DEPLOYMENT: "true" From bb60640784d0e4c7a7227c8017cea39fe201f2d3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 06:08:41 +0000 Subject: [PATCH 3/7] Fix configuration loading precedence and add missing environment variables Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- lib/hooks/core/config_loader.rb | 47 ++++++++++++++++++++++++++------- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index 68a47e38..3e2a7fd0 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -30,17 +30,29 @@ class ConfigLoader # @return [Hash] Merged configuration def self.load(config_path: nil) config = DEFAULT_CONFIG.dup + overrides = [] # Load from file if path provided if config_path.is_a?(String) && File.exist?(config_path) file_config = load_config_file(config_path) - config.merge!(file_config) if file_config - elsif config_path.is_a?(Hash) - config.merge!(config_path) + if file_config + overrides << "file config" + config.merge!(file_config) + end + end + + # Override with environment variables (before programmatic config) + env_config = load_env_config + if env_config.any? + overrides << "environment variables" + config.merge!(env_config) end - # Override with environment variables - config.merge!(load_env_config) + # Programmatic config has highest priority + if config_path.is_a?(Hash) + overrides << "programmatic config" + config.merge!(config_path) + end # Convert string keys to symbols for consistency config = symbolize_keys(config) @@ -51,6 +63,13 @@ def self.load(config_path: nil) config[:production] = false end + # Log overrides if any were made + if overrides.any? + # Use puts for now since logger might not be initialized yet + # This will be replaced with proper logging once the logger is available + puts "INFO: Configuration overrides applied from: #{overrides.join(', ')}" + end + return config end @@ -93,8 +112,9 @@ def self.load_config_file(file_path) end result - rescue => _e - # In production, we'd log this error + rescue => e + # Log this error with meaningful information + puts "ERROR: Failed to load config file '#{file_path}': #{e.message}" nil end @@ -105,8 +125,11 @@ def self.load_env_config env_config = {} env_mappings = { + "HOOKS_HANDLER_DIR" => :handler_dir, "HOOKS_HANDLER_PLUGIN_DIR" => :handler_plugin_dir, "HOOKS_AUTH_PLUGIN_DIR" => :auth_plugin_dir, + "HOOKS_LIFECYCLE_PLUGIN_DIR" => :lifecycle_plugin_dir, + "HOOKS_INSTRUMENTS_PLUGIN_DIR" => :instruments_plugin_dir, "HOOKS_LOG_LEVEL" => :log_level, "HOOKS_REQUEST_LIMIT" => :request_limit, "HOOKS_REQUEST_TIMEOUT" => :request_timeout, @@ -114,17 +137,23 @@ def self.load_env_config "HOOKS_HEALTH_PATH" => :health_path, "HOOKS_VERSION_PATH" => :version_path, "HOOKS_ENVIRONMENT" => :environment, - "HOOKS_ENDPOINTS_DIR" => :endpoints_dir + "HOOKS_ENDPOINTS_DIR" => :endpoints_dir, + "HOOKS_USE_CATCHALL_ROUTE" => :use_catchall_route, + "HOOKS_SYMBOLIZE_PAYLOAD" => :symbolize_payload, + "HOOKS_NORMALIZE_HEADERS" => :normalize_headers } env_mappings.each do |env_key, config_key| value = ENV[env_key] next unless value - # Convert numeric values + # Convert values to appropriate types case config_key when :request_limit, :request_timeout env_config[config_key] = value.to_i + when :use_catchall_route, :symbolize_payload, :normalize_headers + # Convert string to boolean + env_config[config_key] = %w[true 1 yes on].include?(value.downcase) else env_config[config_key] = value end From 717db60059c8bfdd485ebe1fe8369b8740c5db31 Mon Sep 17 00:00:00 2001 From: Grant Birkinbine Date: Wed, 11 Jun 2025 23:11:36 -0700 Subject: [PATCH 4/7] Update lib/hooks/core/config_loader.rb Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- lib/hooks/core/config_loader.rb | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index 3e2a7fd0..cb190b96 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -65,9 +65,7 @@ def self.load(config_path: nil) # Log overrides if any were made if overrides.any? - # Use puts for now since logger might not be initialized yet - # This will be replaced with proper logging once the logger is available - puts "INFO: Configuration overrides applied from: #{overrides.join(', ')}" + logger.info("Configuration overrides applied from: #{overrides.join(', ')}") end return config From a28848e567a77a8c837c5fe318cbd166312da3fc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 06:16:45 +0000 Subject: [PATCH 5/7] Fix logger usage in ConfigLoader - replace logger.info with puts to avoid dependency on uninitialized logger Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- lib/hooks/core/config_loader.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index cb190b96..14f4e91e 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -65,7 +65,7 @@ def self.load(config_path: nil) # Log overrides if any were made if overrides.any? - logger.info("Configuration overrides applied from: #{overrides.join(', ')}") + puts "INFO: Configuration overrides applied from: #{overrides.join(', ')}" end return config From abd2a772ec148357c093f98dbac87e7c2f96b7c2 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 12 Jun 2025 10:23:42 -0700 Subject: [PATCH 6/7] fix bundle --- .bundle/config | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.bundle/config b/.bundle/config index f9263841..0146a1ce 100644 --- a/.bundle/config +++ b/.bundle/config @@ -1,8 +1,7 @@ --- BUNDLE_BIN: "bin" -BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle" +BUNDLE_PATH: "vendor/gems" BUNDLE_CACHE_PATH: "vendor/cache" BUNDLE_CACHE_ALL: "true" BUNDLE_SPECIFIC_PLATFORM: "true" BUNDLE_NO_INSTALL: "true" -BUNDLE_DEPLOYMENT: "true" From c0a8ff8303390d0a9b10bb640c71400ec46af30b Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Thu, 12 Jun 2025 10:31:54 -0700 Subject: [PATCH 7/7] Add environment variable to silence config loader messages --- lib/hooks/core/config_loader.rb | 8 ++++++-- spec/integration/global_lifecycle_hooks_spec.rb | 2 ++ spec/integration/hooks_integration_spec.rb | 2 ++ spec/unit/spec_helper.rb | 1 + 4 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index 14f4e91e..fc3df9cf 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -24,6 +24,10 @@ class ConfigLoader normalize_headers: true }.freeze + SILENCE_CONFIG_LOADER_MESSAGES = ENV.fetch( + "HOOKS_SILENCE_CONFIG_LOADER_MESSAGES", "false" + ).downcase == "true".freeze + # Load and merge configuration from various sources # # @param config_path [String, Hash] Path to config file or config hash @@ -65,7 +69,7 @@ def self.load(config_path: nil) # Log overrides if any were made if overrides.any? - puts "INFO: Configuration overrides applied from: #{overrides.join(', ')}" + puts "INFO: Configuration overrides applied from: #{overrides.join(', ')}" unless SILENCE_CONFIG_LOADER_MESSAGES end return config @@ -112,7 +116,7 @@ def self.load_config_file(file_path) result rescue => e # Log this error with meaningful information - puts "ERROR: Failed to load config file '#{file_path}': #{e.message}" + puts "ERROR: Failed to load config file '#{file_path}': #{e.message}" unless SILENCE_CONFIG_LOADER_MESSAGES nil end diff --git a/spec/integration/global_lifecycle_hooks_spec.rb b/spec/integration/global_lifecycle_hooks_spec.rb index 03b3c38f..cecf02c8 100644 --- a/spec/integration/global_lifecycle_hooks_spec.rb +++ b/spec/integration/global_lifecycle_hooks_spec.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +ENV["HOOKS_SILENCE_CONFIG_LOADER_MESSAGES"] = "true" # Silence config loader messages in tests + require_relative "../../lib/hooks" require "rack/test" require "json" diff --git a/spec/integration/hooks_integration_spec.rb b/spec/integration/hooks_integration_spec.rb index 74de3224..7270d9b8 100644 --- a/spec/integration/hooks_integration_spec.rb +++ b/spec/integration/hooks_integration_spec.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +ENV["HOOKS_SILENCE_CONFIG_LOADER_MESSAGES"] = "true" # Silence config loader messages in tests + require_relative "../../lib/hooks" require "rack/test" require "json" diff --git a/spec/unit/spec_helper.rb b/spec/unit/spec_helper.rb index 3abf307a..47bf2956 100644 --- a/spec/unit/spec_helper.rb +++ b/spec/unit/spec_helper.rb @@ -12,6 +12,7 @@ ENV["GITHUB_WEBHOOK_SECRET"] = FAKE_HMAC_SECRET ENV["ALT_WEBHOOK_SECRET"] = FAKE_ALT_HMAC_SECRET +ENV["HOOKS_SILENCE_CONFIG_LOADER_MESSAGES"] = "true" COV_DIR = File.expand_path("../../coverage", File.dirname(__FILE__))