Skip to content

Commit f6bc318

Browse files
JusterZhuclaude
andcommitted
fix: reorder hooks after asset build, compute LastVersion deterministically
- Move SafeOnBeforeUpdateAsync and SafeReportUpdateStartedAsync to after asset list construction so hooks see the real TargetVersion instead of an empty value (fixes Copilot comment 5). - Compute LastVersion via Max(new Version(...)) instead of relying on array ordering (assets[^1].Version). This is correct for both the local JSON path and custom IDownloadSource implementations whose ListAsync() may not return sorted results (fixes Copilot comment 4). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 750fe15 commit f6bc318

1 file changed

Lines changed: 17 additions & 11 deletions

File tree

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

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -324,15 +324,6 @@ private async Task ExecuteUpgradeAsync()
324324
if (!File.Exists(jsonPath) && DownloadSource == null)
325325
throw new FileNotFoundException($"Version config not found: {jsonPath}");
326326

327-
// Hooks: allow cancellation before download
328-
if (!await SafeOnBeforeUpdateAsync(ctx).ConfigureAwait(false))
329-
{
330-
GeneralTracer.Info("OssStrategy (upgrade): cancelled by hook.");
331-
return;
332-
}
333-
334-
await SafeReportUpdateStartedAsync(ctx).ConfigureAwait(false);
335-
336327
// Build download assets from version config or injected source
337328
List<DownloadAsset> assets;
338329
if (DownloadSource != null)
@@ -365,10 +356,25 @@ private async Task ExecuteUpgradeAsync()
365356
if (assets.Count == 0)
366357
throw new InvalidOperationException("No assets to download.");
367358

368-
// Update LastVersion so hooks (OnAfterUpdate, etc.) know the target version.
369-
_configInfo.LastVersion = assets[assets.Count - 1].Version;
359+
// Compute LastVersion deterministically via Version comparison
360+
// so hooks see the correct TargetVersion regardless of source ordering.
361+
_configInfo.LastVersion = assets
362+
.Select(a => new Version(a.Version))
363+
.Max()!
364+
.ToString();
370365
ctx = BuildUpdateContext();
371366

367+
// Hooks: allow cancellation before download. Called after assets are
368+
// built and LastVersion is set so OnBeforeUpdateAsync sees the real
369+
// TargetVersion for decision-making.
370+
if (!await SafeOnBeforeUpdateAsync(ctx).ConfigureAwait(false))
371+
{
372+
GeneralTracer.Info("OssStrategy (upgrade): cancelled by hook.");
373+
return;
374+
}
375+
376+
await SafeReportUpdateStartedAsync(ctx).ConfigureAwait(false);
377+
372378
GeneralTracer.Debug($"OssStrategy (upgrade): downloading {assets.Count} asset(s).");
373379
await DownloadAssetsAsync(assets, installPath).ConfigureAwait(false);
374380

0 commit comments

Comments
 (0)