fix(config,tray): review findings: profile seed, discard failures, phantom profiles, tray dialogs (#496)

docs/architecture/08-config-profiles.md

@@ -132,7 +132,10 @@ a node test (`npm run test:rules`) both read.
   unless `%LOCALAPPDATA%\Wind\session.keep` marks a restart Wind triggered itself (Restart Wind, or
   an engine change or profile switch with a model change while Wind runs one engine). It is
   consumed once.
-- Tray Quit compares the files and prompts Save, Discard or Cancel.
+- Tray Quit compares the files and prompts Save, Discard or Cancel. After the prompt it re-reads the
+  live ini under the write lock and stops if the active profile changed meanwhile; Discard re-reads
+  the profile file. A prompt that fails to open stops Quit (never a silent drop), and the prompt
+  cancels itself when the served Wind exits (`ServedWindExited`), so it never acts on a new Wind.
 
 **Switching** (Settings or the tray flyout, one implementation: `wind::SwitchProfile` in
 `src/config/profile_switch.h`; each surface only maps its result to its own messages):
@@ -161,11 +164,15 @@ outgoing profile is not mirrored into (`DoSwitchProfile(name, mirrorOutgoing=fal
 
 **Seeding.** `EnsureProfilesSeeded` runs at startup before the ini mtime is recorded. With no
 `profiles\` directory it creates one, saves the current settings as `Default.ini` and writes
-`profile=Default`. The directory is the latch, so a failed `Default.ini` write removes it again. On a
+`profile=Default`. The directory is the latch, so a failed `Default.ini` write removes it again, and
+the function returns false: `createProfile` then fails rather than create the directory itself. On a
 first run with no ini at all the seed text is `DefaultIniText()` (the commented template), so the
 live ini is the template plus `profile=Default`, not a one-line file that `LoadConfig` would accept
 as complete.
 
+**Listing** (`ListProfileFiles`) keeps only long names ending in `.ini`: `*.ini` also matches 8.3
+short names, so a backup such as `Work.ini_bak` would otherwise list as a phantom profile.
+
 **Names** become file names: `ProfileNameError` rejects path and control characters, leading or
 trailing dots and spaces, reserved device names and names over 40 characters. Matching is
 case-insensitive (`SameProfileName`).

docs/architecture/09-settings-ui.md

@@ -60,7 +60,7 @@ newline makes the reply invalid and the page waits forever.
 | `setConfig` | fire | Atomic write of one key to the live ini |
 | `setConfigPersist` | fire | Same, and the key in the active profile file (keybind captures) |
 | `saveSession` | reply `sessionSaved` | Write `MakeProfileText(live)` over the profile |
-| `discardSession` | reply `config` | Rewrite the live ini from the profile |
+| `discardSession` | reply `config` | Rewrite the live ini from the profile. The reply carries `discarded` (`false` when the ini or profile could not be read or the write failed; the page then shows "Couldn't discard" and stops the switch, close or quit it was for) |
 | `ready` | fire | Two frames after mount; the host logs launch-to-paint |
 | `window` | fire | `minimize`, `maximize` (toggles restore), `close`, `quitWind`, `restartWind` (writes `session.keep` first; replies `restartFailed` when Wind cannot be launched) |
 | `dirty` | fire | Unsaved flag, so `WM_CLOSE` can prompt |

src/config/profiles_io.h

@@ -38,8 +38,11 @@ inline std::vector<std::wstring> ListProfileFiles(const std::wstring& dir) {
     if (h == INVALID_HANDLE_VALUE) return names;
     do {
         if (fd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) continue;
+        // The pattern also matches 8.3 short names, so "Work.ini_bak" (short name WORK~1.INI) would
+        // come back here: accept only a long name that really ends in ".ini".
         std::wstring n = fd.cFileName;
-        if (n.size() > 4) names.push_back(n.substr(0, n.size() - 4));  // strip ".ini"
+        if (n.size() > 4 && _wcsicmp(n.c_str() + n.size() - 4, L".ini") == 0)
+            names.push_back(n.substr(0, n.size() - 4));  // strip ".ini"
     } while (FindNextFileW(h, &fd));
     FindClose(h);
     std::sort(names.begin(), names.end(),
@@ -216,12 +219,15 @@ inline void MirrorLiveToActiveProfile(const std::wstring& iniPath, const std::st
 // ini gets profile=Default. Runs before the tick loop records the ini mtime, so the write does not
 // trigger a spurious hot-reload. Idempotent: the dir existing (even empty) means never seed again -
 // which is why a failed Default.ini capture rolls the (still empty) dir back, so the seed retries on
-// the next launch instead of leaving a permanent half-migrated state.
-inline void EnsureProfilesSeeded(const std::wstring& iniPath) {
+// the next launch instead of leaving a permanent half-migrated state. Returns true when the profiles
+// dir exists afterwards (already seeded, or seeded now); false when the seed failed and was rolled
+// back, so a caller must not create the dir itself (that would set the latch with no Default.ini).
+inline bool EnsureProfilesSeeded(const std::wstring& iniPath) {
     SweepStaleIniTmp(iniPath);
     std::wstring dir = ProfilesDirFromIni(iniPath);
-    if (GetFileAttributesW(dir.c_str()) != INVALID_FILE_ATTRIBUTES) return;
-    if (!CreateDirectoryW(dir.c_str(), nullptr)) return;
+    if (GetFileAttributesW(dir.c_str()) != INVALID_FILE_ATTRIBUTES) return true;
+    // ERROR_ALREADY_EXISTS: another process created the dir in the race; it owns the seed.
+    if (!CreateDirectoryW(dir.c_str(), nullptr)) return GetLastError() == ERROR_ALREADY_EXISTS;
     std::string live;
     // First run: no ini yet. Seed from the documented default template (what LoadConfig would have
     // created), so the live ini is the commented file users expect rather than the single line
@@ -232,9 +238,10 @@ inline void EnsureProfilesSeeded(const std::wstring& iniPath) {
     if (!readOk ||
         !WriteTextFileAtomic(dir + L"\\Default.ini", MakeProfileText(live))) {
         RemoveDirectoryW(dir.c_str());   // dir is still empty; retry the whole seed next launch
-        return;
+        return false;
     }
     WriteTextFileAtomic(iniPath, UpdateIniText(live, "profile", "Default"));
+    return true;
 }
 // One-time ini migrations (#318), run at start before ResetSessionToProfile: zoomWheelMods moves into
 // the zoom button slots (MigrateWheelMods) in the live ini AND in every profile file, so the session

src/config_ui/main.cpp

@@ -259,14 +259,15 @@ static bool g_loggedNav = false, g_loggedReady = false;
 
 // Reply to getConfig / discardSession with the session, or with configUnreadable when the ini cannot
 // be read right now (the page retries; it must never be handed an empty ini as "all defaults").
-static void PostConfig(ICoreWebView2* wv) {
+// `extra` is appended to either reply as further JSON fields (discardSession's "discarded").
+static void PostConfig(ICoreWebView2* wv, const std::string& extra = std::string()) {
     std::string live;
     if (!ReadIni(live)) {
         wind::Log(wind::LogLevel::Warn, "config", "getConfig: ini unreadable");
-        wv->PostWebMessageAsJson(L"{\"type\":\"configUnreadable\"}");
+        wv->PostWebMessageAsJson(Widen("{\"type\":\"configUnreadable\"" + extra + "}").c_str());
         return;
     }
-    wv->PostWebMessageAsJson(Widen("{\"type\":\"config\"," + SessionPayload(live) + "}").c_str());
+    wv->PostWebMessageAsJson(Widen("{\"type\":\"config\"," + SessionPayload(live) + extra + "}").c_str());
 }
 // Diagnostics export (flush, copy every log and minidump, zip through PowerShell) takes seconds and
 // up to ~31 s worst case, so it runs on a worker thread; the window thread gets the result by message.
@@ -334,17 +335,33 @@ static void OnSaveSession(ICoreWebView2* wv) {
 }
 static void OnDiscardSession(ICoreWebView2* wv) {
     // Discard: the live ini goes back to the saved profile (globals kept); reply with fresh state.
-    wind::IniWriteLock iniLock;   // item 71
-    std::string live;
-    if (ReadIni(live)) {
-        const std::string active = ActiveProfileName(live);
-        std::string prof;
-        if (!active.empty() && SafeName(active) &&
-            GetFileAttributesW(ProfilePath(active).c_str()) != INVALID_FILE_ATTRIBUTES &&
-            wind::ReadTextFileOk(ProfilePath(active), prof))
-            WriteFileAtomic(IniPath(), wind::MakeLiveText(prof, live, active));
+    // "discarded":false when the session is still there (ini or profile unreadable, or the write
+    // failed): the page must then stop, or a following switch would mirror the unsaved session into
+    // the outgoing profile and turn Discard into Save. No active profile or no profile file means
+    // there is no saved state to go back to (and nothing a switch would mirror into): not a failure.
+    bool ok = false;
+    DWORD err = 0;   // captured under the lock: the lock release and later calls overwrite it
+    {
+        wind::IniWriteLock iniLock;   // item 71
+        std::string live;
+        if (!ReadIni(live)) {
+            err = GetLastError();
+        } else {
+            const std::string active = ActiveProfileName(live);
+            std::string prof;
+            if (active.empty() || !SafeName(active) ||
+                GetFileAttributesW(ProfilePath(active).c_str()) == INVALID_FILE_ATTRIBUTES)
+                ok = true;
+            else if (!wind::ReadTextFileOk(ProfilePath(active), prof))
+                err = GetLastError();
+            else {
+                ok = WriteFileAtomic(IniPath(), wind::MakeLiveText(prof, live, active));
+                if (!ok) err = GetLastError();
+            }
+        }
     }
-    PostConfig(wv);
+    if (!ok) wind::Log(wind::LogLevel::Warn, "config", "discardSession failed (err=%lu)", err);
+    PostConfig(wv, ok ? ",\"discarded\":true" : ",\"discarded\":false");
 }
 static void OnWindowAction(ICoreWebView2* wv, const std::string& j) {
     std::string action = JsonField(j, "action");
@@ -431,11 +448,12 @@ static void OnCreateProfile(ICoreWebView2* wv, const std::string& j) {
         // Pre-migration guard: creating a profile before the core ever seeded would create the
         // profiles dir (the seed latch) and then wipe the user's settings with no Default.ini
         // capture, permanently. Seed Default from the CURRENT settings first; no-op once seeded.
-        wind::EnsureProfilesSeeded(IniPath());
-        CreateDirectoryW(wind::ProfilesDirFromIni(IniPath()).c_str(), nullptr);
+        // A failed seed rolls the dir back; creating it here would set the latch with no Default.ini.
+        if (!wind::EnsureProfilesSeeded(IniPath()))
+            err = "Could not set up profiles (the config file may be locked); try again in a moment";
         std::string seed = "; Wind profile. Keys absent here fall back to Wind's built-in defaults.\n"
                            "model=hybrid\n";
-        if (fromCurrent) {
+        if (err.empty() && fromCurrent) {
             std::string live;
             if (!ReadIni(live)) err = "Could not read the config file";
             else seed = wind::MakeProfileText(live);

src/tray_app/main.cpp

@@ -18,9 +18,12 @@
 namespace wind { namespace TrayApp {
 
 static TrayShared*  g_block = nullptr;
+static HANDLE       g_hWind = nullptr;   // the Wind we serve (SYNCHRONIZE)
 
 TrayShared* Block() { return g_block; }
 
+bool ServedWindExited() { return g_hWind && WaitForSingleObject(g_hWind, 0) == WAIT_OBJECT_0; }
+
 void RequestWindQuit() {
     // The same clean-exit path the installer and Settings use: Wind restores cursor, clip and
     // Magnifier state as on any quit. A window message could not reach Wind anyway (UIPI).
@@ -117,6 +120,8 @@ int WINAPI wWinMain(HINSTANCE hInst, HINSTANCE, PWSTR cmdLine, int) {
         return 0;
     }
 
+    TrayApp::g_hWind = hWind;
+
     // Single instance. On a model relaunch the old Wind's tray is still exiting when the new Wind
     // starts us, so wait for it rather than giving up (it exits as soon as its Wind is gone).
     HANDLE mtx = CreateMutexW(nullptr, FALSE, kTrayMutexName);

src/tray_app/tray_app.h

@@ -50,6 +50,7 @@ void OpenSettings();
 // main.cpp
 TrayShared* Block();          // the shared status block, or nullptr when Wind did not create one
 void RequestWindQuit();       // sets Local\Wind_QuitRequest: Wind's clean-exit path
+bool ServedWindExited();      // the Wind this tray serves has exited (a modal prompt's loop never sees it)
 
 }  // namespace TrayApp
 }  // namespace wind

src/tray_app/tray_menu.cpp

@@ -35,6 +35,13 @@ void OpenSettings() {
 // Save writes the session to the active profile; Discard drops it (a switch resets the live ini to
 // the profile right here, so the switch's own capture of the outgoing live settings cannot save
 // them after all). Returns false when the user cancels.
+// The prompt's own modal loop never sees the served Wind exit (the main loop's wait does), so a
+// timer callback cancels it when Wind is gone: a restarted Wind needs this tray's mutex, and a late
+// Save or Discard would act on the NEW Wind's session (and RequestWindQuit would quit it).
+static HRESULT CALLBACK PromptCallback(HWND h, UINT msg, WPARAM, LPARAM, LONG_PTR) {
+    if (msg == TDN_TIMER && ServedWindExited()) SendMessageW(h, TDM_CLICK_BUTTON, IDCANCEL, 0);
+    return S_OK;
+}
 static bool ConfirmUnsaved(const std::wstring& ini, bool forSwitch) {
     // An unreadable ini is not a clean one: skipping the prompt would let Quit or a switch drop the
     // session unasked. Stop and let the user retry (a missing ini reads as empty and passes).
@@ -56,7 +63,8 @@ static bool ConfirmUnsaved(const std::wstring& ini, bool forSwitch) {
     const TASKDIALOG_BUTTON buttons[] = { { ID_SAVE, L"Save" }, { ID_DISCARD, L"Discard" } };
     TASKDIALOGCONFIG cfg{};
     cfg.cbSize = sizeof(cfg);
-    cfg.dwFlags = TDF_ALLOW_DIALOG_CANCELLATION;
+    cfg.dwFlags = TDF_ALLOW_DIALOG_CANCELLATION | TDF_CALLBACK_TIMER;
+    cfg.pfCallback = PromptCallback;
     cfg.dwCommonButtons = TDCBF_CANCEL_BUTTON;
     cfg.pszWindowTitle = L"Wind";
     cfg.pszMainInstruction = L"You have unsaved settings";
@@ -66,7 +74,18 @@ static bool ConfirmUnsaved(const std::wstring& ini, bool forSwitch) {
     cfg.pButtons = buttons;
     cfg.nDefaultButton = ID_SAVE;
     int pressed = 0;
-    if (FAILED(TaskDialogIndirect(&cfg, &pressed, nullptr, nullptr))) return true;
+    const HRESULT hr = TaskDialogIndirect(&cfg, &pressed, nullptr, nullptr);
+    if (FAILED(hr)) {
+        // The session differs and the user was never asked: going ahead would drop it unasked.
+        wind::Log(wind::LogLevel::Warn, "profile", "unsaved prompt failed (hr=0x%08lx)", static_cast<unsigned long>(hr));
+        Notify(L"Wind", forSwitch ? L"Could not ask about the unsaved settings; profile not switched."
+                                  : L"Could not ask about the unsaved settings; Wind keeps running.");
+        return false;
+    }
+    if (ServedWindExited()) {
+        wind::Log(wind::LogLevel::Info, "profile", "unsaved prompt dropped: the served Wind exited");
+        return false;
+    }
     if (pressed != ID_SAVE && pressed != ID_DISCARD) return false;
     // The dialog blocks for as long as the user takes; Settings, the flyout or a hand edit may have
     // written the ini meanwhile. Act on the CURRENT text, not the snapshot the prompt was built from
@@ -76,6 +95,18 @@ static bool ConfirmUnsaved(const std::wstring& ini, bool forSwitch) {
         Notify(L"Wind", L"Could not read the settings (config file is locked); nothing was changed.");
         return false;
     }
+    // `pp` and the profile snapshot belong to the profile that was active when the prompt opened.
+    // If Settings switched profile meanwhile, Save would store the new session in the OLD profile
+    // and Discard would undo the switch: stop instead.
+    {
+        auto now = wind::ReadIniValues(live);
+        auto nit = now.find("profile");
+        if (nit == now.end() || !wind::SameProfileName(nit->second, it->second)) {
+            wind::Log(wind::LogLevel::Info, "profile", "unsaved prompt: the active profile changed meanwhile");
+            Notify(L"Wind", L"The active profile changed while you were asked; nothing was changed.");
+            return false;
+        }
+    }
     if (pressed == ID_SAVE) {
         if (!wind::WriteTextFileAtomic(pp, wind::MakeProfileText(live))) {
             wind::Log(wind::LogLevel::Warn, "profile", "quit save failed (err=%lu)", GetLastError());
@@ -85,7 +116,11 @@ static bool ConfirmUnsaved(const std::wstring& ini, bool forSwitch) {
         }
         return true;
     }
-    if (forSwitch && !wind::WriteTextFileAtomic(ini, wind::MakeLiveText(profile, live, it->second))) {
+    if (!forSwitch) return true;
+    // Discard resets the live ini to the profile as saved NOW (a keybind capture in Settings writes
+    // straight into the profile file, so the pre-prompt snapshot may be stale).
+    if (!wind::ReadTextFileOk(pp, profile) ||
+        !wind::WriteTextFileAtomic(ini, wind::MakeLiveText(profile, live, it->second))) {
         Notify(L"Wind", L"Could not discard the settings; the profile stays as it is.");
         return false;
     }

tests/test_profiles_io.cpp

@@ -95,3 +95,37 @@ TEST_CASE("ListProfileFiles lists .ini names case-insensitively sorted") {
     CHECK(n[0] == L"Alpha");
     CHECK(n[1] == L"beta");
 }
+
+TEST_CASE("ListProfileFiles skips names that match *.ini only through their 8.3 short name") {
+    TempDir d;
+    WriteTextFileAtomic(d.path + L"\\Work.ini", "x");
+    WriteTextFileAtomic(d.path + L"\\Gaming.INI", "x");
+    // Editor backups: with 8.3 names on, these get short names like WORK~1.INI that match "*.ini".
+    WriteTextFileAtomic(d.path + L"\\Work.ini_bak", "x");
+    WriteTextFileAtomic(d.path + L"\\Gaming.ini~", "x");
+    auto n = ListProfileFiles(d.path);
+    REQUIRE(n.size() == 2);
+    CHECK(n[0] == L"Gaming");
+    CHECK(n[1] == L"Work");
+}
+
+TEST_CASE("EnsureProfilesSeeded reports whether the profiles dir is in place") {
+    TempDir d; const std::wstring ini = d.path + L"\\magnifier.ini";
+    const std::wstring dir = d.path + L"\\profiles";
+    REQUIRE(WriteTextFileAtomic(ini, "maxLevel=7\r\n"));
+    {
+        // A locked live ini: the seed rolls back and says so, leaving no dir (the latch) behind.
+        HANDLE h = CreateFileW(ini.c_str(), GENERIC_READ, 0, nullptr, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, nullptr);
+        REQUIRE(h != INVALID_HANDLE_VALUE);
+        CHECK_FALSE(EnsureProfilesSeeded(ini));
+        CHECK(GetFileAttributesW(dir.c_str()) == INVALID_FILE_ATTRIBUTES);
+        CloseHandle(h);
+    }
+    CHECK(EnsureProfilesSeeded(ini));
+    std::string def;
+    CHECK(ReadTextFileOk(dir + L"\\Default.ini", def));
+    CHECK(def.find("maxLevel=7") != std::string::npos);
+    CHECK(EnsureProfilesSeeded(ini));   // already seeded
+    DeleteFileW((dir + L"\\Default.ini").c_str());
+    RemoveDirectoryW(dir.c_str());
+}