diff --git a/src/RemSound.App/MainForm.cs b/src/RemSound.App/MainForm.cs index 1605eec..787a6fb 100644 --- a/src/RemSound.App/MainForm.cs +++ b/src/RemSound.App/MainForm.cs @@ -1580,28 +1580,26 @@ public sealed class MainForm : Form return n; } - /// If the user opted in () 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. + /// 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 ( + /// via ) — 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. 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 */ } } } diff --git a/src/RemSound.App/RemSoundUpdater.cs b/src/RemSound.App/RemSoundUpdater.cs index ce5b70f..4c5622b 100644 --- a/src/RemSound.App/RemSoundUpdater.cs +++ b/src/RemSound.App/RemSoundUpdater.cs @@ -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. public const string ResumeProfileSentinelName = "_resume-after-update.txt"; + /// Filename of the one-shot "an update just succeeded — show what's new once" marker. + /// Written into the install folder by 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.) + public const string WhatsNewMarkerName = "_whats-new-after-update.txt"; + /// Download the update ZIP, stage it to a per-user temp folder, and launch the new /// version's in-app installer () 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)) diff --git a/src/RemSound.App/SelfTest.cs b/src/RemSound.App/SelfTest.cs index 2ec3893..2e0e653 100644 --- a/src/RemSound.App/SelfTest.cs +++ b/src/RemSound.App/SelfTest.cs @@ -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"; } + /// 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). + 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 */ } + } + } + /// 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. diff --git a/src/RemSound.App/UpdateApplier.cs b/src/RemSound.App/UpdateApplier.cs index 040ab6f..4e0ec2f 100644 --- a/src/RemSound.App/UpdateApplier.cs +++ b/src/RemSound.App/UpdateApplier.cs @@ -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); diff --git a/src/RemSound.App/WhatsNewMarker.cs b/src/RemSound.App/WhatsNewMarker.cs new file mode 100644 index 0000000..07aaf79 --- /dev/null +++ b/src/RemSound.App/WhatsNewMarker.cs @@ -0,0 +1,46 @@ +namespace RemSound.App; + +/// +/// The one-shot "an update just succeeded" marker that drives the "what's new" popup. The in-app +/// updater () 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. +/// +internal static class WhatsNewMarker +{ + private static string PathFor(string baseDir) => Path.Combine(baseDir, RemSoundUpdater.WhatsNewMarkerName); + + /// True if the success marker is present in (the install folder). + public static bool Exists(string baseDir) + { + try { return File.Exists(PathFor(baseDir)); } + catch { return false; } + } + + /// Write the marker into . Called by the updater on success only. + 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"); + } + + /// 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. + 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; } + } +}