Introduce JsonManager for Auto-Save

Author: carlos-zamora

src/cascadia/TerminalApp/AppLogic.cpp

@@ -302,38 +302,30 @@ namespace winrt::TerminalApp::implementation
     // - <none>
     void AppLogic::_RegisterSettingsChange()
     {
-        const std::filesystem::path settingsPath{ std::wstring_view{ CascadiaSettings::SettingsPath() } };
-        _reader.create(
-            settingsPath.parent_path().c_str(),
-            false,
-            // We want file modifications, AND when files are renamed to be
-            // settings.json. This second case will often happen with text
-            // editors, who will write a temp file, then rename it to be the
-            // actual file you wrote. So listen for that too.
-            wil::FolderChangeEvents::FileName | wil::FolderChangeEvents::LastWriteTime,
-            [this, settingsBasename = settingsPath.filename()](wil::FolderChangeEvent, PCWSTR fileModified) {
-                // DO NOT create a static reference to ApplicationState::SharedInstance here.
-                //
-                // ApplicationState::SharedInstance already caches its own
-                // static ref. If _we_ keep a static ref to the member in
-                // AppState, then our reference will keep ApplicationState alive
-                // after the `ActionToStringMap` gets cleaned up. Then, when we
-                // try to persist the actions in the window state, we won't be
-                // able to. We'll try to look up the action and the map just
-                // won't exist. We'll explode, even though the Terminal is
-                // tearing down anyways. So we'll just die, but still invoke
-                // WinDBG's post-mortem debugger, who won't be able to attach to
-                // the process that's already exiting.
-                //
-                // So DON'T ~give a mouse a cookie~ take a static ref here.
-
-                const auto modifiedBasename = std::filesystem::path{ fileModified }.filename();
-
-                if (modifiedBasename == settingsBasename)
-                {
-                    ReloadSettingsThrottled();
-                }
-            });
+        // The JsonManager owns the settings.json watcher and the auto-save write
+        // path. It raises SettingsChangedExternally (on a background
+        // thread) when it detects an edit it didn't make; we marshal a throttled
+        // reload onto the UI thread in response.
+        _jsonManager = Settings::Model::JsonManager{};
+        _jsonManager.SettingsChangedExternally([weakSelf = get_weak()](auto&&, auto&&) {
+            if (auto self{ weakSelf.get() })
+            {
+                self->ReloadSettingsThrottled();
+            }
+        });
+        _jsonManager.Start();
+
+        // Bind the live tree for auto-save -- but only if the initial load
+        // actually succeeded. We must never auto-save the defaults fallback over
+        // a user's broken settings.json.
+        if (SUCCEEDED(_settingsLoadedResult))
+        {
+            _jsonManager.SetLiveSettings(_settings);
+        }
+        else
+        {
+            _jsonManager.ClearLiveSettings();
+        }
     }
 
     void AppLogic::_ApplyLanguageSettingChange() noexcept
