Fix what's-new re-appearing after a failed update (held for next release)
Replace the running-version-vs-saved-version trigger for the "what's new" popup with a one-shot marker the updater writes ONLY on a successful update (UpdateApplier success path). A failed/rolled-back update never writes it (and clears any stale one), so it can no longer re-trigger what's-new — the old best-effort flag save could lose a race during the update churn and leave the version mismatched, which was the bug. New WhatsNewMarker seam (Write/Exists/Consume) + a SelfTest case for the consume-once contract. MaybeShowWhatsNewAfterUpdate now shows iff the marker is present, then deletes it; still records LastWhatsNewVersion for the import-offer's upgrade detection. Not released yet — bundling with the connection-retry (#15) work in the next release. No version bump, no manual change (internal fix). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
36431d6d39
commit
e710834458
@@ -1580,28 +1580,26 @@ public sealed class MainForm : Form
|
||||
return n;
|
||||
}
|
||||
|
||||
/// <summary>If the user opted in (<see cref="AppConfig.ShowWhatsNewAfterUpdate"/>) and the
|
||||
/// running version changed since the last launch we recorded, open the About box once so
|
||||
/// they see what changed in the update just installed. Always records the current version
|
||||
/// so the change is detected exactly once. A fresh install (no version recorded yet) does
|
||||
/// NOT count as an update. 2026-05-31.</summary>
|
||||
/// <summary>Show the About box once after a SUCCESSFUL in-app update, if the user opted in. Driven by
|
||||
/// a one-shot marker the updater writes only on success (<see cref="RemSoundUpdater.WhatsNewMarkerName"/>
|
||||
/// via <see cref="WhatsNewMarker"/>) — NOT by a running-version-vs-saved-version compare, which could
|
||||
/// re-fire after a FAILED update when its best-effort flag save lost a race during the update churn
|
||||
/// (that was the bug). The marker is consumed (deleted) here exactly once. Separately records
|
||||
/// LastWhatsNewVersion as the "a version has run here" signal the keyboard-shortcut import offer uses
|
||||
/// to tell an upgrade from a fresh install. 2026-06-23.</summary>
|
||||
private void MaybeShowWhatsNewAfterUpdate()
|
||||
{
|
||||
if (IsDisposed) return;
|
||||
var current = System.Reflection.Assembly.GetExecutingAssembly().GetName().Version?.ToString(3) ?? "";
|
||||
|
||||
AppConfig cfg;
|
||||
try { cfg = AppConfig.Load(); }
|
||||
catch { return; }
|
||||
var justUpdated = WhatsNewMarker.Exists(AppContext.BaseDirectory);
|
||||
var cfg = AppConfig.Load();
|
||||
|
||||
var versionChanged = !string.IsNullOrEmpty(cfg.LastWhatsNewVersion)
|
||||
&& cfg.LastWhatsNewVersion != current;
|
||||
|
||||
if (cfg.ShowWhatsNewAfterUpdate && versionChanged)
|
||||
if (justUpdated && cfg.ShowWhatsNewAfterUpdate)
|
||||
{
|
||||
try
|
||||
{
|
||||
logFile.Event($"what's new: opening About after update {cfg.LastWhatsNewVersion} -> {current}");
|
||||
logFile.Event($"what's new: opening About after a successful update (now v{current})");
|
||||
using var dlg = new AboutDialog();
|
||||
ForegroundDialog.Show(owner => dlg.ShowDialog(owner));
|
||||
}
|
||||
@@ -1611,8 +1609,15 @@ public sealed class MainForm : Form
|
||||
}
|
||||
}
|
||||
|
||||
// Record the current version so the next change is detected once. Reload first so we
|
||||
// don't clobber a concurrent config write (e.g. the startup update-check timestamp).
|
||||
// Consume the marker so what's-new shows exactly once. Deleted whether or not we showed it, and
|
||||
// only ever present after a genuine success — a failed update simply has nothing here to re-trigger.
|
||||
if (justUpdated && !WhatsNewMarker.Consume(AppContext.BaseDirectory))
|
||||
{
|
||||
logFile.Event("what's new: could not delete the update marker (will re-show next launch)");
|
||||
}
|
||||
|
||||
// Record "a version has run on this machine" for the upgrade-vs-fresh-install detection used by
|
||||
// MaybeOfferKeyboardShortcutImport. Best-effort; no longer drives the what's-new popup.
|
||||
if (cfg.LastWhatsNewVersion != current)
|
||||
{
|
||||
try
|
||||
@@ -1621,7 +1626,7 @@ public sealed class MainForm : Form
|
||||
fresh.LastWhatsNewVersion = current;
|
||||
fresh.Save();
|
||||
}
|
||||
catch { /* harmless — at worst we re-show next launch */ }
|
||||
catch { /* harmless */ }
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -168,6 +168,14 @@ internal sealed class RemSoundUpdater : IDisposable
|
||||
/// drops the user back into the same profile they were running rather than at the picker.</summary>
|
||||
public const string ResumeProfileSentinelName = "_resume-after-update.txt";
|
||||
|
||||
/// <summary>Filename of the one-shot "an update just succeeded — show what's new once" marker.
|
||||
/// Written into the install folder by <see cref="UpdateApplier"/> ONLY on a successful update
|
||||
/// (never on a failed/rolled-back one); read and deleted by MainForm on the next startup. This is
|
||||
/// the positive signal that drives the "what's new after an update" popup — so a FAILED update can't
|
||||
/// trigger it. (The old running-version-vs-saved-version compare could re-fire after a failure when
|
||||
/// its best-effort flag save lost a race during the update churn — that was the bug.)</summary>
|
||||
public const string WhatsNewMarkerName = "_whats-new-after-update.txt";
|
||||
|
||||
/// <summary>Download the update ZIP, stage it to a per-user temp folder, and launch the new
|
||||
/// version's in-app installer (<see cref="UpdateApplier"/>) to take over once this process
|
||||
/// exits. Returns true if the installer was launched (caller should Application.Exit
|
||||
@@ -196,9 +204,11 @@ internal sealed class RemSoundUpdater : IDisposable
|
||||
var zipPath = Path.Combine(stageRoot, $"RemSound-update-{info.Tag}.zip");
|
||||
Directory.CreateDirectory(appDir);
|
||||
|
||||
// This attempt starts clean: clear any stale failure marker / resume sentinel in the install.
|
||||
// This attempt starts clean: clear any stale failure marker / resume sentinel / what's-new
|
||||
// marker in the install.
|
||||
TryDelete(Path.Combine(installDir, "update-failed.txt"));
|
||||
TryDelete(Path.Combine(installDir, ResumeProfileSentinelName));
|
||||
TryDelete(Path.Combine(installDir, WhatsNewMarkerName));
|
||||
|
||||
Log?.Invoke($"updater: downloading {info.DownloadUrl}");
|
||||
await using (var src = await http.GetStreamAsync(info.DownloadUrl, token).ConfigureAwait(false))
|
||||
|
||||
@@ -57,6 +57,7 @@ internal static class SelfTest
|
||||
RunStep(results, "Server wire-format compatibility", ServerWireCompat);
|
||||
RunStep(results, "App settings save and reload", SettingsRoundTrip);
|
||||
RunStep(results, "Profile save and reload", ProfileRoundTrip);
|
||||
RunStep(results, "What's-new update marker", WhatsNewMarkerRoundTrip);
|
||||
RunStep(results, "Diagnostics report privacy", DiagnosticsPrivacy);
|
||||
RunStep(results, "Bundled resources present", ResourcesPresent);
|
||||
RunStep(results, "Dialog accessibility (names + mnemonics)", AccessibilityAudit);
|
||||
@@ -131,6 +132,29 @@ internal static class SelfTest
|
||||
return "AES-256-GCM, PBKDF2 fingerprint, on-disk scramble";
|
||||
}
|
||||
|
||||
/// <summary>The "what's new after a successful update" marker round-trips: present after Write,
|
||||
/// Consume removes it exactly once, and a second Consume is a no-op. This is the contract the bug
|
||||
/// fix rests on — a failed update writes no marker (no popup); a success writes one (shown once).</summary>
|
||||
private static string? WhatsNewMarkerRoundTrip()
|
||||
{
|
||||
var dir = Path.Combine(Path.GetTempPath(), "rs-selftest-whatsnew-" + Guid.NewGuid().ToString("N"));
|
||||
Directory.CreateDirectory(dir);
|
||||
try
|
||||
{
|
||||
Check(!WhatsNewMarker.Exists(dir), "a fresh folder must have no marker");
|
||||
WhatsNewMarker.Write(dir);
|
||||
Check(WhatsNewMarker.Exists(dir), "marker must exist after Write");
|
||||
Check(WhatsNewMarker.Consume(dir), "Consume must report it removed the marker");
|
||||
Check(!WhatsNewMarker.Exists(dir), "marker must be gone after Consume");
|
||||
Check(!WhatsNewMarker.Consume(dir), "a second Consume must be a no-op (shown exactly once)");
|
||||
return "write / exists / consume-once / idempotent";
|
||||
}
|
||||
finally
|
||||
{
|
||||
try { Directory.Delete(dir, recursive: true); } catch { /* best-effort temp cleanup */ }
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>The packet header writes and reads back for every type, and malformed packets
|
||||
/// (too short, bad magic, wrong version) are rejected rather than mis-parsed. Plus the PCM
|
||||
/// multi-part sub-header round-trips.</summary>
|
||||
|
||||
@@ -72,6 +72,7 @@ internal static class UpdateApplier
|
||||
RollBack(moved, created, Log);
|
||||
TryDeleteDirectory(backupDir); // restored files were moved back out; drop the empty backup tree
|
||||
WriteFailureMarker(target, logPath);
|
||||
WhatsNewMarker.Consume(target); // a rolled-back update must never trigger "what's new"
|
||||
Log("rolled back to previous version" + (noRestart ? "" : "; restarting it"));
|
||||
if (!noRestart) RestartApp(target, resumeProfile, Log); // old version restored intact — safe to relaunch
|
||||
CleanupStage(stageRoot, Log);
|
||||
@@ -80,6 +81,10 @@ internal static class UpdateApplier
|
||||
|
||||
TryDeleteDirectory(backupDir);
|
||||
WriteResumeSentinel(target, resumeProfile, Log);
|
||||
// Positive one-shot signal that the update genuinely succeeded — drives the next launch's
|
||||
// "what's new" popup. Written ONLY here, on the success path.
|
||||
try { WhatsNewMarker.Write(target); Log("wrote what's-new marker"); }
|
||||
catch (Exception ex) { Log($"could not write what's-new marker: {ex.Message}"); }
|
||||
Log("apply-update OK" + (noRestart ? "" : " — restarting RemSound"));
|
||||
if (!noRestart) RestartApp(target, resumeProfile, Log);
|
||||
CleanupStage(stageRoot, Log);
|
||||
|
||||
@@ -0,0 +1,46 @@
|
||||
namespace RemSound.App;
|
||||
|
||||
/// <summary>
|
||||
/// The one-shot "an update just succeeded" marker that drives the "what's new" popup. The in-app
|
||||
/// updater (<see cref="UpdateApplier"/>) writes it into the install folder ONLY when a swap completes
|
||||
/// successfully — never on a failed/rolled-back update — and MainForm consumes it (shows About once,
|
||||
/// then deletes it) on the next launch. Because the signal is positive and written only on success, a
|
||||
/// failed update can't re-trigger what's-new, which the old version-compare flag could.
|
||||
///
|
||||
/// Kept as a tiny seam (rather than inline file calls) so the consume contract is unit-testable: write
|
||||
/// it into a throwaway folder, assert Exists, Consume once (true), then Exists is false and a second
|
||||
/// Consume is false. See the SelfTest "what's-new marker" case.
|
||||
/// </summary>
|
||||
internal static class WhatsNewMarker
|
||||
{
|
||||
private static string PathFor(string baseDir) => Path.Combine(baseDir, RemSoundUpdater.WhatsNewMarkerName);
|
||||
|
||||
/// <summary>True if the success marker is present in <paramref name="baseDir"/> (the install folder).</summary>
|
||||
public static bool Exists(string baseDir)
|
||||
{
|
||||
try { return File.Exists(PathFor(baseDir)); }
|
||||
catch { return false; }
|
||||
}
|
||||
|
||||
/// <summary>Write the marker into <paramref name="baseDir"/>. Called by the updater on success only.</summary>
|
||||
public static void Write(string baseDir)
|
||||
{
|
||||
File.WriteAllText(PathFor(baseDir),
|
||||
"One-shot marker: RemSound finished a successful update. \"What's new\" shows once on the next\r\n"
|
||||
+ "launch, then this file is deleted. Safe to delete.\r\n");
|
||||
}
|
||||
|
||||
/// <summary>Delete the marker if present, so what's-new shows exactly once. Returns true if a marker
|
||||
/// was there (and is now gone); a delete is reliable in a way the old best-effort config-save wasn't.</summary>
|
||||
public static bool Consume(string baseDir)
|
||||
{
|
||||
try
|
||||
{
|
||||
var path = PathFor(baseDir);
|
||||
if (!File.Exists(path)) return false;
|
||||
File.Delete(path);
|
||||
return true;
|
||||
}
|
||||
catch { return false; }
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user