Skip to content

Commit 5363fed

Browse files
JusterZhuclaude
andcommitted
refactor: make SilentPollOrchestrator delegate to ClientStrategy instead of reimplementing update logic
The silent update flow was a completely separate reimplementation of the standard ClientStrategy workflow, leading to three critical bugs: 1. DiffPipeline was never injected into the strategy, causing PatchMiddleware to throw InvalidOperationException on every poll cycle (infinite retry loop) 2. OnProcessExit unconditionally launched the upgrade process even when only Upgrade packages were applied (unnecessary app restart) 3. Download orchestrator was created without DownloadOrchestratorOptions, ignoring user-configured timeout, concurrency, retry, and checksum settings Solution: - ClientStrategy: add LaunchAfterPrepare flag (default true) and HasPreparedClientUpdate property - GeneralUpdateBootstrap: extract ConfigureStrategy() shared by standard and silent paths, ensuring identical extension injection - SilentPollOrchestrator: rewritten from ~600 to ~175 lines; now a thin scheduling layer that delegates all update work to ClientStrategy - UpdateOptions: add LaunchClientAfterUpdate option for silent mode Files changed: 5 | +208 -596 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 39a2c75 commit 5363fed

5 files changed

Lines changed: 208 additions & 596 deletions

File tree

src/c#/GeneralUpdate.Core/Bootstrap/GeneralUpdateBootstrap.cs

