From 02f507ccb11a7cab2e5c69935a5fb49ffaa1bac6 Mon Sep 17 00:00:00 2001
From: jkdevito <jdevito@pepperdash.com>
Date: Wed, 8 Jul 2026 10:07:20 -0500
Subject: [PATCH] fix(mobile-control): address PR review feedback on version
 tracking

- ConnectedClientVersions now deep-copies each ConnectedClientVersionInfo
  when snapshotting, so external callers can't mutate the internally
  tracked, lock-protected instances (Copilot review).
- ShowInfo now prints "(not configured)" for an empty ExpectedAppVersion,
  matching the match/mismatch calculation which already treats empty the
  same as not-configured (Copilot review).
- GetPackageManifestRequestHandler.PopulatePackages filters out null
  entries from the config-supplied packages list before processing, so a
  malformed "packages": [null, ...] in user-edited config JSON degrades
  gracefully instead of throwing (Copilot review).
---
 .../GetPackageManifestRequestHandler.cs            |  6 +++++-
 .../ConnectedClientVersionInfo.cs                  | 14 ++++++++++++++
 .../MobileControlSystemController.cs               |  4 ++--
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/src/PepperDash.Essentials.Core/Web/RequestHandlers/GetPackageManifestRequestHandler.cs b/src/PepperDash.Essentials.Core/Web/RequestHandlers/GetPackageManifestRequestHandler.cs
index 8dc4a751..eeea75cd 100644
--- a/src/PepperDash.Essentials.Core/Web/RequestHandlers/GetPackageManifestRequestHandler.cs
+++ b/src/PepperDash.Essentials.Core/Web/RequestHandlers/GetPackageManifestRequestHandler.cs
@@ -122,7 +122,11 @@ namespace PepperDash.Essentials.Core.Web.RequestHandlers
 		/// </summary>
 		private static void PopulatePackages(VersionData result)
 		{
-			var configPackages = result.Packages ?? new System.Collections.Generic.List<NugetVersion>();
+			// Filter out null entries defensively - the packages list is deserialized from user-editable
+			// config JSON, so a malformed "packages": [null, ...] shouldn't throw and 500 the endpoint.
+			var configPackages = (result.Packages ?? new System.Collections.Generic.List<NugetVersion>())
+				.Where(p => p != null)
+				.ToList();
 			var matchedConfigPackages = new System.Collections.Generic.HashSet<NugetVersion>();
 			var mergedPackages = new System.Collections.Generic.List<NugetVersion>();
 
diff --git a/src/PepperDash.Essentials.MobileControl/ConnectedClientVersionInfo.cs b/src/PepperDash.Essentials.MobileControl/ConnectedClientVersionInfo.cs
index 4107d120..68f5e36e 100644
--- a/src/PepperDash.Essentials.MobileControl/ConnectedClientVersionInfo.cs
+++ b/src/PepperDash.Essentials.MobileControl/ConnectedClientVersionInfo.cs
@@ -43,5 +43,19 @@ namespace PepperDash.Essentials
         /// </summary>
         [JsonProperty("lastSeen")]
         public DateTime LastSeen { get; set; }
+
+        /// <summary>
+        /// Returns a copy of this instance, safe for callers outside the owning lock to hold/mutate
+        /// without affecting the internally tracked instance
+        /// </summary>
+        public ConnectedClientVersionInfo Clone() => new ConnectedClientVersionInfo
+        {
+            ClientId = ClientId,
+            RoomKey = RoomKey,
+            TouchpanelKey = TouchpanelKey,
+            AppVersion = AppVersion,
+            ExpectedAppVersion = ExpectedAppVersion,
+            LastSeen = LastSeen
+        };
     }
 }
diff --git a/src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs b/src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs
index 91fc55c5..c17cf3b9 100644
--- a/src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs
+++ b/src/PepperDash.Essentials.MobileControl/MobileControlSystemController.cs
@@ -89,7 +89,7 @@ namespace PepperDash.Essentials
                 lock (_connectedClientVersionsLock)
                 {
                     return new ReadOnlyDictionary<string, ConnectedClientVersionInfo>(
-                        new Dictionary<string, ConnectedClientVersionInfo>(_connectedClientVersions)
+                        _connectedClientVersions.ToDictionary(kv => kv.Key, kv => kv.Value.Clone())
                     );
                 }
             }
@@ -1820,7 +1820,7 @@ namespace PepperDash.Essentials
 
                     CrestronConsole.ConsoleCommandResponse(
                         $"  Client: {v.ClientId}  Touchpanel: {v.TouchpanelKey}  Room: {v.RoomKey}\r\n" +
-                        $"    Reported: {v.AppVersion}  Expected: {v.ExpectedAppVersion ?? "(not configured)"}  Match: {(match ? "Yes" : "NO - MISMATCH")}\r\n" +
+                        $"    Reported: {v.AppVersion}  Expected: {(string.IsNullOrEmpty(v.ExpectedAppVersion) ? "(not configured)" : v.ExpectedAppVersion)}  Match: {(match ? "Yes" : "NO - MISMATCH")}\r\n" +
                         $"    Last Seen (UTC): {v.LastSeen:yyyy-MM-dd HH:mm:ss}\r\n"
                     );
                 }