A second window from --new-instance no longer duplicates or overwrites the first window's session (F9)

src/PlanViewer.App/MainWindow.FileOps.cs

@@ -583,9 +583,18 @@ internal List<string> CollectOpenTabEntries()
     /// <summary>
     /// Saves the restore entries of all currently open tabs — file paths, and since #496
     /// scratch buffer entries — docked and detached alike (#490).
+    ///
+    /// <para>Not in a secondary instance (<see cref="SingleInstance.IsSecondaryInstance"/>):
+    /// the list on disk is the owner's, and this is the one writer that would replace it
+    /// with the secondary's own tabs. Every write of the list comes through here — the
+    /// debounce, the final write in <see cref="OnClosed"/>, and the update restart — so the
+    /// one guard covers all three.</para>
     /// </summary>
     private void SaveOpenPlans()
     {
+        if (SingleInstance.IsSecondaryInstance)
+            return;
+
         _appSettings.OpenTabs.Clear();
         _appSettings.OpenTabs.AddRange(CollectOpenTabEntries());
 
@@ -627,6 +636,12 @@ timer against a window being torn down. */
         if (IsShuttingDown)
             return;
 
+        /* A secondary instance writes no session state (see SaveOpenPlans), so there is
+           nothing to schedule: no pending flag, and no timer that would tick just to find
+           the write refused. */
+        if (SingleInstance.IsSecondaryInstance)
+            return;
+
         _sessionPersistPending = true;
 
         /* No real timer under the test host, in #451's pattern: the suite shares one
@@ -741,6 +756,11 @@ internal void PersistSessionForRestart()
     /// the poison defense wholesale — the entry is already off the cleared list before its
     /// buffer is read, and a buffer that fails to load is skipped, never re-added, and
     /// deleted (<see cref="TryRestoreScratchTab"/>).</para>
+    ///
+    /// <para><b>Never in a secondary instance.</b> Everything above is written for the one
+    /// instance that owns the saved session: the clear-and-save empties the owner's list on
+    /// disk, and the sweep deletes buffers the owner still needs. <see cref="OpenFromStartupArgs"/>
+    /// skips this method when <see cref="SingleInstance.IsSecondaryInstance"/> is set.</para>
     /// </summary>
     /// <param name="createFallbackTab">
     /// Whether an empty restore opens a fresh query tab. False when the caller is about to

src/PlanViewer.App/MainWindow.ScratchPersist.cs

@@ -99,6 +99,16 @@ different questions (staleness against on-disk changes, most of all). That is #4
     /// </summary>
     private void HookScratchPersistence(QuerySessionControl session)
     {
+        /* A secondary instance never persists scratch content. The buffer folder and the ids
+           in it belong to the instance that owns the slot: writing there would put this
+           window's text under names the owner also writes, drops and sweeps. With no
+           subscription nothing is ever queued, no buffer id is minted, and every place that
+           drops a buffer finds no id and does nothing — the drop sites need no guard of
+           their own. The cost is crash recovery for this window's scratch tabs; the close
+           prompts do not depend on it. */
+        if (SingleInstance.IsSecondaryInstance)
+            return;
+
         /* Only sessions that are scratch NOW. SourceFilePath moves null→path exactly once
            (the save) and never back, so a session arriving here file-backed can never need
            this hook later — the scope fence again, applied at subscription time. It also

src/PlanViewer.App/MainWindow.axaml.cs

@@ -70,7 +70,12 @@ public MainWindow()
         // Not in the test host (#451): every test window would grab the machine's single
         // SQLPerformanceStudio_OpenFile pipe slot and never release it — OnClosed never
         // runs there — racing any real Studio instance on the same box.
-        if (!AppRuntimeMode.IsTestHost)
+        // Not in a secondary instance either: the pipe belongs to the instance that owns the
+        // slot. The listener retries until it gets the pipe, so a secondary would take it once
+        // the owner exits, and later launches would hand their files to a window whose tabs are
+        // never saved. Without it, such a launch finds no pipe, claims the slot, and runs as the
+        // new owner.
+        if (!AppRuntimeMode.IsTestHost && !SingleInstance.IsSecondaryInstance)
             StartPipeServer();
 
         InitializeComponent();
@@ -224,7 +229,23 @@ this only changes the cold start. The restore's new-tab fallback stays out of th
            way when a file is about to open — a stray empty scratch tab beside the file the
            user asked for is nobody's intent. */
         var hasFileArg = args.Length > 1 && File.Exists(args[1]);
-        RestoreOpenPlans(createFallbackTab: !hasFileArg);
+
+        if (SingleInstance.IsSecondaryInstance)
+        {
+            /* A secondary instance (--new-instance beside a running one) does not restore.
+               The saved list belongs to the instance that owns the slot, which rewrites it on
+               every tab change: restoring it here would open a copy of every one of its tabs
+               and share its scratch buffer ids. RestoreOpenPlans is skipped whole, not called
+               with a flag, because it also clears and saves the list and sweeps the buffer
+               folder — both of which would land on the owner's files. What is left is what a
+               launch with nothing to restore does: the file it was given, else a new tab. */
+            if (!hasFileArg)
+                NewQuery_Click(this, new RoutedEventArgs());
+        }
+        else
+        {
+            RestoreOpenPlans(createFallbackTab: !hasFileArg);
+        }
 
         if (hasFileArg)
             OpenFileByExtension(args[1]);

src/PlanViewer.App/Program.cs

@@ -50,7 +50,10 @@ other setting (AtomicFile prevents torn writes, not lost updates). File-argument
            launches already forwarded to the running instance over the pipe; a BARE second
            launch ran a full instance and was exactly the clobber case. So unless the user
            explicitly asks for a second instance, a launch that finds one running hands it
-           its work — a file path, or a bare "surface yourself" — and exits. */
+           its work — a file path, or a bare "surface yourself" — and exits. A launch that
+           does ask (--new-instance) still gets its own window, but it claims the slot first
+           and, if another instance already holds it, runs as a secondary that leaves the
+           saved session alone — see ClaimSlotForNewInstance. */
         var newInstanceRequested = SingleInstance.NewInstanceRequested(args);
 
         // The flag is a launcher directive, not a file: strip it so nothing downstream can
@@ -101,6 +104,11 @@ when the loser's retries run out before the winner's pipe exists. The
                    pre-#489 last-write-wins behavior, which is the accepted floor. */
             }
         }
