Skip to content

Commit 94c7a72

Browse files
JusterZhuclaude
andcommitted
fix: delegate upgrade process launch to ClientStrategy in silent mode
OnProcessExit was manually resolving the updater path and calling Process.Start, bypassing the configured OS strategy. This meant: - Custom strategies registered via Strategy<T>() were ignored for path resolution - The OnBeforeStartAppAsync lifecycle hook (e.g. UnixPermissionHooks for chmod +x) was never invoked in silent mode Solution: - AbstractStrategy: add internal StartProcess() that reuses ResolveAppPath - ClientStrategy: add LaunchUpgradeProcessSync() that runs the pre-launch hook and delegates path resolution + process start to the OS strategy - SilentPollOrchestrator: OnProcessExit now calls the strategy instead of hand-rolling path logic Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 14e88b2 commit 94c7a72

3 files changed

Lines changed: 69 additions & 18 deletions

File tree

src/c#/GeneralUpdate.Core/Silent/SilentPollOrchestrator.cs

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,4 @@
11
using System;
2-
using System.Diagnostics;
3-
using System.IO;
42
using System.Threading;
53
using System.Threading.Tasks;
64
using GeneralUpdate.Core.Configuration;
@@ -142,7 +140,9 @@ private async Task PollLoopAsync(CancellationToken token)
142140
/// </summary>
143141
/// <remarks>
144142
/// The encrypted IPC file was already written by <see cref="ClientStrategy.SendProcessIpc"/>
145-
/// during the last successful poll cycle. This handler only starts the upgrade executable.
143+
/// during the last successful poll cycle. This handler delegates the actual process
144+
/// launch to <see cref="ClientStrategy.LaunchUpgradeProcessSync"/>, which reuses the
145+
/// configured OS strategy for path resolution and runs the pre-launch lifecycle hook.
146146
/// </remarks>
147147
private void OnProcessExit(object? sender, EventArgs e)
148148
{
@@ -159,21 +159,8 @@ private void OnProcessExit(object? sender, EventArgs e)
159159
return;
160160
}
161161

162-
var updaterDir = !string.IsNullOrWhiteSpace(_configInfo.UpdatePath)
163-
? (Path.IsPathRooted(_configInfo.UpdatePath)
164-
? _configInfo.UpdatePath
165-
: Path.Combine(_configInfo.InstallPath, _configInfo.UpdatePath))
166-
: _configInfo.InstallPath;
167-
var updaterPath = Path.Combine(updaterDir, _configInfo.UpdateAppName);
168-
169-
if (!File.Exists(updaterPath))
170-
{
171-
GeneralTracer.Warn($"SilentPollOrchestrator: updater not found at {updaterPath}, cannot launch.");
172-
return;
173-
}
174-
175-
Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = updaterPath });
176-
GeneralTracer.Info($"SilentPollOrchestrator: launched updater {updaterPath}");
162+
_strategy.LaunchUpgradeProcessSync();
163+
GeneralTracer.Info("SilentPollOrchestrator: upgrade process launched via ClientStrategy.");
177164
}
178165
catch (Exception ex)
179166
{

src/c#/GeneralUpdate.Core/Strategy/AbstractStrategy.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Diagnostics;
23
using System.IO;
34
using System.Linq;
45
using System.Threading.Tasks;
@@ -322,6 +323,23 @@ protected string ResolveAppPath(string name, bool preferUpdatePath = false)
322323
return CheckPath(_configinfo.InstallPath, name);
323324
}
324325

