Use T.unsafe only where Sorbet can't type the code, and remove mailer metadata that never collected anything

.cursor/rules/sorbet.mdc

@@ -1,6 +1,6 @@
 ---
 description: Working with Sorbet and Rails constants
-globs: 
+globs:
 alwaysApply: true
 ---
 - `T::Sig` is not available globally, we must extend it whenever we need it (we're building a library, not an application.)
@@ -13,7 +13,7 @@ alwaysApply: true
 
 - Use Sorbet type annotations for all methods
 - Ensure all files have the appropriate `# typed:` annotation
-- **NEVER use `T.unsafe` calls**. Instead, properly type your code or use appropriate type assertions with `T.let` or `T.cast` when absolutely necessary.
+- Use `T.unsafe` only in the cases listed in AGENTS.md (Code Style), with a comment saying why. Properly type your code first, or use `T.let` or `T.cast`.
 - `T.untyped` is generally ok for Hash values when they come from unknown sources.
 - When dealing with external libraries, create proper type signatures or use extension methods rather than resorting to `T.unsafe`.
 - **NEVER use `class.name`** anywhere - this is a Sorbet quirk that hides the `name` method from all base classes. Prefer just using Classes themselves as the type. `"#{class}"` will automatically call `.to_s`. Similarly, `to_json` will automatically call `.to_s` - but you can call `.to_s` manually if you really need it.
@@ -32,7 +32,7 @@ end
 
 # BAD
 SomeGem.configure do
-  T.unsafe(self).add_option "value"  # NEVER do this!
+  T.unsafe(self).add_option "value"  # use T.bind instead
 end
 ```
 

AGENTS.md

@@ -144,7 +144,18 @@ You do not need to check if these are defined with `defined?` - they are guarant
 - Method signatures must include proper return types
 - Use `::` prefixes for external modules (Rails/third-party gems)
 - `T.must` may be used very sparingly but only in tests - e.g. `T.must(log_file.path)`
-- `T.unsafe` must NEVER be used. Use proper typing or `T.let`/`T.cast` when necessary
+- `T.unsafe` turns off type checking for one value. Use it only where Sorbet can't type the code another way,
+  with a short comment saying why. Try these first:
+  - Nothing: `const_get`, `instance_variable_get`, `send`/`public_send` and `JSON.parse` already return
+    `T.untyped`, so wrapping their results adds nothing.
+  - Narrowing with `is_a?` or `case`, or `T.cast`/`T.let` when you know the type.
+  - `requires_ancestor` in a module that's included in or prepended to another class.
+  - `T.bind(self, SomeClass)` in a block that runs with a known `self`, or an override in `sorbet/rbi/overrides/`
+    when the block's `self` varies (`ActiveSupport.on_load`).
+  - A shim in `sorbet/rbi/shims/` for an optional gem's API that Tapioca can't see (Shrine's plugins).
+
+  What's left: splatting an Array or Hash whose size Sorbet doesn't know, a duck-typed call after `respond_to?` on
+  another library's object, a `Minitest::Mock`, and a test that passes a wrong type on purpose.
 - Don't call `def` inside a method definition in tests (or anywhere). Use mocks or stubs.
 - For modules included in other classes, use `requires_ancestor`
 - Custom type overrides belong in `sorbet/rbi/overrides/`

lib/log_struct/integrations/action_mailer.rb

@@ -32,7 +32,7 @@ def self.setup(config)
         # if ActionMailer::Base is already loaded.
         ActiveSupport.on_load(:action_mailer) do
           # Silence default ActionMailer logs (we use our own structured logging)
-          T.unsafe(self).logger = ::Logger.new(File::NULL)
+          self.logger = ::Logger.new(File::NULL)
 
           prepend LogStruct::Integrations::ActionMailer::EventLogging
           prepend LogStruct::Integrations::ActionMailer::ErrorHandling

lib/log_struct/integrations/action_mailer/error_handling.rb

@@ -264,7 +264,7 @@ def build_exception_data(error, level, context_data)
 
         sig { params(error: StandardError).returns(String) }
         def recipients(error)
-          # Extract recipient info if available
+          # Some delivery errors (Postmark's, for example) have the recipients
           if error.respond_to?(:recipients) && T.unsafe(error).recipients.present?
             T.unsafe(error).recipients.join(", ")
           else

lib/log_struct/integrations/action_mailer/metadata_collection.rb

@@ -25,9 +25,6 @@ def self.add_message_metadata(mailer, log_data)
         def self.add_context_metadata(mailer, log_data)
           # Add account ID information if available (but not user email)
           extract_ids_to_log_data(mailer, log_data)
-
-          # Add any current tags from ActiveJob or ActionMailer
-          add_current_tags_to_log_data(log_data)
         end
 
         sig { params(mailer: T.untyped, log_data: T::Hash[Symbol, T.untyped]).void }
@@ -43,23 +40,6 @@ def self.extract_ids_to_log_data(mailer, log_data)
             log_data[log_key] = obj.id if obj.respond_to?(:id)
           end
         end
-
-        sig { params(log_data: T::Hash[Symbol, T.untyped]).void }
-        def self.add_current_tags_to_log_data(log_data)
-          # Get current tags from thread-local storage or ActiveSupport::TaggedLogging
-          tags = if ::ActiveSupport::TaggedLogging.respond_to?(:current_tags)
-            T.unsafe(::ActiveSupport::TaggedLogging).current_tags
-          else
-            Thread.current[:activesupport_tagged_logging_tags] || []
-          end
-          log_data[:tags] = tags if tags.present?
-
-          # Get job_id from ActiveJob if available
-          if defined?(::ActiveJob::Logging) && ::ActiveJob::Logging.respond_to?(:job_id) &&
-              T.unsafe(::ActiveJob::Logging).job_id.present?
-            log_data[:job_id] = T.unsafe(::ActiveJob::Logging).job_id
-          end
-        end
       end
     end
   end

lib/log_struct/integrations/ahoy.rb

@@ -43,7 +43,7 @@ def track(name, properties = nil, options = {})
             end
           end
 
-          T.unsafe(::Ahoy::Tracker).prepend(mod)
+          ::Ahoy::Tracker.prepend(mod)
         end
 
         true

lib/log_struct/integrations/dotenv.rb

@@ -130,8 +130,8 @@ def self.setup_boot
         return unless defined?(::ActiveSupport::Notifications)
 
         instrumenter = if Object.const_defined?(:Dotenv)
-          dm = T.unsafe(Object.const_get(:Dotenv))
-          dm.respond_to?(:instrumenter) ? T.unsafe(dm).instrumenter : ::ActiveSupport::Notifications
+          dm = Object.const_get(:Dotenv)
+          dm.respond_to?(:instrumenter) ? dm.instrumenter : ::ActiveSupport::Notifications
         else
           ::ActiveSupport::Notifications
         end
@@ -183,9 +183,9 @@ def self.intercept_logger_setter!
         return unless Object.const_defined?(:Dotenv)
         # Do not intercept when LogStruct is disabled; allow original dotenv replay
         return unless LogStruct.enabled?
-        dotenv_mod = T.unsafe(Object.const_get(:Dotenv))
+        dotenv_mod = Object.const_get(:Dotenv)
         return unless dotenv_mod.const_defined?(:Rails)
-        klass = T.unsafe(dotenv_mod.const_get(:Rails))
+        klass = dotenv_mod.const_get(:Rails)
         return if klass.instance_variable_defined?(:@_logstruct_replay_patched)
 
         original = klass.instance_method(:logger=)
@@ -212,15 +212,15 @@ def self.intercept_logger_setter!
       def self.resolve_boot_logs!
         # If LogStruct is disabled, do not alter dotenv behavior at all
         return unless LogStruct.enabled?
-        dotenv_mod = Object.const_defined?(:Dotenv) ? T.unsafe(Object.const_get(:Dotenv)) : nil
-        klass = dotenv_mod&.const_defined?(:Rails) ? T.unsafe(dotenv_mod.const_get(:Rails)) : nil
+        dotenv_mod = Object.const_defined?(:Dotenv) ? Object.const_get(:Dotenv) : nil
+        klass = dotenv_mod&.const_defined?(:Rails) ? dotenv_mod.const_get(:Rails) : nil
 
         pending_logger = nil
         railtie_instance = nil
         if klass&.respond_to?(:instance)
           railtie_instance = klass.instance
           if railtie_instance.respond_to?(:logstruct_pending_dotenv_logger)
-            pending_logger = T.unsafe(railtie_instance).logstruct_pending_dotenv_logger
+            pending_logger = railtie_instance.logstruct_pending_dotenv_logger
           end
         end
 
@@ -242,7 +242,7 @@ def self.resolve_boot_logs!
 
           # Detach original subscriber and subscribe runtime structured
           if dotenv_mod&.const_defined?(:LogSubscriber)
-            T.unsafe(dotenv_mod.const_get(:LogSubscriber)).detach_from(:dotenv)
+            dotenv_mod.const_get(:LogSubscriber).detach_from(:dotenv)
           end
           LogStruct::Integrations::Dotenv.subscribe!
 

lib/log_struct/integrations/puma.rb

@@ -98,16 +98,16 @@ def install_patches!
             rescue => e
               handle_integration_error(e)
             end
-            puma_mod = ::Object.const_defined?(:Puma) ? T.unsafe(::Object.const_get(:Puma)) : nil # rubocop:disable Sorbet/ConstantsFromStrings
+            puma_mod = ::Object.const_defined?(:Puma) ? ::Object.const_get(:Puma) : nil # rubocop:disable Sorbet/ConstantsFromStrings
             # rubocop:disable Sorbet/ConstantsFromStrings
             if puma_mod&.const_defined?(:LogWriter)
-              T.unsafe(::Object.const_get("Puma::LogWriter")).prepend(LogWriterPatch)
+              ::Object.const_get("Puma::LogWriter").prepend(LogWriterPatch)
             end
             if puma_mod&.const_defined?(:ErrorLogger)
-              T.unsafe(::Object.const_get("Puma::ErrorLogger")).prepend(ErrorLoggerPatch)
+              ::Object.const_get("Puma::ErrorLogger").prepend(ErrorLoggerPatch)
             end
             if puma_mod&.const_defined?(:Events)
-              ev = T.unsafe(::Object.const_get("Puma::Events"))
+              ev = ::Object.const_get("Puma::Events")
               ev.prepend(EventsPatch)
             end
             if puma_mod&.const_defined?(:Cluster)
@@ -119,11 +119,11 @@ def install_patches!
             end
             # Patch Rack::Handler::Puma.run to emit lifecycle logs using options
             if ::Object.const_defined?(:Rack)
-              rack_mod = T.unsafe(::Object.const_get(:Rack))
+              rack_mod = ::Object.const_get(:Rack)
               if rack_mod.const_defined?(:Handler)
-                handler_mod = T.unsafe(rack_mod.const_get(:Handler))
+                handler_mod = rack_mod.const_get(:Handler)
                 if handler_mod.const_defined?(:Puma)
-                  handler = T.unsafe(handler_mod.const_get(:Puma))
+                  handler = handler_mod.const_get(:Puma)
                   handler.singleton_class.prepend(RackHandlerPatch)
                 end
               end
@@ -369,7 +369,7 @@ def puts(msg)
             return nil
           end
           if ::Kernel.instance_variables.include?(:@stdout)
-            io = T.unsafe(::Kernel.instance_variable_get(:@stdout))
+            io = ::Kernel.instance_variable_get(:@stdout)
             return io.puts(msg)
           end
           super

lib/log_struct/integrations/shrine.rb

@@ -25,7 +25,7 @@ def self.setup(config)
 
         # Create a structured log subscriber for Shrine
         # ActiveSupport::Notifications::Event has name, time, end, transaction_id, payload, and duration
-        shrine_log_subscriber = T.unsafe(lambda do |event|
+        shrine_log_subscriber = lambda do |event|
           payload = event.payload.except(:io, :metadata, :name).dup
 
           # Map event name to Event type
@@ -86,14 +86,14 @@ def self.setup(config)
           # Log directly through SemanticLogger, NOT through Shrine.logger
           # Shrine.logger is a basic Logger that would just call .to_s on the struct
           ::SemanticLogger[::Shrine].info(log_data)
-        end)
+        end
 
         # Load the instrumentation plugin without Shrine's default log subscriber, which logs
         # each event as a line of text (a no-op if the app has loaded it already). Shrine
         # ignores a subscriber that's already subscribed.
         ::Shrine.plugin :instrumentation, log_subscriber: nil
         SHRINE_EVENTS.each do |event_name|
-          T.unsafe(::Shrine).subscribe(event_name, &shrine_log_subscriber)
+          ::Shrine.subscribe(event_name, &shrine_log_subscriber)
         end
 
         # The app may load the plugin with the default log subscriber too, before or after this

lib/log_struct/monkey_patches/active_support/tagged_logging/formatter.rb

@@ -31,14 +31,13 @@ def call(severity, time, progname, data)
         # Preserve original Rails behavior when LogStruct is disabled
         return super unless ::LogStruct.enabled?
 
-        # Get current tags
-        tags = T.unsafe(self).respond_to?(:current_tags) ? current_tags : (Thread.current[:activesupport_tagged_logging_tags] || [])
+        tags = current_tags
 
         # Add tags to data as hash entry (not text prefix)
         data_with_tags = if data.is_a?(Hash)
           tags.present? ? data.merge(tags: tags) : data
         elsif data.is_a?(::LogStruct::Log::Interfaces::CommonFields) || (data.is_a?(T::Struct) && data.respond_to?(:serialize))
-          hash = T.unsafe(data).serialize
+          hash = data.serialize
           tags.present? ? hash.merge(tags: tags) : hash
         else
           base = {LogStruct::LogField::Message.serialize => data.to_s}

lib/log_struct/rails_boot_banner_silencer.rb

@@ -51,6 +51,8 @@ def self.patch_server_command(server_command)
 
     module ServerCommandSilencer
       extend T::Sig
+      extend T::Helpers
+      requires_ancestor { Object }
 
       sig { params(args: T.untyped, block: T.nilable(T.proc.returns(T.untyped))).returns(T.untyped) }
       def perform(*args, &block)
@@ -105,9 +107,9 @@ def build_rails_banner_line(url)
 
       sig { returns(String) }
       def lookup_executable
-        return "rails" unless T.unsafe(self).respond_to?(:executable, true)
+        return "rails" unless respond_to?(:executable, true)
 
-        T.cast(T.unsafe(self).send(:executable), String)
+        T.cast(send(:executable), String)
       rescue
         "rails"
       end

lib/log_struct/semantic_logger/formatter.rb

@@ -44,7 +44,7 @@ module SemanticLogger
     # The formatter produces consistent, parseable JSON that includes:
     # - Standard log fields (timestamp, level, message, logger name)
     # - LogStruct-specific fields (source, event, context)
-    # - SemanticLogger metadata (process ID, thread ID, tags)
+    # - The request ID and the running Sidekiq job's context, from SemanticLogger's named tags
     # - Application-specific payload data
     #
     # This combination provides the performance benefits of SemanticLogger with

lib/log_struct/semantic_logger/logger.rb

@@ -107,11 +107,11 @@ def stop_broadcasting_to(logger)
       # Support for tagged logging
       sig { params(tags: T.untyped, block: T.proc.returns(T.untyped)).returns(T.untyped) }
       def tagged(*tags, &block)
-        # Convert tags to array and pass individually to avoid splat issues
         tag_array = tags.flatten
         if tag_array.empty?
           super(&block)
         else
+          # Sorbet can't check a splat of an Array whose size it doesn't know
           super(*T.unsafe(tag_array), &block)
         end
       end

lib/log_struct/semantic_logger/setup.rb

@@ -259,7 +259,7 @@ def self.replace_rails_logger(app)
         # initializers run, e.g. ActiveRecord would copy an empty filter_parameters into
         # filter_attributes.
         COMPONENT_LOAD_HOOKS.each do |hook|
-          ActiveSupport.on_load(hook) { T.unsafe(self).logger = logger }
+          ActiveSupport.on_load(hook) { self.logger = logger }
         end
 
         # Store reference in app config
@@ -282,7 +282,7 @@ def self.restore_rails_logger(app)
         # components that are already loaded
         COMPONENT_LOAD_HOOKS.each do |hook|
           ActiveSupport.on_load(hook) do
-            T.unsafe(self).logger = original_logger if T.unsafe(self).logger.equal?(logger)
+            self.logger = original_logger if self.logger.equal?(logger)
           end
         end
 

rails_test_app/templates/app/controllers/logging_controller.rb

@@ -108,11 +108,11 @@ def test_job
   def test_context
     # TODO: Fix types for the tagged method
     # Test Rails' built-in tagged logging
-    T.unsafe(Rails.logger).tagged("REQUEST_ID_123", "USER_456") do
+    Rails.logger.tagged("REQUEST_ID_123", "USER_456") do
       Rails.logger.info("Message with tags")
 
       # Nested tags
-      T.unsafe(Rails.logger).tagged("NESTED") do
+      Rails.logger.tagged("NESTED") do
         Rails.logger.warn("Message with nested tags")
       end
     end

sorbet/rbi/overrides/active_support.rbi

@@ -39,3 +39,10 @@ module ActiveSupport
     end
   end
 end
+
+module ActiveSupport::LazyLoadHooks
+  # The block runs with self set to the class that loaded (ActionController::Base,
+  # ActiveRecord::Base, ...), so Sorbet can't type calls on self inside it
+  sig { params(name: Symbol, options: T::Hash[Symbol, T.untyped], block: T.proc.bind(T.untyped).void).void }
+  def on_load(name, options = {}, &block); end
+end

sorbet/rbi/shims/shrine.rbi

@@ -6,3 +6,9 @@ module Shrine::Plugins::Instrumentation
     def self.call(event); end
   end
 end
+
+class Shrine
+  # Added by the instrumentation plugin, which Shrine loads on demand
+  sig { params(event_name: Symbol, subscriber: T.proc.params(event: T.untyped).void).void }
+  def self.subscribe(event_name, &subscriber); end
+end

test/log_struct/configuration_test.rb

@@ -525,9 +525,9 @@ def test_enabled_for_puma_via_argv_in_production
     private
 
     def with_rails_filter_parameters(filter_params, &block)
-      mock_config = T.unsafe(Object.new)
+      mock_config = Object.new
       mock_config.define_singleton_method(:filter_parameters) { filter_params }
-      mock_app = T.unsafe(Object.new)
+      mock_app = Object.new
       mock_app.define_singleton_method(:config) { mock_config }
       LogStruct.config.filters.filter_keys = []
 

test/log_struct/error_handling_test.rb

@@ -41,14 +41,14 @@ def test_handle_exception_raise
   def test_handle_exception_log
     LogStruct.config.error_handling_modes.standard_errors = LogStruct::ErrorHandlingMode::Log
     error = RuntimeError.new("log me")
-    logged = []
+    logged = T.let([], T::Array[LogStruct::Log::Error])
 
     LogStruct.stub(:error, ->(log) { logged << log }) do
       LogStruct.handle_exception(error, source: LogStruct::Source::App)
     end
 
     assert_equal 1, logged.length
-    log_entry = T.unsafe(logged).first
+    log_entry = T.must(logged.first)
 
     assert_equal "log me", log_entry.message
   end