+        else
+        {
+            // --new-instance: never handed to a running instance, but it still claims the slot.
+            ClaimSlotForNewInstance();
+        }
 
         BuildAvaloniaApp()
             .StartWithClassicDesktopLifetime(effectiveArgs);
@@ -118,16 +126,40 @@ public static AppBuilder BuildAvaloniaApp()
             .WithInterFont()
             .LogToTrace();
 
+    /// <summary>
+    /// What <c>--new-instance</c> does about the slot. It never hands its launch to the
+    /// running instance — the user asked for a window of their own — but it no longer skips
+    /// the slot either. It claims it the same way an ordinary launch does:
+    /// <list type="bullet">
+    /// <item>Slot free: no other instance is running, so this launch is an ordinary one. It
+    /// restores, persists and owns the slot, and later launches hand their work to it.</item>
+    /// <item>Slot held: another instance owns the saved session, so this process runs as a
+    /// secondary (<see cref="SingleInstance.IsSecondaryInstance"/>) and leaves that session
+    /// alone. Before this, it restored the owner's list into a second window, and the two
+    /// windows then wrote, dropped and swept the same scratch buffers.</item>
+    /// </list>
+    /// Split from <see cref="Main"/> so the decision can be tested with a mutex name of the
+    /// test's own; the harness cannot start a second app process to hold the real one.
+    ///
+    /// <para>Where named mutexes do not work at all, <see cref="TryBecomeSingleInstanceOwner"/>
+    /// answers true so that the launch still runs, and this launch then acts as an owner: it
+    /// restores the saved session even if another instance is running. That is how every
+    /// <c>--new-instance</c> launch behaved before, and it only happens where the mutex
+    /// machinery itself fails.</para>
+    /// </summary>
+    internal static void ClaimSlotForNewInstance(string mutexName = SingleInstance.MutexName) =>
+        SingleInstance.IsSecondaryInstance = !TryBecomeSingleInstanceOwner(mutexName);
+
     /// <summary>
     /// Tries to claim the single-instance slot (#489). True means this process is the
     /// owner and should run; false means another instance holds the slot and this launch
-    /// should hand its work over instead.
+    /// should hand its work over instead (or, for <c>--new-instance</c>, run as a secondary).
     /// </summary>