@@ -389,6 +381,11 @@ namespace winrt::TerminalApp::implementation
             if (initialLoad)
             {
                 _settings = CascadiaSettings::LoadDefaults();
+                // Don't auto-save the defaults over the user's broken file.
+                if (_jsonManager)
+                {
+                    _jsonManager.ClearLiveSettings();
+                }
             }
             else
             {
@@ -409,6 +406,11 @@ namespace winrt::TerminalApp::implementation
         else
         {
             _settings.LogSettingChanges(true);
+            // Rebind the freshly-loaded tree for auto-save.
+            if (_jsonManager)
+            {
+                _jsonManager.SetLiveSettings(_settings);
+            }
         }
 
         _ApplyLanguageSettingChange();

src/cascadia/TerminalApp/AppLogic.h

@@ -72,7 +72,9 @@ namespace winrt::TerminalApp::implementation
         // These fields invoke _reloadSettings and must be destroyed before _reloadSettings.
         // (C++ destroys members in reverse-declaration-order.)
         winrt::com_ptr<LanguageProfileNotifier> _languageProfileNotifier;
-        wil::unique_folder_change_reader_nothrow _reader;
+        // Owns the settings.json watcher + auto-save write path.
+        // Raises SettingsChangedExternally, which we handle by reloading.
+        Microsoft::Terminal::Settings::Model::JsonManager _jsonManager{ nullptr };
 
         TerminalApp::ContentManager _contentManager{ winrt::make<implementation::ContentManager>() };
 

src/cascadia/TerminalSettingsModel/CascadiaSettings.h

@@ -170,6 +170,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         void ResetApplicationState() const;
         void ResetToDefaultSettings();
         bool WriteSettingsToDisk();
+        void SetWriteHandler(std::function<void()> handler);
+
         Json::Value ToJson() const;
         Model::Profile ProfileDefaults() const;
         Model::Profile CreateNewProfile();
@@ -208,6 +210,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         void _refreshDefaultTerminals();
         void _writeSettingsToDisk(std::string_view contents);
 
+        // Auto-save (GH#12424): the shared "request auto-save" sink installed on
+        // every inheritable object in this live tree. Empty until SetWriteHandler.
+        std::shared_ptr<std::function<void()>> _writeSink;
+        void _installWriteSink();
+
         void _resolveDefaultProfile() const;
         void _resolveNewTabMenuProfiles() const;
         void _resolveNewTabMenuProfilesSet(const winrt::Windows::Foundation::Collections::IVectorView<Model::NewTabMenuEntry>& entries, winrt::Windows::Foundation::Collections::IMap<int, Model::Profile>& remainingProfiles, Model::RemainingProfilesEntry& remainingProfilesEntry) const;

src/cascadia/TerminalSettingsModel/CascadiaSettingsSerialization.cpp

@@ -1484,6 +1484,62 @@ CascadiaSettings::CascadiaSettings(SettingsLoader&& loader) :
     _validateSettings();
 
     ExpandCommands();
+
+    _installWriteSink();
+}
+
+// Auto-save: create one shared sink and install it on every
+// inheritable object in the live tree so that mutating any setting requests a
+// debounced save. The sink stays a no-op until SetWriteHandler() binds a
+// handler (which JsonManager only does for a successfully-loaded live tree).
+// Copy()/clone paths construct fresh objects without a sink, so editor clones
+// and the defaults fallback never auto-save.
+void CascadiaSettings::_installWriteSink()
+{
+    _writeSink = std::make_shared<std::function<void()>>();
+
+    const auto installForProfile = [&](implementation::Profile* prof) {
+        if (!prof)
+        {
+            return;
+        }
+        prof->SetWriteSettingsSink(_writeSink);
+        if (const auto da = prof->DefaultAppearance())
+        {
+            winrt::get_self<AppearanceConfig>(da)->SetWriteSettingsSink(_writeSink);
+        }
+        if (const auto ua = prof->UnfocusedAppearance())
+        {
+            winrt::get_self<AppearanceConfig>(ua)->SetWriteSettingsSink(_writeSink);
+        }
+        if (const auto fi = prof->FontInfo())
+        {
+            winrt::get_self<FontConfig>(fi)->SetWriteSettingsSink(_writeSink);
+        }
+    };
+
+    if (_globals)
+    {
+        _globals->SetWriteSettingsSink(_writeSink);
+        if (const auto ntm = _globals->NewTabMenu())
+        {
+            winrt::get_self<implementation::NewTabMenu>(ntm)->SetWriteSettingsSink(_writeSink);
+        }
+    }
+
+    installForProfile(_baseLayerProfile.get());
+    for (const auto& p : _allProfiles)
+    {
+        installForProfile(winrt::get_self<implementation::Profile>(p));
+    }
+}
+
+void CascadiaSettings::SetWriteHandler(std::function<void()> handler)
+{
+    if (_writeSink)
+    {
+        *_writeSink = std::move(handler);
+    }
 }
 
 // Method Description:
@@ -1513,10 +1569,7 @@ const std::filesystem::path& CascadiaSettings::_releaseSettingsPath()
 // Returns a has (approximately) uniquely identifying the settings.json contents on disk.
 winrt::hstring CascadiaSettings::_calculateHash(std::string_view settings, const FILETIME& lastWriteTime)
 {
-    const auto fileHash = til::hash(settings);
-    const ULARGE_INTEGER fileTime{ lastWriteTime.dwLowDateTime, lastWriteTime.dwHighDateTime };
-    const auto hash = fmt::format(FMT_COMPILE(L"{:016x}-{:016x}"), fileHash, fileTime.QuadPart);
-    return winrt::hstring{ hash };
+    return CalculateSettingsHash(settings, lastWriteTime);
 }
 
 // This returns something akin to %LOCALAPPDATA%\Packages\WindowsTerminalDev_8wekyb3d8bbwe\LocalState