326+
/// <summary>
327+
/// Resolves and starts the target executable using the strategy's platform-specific
328+
/// path resolution logic. Does not perform any additional work (no hooks, no shutdown).
329+
/// For use by callers that only need process launch, e.g. <see cref="Silent.SilentPollOrchestrator"/>.
330+
/// </summary>
331+
/// <param name="appName">The name of the executable to launch.</param>
332+
/// <param name="preferUpdatePath">When <c>true</c>, checks <c>UpdatePath</c> first.</param>
333+
/// <exception cref="FileNotFoundException">Thrown when the executable cannot be resolved.</exception>
334+
internal void StartProcess(string appName, bool preferUpdatePath = false)
335+
{
336+
var appPath = ResolveAppPath(appName, preferUpdatePath);
337+
if (string.IsNullOrEmpty(appPath))
338+
throw new FileNotFoundException($"Can't find the app {appName}!");
339+
GeneralTracer.Info($"AbstractStrategy.StartProcess: launching {appPath}");
340+
Process.Start(appPath);
341+
}
342+
325343
/// <summary>
326344
/// Resolves the target application path for the update package. Determines whether to use the update path or install path
327345
/// based on the version's application type.

src/c#/GeneralUpdate.Core/Strategy/ClientStrategy.cs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -674,6 +674,52 @@ private async Task LaunchUpgradeProcessAsync()
674674
await _osStrategy!.StartAppAsync();
675675
}
676676

677+
/// <summary>
678+
/// Synchronously launches the upgrade process via the configured OS strategy
679+
/// after running the pre-launch lifecycle hook. Designed for
680+
/// <see cref="Silent.SilentPollOrchestrator"/> to call from
681+
/// <see cref="AppDomain.ProcessExit"/>, where the process is already shutting
682+
/// down and only path resolution + hook + process start are needed.
683+
/// </summary>
684+
/// <remarks>
685+
/// Unlike <see cref="LaunchUpgradeProcessAsync"/>, this method does NOT call
686+
/// <see cref="AbstractStrategy.StartAppAsync"/> — it only resolves the path
687+
/// via the strategy's <see cref="AbstractStrategy.ResolveAppPath"/> and starts
688+
/// the process, without the extra shutdown / Bowl / tracer-dispose work that
689+
/// is only appropriate when the current process is about to exit voluntarily.
690+
/// </remarks>
691+
internal void LaunchUpgradeProcessSync()
692+
{
693+
// Run the pre-launch lifecycle hook (e.g. UnixPermissionHooks for chmod +x).
694+
// In the standard flow this runs inside ExecuteStandardWorkflowAsync; in
695+
// silent mode it was deferred and must run now, before the process starts.
696+
var ctx = BuildUpdateContext();
697+
SafeOnBeforeStartAppAsync(ctx).GetAwaiter().GetResult();
698+
699+
if (_osStrategy is AbstractStrategy abs)
700+
{
701+
abs.LaunchAppName = _configInfo!.UpdateAppName;
702+
abs.LaunchBowl = false;
703+
abs.UseUpdatePath = !string.IsNullOrWhiteSpace(_configInfo.UpdatePath);
704+
abs.StartProcess(abs.LaunchAppName!, abs.UseUpdatePath);
705+
return;
706+
}
707+
708+
// Fallback: custom IStrategy (non-AbstractStrategy).
709+
// For a custom strategy we can't use the platform path resolution, so we
710+
// fall back to a simple InstallPath + UpdateAppName lookup.
711+
var updaterDir = !string.IsNullOrWhiteSpace(_configInfo!.UpdatePath)
712+
? (Path.IsPathRooted(_configInfo.UpdatePath)
713+
? _configInfo.UpdatePath
714+
: Path.Combine(_configInfo.InstallPath, _configInfo.UpdatePath))
715+
: _configInfo.InstallPath;
716+
var appPath = Path.Combine(updaterDir, _configInfo.UpdateAppName);
717+
if (!File.Exists(appPath))
718+
throw new FileNotFoundException($"Upgrade application not found: {appPath}");
719+
GeneralTracer.Info($"ClientStrategy: launching upgrade process {appPath}");
720+
Process.Start(appPath);
721+
}
722+
677723
#endregion
678724

679725
#region Helpers

0 commit comments

Comments
 (0)