-    private static bool TryBecomeSingleInstanceOwner()
+    private static bool TryBecomeSingleInstanceOwner(string mutexName = SingleInstance.MutexName)
     {
         try
         {
-            var mutex = new Mutex(initiallyOwned: true, SingleInstance.MutexName, out var createdNew);
+            var mutex = new Mutex(initiallyOwned: true, mutexName, out var createdNew);
             if (createdNew)
             {
                 _singleInstanceMutex = mutex;

src/PlanViewer.App/Services/AppSettingsService.cs

@@ -193,6 +193,18 @@ public static AppSettings Load()
     /// while <see cref="Load"/> found the file unreadable, which would otherwise overwrite
     /// content this process has never actually seen. The Settings window reports that case
     /// through <see cref="SaveBlocked"/>.
+    ///
+    /// <para>In a secondary instance (<see cref="SingleInstance.IsSecondaryInstance"/>) the
+    /// saved open-tab list is kept as the disk has it. The whole file is written, so the
+    /// settings object's list goes with it — and a secondary's copy is the one it read at its
+    /// own startup, long out of date next to the list the owner rewrites on every tab change.
+    /// Recent plans, the Settings dialog and the server-filter toggle all save through here.
+    /// The list is read off the disk just before the write, so the write hands back what the
+    /// owner last saved. An owner save that lands between that read and the write is still
+    /// lost; nothing locks the file across processes, and the gap is milliseconds. Every other
+    /// setting stays last-write-wins, which is what two instances on purpose has always meant
+    /// (see Program.Main). If the file cannot be read there is no list to keep, so the save is
+    /// skipped rather than written with a guess.</para>
     /// </summary>
     public static void Save(AppSettings settings)
     {
@@ -201,6 +213,9 @@ public static void Save(AppSettings settings)
 
         try
         {
+            if (SingleInstance.IsSecondaryInstance && !TryAdoptSavedOpenTabs(settings))
+                return;
+
             Directory.CreateDirectory(SettingsDir);
             var json = JsonSerializer.Serialize(settings, JsonOptions);
             AtomicFile.WriteAllText(SettingsPath, json);
@@ -212,6 +227,32 @@ public static void Save(AppSettings settings)
         }
     }
 
+    /// <summary>
+    /// Puts the open-tab list that is on disk right now into <paramref name="settings"/>, for a
+    /// secondary instance about to write the whole file (see <see cref="Save"/>). Read the way
+    /// <see cref="Load"/> reads, so a file that will not parse is moved aside rather than
+    /// overwritten. False when the file exists but cannot be read — the caller must not write.
+    /// </summary>
+    private static bool TryAdoptSavedOpenTabs(AppSettings settings)
+    {
+        var outcome = SettingsFileStore.Read<AppSettings>(
+            SettingsPath,
+            nameof(AppSettingsService),
+            json => JsonSerializer.Deserialize<AppSettings>(json, JsonOptions),
+            out var onDisk);
+
+        if (outcome == SettingsFileStore.ReadOutcome.Unreadable)
+            return false;
+
+        if (onDisk != null)
+            MigrateOpenTabs(onDisk);
+
+        // Missing, or unparseable and just moved aside: there is no list on disk to keep, and
+        // an empty one is a truer thing to write than a stale one.
+        settings.OpenTabs = onDisk?.OpenTabs ?? new List<string>();
+        return true;
+    }
+
     /// <summary>
     /// Moves an "open_plans" list written by an older version onto <see cref="AppSettings.OpenTabs"/>,
     /// so upgrading does not cost the user the tabs they had open. Only fills an empty OpenTabs:

src/PlanViewer.App/SingleInstance.cs

@@ -47,12 +47,42 @@ internal static class SingleInstance
     internal const string MutexName = "SQLPerformanceStudio_SingleInstance";
 
     /// <summary>
-    /// Escape hatch (#489): skip the single-instance check and run a full second instance.
-    /// A user who runs two on purpose accepts settings last-write-wins as their informed
-    /// choice. Stripped from argv before any file-open logic sees it.
+    /// Escape hatch (#489): open a window of its own instead of handing the launch to the
+    /// running instance. A user who runs two on purpose accepts settings last-write-wins as
+    /// their informed choice — but not a duplicated session: the launch claims the slot
+    /// first, and when another instance already holds it this process runs as a secondary
+    /// (see <see cref="IsSecondaryInstance"/>). Stripped from argv before any file-open
+    /// logic sees it.
     /// </summary>
     internal const string NewInstanceFlag = "--new-instance";
 
+    /// <summary>
+    /// True in a process started with <see cref="NewInstanceFlag"/> while another instance
+    /// already owned the single-instance slot. Set once by <see cref="Program"/> before any
+    /// window exists; nothing else in the product sets it. Tests that set it belong in the
+    /// serial collection, because it changes what every settings save writes.
+    ///
+    /// <para><b>Why a secondary must not own the session.</b> The saved open-tab list and the
+    /// scratch buffer folder have one writer by design: the owner rewrites the list on every
+    /// tab change, names buffers by ids only it knows, and sweeps any buffer its own list
+    /// does not name. A second process that restored that list would open a copy of every
+    /// tab, share the owner's buffer ids so that both windows write, drop and sweep the same
+    /// files, and whichever window closed last would overwrite the other's list.</para>
+    ///
+    /// <para><b>What a secondary does instead.</b> It restores nothing (a file argument still
+    /// opens; otherwise the usual new tab), never writes the list or a scratch buffer, never
+    /// sweeps the buffer folder, and keeps the list already on disk when it saves settings
+    /// (see <c>AppSettingsService.Save</c>). What it loses is session restore for its own
+    /// tabs: they are not reopened at the next start, whether it closed cleanly or crashed,
+    /// and its scratch tabs have no crash recovery. The unsaved-changes prompts do not depend
+    /// on persistence and work as they always have.</para>
+    ///
+    /// <para>Not set on the launch path where a non-owner runs fully after the pipe hand-off
+    /// failed (see <c>Program.Main</c>): that launch never asked for a second window, so it
+    /// keeps the pre-#489 behavior.</para>
+    /// </summary>
+    internal static bool IsSecondaryInstance { get; set; }
+
     /// <summary>
     /// The line a bare second launch sends to mean "surface your main window". The
     /// double-colons make it unrepresentable as a Windows file name on purpose — see the