@@ -1613,7 +1666,9 @@ bool CascadiaSettings::WriteSettingsToDisk()
 void CascadiaSettings::_writeSettingsToDisk(std::string_view contents)
 {
     FILETIME lastWriteTime{};
-    til::io::write_utf8_string_to_file_atomic(_settingsPath(), contents, &lastWriteTime);
+    // Auto-save (GH#12424): write atomically via a process-unique temp file and
+    // keep a best-effort "settings.json.bak" backup of the prior contents.
+    WriteSettingsFile(_settingsPath(), contents, /*makeBackup*/ true, &lastWriteTime);
 
     _hash = _calculateHash(contents, lastWriteTime);
 

src/cascadia/TerminalSettingsModel/FileUtils.cpp

@@ -8,6 +8,10 @@
 #include <shlobj.h>
 #include <WtExeUtils.h>
 
+#include <fmt/compile.h>
+#include <til/io.h>
+#include <til/hash.h>
+
 static constexpr std::wstring_view UnpackagedSettingsFolderName{ L"Microsoft\\Windows Terminal\\" };
 static constexpr std::wstring_view ReleaseSettingsFolder{ L"Packages\\Microsoft.WindowsTerminal_8wekyb3d8bbwe\\LocalState\\" };
 static constexpr std::wstring_view PortableModeMarkerFile{ L".portable" };
@@ -84,4 +88,66 @@ namespace winrt::Microsoft::Terminal::Settings::Model
         }();
         return baseSettingsPath;
     }
+
+    // Computes the hash that (approximately) uniquely identifies a settings
+    // file's contents on disk. Combines a content hash with the file's last
+    // write time.
+    winrt::hstring CalculateSettingsHash(std::string_view content, const FILETIME& lastWriteTime)
+    {
+        const auto fileHash = til::hash(content);
+        const ULARGE_INTEGER fileTime{ lastWriteTime.dwLowDateTime, lastWriteTime.dwHighDateTime };
+        const auto hash = fmt::format(FMT_COMPILE(L"{:016x}-{:016x}"), fileHash, fileTime.QuadPart);
+        return winrt::hstring{ hash };
+    }
+
+    // Atomically writes settings content to disk:
+    //   1. (optional) best-effort backup of the current file to "<path>.bak"
+    //   2. write to a process-unique temp file in the same directory
+    //   3. atomically replace the target via rename
+    // The unique temp name avoids cross-process temp clobbering when multiple
+    // Terminal instances auto-save concurrently. The ".bak" is best-effort and
+    // diagnostic only -- it is not a reliable conflict-resolution mechanism.
+    void WriteSettingsFile(const std::filesystem::path& path, std::string_view content, bool makeBackup, FILETIME* lastWriteTime)
+    {
+        // GH#10787: rename() replaces symbolic links themselves, not the path
+        // they point at. Resolve them first before generating the temp path.
+        std::error_code ec;
+        const auto resolvedPath = std::filesystem::is_symlink(path) ? std::filesystem::canonical(path, ec) : path;
+        if (ec)
+        {
+            if (ec.value() != ERROR_FILE_NOT_FOUND)
+            {
+                THROW_WIN32_MSG(ec.value(), "failed to compute canonical settings path");
+            }
+            // The original file is a symbolic link whose target doesn't exist.
+            // Fall back to a direct (non-atomic) write; this is an edge case.
+            til::io::write_utf8_string_to_file(path, content, false, lastWriteTime);
+            return;
+        }
+
+        if (makeBackup)
+        {
+            std::error_code backupEc;
+            auto backupPath = resolvedPath;
+            backupPath += L".bak";
+            // Best-effort: ignore failures (e.g. the file doesn't exist yet).
+            std::filesystem::copy_file(resolvedPath, backupPath, std::filesystem::copy_options::overwrite_existing, backupEc);
+        }
+
+        // Process- and call-unique temp name to avoid cross-process clobber.
+        static std::atomic<uint32_t> counter{ 0 };
+        auto tmpPath = resolvedPath;
+        tmpPath += fmt::format(FMT_COMPILE(L".{}.{}.tmp"), GetCurrentProcessId(), counter.fetch_add(1, std::memory_order_relaxed));
+
+        til::io::write_utf8_string_to_file(tmpPath, content, false, lastWriteTime);
+
+        // renaming is (close enough to) atomic.
+        std::filesystem::rename(tmpPath, resolvedPath, ec);
+        if (ec)
+        {
+            std::error_code cleanupEc;
+            std::filesystem::remove(tmpPath, cleanupEc);
+            THROW_WIN32_MSG(ec.value(), "failed to write settings file");
+        }
+    }
 }

src/cascadia/TerminalSettingsModel/FileUtils.h

@@ -6,4 +6,6 @@ namespace winrt::Microsoft::Terminal::Settings::Model
     bool IsPortableMode();
     std::filesystem::path GetBaseSettingsPath();
     std::filesystem::path GetReleaseSettingsPath();
+    winrt::hstring CalculateSettingsHash(std::string_view content, const FILETIME& lastWriteTime);
+    void WriteSettingsFile(const std::filesystem::path& path, std::string_view content, bool makeBackup, FILETIME* lastWriteTime = nullptr);
 }