From 414ceb76eb54aa46bed6d4669a615999e6b2e13c Mon Sep 17 00:00:00 2001
From: Connor Shea <2977353+connorshea@users.noreply.github.com>
Date: Fri, 24 Jul 2026 22:21:10 +0000
Subject: [PATCH] fix: reuse the logger instead of reopening the log file on
 every call

When KNAPSACK_PRO_LOG_DIR was set, `KnapsackPro.logger` built a new
`Logger` and reopened the log file on *every* call, because the log_dir
branch ran unconditionally instead of only when no logger existed yet.
A CI node leaked a Logger object and a file handle per log call.

The stdout logger was already memoized; the log_dir logger now is too.

Note one consequence worth a second opinion: assigning a custom logger
(`KnapsackPro.logger = Rails.logger`) while KNAPSACK_PRO_LOG_DIR is set
used to be silently overridden on the next `logger` call, and now wins.
That looks like the intended behaviour of a public writer, but it is a
change for anyone relying on the old precedence.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 CHANGELOG.md              |  2 ++
 lib/knapsack_pro.rb       | 20 ++++++++++----------
 spec/knapsack_pro_spec.rb | 13 +++++++++++++
 3 files changed, 25 insertions(+), 10 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 59728345..d1398292 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,8 @@
 
 ### Unreleased
 
+* (patch) Fix `KnapsackPro.logger` building a new logger and reopening the log file on every call when `KNAPSACK_PRO_LOG_DIR` is set.
+
 ### 10.0.1
 
 * Add support for [File Paths Encryption](https://docs.knapsackpro.com/ruby/encryption/) to [Retry only Failures](https://docs.knapsackpro.com/ruby/retry-only-failures/).
diff --git a/lib/knapsack_pro.rb b/lib/knapsack_pro.rb
index 4820cea3..736bf134 100644
--- a/lib/knapsack_pro.rb
+++ b/lib/knapsack_pro.rb
@@ -110,17 +110,17 @@ def root
     end
 
     def logger
-      if KnapsackPro::Config::Env.log_dir
-        default_logger = Logger.new("#{KnapsackPro::Config::Env.log_dir}/knapsack_pro_node_#{KnapsackPro::Config::Env.ci_node_index}.log")
-        default_logger.level = KnapsackPro::Config::Env.log_level
-        self.logger = default_logger
-      end
+      return @logger if @logger
 
-      unless @logger
-        default_logger = ::Logger.new(stdout)
-        default_logger.level = KnapsackPro::Config::Env.log_level
-        self.logger = default_logger
-      end
+      log_dir = KnapsackPro::Config::Env.log_dir
+      default_logger =
+        if log_dir
+          ::Logger.new("#{log_dir}/knapsack_pro_node_#{KnapsackPro::Config::Env.ci_node_index}.log")
+        else
+          ::Logger.new(stdout)
+        end
+      default_logger.level = KnapsackPro::Config::Env.log_level
+      self.logger = default_logger
 
       @logger
     end
diff --git a/spec/knapsack_pro_spec.rb b/spec/knapsack_pro_spec.rb
index c1694b4a..a958f1d5 100644
--- a/spec/knapsack_pro_spec.rb
+++ b/spec/knapsack_pro_spec.rb
@@ -51,6 +51,19 @@
           end
         end
       end
+
+      it 'reuses the logger instead of reopening the log file on every call' do
+        Dir.mktmpdir do |dir|
+          stub_const('ENV', 'KNAPSACK_PRO_LOG_DIR' => dir)
+
+          expect(::Logger).to receive(:new).once.and_call_original
+
+          logger = described_class.logger
+
+          expect(described_class.logger).to be(logger)
+          expect(described_class.logger).to be(logger)
+        end
+      end
     end
 
     context 'with the default logger' do