Harden helper deploy against partial-copy UAC loop (#97)

XboxGamingBarHelper/ElevationBootstrapper.cs

@@ -304,6 +304,15 @@ public static bool PerformSetup()
                 Logger.Info($"Target directory: {HelperDeploymentService.HelperFolder}");
                 Logger.Info($"Package version: {HelperDeploymentService.GetCurrentPackageVersion()}");
 
+                // Step 0: stop any running/zombie helper instances first (#97). A helper
+                // still running from the deployed folder keeps its exe/DLLs locked, so
+                // every copy attempt fails, .version is never stamped, and each launch
+                // re-fires the UAC setup loop. We're elevated here, so we have the rights
+                // to kill them regardless of who started them.
+                Logger.Info("Step 0: Stopping running helper instances...");
+                DebugLog("Step 0: Killing running helper instances...");
+                KillRunningHelperInstances(DebugLog);
+
                 // Step 1: Deploy helper files
                 Logger.Info("Step 1: Deploying helper files...");
                 DebugLog("Step 1: Deploying...");
@@ -344,6 +353,52 @@ public static bool PerformSetup()
             }
         }
 
+        /// <summary>
+        /// Kills every other XboxGamingBarHelper process so the deploy that follows
+        /// doesn't fail on locked files (#97). All sessions, not just ours — a stale
+        /// scheduled-task helper holds the same locks wherever it lives. The launcher
+        /// instance that spawned this setup has already exited by the time UAC consent
+        /// completes, so "everything except self" is safe.
+        /// </summary>
+        private static void KillRunningHelperInstances(Action<string> debugLog)
+        {
+            try
+            {
+                int selfPid = Process.GetCurrentProcess().Id;
+                var peers = Process.GetProcessesByName("XboxGamingBarHelper");
+                foreach (var p in peers)
+                {
+                    try
+                    {
+                        if (p.Id == selfPid || p.HasExited)
+                        {
+                            continue;
+                        }
+
+                        Logger.Info($"Killing running helper instance (PID={p.Id}) before deploy");
+                        debugLog?.Invoke($"Killing helper PID={p.Id}");
+                        p.Kill();
+                        if (!p.WaitForExit(5000))
+                        {
+                            Logger.Warn($"Helper PID={p.Id} did not exit within 5s; deploy may hit locked files");
+                        }
+                    }
+                    catch (Exception ex)
+                    {
+                        Logger.Warn($"Could not kill helper PID={p.Id}: {ex.Message}");
+                    }
+                    finally
+                    {
+                        try { p.Dispose(); } catch { }
+                    }
+                }
+            }
+            catch (Exception ex)
+            {
+                Logger.Warn($"Could not enumerate helper instances: {ex.Message}");
+            }
+        }
+
         /// <summary>
         /// Checks if the current process is running with administrator privileges.
         /// </summary>

XboxGamingBarHelper/Services/HelperDeploymentService.cs

@@ -128,9 +128,13 @@ public static string GetMsixHelperPath()
         };
 
         /// <summary>
-        /// Gets the current package version from MSIX
+        /// Gets the package version from MSIX identity, or null when the process has no
+        /// package identity (e.g. running from the deployed LocalCache copy). Use this
+        /// wherever a wrong-but-plausible version is worse than no version at all —
+        /// the assembly-version fallback below is a hardcoded 1.0.0.0 and stamping it
+        /// into .version would wedge IsDeploymentNeeded into a permanent mismatch.
         /// </summary>
-        public static string GetCurrentPackageVersion()
+        public static string GetPackageVersionFromIdentity()
         {
             try
             {
@@ -141,17 +145,31 @@ public static string GetCurrentPackageVersion()
             catch (Exception ex)
             {
                 Logger.Debug($"Could not get package version (not running in MSIX?): {ex.Message}");
-                // Fall back to assembly version
-                try
-                {
-                    var assembly = Assembly.GetExecutingAssembly();
-                    var version = assembly.GetName().Version;
-                    return version?.ToString() ?? "0.0.0.0";
-                }
-                catch
-                {
-                    return "0.0.0.0";
-                }
+                return null;
+            }
+        }
+
+        /// <summary>
+        /// Gets the current package version from MSIX
+        /// </summary>
+        public static string GetCurrentPackageVersion()
+        {
+            var identityVersion = GetPackageVersionFromIdentity();
+            if (identityVersion != null)
+            {
+                return identityVersion;
+            }
+
+            // Fall back to assembly version
+            try
+            {
+                var assembly = Assembly.GetExecutingAssembly();
+                var version = assembly.GetName().Version;
+                return version?.ToString() ?? "0.0.0.0";
+            }
+            catch
+            {
+                return "0.0.0.0";
             }
         }
 
@@ -284,7 +302,29 @@ public static bool DeployHelper()
                 if (PathsAreSame(sourceDir, HelperFolder))
                 {
                     Logger.Warn($"Deploy source equals deployed folder ({HelperFolder}); skipping self-copy (files already in place).");
-                    return File.Exists(DeployedExePath);
+
+                    // #97: if the files are in place but .version is missing/stale, this
+                    // no-op path used to leave the mismatch in place forever. Re-stamp —
+                    // but only with a real package-identity version; the assembly-version
+                    // fallback (1.0.0.0) would poison the stamp and recreate the loop.
+                    if (File.Exists(DeployedExePath))
+                    {
+                        var identityVersion = GetPackageVersionFromIdentity();
+                        if (identityVersion != null && GetDeployedVersion() != identityVersion)
+                        {
+                            try
+                            {
+                                File.WriteAllText(VersionFilePath, identityVersion);
+                                Logger.Info($"Version file re-stamped on self-copy no-op: {identityVersion}");
+                            }
+                            catch (Exception ex)
+                            {
+                                Logger.Warn($"Could not re-stamp version file: {ex.Message}");
+                            }
+                        }
+                        return true;
+                    }
+                    return false;
                 }
 
                 // Create target directory
@@ -372,7 +412,11 @@ public static bool DeployHelper()
                 }
 
                 Logger.Info($"Deployment complete: {successCount} files copied, {failCount} failures");
-                return successCount > 0 && File.Exists(DeployedExePath);
+                // #97: success must match the .version stamp condition. Returning true on a
+                // partial deploy (failCount>0) let PerformSetup report "Setup Complete" while
+                // .version was never written — every launch then re-detected a mismatch and
+                // re-fired UAC, forever. Fail loudly instead so setup surfaces the error.
+                return deployOk;
             }
             catch (Exception ex)
             {