Lines changed: 88 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -135,76 +135,10 @@ private async Task<GeneralUpdateBootstrap> LaunchWithStrategy(IStrategy roleStra
135135
var authProvider = ResolveExtension<Security.IHttpAuthProvider>();
136136
if (authProvider != null) Network.VersionService.SetDefaultAuthProvider(authProvider);
137137

138-
// Resolve hooks from extensions
139-
var hooks = ResolveExtension<Hooks.IUpdateHooks>() ?? new Hooks.NoOpUpdateHooks();
140-
roleStrategy.Hooks = hooks;
138+
ConfigureStrategy(roleStrategy);
141139

142-
// Resolve reporter from extensions; default to HttpUpdateReporter.
143-
// When ReportUrl is not configured, HttpUpdateReporter is effectively a no-op.
144-
var reporter = ResolveExtension<Download.Reporting.IUpdateReporter>() ??
145-
new Download.Reporting.HttpUpdateReporter();
146-
147-
if (reporter is Download.Reporting.HttpUpdateReporter httpReporter)
148-
httpReporter.ReportUrl = _configInfo.ReportUrl;
149-
150-
roleStrategy.Reporter = reporter;
151-
152-
// ── Download components ──
153-
var downloadOrchestrator = ResolveExtension<Download.Abstractions.IDownloadOrchestrator>();
154-
var downloadPolicy = ResolveExtension<Download.Abstractions.IDownloadPolicy>();
155-
var downloadExecutor = ResolveExtension<Download.Abstractions.IDownloadExecutor>();
156-
157-
// Build download pipeline factory from registered extension type
158-
Func<string?, Download.Abstractions.IDownloadPipeline>? downloadPipelineFactory = null;
159-
var pipelineType = ResolveExtensionType<Download.Abstractions.IDownloadPipeline>();
160-
if (pipelineType != null)
161-
{
162-
var stringCtor = pipelineType.GetConstructor([typeof(string)]);
163-
if (stringCtor != null)
164-
downloadPipelineFactory = hash => (Download.Abstractions.IDownloadPipeline)stringCtor.Invoke([hash]);
165-
else
166-
downloadPipelineFactory = _ => (Download.Abstractions.IDownloadPipeline)Activator.CreateInstance(pipelineType);
167-
}
168-
169-
var diffPipeline = BuildDiffPipeline();
170-
171-
switch (roleStrategy)
172-
{
173-
case ClientStrategy cs:
174-
cs.DownloadSource = ResolveExtension<Download.Abstractions.IDownloadSource>();
175-
176-
if (_updatePrecheck != null)
177-
cs.UseUpdatePrecheck(_updatePrecheck);
178-
179-
await CallSmallBowlHomeAsync(_configInfo.Bowl).ConfigureAwait(false);
180-
181-
cs.SetDiffPipeline(diffPipeline);
182-
if (downloadOrchestrator != null) cs.SetOrchestrator(downloadOrchestrator);
183-
if (downloadPolicy != null) cs.SetDownloadPolicy(downloadPolicy);
184-
if (downloadExecutor != null) cs.SetDownloadExecutor(downloadExecutor);
185-
if (downloadPipelineFactory != null) cs.SetDownloadPipelineFactory(downloadPipelineFactory);
186-
break;
187-
188-
case UpdateStrategy us:
189-
us.SetDiffPipeline(diffPipeline);
190-
us.SetReportType(_configInfo.ReportType);
191-
break;
192-
}
193-
194-
// Inject custom OS-level strategy if registered via Strategy<T>()
195-
var customOsStrategy = ResolveExtension<IStrategy>();
196-
if (customOsStrategy != null)
197-
{
198-
switch (roleStrategy)
199-
{
200-
case ClientStrategy cs:
201-
cs.SetOsStrategy(customOsStrategy);
202-
break;
203-
case UpdateStrategy us:
204-
us.SetOsStrategy(customOsStrategy);
205-
break;
206-
}
207-
}
140+
if (roleStrategy is ClientStrategy cs)
141+
await CallSmallBowlHomeAsync(_configInfo.Bowl).ConfigureAwait(false);
208142

209143
roleStrategy.Create(_configInfo);
210144

@@ -420,34 +354,103 @@ private void ApplyRuntimeOptions()
420354

421355
/// <summary>
422356
/// Silent update mode — starts a background poll loop and returns immediately.
423-
/// The orchestrator checks for updates periodically and prepares them.
424-
/// When the host process exits, the prepared update is applied.
357+
/// Uses the same fully-configured <see cref="ClientStrategy"/> as the standard flow;
358+
/// the orchestrator only adds polling and deferred-upgrade-on-exit on top.
425359
/// </summary>
426-
private async Task LaunchSilentAsync()
360+
private async Task<GeneralUpdateBootstrap> LaunchSilentAsync()
427361
{
428362
GeneralTracer.Info("GeneralUpdateBootstrap: starting silent update mode.");
363+
364+
ApplyRuntimeOptions();
365+
366+
// Network-level extensions (global, applied before any HTTP call)
367+
var sslPolicy = ResolveExtension<Security.ISslValidationPolicy>();
368+
if (sslPolicy != null) Network.VersionService.SetSslValidationPolicy(sslPolicy);
369+
var authProvider = ResolveExtension<Security.IHttpAuthProvider>();
370+
if (authProvider != null) Network.VersionService.SetDefaultAuthProvider(authProvider);
371+
372+
var strategy = new ClientStrategy();
373+
ConfigureStrategy(strategy);
374+
375+
strategy.LaunchAfterPrepare = false;
376+
429377
var pollMinutes = GetOption(UpdateOptions.SilentPollIntervalMinutes);
378+
var launchClient = GetOption(UpdateOptions.LaunchClientAfterUpdate);
430379
var silentOptions = new Silent.SilentOptions
431380
{
432-
PollInterval = TimeSpan.FromMinutes(pollMinutes)
381+
PollInterval = TimeSpan.FromMinutes(pollMinutes),
382+
LaunchClientAfterUpdate = launchClient
433383
};
384+
385+
var orchestrator = new Silent.SilentPollOrchestrator(strategy, _configInfo, silentOptions);
386+
await orchestrator.StartAsync().ConfigureAwait(false);
387+
GeneralTracer.Info("GeneralUpdateBootstrap: silent update mode started, returning to caller.");
388+
389+
return this;
390+
}
391+
392+
/// <summary>
393+
/// Applies all registered extensions (hooks, reporter, download components,
394+
/// DiffPipeline, custom OS strategy) to a role strategy. Shared by the standard
395+
/// and silent boot paths so they are always configured identically.
396+
/// </summary>
397+
private void ConfigureStrategy(IStrategy roleStrategy)
398+
{
434399
var hooks = ResolveExtension<Hooks.IUpdateHooks>() ?? new Hooks.NoOpUpdateHooks();
400+
roleStrategy.Hooks = hooks;
401+
435402
var reporter = ResolveExtension<Download.Reporting.IUpdateReporter>() ??
436403
new Download.Reporting.HttpUpdateReporter();
437-
if (reporter is Download.Reporting.HttpUpdateReporter httpRep)
438-
httpRep.ReportUrl = _configInfo.ReportUrl;
439-
var sslPolicy = ResolveExtension<Security.ISslValidationPolicy>();
440-
if (sslPolicy != null) Network.VersionService.SetSslValidationPolicy(sslPolicy);
441-
var authProvider = ResolveExtension<Security.IHttpAuthProvider>();
442-
if (authProvider != null) Network.VersionService.SetDefaultAuthProvider(authProvider);
404+
if (reporter is Download.Reporting.HttpUpdateReporter httpReporter)
405+
httpReporter.ReportUrl = _configInfo.ReportUrl;
406+
roleStrategy.Reporter = reporter;
443407

444-
var orchestrator = new Silent.SilentPollOrchestrator(_configInfo, silentOptions)
445-
.WithHooks(hooks)
446-
.WithReporter(reporter)
447-
.WithOsStrategy(ResolveExtension<IStrategy>());
408+
var diffPipeline = BuildDiffPipeline();
448409

449-
await orchestrator.StartAsync().ConfigureAwait(false);
450-
GeneralTracer.Info("GeneralUpdateBootstrap: silent update mode started, returning to caller.");
410+
// Download components
411+
var downloadOrchestrator = ResolveExtension<Download.Abstractions.IDownloadOrchestrator>();
412+
var downloadPolicy = ResolveExtension<Download.Abstractions.IDownloadPolicy>();
413+
var downloadExecutor = ResolveExtension<Download.Abstractions.IDownloadExecutor>();
414+
415+
Func<string?, Download.Abstractions.IDownloadPipeline>? downloadPipelineFactory = null;
416+
var pipelineType = ResolveExtensionType<Download.Abstractions.IDownloadPipeline>();
417+
if (pipelineType != null)
418+
{
419+
var stringCtor = pipelineType.GetConstructor([typeof(string)]);
420+
if (stringCtor != null)
421+
downloadPipelineFactory = hash => (Download.Abstractions.IDownloadPipeline)stringCtor.Invoke([hash]);
422+
else
423+
downloadPipelineFactory = _ => (Download.Abstractions.IDownloadPipeline)Activator.CreateInstance(pipelineType);
424+
}
425+
426+
switch (roleStrategy)
427+
{
428+
case ClientStrategy cs:
429+
cs.DownloadSource = ResolveExtension<Download.Abstractions.IDownloadSource>();
430+
cs.SetDiffPipeline(diffPipeline);
431+
if (downloadOrchestrator != null) cs.SetOrchestrator(downloadOrchestrator);
432+
if (downloadPolicy != null) cs.SetDownloadPolicy(downloadPolicy);
433+
if (downloadExecutor != null) cs.SetDownloadExecutor(downloadExecutor);
434+
if (downloadPipelineFactory != null) cs.SetDownloadPipelineFactory(downloadPipelineFactory);
435+
if (_updatePrecheck != null) cs.UseUpdatePrecheck(_updatePrecheck);
436+
break;
437+
438+
case UpdateStrategy us:
439+
us.SetDiffPipeline(diffPipeline);
440+
us.SetReportType(_configInfo.ReportType);
441+
break;
442+
}
443+
444+
// Custom OS-level strategy (registered via Strategy<T>())
445+
var customOsStrategy = ResolveExtension<IStrategy>();
446+
if (customOsStrategy != null)
447+
{
448+
switch (roleStrategy)
449+
{
450+
case ClientStrategy cs: cs.SetOsStrategy(customOsStrategy); break;
451+
case UpdateStrategy us: us.SetOsStrategy(customOsStrategy); break;
452+
}
453+
}
451454
}
452455

453456
private DiffPipeline BuildDiffPipeline()

src/c#/GeneralUpdate.Core/Configuration/UpdateOptions.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ public static class UpdateOptions
113113
/// </summary>
114114
public static UpdateOption<int> SilentPollIntervalMinutes { get; } = UpdateOption.ValueOf<int>("SILENTPOLLINTERVALMINUTES", 60);
115115

116+
/// <summary>
117+
/// Whether to automatically launch the client application after the upgrade process completes in silent mode.
118+
/// Defaults to <c>true</c>.
119+
/// </summary>
120+
public static UpdateOption<bool> LaunchClientAfterUpdate { get; } = UpdateOption.ValueOf<bool>("LAUNCHCLIENTAFTERUPDATE", true);
121+
116122
// ════════════════════════════════════════
117123
// Concurrency and Resume Options
118124
// ════════════════════════════════════════

0 commit comments

Comments
 (0)