From bf86b3def68cb28960c455686541cbeec3cd8cf4 Mon Sep 17 00:00:00 2001
From: Joseph Doherty <dohejw01@gmail.com>
Date: Wed, 10 Jun 2026 11:56:51 -0400
Subject: [PATCH] fix(scripting): explicit companion logger + disposable
 ScriptRootLogger (T2 review)

---
 .../ScriptRootLogger.cs                       | 24 ++++++++++++-------
 .../Logging/ScriptRootLoggerFactory.cs        | 11 +++++++--
 src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs  |  2 +-
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptRootLogger.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptRootLogger.cs
index 07f000c0..76db74aa 100644
--- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptRootLogger.cs
+++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Scripting/ScriptRootLogger.cs
@@ -11,15 +11,23 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Scripting;
 ///     file sink, the <see cref="ScriptLogCompanionSink"/> mirroring errors to the main log,
 ///     and the <see cref="ScriptLogTopicSink"/> fanning entries onto the cluster
 ///     <c>script-logs</c> topic for the live Script-log Admin UI page.
+///     <para>
+///         Implements <see cref="IDisposable"/> so the DI container flushes the rolling-file
+///         sink on graceful host shutdown.
+///     </para>
 /// </remarks>
-public sealed class ScriptRootLogger
+public sealed class ScriptRootLogger : IDisposable
 {
-    /// <summary>Gets the composed root script logger that evaluators derive per-script loggers from.</summary>
-    public Serilog.ILogger Logger { get; }
+    private readonly Serilog.Core.Logger _logger;
 
-    /// <summary>Initializes a new instance of the <see cref="ScriptRootLogger"/> class.</summary>
-    /// <param name="logger">The composed root script logger. Must not be <c>null</c>.</param>
-    /// <exception cref="ArgumentNullException">Thrown when <paramref name="logger"/> is <c>null</c>.</exception>
-    public ScriptRootLogger(Serilog.ILogger logger) =>
-        Logger = logger ?? throw new ArgumentNullException(nameof(logger));
+    /// <summary>The composed root script logger (file + companion mirror + DPS topic sink).</summary>
+    public Serilog.ILogger Logger => _logger;
+
+    /// <summary>Initializes a new <see cref="ScriptRootLogger"/>.</summary>
+    /// <param name="logger">The composed Serilog logger; owned + disposed by this wrapper.</param>
+    public ScriptRootLogger(Serilog.Core.Logger logger) =>
+        _logger = logger ?? throw new ArgumentNullException(nameof(logger));
+
+    /// <summary>Disposes the underlying logger, flushing the rolling-file sink.</summary>
+    public void Dispose() => _logger.Dispose();
 }
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Logging/ScriptRootLoggerFactory.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Logging/ScriptRootLoggerFactory.cs
index 1796417a..810d43b0 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Logging/ScriptRootLoggerFactory.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Logging/ScriptRootLoggerFactory.cs
@@ -27,12 +27,19 @@ public static class ScriptRootLoggerFactory
     ///     Minimum level for events forwarded to the <c>script-logs</c> topic; lower-level events
     ///     still land in the file sink but are not pushed onto the cluster bus.
     /// </param>
+    /// <param name="companionLogger">
+    ///     The application's main logger that receives mirrored Error-or-higher script events at
+    ///     Warning level. Passed explicitly so callers control the source rather than reading the
+    ///     global <see cref="Serilog.Log.Logger"/> static inside the factory.
+    /// </param>
     /// <returns>The composed root script logger.</returns>
-    public static Serilog.ILogger Build(IScriptLogPublisher publisher, string filePath, LogEventLevel topicMinLevel)
+    public static Serilog.Core.Logger Build(
+        IScriptLogPublisher publisher, string filePath, LogEventLevel topicMinLevel,
+        Serilog.ILogger companionLogger)
         => new LoggerConfiguration()
             .MinimumLevel.Verbose()
             .WriteTo.File(filePath, rollingInterval: RollingInterval.Day)
-            .WriteTo.Sink(new ScriptLogCompanionSink(Serilog.Log.Logger))
+            .WriteTo.Sink(new ScriptLogCompanionSink(companionLogger))
             .WriteTo.Sink(new ScriptLogTopicSink(publisher, topicMinLevel))
             .CreateLogger();
 }
diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs
index fb7cf8b5..66a02c3b 100644
--- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs
+++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Program.cs
@@ -131,7 +131,7 @@ if (hasDriver)
         new DpsScriptLogPublisher(() => sp.GetRequiredService<ActorSystem>()));
     builder.Services.AddSingleton(sp => new ScriptRootLogger(
         ScriptRootLoggerFactory.Build(
-            sp.GetRequiredService<IScriptLogPublisher>(), scriptLogFilePath, scriptLogTopicMinLevel)));
+            sp.GetRequiredService<IScriptLogPublisher>(), scriptLogFilePath, scriptLogTopicMinLevel, Serilog.Log.Logger)));
 
     builder.Services.AddValidatedOptions<LdapOptions, LdapOptionsValidator>(builder.Configuration, LdapOptions.SectionName);
     // TryAdd so a fused admin+driver node (where AddOtOpcUaAuth also registers these) ends up